| zaro | asselin: i believe i've got my changes in but have an error, don't understand the message though. https://review.openstack.org/#/c/199778/ | 00:02 |
|---|---|---|
| * asselin looks | 00:05 | |
| * zaro needs to head out. | 00:08 | |
| zaro | asselin: will be online later tonight to correct those if you can tell me what the error means. | 00:10 |
| asselin | zaro, sure | 00:10 |
| asselin | zaro, reviewed | 00:17 |
| anteaya | so let's hold off rechecks until our git farm is back again | 00:20 |
| fungi | so for sprint purposes, someone may want to work on a new https://review.openstack.org/182394 (we reverted it) and test it on centos 6 | 00:27 |
| fungi | in particular, it looks like you need mod_version enabled to use ifversion containers | 00:27 |
| fungi | so it was falling back to loading everything inside the ifversion >= 2.4 blocks even though it was running apache 2.2 | 00:28 |
| fungi | fun story | 00:28 |
| asselin | fungi, is that what broke git.openstack.org? | 00:28 |
| fungi | yeppers | 00:28 |
| anteaya | I'll give it a shot | 00:39 |
| *** jkomg has joined #openstack-sprint | 00:40 | |
| anteaya | any idea what email yolanda wants me to use for her co-authored by credit? | 00:45 |
| anteaya | this one: https://review.openstack.org/#/q/owner:info%2540ysoft.biz+status:open,n,z | 00:45 |
| *** jkomg has quit IRC | 00:45 | |
| fungi | i usually just cut-n-paste from the git commit info in gerrit | 00:50 |
| fungi | (or in git really) | 00:50 |
| fungi | git log, cut, paste | 00:50 |
| asselin | zaro, why so many patches? it's getting confusing. Can you update them to one puppet-openstackci and one related system-config patch? | 00:57 |
| anteaya | you way is better | 01:00 |
| anteaya | your way | 01:00 |
| asselin | jhesketh, yolanda jesusaurus I updated the etherpad with nodepool patches | 01:05 |
| zaro | asselin: it's because i thought those are 2 distinct changes. one to move and one to rename params. | 01:07 |
| asselin | zaro, ok...so what order should I look at them? | 01:07 |
| zaro | asselin: also i didn't want to reuse the previous change because i thought reviewers would get confused due to all the comments. | 01:08 |
| zaro | and doing both at once would have been more confusing for me. | 01:09 |
| asselin | ok....so help me sort them so I can review | 01:09 |
| zaro | asselin: start with https://review.openstack.org/#/c/199778/ => https://review.openstack.org/#/c/199784/ => https://review.openstack.org/#/c/199780/ | 01:10 |
| asselin | on the etherpad | 01:10 |
| zaro | hold on, i've lost the ability to copy/paste from irc | 01:12 |
| asselin | zaro, ok, so 184919 and 199335 would be abandoned? | 01:12 |
| zaro | yes. | 01:13 |
| asselin | zaro, ok, so one issue we'll have is that system-config is already consuming openstackci:jenkins_master | 01:16 |
| asselin | so each change will need to work with system-config as it merges | 01:17 |
| asselin | zaro, I think we can do that by setting manage_jenkins_jobs=false in the first patch | 01:18 |
| asselin | 199778 | 01:18 |
| zaro | so either go back to single change for system-config and openstackci or 2 changes for each? | 01:18 |
| asselin | I'm looking for a path forward witn what you proposed | 01:19 |
| zaro | maybe i'll just do the single change for each since that seems simpler in this case. | 01:19 |
| asselin | zaro, up to you. Either way, I think we'll need to set the default manage_jenkins_jobs=false | 01:20 |
| asselin | then we align the system config change with the correct params, and pass in manage_jenkins_jobs=true | 01:21 |
| zaro | why default to false? | 01:21 |
| asselin | then on a new patch we can change the default back to false | 01:21 |
| zaro | but yeah, i'll munge the move and rename in one. | 01:21 |
| asselin | zaro, b/c we don't want the part in the if stament to run | 01:22 |
| asselin | https://review.openstack.org/#/c/199784/1/manifests/jenkins_master.pp | 01:22 |
| zaro | yah, i get it. | 01:22 |
| asselin | until after system config passes in its values | 01:22 |
| asselin | ok | 01:22 |
| *** TravT has joined #openstack-sprint | 01:51 | |
| *** TravT is now known as TravT_away | 01:51 | |
| *** crinkle has quit IRC | 02:08 | |
| *** crinkle has joined #openstack-sprint | 02:16 | |
| *** asselin__ has joined #openstack-sprint | 03:41 | |
| *** larainema has joined #openstack-sprint | 04:43 | |
| *** larainema has left #openstack-sprint | 04:49 | |
| *** ig0r_ has joined #openstack-sprint | 05:51 | |
| *** ig0r__ has quit IRC | 05:55 | |
| zaro | asselin: squashed my changes into one, abandoned my previous changes, and updated the etherpad. | 06:02 |
| yolanda | hi there, picking all conversations from the sprint, thx for all the efforts on prev patches i sent | 06:06 |
| *** TravT_away has quit IRC | 07:08 | |
| *** mmasaki has quit IRC | 08:31 | |
| *** ctlaugh_ has quit IRC | 08:47 | |
| *** TravT_away has joined #openstack-sprint | 09:01 | |
| *** ctlaugh_ has joined #openstack-sprint | 09:01 | |
| *** BobBall has joined #openstack-sprint | 09:04 | |
| asselin__ | yolanda, hi | 12:49 |
| yolanda | hi asselin_ | 12:49 |
| *** pserebryakov has joined #openstack-sprint | 12:50 | |
| yolanda | i amended the openstack-ci nodepool patch, based on our previous secure.conf patches, and the oscc from nibz | 12:52 |
| yolanda | it's a very long queue of patches however | 12:52 |
| asselin__ | yolanda, which one is the oscc patch? | 13:12 |
| asselin__ | you didn't update the commit message here https://review.openstack.org/#/c/188325/16 | 13:13 |
| yolanda | mm, i guess i did in wrong patchset | 13:14 |
| yolanda | asselin, weird, i was using gerrit editor for that | 13:15 |
| yolanda | did again | 13:15 |
| yolanda | asselin, oscc patch is https://review.openstack.org/#/c/199674/ | 13:20 |
| asselin__ | yolanda, https://review.openstack.org/#/c/188325/ commented | 13:25 |
| yolanda | cool, i'll take a look | 13:27 |
| asselin__ | yolanda, for this, I think the depends on is indirectly there: https://review.openstack.org/#/c/199750/ via the gerrit depends-on change's commit message depends on | 13:27 |
| yolanda | yes, there is a chain | 13:28 |
| yolanda | but i see that more clear, if that is referenced in the commit message, don't you think? | 13:29 |
| yolanda | asselin_, updated doc on secure.conf change | 13:33 |
| asselin__ | yolanda, That patch is the final change to transition to the new nodepool configurations. Anyway, I wouldn't -1 for just that. | 13:40 |
| yolanda | k, let me add it myself and we are ok | 13:45 |
| yolanda | done | 13:49 |
| *** pserebryakov has quit IRC | 14:00 | |
| *** TravT_away is now known as TravT | 14:01 | |
| *** TravT has quit IRC | 15:03 | |
| *** TravT has joined #openstack-sprint | 15:04 | |
| jeblair | yolanda: i left a comment on https://review.openstack.org/189762 | 15:31 |
| yolanda | checking | 15:32 |
| yolanda | mm, this one involves a bit of rewrite, but it makes sense | 15:32 |
| jeblair | yeah, sorry i didn't catch that earlier :( | 15:33 |
| yolanda | no problem | 15:33 |
| yolanda | i updated the patches | 15:54 |
| jeblair | nibalizer, yolanda: see crinkle's note about license url in https://review.openstack.org/199653 | 15:55 |
| jeblair | i'm going to aprv it anyway because it matches all the other modules, but that might be a project-wide cleanup at some point | 15:55 |
| jeblair | nibalizer, mordred: when you are both around, can we chat about https://review.openstack.org/199674 ? i have two thoughts: 1) i don't know if i think changing behavior based on invoking uid is a good idea (this is a philisophical point). 2) i don't know if that will actually do what we want -- don't we want to write ~nodepool's clouds.yaml while running as root? (this is a practical point) | 16:00 |
| *** Somay has joined #openstack-sprint | 16:06 | |
| *** Somay has left #openstack-sprint | 16:13 | |
| *** TravT_ has joined #openstack-sprint | 16:13 | |
| mordred | jeblair: morning! | 16:14 |
| * zaro continues reviewing | 16:15 | |
| *** TravT has quit IRC | 16:16 | |
| *** TravT_ is now known as TravT | 16:16 | |
| mordred | jeblair, nibalizer: I'm not 100% sure I understand how the structure of that is going to help the general problem ... that being that we have some public and some private data here - unless we want to just consider all of our cloud config to be private | 16:16 |
| jeblair | mordred: i think i was assuming it was all private... what's public? | 16:17 |
| mordred | well, the only thing that's secret is the password | 16:17 |
| mordred | there is nothing secret about any of the rest of the data in the file | 16:17 |
| mordred | there also might be things, such as cache settings, that we would want to express changes to in a code review | 16:18 |
| jeblair | mordred: my understanding of the problem this is trying to solve is that we have multiple puppet classes that may want to have a clouds.yaml file written, so this is generalizing that pattern into a reusable component | 16:18 |
| mordred | totally | 16:18 |
| mordred | I understand the reason for the existence of the module | 16:18 |
| jeblair | ok, i may be talking to myself then. bear with me. :) | 16:18 |
| mordred | I'm not sure I fully grok how, with its current structure, it would be consumed | 16:19 |
| mordred | jeblair: (I may be too :) ) | 16:19 |
| jeblair | mordred: we would still be able to review changes like that in code review right? they would be changes to system-config.... put another way: | 16:19 |
| jeblair | mordred: the private/public decision becomes one of what we put in our secret hiera file and what we put in puppet manifests or public hiera files | 16:20 |
| jeblair | yeah? | 16:20 |
| asselin | yolanda, ran into internal issues with testing nodepool patches. Switching back to JJB. | 16:20 |
| nibalizer | mordred: you can build the hash in puppet then add in the password by doimg a hiera lookup | 16:27 |
| mordred | nibalizer: yeah - that was the thing that seemed ick to me | 16:28 |
| mordred | like, there is already a structured format, managing that structured format in a different structured format just seems mildly bonkers to the way my brain works (sort of like why I don't like the newer puppet-apache) ... but Iwill admit I do not have a better suggestion | 16:29 |
| nibalizer | mordred: well new apache models the apache syntax in the puppet language | 16:31 |
| nibalizer | the clouds.yaml file is just a big hash with sub hashes and arrays | 16:31 |
| nibalizer | note that the template just calls @var.to_yaml so it is exactly the same data, no manipulation | 16:32 |
| mordred | sure | 16:32 |
| nibalizer | no translation or anything icky like that | 16:32 |
| mordred | I know - I just have to manage it now in puppet format, rather than yaml format | 16:32 |
| nibalizer | not really | 16:32 |
| mordred | so it's a bunch of extra syntax | 16:32 |
| nibalizer | but you can | 16:32 |
| mordred | no? | 16:32 |
| mordred | ok - that's the part where I think if I saw consumption of it I'd understand betterer | 16:32 |
| nibalizer | I mean you can/should I think | 16:33 |
| nibalizer | but you could look up the entire hash from hiera | 16:33 |
| nibalizer | $clouds = hiera_hash('nodepool_clouds') | 16:33 |
| nibalizer | and if nodepool_clouds is a hash it will just work | 16:33 |
| mordred | so - how does hiera handle merging hashes | 16:33 |
| nibalizer | https://docs.puppetlabs.com/hiera/2.0/lookup_types.html#hash-merge | 16:34 |
| mordred | like, can I have a nodepool_clouds in private hiera AND a nodepool_clouds in public hiera and it'll pull cloud['rax']['auth']['password'] from private and the rest from public? | 16:34 |
| nibalizer | yep thats exactly what you can do | 16:34 |
| mordred | ok | 16:34 |
| mordred | jeblair: ^^ I believe this gets me the thing that woudl make me happy with consuming this | 16:34 |
| mordred | jeblair: I realize I have not answered _any_ of your questions - so thanks for letting me hijack that :) | 16:35 |
| jeblair | mordred: neato! so we need, er, hierarchical, hiera to do that, yeah? | 16:35 |
| jeblair | mordred: that's okay, i've barred the door! | 16:35 |
| nibalizer | also the behaviour change stuff on $::id in that change could be ripped out | 16:35 |
| nibalizer | I'm not particularly attached to it | 16:36 |
| mordred | jeblair, nibalizer: I could see a user parameter that defaults to $::id ... | 16:36 |
| mordred | jeblair: to question 2 - why would we want to write out nodepool's clouds.yaml as root? | 16:37 |
| mordred | oh - that was ... | 16:37 |
| mordred | nevermind | 16:37 |
| mordred | that was another facet of why the id thing might not work | 16:38 |
| mordred | nibalizer, jeblair: k. I don't need us to manage it all in hiera files immediately. knowing that we _can_ once we have a strong in-tree hiera story landed is fine by me | 16:40 |
| jeblair | mordred: yeah, i meant "we run puppet as root, it wants to write ~nodepool's hiera file, so $::id wourd dtwt" | 16:42 |
| mordred | yup | 16:42 |
| * mordred groks now | 16:42 | |
| nibalizer | i tried to make it so that it would be easy to set the group of the clouds.yaml file so that you could give the nodepool user access to it | 16:42 |
| nibalizer | im also okay with that module knownign how to manage a 'cloud' group or something that we'd add users too | 16:43 |
| nibalizer | sortof like the dailup user | 16:43 |
| nibalizer | er group | 16:43 |
| nibalizer | mordred: another thing is I've kinda already done the puppetification of the data structure | 16:45 |
| nibalizer | so that can be copypastad where it needs to go then manipulated | 16:45 |
| nibalizer | also right now the way it works is that if you dont pass a client or a cache object you'll get an empty client hash in the config file | 16:45 |
| nibalizer | that wouldn't break anything right? | 16:45 |
| mordred | it _shouldn't_? I should test that | 16:46 |
| mordred | nibalizer: nope. doesn't break anything | 16:46 |
| nibalizer | noice | 16:47 |
| *** asselin_ has joined #openstack-sprint | 16:58 | |
| *** asselin__ has quit IRC | 17:00 | |
| pabelanger | nibalizer, took me a while, but http://logs.openstack.org/19/198819/7/check/gate-infra-puppet-apply-precise/f0b1b3f/console.html#_2015-07-09_02_30_26_962 | 17:07 |
| pabelanger | need to come up with new names for the puppet-httpd service and package | 17:07 |
| pabelanger | since they conflict with puppet-apache 0.0.4 being installed | 17:08 |
| nibalizer | that shouldn't conflict on a single node though right? | 17:09 |
| nibalizer | also, unfortunately, I can't be as active today | 17:09 |
| nibalizer | gotta do some internal stuff | 17:09 |
| pabelanger | nibalizer, ya, it is bombing the bare-precise node | 17:09 |
| pabelanger | I need to see what is it pulling in for apache | 17:10 |
| pabelanger | but, right now we cannot do both puppet-apache and puppet-httpd on same node | 17:10 |
| nibalizer | thats an expected failure | 17:10 |
| nibalizer | they can share the modulepath | 17:10 |
| nibalizer | but you can only include httpd or include apache once | 17:10 |
| clarkb | uh | 17:12 |
| clarkb | include should allow infinite includes? | 17:12 |
| clarkb | oh you mean you can't mix them | 17:12 |
| *** TravT_ has joined #openstack-sprint | 17:13 | |
| *** TravT_ has quit IRC | 17:13 | |
| *** TravT_ has joined #openstack-sprint | 17:14 | |
| nibalizer | https://github.com/wcooley/puppet-puppet_mutex or maybe something less generic could be added to puppet-httpd | 17:16 |
| *** TravT has quit IRC | 17:16 | |
| pabelanger | nibalizer, I can fix it by adding a Depends-On message in system-config | 17:18 |
| nibalizer | right well we could have the puppet-httpd class check to see if an 'apache' class has been instantiated on the node and bail if so | 17:23 |
| jesusaurus | but that would probably lead to unintended behaviour, i think that getting an error when including both is probably good | 17:25 |
| *** TravT_ is now known as TravT | 17:25 | |
| pabelanger | jeblair, have you had a chance to look at https://review.openstack.org/#/c/199794/ ? | 17:26 |
| pabelanger | regarding the git package for centos 6 | 17:26 |
| clarkb | pabelanger: can we wait and see if ianw's fix gets in? | 17:29 |
| clarkb | or do we not expect it to make it to the normal repos? | 17:30 |
| pabelanger | clarkb, did some research yesterday, people been saying RHEL 6.8 | 17:30 |
| clarkb | (that appears to be ianw's fix just consumed in a round about way) | 17:30 |
| pabelanger | Right | 17:30 |
| asselin | yolanda, btw, one issue with the nodepool is that e.g. nodepool list requires sudo / being nodepool user | 17:31 |
| clarkb | asselin: is that an issue? | 17:31 |
| asselin | maybe that's a feature | 17:32 |
| jeblair | clarkb, pabelanger: so "saying rhel 6.8" means.... some unknown point in either the near or distant future? :) | 17:32 |
| clarkb | jeblair: I have no idea :) | 17:32 |
| clarkb | is there no way to get it into epel now? | 17:32 |
| clarkb | (I really have no clue how rhel/centos repos are managed) | 17:32 |
| pabelanger | jeblair, Ya, distant future. Since 6.7 isn't even released | 17:32 |
| pabelanger | clarkb, never do I, to be honest | 17:33 |
| pabelanger | but, asking around, it likely will be some time before consumed upstream | 17:33 |
| clarkb | something something maybe its time to figure out shallow clones with newer git and drop centos6? | 17:33 |
| clarkb | oh hrm we need them for juno python26 testing | 17:34 |
| pabelanger | ya, was also going to move into centos7 puppet jobs next week too | 17:34 |
| jeblair | clarkb: if we want to go down that path, we could look into what's doing https cloning | 18:00 |
| jeblair | since i thought most of our ggp and zuul-cloner stuff used git:// for this reason | 18:01 |
| jeblair | pabelanger: ^ | 18:01 |
| jeblair | having said that, i'm also okay with https://review.openstack.org/199794 | 18:01 |
| clarkb | jeblair: good point, we are affected because we use https in the puppet manifests | 18:01 |
| clarkb | ya using ianw's patched git is likely simplest and if that is the way to consume it then ok | 18:02 |
| pabelanger | jeblair, we'd need to do some .gitconfig major to map https:// into git:// since some external puppet modules are hosted on github over https | 18:02 |
| jeblair | pabelanger: but for some reason, github https isn't as slow, right? | 18:02 |
| pabelanger | jeblair, it was hit or miss. Had same issue with performance, less random | 18:03 |
| jeblair | pabelanger, clarkb: switching to -infra | 18:05 |
| asselin | nibalizer, can you review https://review.openstack.org/#/c/199784/ | 18:06 |
| asselin | nibalizer, and the related system-config change is also ready: https://review.openstack.org/#/c/199780/ | 18:08 |
| * asselin switches back to testing nodepool changes | 18:09 | |
| nibalizer | asselin: sorry, I can't really sprint today :( | 18:15 |
| asselin | :( | 18:16 |
| nibalizer | gotta HP it up | 18:16 |
| asselin | nibalizer, I guess I'll do the 3rd party ci task then | 18:16 |
| nibalizer | ya | 18:18 |
| nibalizer | soz | 18:18 |
| asselin | pabelanger jesusaurus BobBall ctlaugh_ please review https://review.openstack.org/#/c/199784/ https://review.openstack.org/#/c/199780/ | 18:19 |
| yolanda | asselin, well, ideally all commands should be executed with nodepool user | 18:20 |
| yolanda | if you are using dib, and you generate images with the wrong user, you'll be on trouble | 18:20 |
| asselin | yolanda, yes, I agree now, no issues | 18:20 |
| asselin | yolanda, can you also review JJB https://review.openstack.org/#/c/199780/ | 18:32 |
| yolanda | sure | 18:34 |
| pabelanger | jeblair, ha, I was just going to do a patch to remove the -e from apply test | 18:35 |
| pabelanger | thanks | 18:35 |
| asselin | crinkle, what do you suggest as a fix for $jenkins_url = "https://${vhost_name}/",? | 18:37 |
| asselin | perhaps no url? | 18:39 |
| crinkle | asselin: you could leave it unset in the params list and create a $_jenkins_url = "https://${vhost_name}/" in the body of the manifest | 18:39 |
| crinkle | asselin: or change the default to "https://${::fqdn}" if that's a valid choice | 18:40 |
| asselin | actually, I'm wondering why the deault can't just be localhost | 18:41 |
| asselin | http://localhost:8080 | 18:41 |
| crinkle | works for me | 18:41 |
| asselin | zaro, ^^ | 18:42 |
| pabelanger | asahlin, http://localhost:8080 WTM | 18:44 |
| pabelanger | I assume we'll drop a proxy at some point | 18:44 |
| pabelanger | drop-in* | 18:44 |
| asselin | pabelanger, WTM? | 18:45 |
| pabelanger | WFM* | 18:45 |
| pabelanger | my bad | 18:45 |
| pabelanger | work for me | 18:45 |
| asselin | ok | 18:45 |
| pabelanger | need some coffee | 18:45 |
| pabelanger | brb | 18:45 |
| asselin | that's what I thought | 18:45 |
| mmedvede | pabelanger: thanks for reviewing https://review.openstack.org/#/c/200186. Are you saying to make separate patch for each class? I could not parse the first sentence of your comment. | 18:56 |
| pabelanger | mmedvede, ya, fat fingers today. I was trying to say you've changed more then just logstash. To make it easier to review and get merge, I'd do each node in a different commit | 18:57 |
| pabelanger | since the break out changes tend to linger for a bit | 18:58 |
| mmedvede | pabelanger: gotcha. Ok, would do | 18:58 |
| pabelanger | crinkle, mind giving me some help with beaker? | 19:01 |
| pabelanger | trying to understand how this is getting installed | 19:01 |
| pabelanger | http://logs.openstack.org/27/198827/3/check/gate-openstackci-beaker-trusty/5bf0e6d/console.html#_2015-07-09_17_52_31_389 | 19:01 |
| pabelanger | would that be coming from the metadata.json dependencies? | 19:02 |
| *** TravT is now known as TravT_away | 19:03 | |
| mmedvede | pabelanger: that might be coming from modules.env | 19:07 |
| mmedvede | only a guess, I am not sure how that test is setup | 19:09 |
| crinkle | pabelanger: it's coming from here http://git.openstack.org/cgit/openstack-infra/puppet-openstackci/tree/spec/spec_helper_acceptance.rb#n29 | 19:09 |
| pabelanger | crinkle, any idea why we hardcode it? | 19:10 |
| pabelanger | crinkle, working on migrating to puppet-httpd and need that get installed | 19:10 |
| crinkle | pabelanger: because https://review.openstack.org/#/c/184905 isn't merged yet | 19:10 |
| crinkle | pabelanger: we are moving toward not hardcoding it | 19:11 |
| pabelanger | crinkle, perfect. thanks for the info | 19:11 |
| crinkle | np | 19:11 |
| pabelanger | crinkle, will depend-on it for my testing, thanks again | 19:12 |
| *** asselin_ has quit IRC | 19:40 | |
| *** asselin_ has joined #openstack-sprint | 19:40 | |
| *** TravT_away has quit IRC | 19:44 | |
| pabelanger | I'll be here today | 19:48 |
| pabelanger | could somebody help me figure out how to disable colors for beaker? | 19:48 |
| pabelanger | I know passing: | 19:49 |
| pabelanger | beaker --no-color is the way to do it | 19:49 |
| pabelanger | by my ruby skills are pretty weak and unsure where it _should_ go | 19:49 |
| pabelanger | trying to get rid of: http://logs.openstack.org/20/198820/4/check/gate-puppet-openstackci-puppet-beaker-rspec-dsvm-centos7/0030584/console.html#_2015-07-09_17_44_51_033 | 19:49 |
| pabelanger | bundle exec rspec spec/acceptance is the entry point it looks like | 19:50 |
| crinkle | pabelanger: as far as i know you can't disable it | 19:50 |
| crinkle | pabelanger: it's :( | 19:50 |
| crinkle | i think nibalizer looked into it ^ | 19:51 |
| pabelanger | guess an upstream patchset is needed | 19:51 |
| pabelanger | something like BEAKER_color=false | 19:51 |
| crinkle | yeah looks like in beaker itself you can https://github.com/puppetlabs/beaker/blob/master/lib/beaker/options/command_line_parser.rb#L118 but it's not exposed in beaker-rspec | 19:52 |
| pabelanger | right | 19:53 |
| nibalizer | right | 19:54 |
| * crinkle pokes it | 19:55 | |
| *** TravT_away has joined #openstack-sprint | 20:00 | |
| crinkle | pabelanger: nibalizer https://github.com/puppetlabs/beaker-rspec/pull/70 | 20:10 |
| pabelanger | crinkle, nice! | 20:10 |
| zaro | asselin, crinkle : sorry, was afk for a little bit. will fix those changes. | 20:11 |
| zaro | asselin, crinkle : how about default username of 'jenkins' and password of 'password'? | 20:16 |
| asselin | zaro, wfm | 20:16 |
| zaro | that will work for unsecured jenkins. | 20:16 |
| crinkle | zaro: i am generally against setting a default password | 20:16 |
| crinkle | someone is going to use it | 20:16 |
| asselin | or password='secret' | 20:17 |
| asselin | crinkle, zaro, do you think this is something that can be done in a follow-up? | 20:18 |
| crinkle | yes it could be fixed later | 20:19 |
| zaro | i'll go with secret then. | 20:19 |
| jesusaurus | zaro: what use case do you think a default password is good for? | 20:19 |
| asselin | I'm ok either way | 20:19 |
| asselin | jesusaurus, for the unsecured jenkins use case | 20:19 |
| zaro | jesusaurus: then user doesn't have to even provide? | 20:20 |
| crinkle | why would you want jenkins to not be secure? | 20:20 |
| asselin | otherwise use has to provide a password, but there isn't any, so it's awkward | 20:20 |
| *** TravT_away is now known as TravT | 20:21 | |
| asselin | crinkle, b/c that's currecntly the default | 20:21 |
| crinkle | why is providing a password hard? | 20:21 |
| asselin | there's no puppet creating the jenkins user with specified name and setting up its password | 20:22 |
| asselin | you have to do that with the UI AFAIK | 20:22 |
| zaro | not hard, just easier if in cases that's unsecured. like maybe for initial ci setup or when testing jenkins | 20:22 |
| zaro | i guess if we make the assumption that all jenkins should be secured then i say require a password. i can see both sides. | 20:23 |
| asselin | fungi, jeblair clarkb how is the jekins "gerrig" user created? | 20:24 |
| zaro | asselin: i believe that's from heira? | 20:25 |
| zaro | ohh wait, you mean how's it's put into jenkins? | 20:26 |
| asselin | zaro, yes | 20:26 |
| clarkb | its from launchpad | 20:26 |
| clarkb | so openid | 20:26 |
| zaro | yup | 20:26 |
| asselin | clarkb, ok and how is jenkins configured to look at launchpad dor openid? | 20:27 |
| clarkb | in the jenkins auth settings you tell it to openid | 20:27 |
| asselin | where are the jenkins auth setting set? | 20:27 |
| clarkb | in the jenkins security settings page | 20:28 |
| asselin | manually or via puppet? | 20:28 |
| clarkb | manually | 20:28 |
| jeblair | asselin: manually -- we have a tarball that we use to do that when we make new masters. there's a lot of stuff that would be really hard to automate with jenkins. | 20:29 |
| asselin | ok, so if it's manual currently, we should assume the default is unsecured jenkins | 20:29 |
| jeblair | i think the best way to approach it with openstackci is one of: a) have some manual steps people do after running puppet the first time; b) see if we can make a tarball of a basic jenkins config that's not insecure and get puppet to deploy that on first run | 20:30 |
| jesusaurus | i would like to see the puppet modules give us a secure deployment by default. security shouldn't be something consumers need to figure out how to set after an initial deployment | 20:30 |
| jeblair | (those are alternatives; i forgot the 'or') | 20:30 |
| crinkle | jesusaurus: ++ | 20:31 |
| jeblair | regardless, i would far rather us spend time on zuulv3 rather than getting the perfect automated jenkins deployment | 20:31 |
| jesusaurus | jeblair: good point | 20:31 |
| asselin | jesusaurus, I agree security should be simpler. I've spent a lot of time trying to figure it out each time. That said, getting something up and running the securing other ways is a good compromise | 20:32 |
| clarkb | also jenkins is insecure period... | 20:32 |
| clarkb | there is no secure jenkins unfortunately | 20:33 |
| jesusaurus | I don't think it's a big deal to make the jenkins user and password required parameters, and I would like us to break the habit of setting them to '' by default | 20:33 |
| clarkb | at least not if you run jobs that run code | 20:33 |
| jesusaurus | there are other modules where we set the user and password to '' as well | 20:34 |
| zaro | ok. new patch is up with default username as 'jenkins' and password as 'secret' | 20:34 |
| clarkb | make password a required param makes sense to me | 20:35 |
| asselin | personally, I'm ok either way | 20:35 |
| asselin | as long as both are required params | 20:35 |
| asselin | or obth defaulted | 20:35 |
| crinkle | i don't think it's important to make them both the same | 20:36 |
| asselin | well I don't have strong opinion on these, so let's just make the password required and go from there. | 20:40 |
| crinkle | +1 | 20:40 |
| zaro | done | 20:41 |
| asselin | zaro, lgtm | 20:42 |
| jesusaurus | is puppet going to complain about a required parameter listed after an option parameter? | 20:43 |
| jesusaurus | s/option/optional/ | 20:43 |
| crinkle | I don't think puppet cares, but the style guide does https://docs.puppetlabs.com/guides/style_guide.html#display-order-of-parameters | 20:44 |
| clarkb | jesusaurus: iirc no because ever parameter is named | 20:44 |
| clarkb | *every | 20:44 |
| jesusaurus | cool | 20:45 |
| zaro | asselin: i remember you said you prefer to not do this? jenkins_url => "https://${vhost_name}/" | 20:46 |
| zaro | in https://review.openstack.org/#/c/199780/4/modules/openstack_project/manifests/jenkins.pp | 20:46 |
| asselin | zaro, correct, you need to pass it in b/c the default is not valid anymore | 20:47 |
| zaro | asselin: i think you wanted 'jenkins_url => $jenkins_url' correct? | 20:47 |
| *** EmilienM has joined #openstack-sprint | 20:47 | |
| asselin | zaro, I copied that from the base | 20:48 |
| zaro | i would set $jenkins_url => "https://${vhost_name}/" at the top but i think crinkle says that doesn't work. | 20:48 |
| asselin | no ned to set it at the top | 20:48 |
| asselin | no need* | 20:48 |
| asselin | $vhost_name is set at the top | 20:49 |
| asselin | you use it below to create the url | 20:49 |
| asselin | so add $jenkins_url => "https://${vhost_name}/" to e.g. line 53 | 20:50 |
| zaro | rogr | 20:50 |
| zaro | done | 20:55 |
| pabelanger | There, just killed a bunch of variable warnings for our puppet modules | 21:01 |
| pabelanger | likely more, but that is first pass | 21:02 |
| crinkle | \o/ | 21:09 |
| pabelanger | crinkle, ServerAlias #{serveraliases}, what would be the proper syntax? | 21:11 |
| crinkle | pabelanger: #{@serveraliases} i think | 21:12 |
| * crinkle tests to make sure | 21:13 | |
| crinkle | pabelanger: yeah "#{@serveraliases}" works | 21:15 |
| pabelanger | crinkle, great thanks | 21:15 |
| asselin | zaro, failed jenkins https://review.openstack.org/#/c/199784/ | 21:19 |
| asselin | crinkle, jesusaurus do you know how to get passed this? http://logs.openstack.org/84/199784/5/check/gate-infra-puppet-apply-precise/80e0733/console.html#_2015-07-09_20_47_09_333 | 21:21 |
| crinkle | seems like maybe the depends-on/needed-by should be reversed? if openstack_project is still trying to use jenkins_jobs_password instead of jenkins_password | 21:25 |
| asselin | crinkle, jesusaurus zaro looks like we need to put in the default password back, then after the system-config change merges, we take it out | 21:26 |
| crinkle | asselin: +1 was just about to suggest that | 21:26 |
| asselin | crinkle, no the dependency needs to stay like that | 21:26 |
| asselin | zaro, ^^ | 21:27 |
| crinkle | okay | 21:27 |
| pabelanger | Ya, you have a circular dependency. I'm running into that issue with puppet-httpd | 21:28 |
| pabelanger | it exposes a edge case in our testing where you need to chance both and depend on each other | 21:28 |
| pabelanger | was going to see if I could update the testing to be better | 21:29 |
| pabelanger | other wise, you need to do a partial commit, allow for some upgrade path, then over merged, remove the upgrade code | 21:29 |
| pabelanger | in your case, you'd need to add a deprecated warning for jenkins_job_password | 21:29 |
| pabelanger | then commit | 21:29 |
| pabelanger | then remove it | 21:30 |
| pabelanger | either way, need some food | 21:30 |
| jesusaurus | asselin: zaro: I think in https://review.openstack.org/#/c/199780/ we need to accept both parameters instead of just renaming jenkins_jobs_password to jenkins_password | 21:30 |
| pabelanger | jesusaurus, yup | 21:30 |
| crinkle | yeah deprecating would be a better approach than just yanking it | 21:30 |
| asselin | can you pass in extra args in puppet? | 21:31 |
| jesusaurus | if it's not listed in the class then you can not pass it in | 21:31 |
| zaro | jesusaurus: hmm, i yanked a few out, i guess i need to put them all back in? | 21:32 |
| jesusaurus | yeah, and I think we'll also need some logic checking which is being passed in | 21:32 |
| asselin | jesusaurus, what about leaving in the default password, then yanking it after the system-config change merges? | 21:32 |
| jesusaurus | asselin: then that default password might get applied to the production server, right? | 21:34 |
| asselin | jesusaurus, no it won't...let me walk through it | 21:35 |
| asselin | currently manage_jenkins_jobs is false https://review.openstack.org/#/c/199784/5/manifests/jenkins_master.pp | 21:35 |
| asselin | so when that merges, the default empty password won't be used | 21:36 |
| asselin | when the system-config change merges, https://review.openstack.org/#/c/199780/5/modules/openstack_project/manifests/jenkins.pp | 21:36 |
| asselin | it sets manage_jenkins_jobs to true and passes in its password | 21:37 |
| jesusaurus | ok, yeah | 21:43 |
| asselin | zaro ^^ | 21:45 |
| zaro | asselin: i'm not getting what supposed to happen. add 'jenkins_jobs_password' and 'jenkins_jobs_username' to param list? | 22:05 |
| asselin | zaro, I think simply revrt back one patch | 22:05 |
| asselin | the current latest patch will be a separate change | 22:06 |
| jesusaurus | zaro: we need to have a default value for the password because adding a new required parameter breaks the api | 22:06 |
| asselin | zaro, yes, patch set 4 | 22:08 |
| asselin | https://review.openstack.org/#/c/199784/4/manifests/jenkins_master.pp | 22:08 |
| zaro | ohh i see jesusaurus was referring to a differnt change so that confused me. | 22:09 |
| asselin | patch set 5 needs to be a new change with depends-on https://review.openstack.org/#/c/199780/ | 22:09 |
| zaro | that seems like a circular dependency, no? | 22:17 |
| asselin | zaro, not if you submit it as a new change | 22:18 |
| asselin | then it's the last of the chain | 22:18 |
| asselin | jesusaurus, crinkle https://review.openstack.org/#/c/200289 | 22:28 |
| asselin | zaro, https://review.openstack.org/#/c/199784 needs attention | 22:47 |
| jesusaurus | asselin: zaro: the stack looks good to me now | 22:56 |
| asselin | cool | 22:56 |
| asselin | me2 | 22:56 |
| asselin | we should get these pushed through today | 22:57 |
| asselin | I need to step away for a bit | 22:57 |
| zaro | excellente! sorry i wasn't setup to test my changes :( will be more prepared next time. | 22:58 |
| jesusaurus | zaro: no worries | 22:58 |
| *** TravT has quit IRC | 23:05 | |
| *** TravT has joined #openstack-sprint | 23:08 | |
| nibalizer | pabelanger: we've generally been doing the remove, land, change, remove process instead of trying to build bidirectional depends | 23:09 |
| nibalizer | especially since the system doesn't really allow you to land two things at once as I understand it | 23:09 |
| jesusaurus | correct, there's no way to land two co-dependent changes at the same time, one has to land first | 23:13 |
| clarkb | (and thats intentional) | 23:14 |
| *** openstack has joined #openstack-sprint | 23:40 | |
| asselin | nibalizer, what do you think about this stack? https://review.openstack.org/#/c/199784/ | 23:56 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!