Monday, 2016-11-21

*** greghaynes has quit IRC00:33
*** greghaynes has joined #zuul00:42
*** harlowja has quit IRC02:57
*** pleia2 has quit IRC02:58
*** dmsimard has quit IRC02:58
*** pleia2 has joined #zuul03:00
*** dmsimard has joined #zuul03:00
*** Zara has quit IRC03:13
*** Zara has joined #zuul03:13
*** rattboi has quit IRC03:14
*** rattboi has joined #zuul03:17
*** tflink has quit IRC03:26
*** elli has joined #zuul03:59
*** elli has quit IRC04:19
*** isla has joined #zuul04:27
*** isla has quit IRC04:30
*** tflink has joined #zuul04:35
*** tflink has quit IRC04:37
*** tflink has joined #zuul04:39
*** abregman has joined #zuul06:29
openstackgerritJoshua Hesketh proposed openstack-infra/zuul: Merge branch 'master' into workingv3  https://review.openstack.org/38947006:37
openstackgerritMerged openstack-infra/zuul: Refactor skip-if tests to use irrelevant-files  https://review.openstack.org/39941507:24
*** saneax-_-|AFK is now known as saneax08:07
*** abregman has quit IRC08:29
*** abregman has joined #zuul08:35
*** abregman has quit IRC08:39
*** abregman has joined #zuul08:43
*** hashar has joined #zuul08:48
*** olaph has quit IRC08:50
*** olaph has joined #zuul08:50
openstackgerritTuan Luong-Anh proposed openstack-infra/zuul: Replace assertEquals with assertEqual  https://review.openstack.org/40011908:53
*** hashar has quit IRC08:55
*** hashar has joined #zuul08:56
*** hashar has quit IRC09:20
*** willthames has quit IRC09:29
*** yolanda has joined #zuul10:32
*** pabelanger has quit IRC12:15
*** pabelanger has joined #zuul12:15
*** hashar has joined #zuul12:17
Shrewspabelanger: hrm, odd13:40
Shrewspabelanger: i'm still at my first cup of coffee, but not sure that rotation test is valid. You're setting state time but not state so I'm not real sure of your intent there13:45
Shrewspabelanger: also, those tests are making assumptions about build IDs that I don't believe is safe13:47
Shrewspabelanger: ZK decides the sequence numbers, and I've seen them start at 0 and I've seen them start at 1. I don't know how it chooses, but we can't know for sure which one it is.13:48
Shrewspabelanger: is the addition of waitForBuild() already merged?13:48
Shrewsah, no. in the previous review13:48
Shrewswill comment there13:48
Shrewsoh, i see what you're trying to do in that test now. caffeine is kicking in13:55
Shrewspabelanger: so, I've tested the rotation myself so logic should be sound (unless there is a race condition I didn't encounter). let's eliminate the build ID issue I mentioned in the review first.13:57
Shrewsto make sure that's not tripping the test up13:58
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool: Print all fields of ZK objects  https://review.openstack.org/40027014:23
Shrewspabelanger: oh, the cleanup code has changed somewhere along the lines. I don't think it's sorting properly14:39
Shrewsi think this broke it: https://review.openstack.org/39930714:42
Shrewsjeblair: Why did you remove the call to getMostRecentBuilds() on line 330 here? https://review.openstack.org/#/c/399307/2/nodepool/builder.py14:44
Shrewsjeblair: i think we need to add that back. the builds need to be sorted by state_time. that code isn't doing that, but the get* call will14:44
Shrewsbut i want to make sure i'm not missing something before i add it back in14:45
Shrewsah, the "consistent view" thing14:48
*** abregman is now known as abregman|mtg14:49
pabelangerShrews: the goal for the rotation test is to confirm we have 2 builds for an image at any given time. Unless that hasn't changes, that is how many we had for nodepoolv2. using the state_time was my way of expiring an build to force a rebuilt.14:51
pabelangeragreed about assumptions about build name / id, will have to change that14:51
pabelangerShrews: Ya, I think sort is the issue too. Was just poking at it a little while waiting for the train14:52
Shrewspabelanger: also, storeBuild() should really not be used w/o imageBuildLock() ... though it's somewhat safe to set 'delete' state14:53
pabelangerk14:54
pabelangerI can clean that up too14:54
openstackgerritTuan Luong-Anh proposed openstack-infra/zuul: Replace assertEquals with assertEqual  https://review.openstack.org/40011914:58
openstackgerritPaul Belanger proposed openstack-infra/nodepool: Add test to validate rebuilds are scheduled  https://review.openstack.org/39997415:01
pabelangerShrews: okay, update to use imageBuildLock^ Still working on build-id issue15:01
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool: Sort existing builds properly in cleanup thread  https://review.openstack.org/40029215:10
Shrewspabelanger: jeblair: ^^^ I think that corrects the sorting issue15:10
Shrewsbut your test still doesn't work for some reason15:11
*** saneax is now known as saneax-_-|AFK15:12
*** dmsimard is now known as dmsimard|away15:24
Shrewspabelanger: oh! i know why it still doesn't work15:25
Shrewspabelanger: so, in addition to keeping the 2 most recent builds, we ALSO have a check to make sure that we keep the two most recent UPLOADS. So both 1 and 2 have uploads, so 1 will never get deleted15:25
* Shrews forgot about that15:26
Shrewspabelanger: that's what that upload recency table is all about15:26
*** markmcd has joined #zuul15:28
Shrewsoh, but 3 should get uploaded. hrm15:30
Shrewsgrr15:30
pabelangerShrews: when I last looked 3 does get uploaded, but then deleted right away15:46
openstackgerritPaul Belanger proposed openstack-infra/nodepool: Enable DIB_CHECKSUM in devstack tests  https://review.openstack.org/40030915:49
Shrewsfor some reason the cleanup code is only ever seeing the upload for 115:50
phschwartzjeblair: when you have some time today I want to talk more about the pipeline dependency tree. I have tried a few different ways of doing it and none feel 100% right15:56
Shrewsphschwartz: lol. indentation bug... patch incoming16:06
Shrewserr, pabelanger ^^^16:06
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool: Fix upload recency table bug  https://review.openstack.org/40032016:08
Shrewspabelanger: ^^^16:08
Shrewspabelanger: rebase your review on top of that stack, it should pass now16:10
pabelangerack16:11
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool: Add test_image_delete test  https://review.openstack.org/39975716:17
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool: Remove image-update based tests  https://review.openstack.org/39976416:17
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool: Re-enable test: test_image_delete_invalid  https://review.openstack.org/39973916:17
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool: Remove image-upload command and tests  https://review.openstack.org/39976616:17
markmcdi noticed in http://inaugust.com/talks/zuulv3.html#/4 a mention of a design for container support, has someone got a link to this design ? thks16:26
pabelangerI don't think that spec is written yet16:28
markmcdpabelanger: thanks, i was thinking that was the case, just wanted to confirm it16:30
pabelangermarkmcd: keep an eye out for a nodepool spec, that's where we'd be adding support for it16:30
pabelangerWe talked about it recently at ansiblefest last month, but future work right now16:31
markmcdpabelanger: perfect, its being developed as part of the zuulv3 effort though ?16:32
pabelangermarkmcd: I believe it is on the table, but mordred knows more about the details.  It is something we want for sure16:33
clarkbfwiw using containers today should just work if using nova + container flavors, or plugging something like jenkins mesos into jenkins. The gotcha there is that doesnt get you containers running speculative git merge based images whoch I think may be the thing mordred is interested in16:37
markmcdpabelanger: ack16:39
pabelangerclarkb: speaking of containers, I'm going to try running disk-image-create within lxc later today.  See how that works, that might be a dirty way to support more then 1 builder on a host16:40
markmcdclarkb: im interested in using speculative merge and all the good stuff zuul offers with containers :)16:40
pabelangerotherwise, I found an old blueprint for DIB to support parallel builds16:41
clarkbpabelanger: you have to open a bunch of perms like mounting iirc16:41
jeblairShrews: yeah, i sorted, but forgot to sort by time, sorry16:42
pabelangerclarkb: Ya, that's what I've been reading16:42
jeblairphschwartz: yes, let's try to catch up on the dependency thing later today16:42
jeblairpabelanger: why do we want more than one builder on a host?16:42
pabelangerjeblair: clarkb was noting we might get better testing times for devstack nodepool job, if we did parallel image builds. Was looking at adding more labels to the devstack job to help catch DIB regressions16:44
jeblairpabelanger: i thought we only made one image in there?16:45
clarkbspecufically I didnt want adding redundant builds to the integration job because they serialize16:45
clarkbjeblair: pabelanger is trying to add another16:45
pabelangerjeblair: yes, trusty, but wanted to add xenial too16:45
jeblairah, that makes more sense if i read pabelanger's sentences in reverse order.  :)16:45
pabelangeror all flavors for diskimage-builder devstack job.16:46
pabelangerhttps://review.openstack.org/#/c/399865/16:46
jeblairSpamapS: over the weekend, i was thinking there's a subtlety we may have overlooked which might make for a simple solution:  we really only intend the job stanzas in the project-pipelines to *modify* existing job definitions.  therefore, if we say that a change has to match one of the standalone job definitions as well as the one in the project-pipeline, then we may have the behavior we want.16:47
jeblairpabelanger, clarkb: re https://review.openstack.org/399642  did we also decide to remove 'min-ready: -1' support?16:59
*** abregman|mtg has quit IRC16:59
clarkbjeblair: yes I am fine with that16:59
pabelangersame16:59
pabelangerI can update the testing today16:59
jeblairokay will -1 that for now then16:59
SpamapSjeblair: That sounds like a good constraint.17:13
mordredclarkb, pabelanger: yes - the 'hard' problem I'm interested in solving is supporting folks using containers like app containers in a kubernetes environment where the containers are single process. using containers like VMs where you can ssh in to them is, I agree, easy17:25
mordredand I think that plugging a mesos provider, for instance, into nodepool in such a way that it returned a container that ansible could ssh in to should be 'easy' and should 'just work'17:25
mordredbut to be able to support testing folks' multi-container orchestrated applications in a way that feels robust is, I think, a thing that we're probably going to have to play with for a while17:27
*** hashar has quit IRC17:43
openstackgerritPaul Belanger proposed openstack-infra/nodepool: Add test for provider uploads enable / disable  https://review.openstack.org/39968117:59
openstackgerritPaul Belanger proposed openstack-infra/nodepool: Re-enable TestWebApp tests  https://review.openstack.org/39971617:59
openstackgerritPaul Belanger proposed openstack-infra/nodepool: Remove waitForBuiltImages() / JobTracker() from nodepool.py  https://review.openstack.org/39972717:59
pabelangerjust a rebase18:00
pabelangerwhile I work on 39964218:00
*** harlowja has joined #zuul18:14
openstackgerritPaul Belanger proposed openstack-infra/nodepool: Add tests for enable / disabled labels  https://review.openstack.org/39964218:19
pabelangerjeblair: clarkb: let me know what you think^, setting providers: [] for a label, means it will be disabled18:20
jeblairpabelanger: how's that different than 0?18:25
jeblairmin-ready=018:25
pabelangerwe upload images still with min-ready 018:26
pabelangerIIRC: we set min-ready: 0, to prime the cloud with images18:27
pabelangerthen toggle it when we want to enable the cloud18:27
jeblairright18:27
jeblairpabelanger: if you don't want an image in a provider, don't put an image stanza for that image on that provider18:28
jeblairif you don't want to create nodes in a provider with a label that uses that image, don't add that provider to the list of providers in the labels18:28
pabelangersure, we can do that18:28
jeblairpabelanger: i think we already do that, that's what my changes last week do18:29
pabelangerjeblair: okay, do you mind commenting on the review with your objections. No issue for me to rework it18:30
jeblairpabelanger: let me write up an etherpad and make sure we're on the same page18:31
pabelangerWFM18:31
jeblairpabelanger: https://etherpad.openstack.org/p/um87VzudKq18:37
jeblairpabelanger: i think that lays out a coherent approach to this18:38
jeblairpabelanger: however, that first section, about diskimages, isn't actually how it behaves now -- i think we need a change to implement that.  i think the second and third sections are the current behavior.18:38
jeblairpabelanger: (but as i was writing the other sections, i thought that changing that first section to match makes sense)18:39
jeblairclarkb: ^ ?18:39
pabelangerreadying now, trying to process18:41
jeblairi will continue to examplify18:42
pabelangerjeblair: okay, I see the pattern now.  Ya, that appears to be much simpler then today18:45
jeblairi think what it doesn't do is allow us to stop updating an image, but still keep it around.18:45
pabelangerya18:46
jeblairi think maybe whatever need we have for that could be solved by the 'pause' think we mentioned last week18:47
pabelangerindeed, we could have used that over the weekend, to stop uploading broken ubuntu-xenial images18:47
pabelangerbut not make a massive nodepool.yaml update18:48
jeblairfor that, maybe we can add an attribute to both the diskimage, as well as the provider-image, that says "don't update this diskimage" and "don't upload new versions of this image to this provider" respectively18:49
jeblairi don't think the second would be used much.  i think our main need is for the first.18:49
jeblairpabelanger: there, updated etherpad to mention pause18:51
jeblairclarkb, Shrews: does https://etherpad.openstack.org/p/um87VzudKq look good?18:52
*** jamielennox is now known as jamielennox|away18:52
pabelangerjeblair: yes, I think that works. Thanks18:53
Shrewslooking18:56
Shrews'pause' attribute?18:57
jeblairShrews: it's a new thing -- we realized friday as we were cranking out broken xenial builds, we need a way to tell nodepool to chill out for a while18:57
jeblairShrews: right now, we can just delete an image and we have a day to fix things.  but new nodepool builder would immediately replace that.18:58
Shrewsooh, then let's rename the attribute 'chill'18:58
pabelangerhaha18:58
Shrewsi guess the etherpad makes sense then18:59
*** jamielennox|away is now known as jamielennox19:00
pabelangerjeblair: that means, min-ready for a label would be 1 or greater, right?19:04
jeblairpabelanger: i think min-ready=0 still means "don't use this label"19:05
jeblairgrr19:06
jeblairpabelanger: yes, that's right -- i just realized i wrote max-servers in the etherpad though19:06
jeblair(etherpad corrected)19:06
pabelangerk, we can update voluptuous for min-ready to ensure it is 1 based19:08
jeblairpabelanger: no min-ready=0 is still a thing19:08
pabelangerah19:09
jeblairjust not min-ready=-119:09
pabelangerDoh, right19:09
Shrewsgeez, my commit msg on 400292 is just all sorts of messed up. i wonder if my brain is broken19:19
mordredShrews: I've wondered that for a long time19:19
Shrewsmordred: now we have documented proof19:20
openstackgerritPaul Belanger proposed openstack-infra/nodepool: Add test to validate when a node build is disabled  https://review.openstack.org/39964219:25
pabelangerjeblair: okay, I believe that is the test to validate min-ready: 0, make sure a node is not launched19:25
*** rcarrillocruz has joined #zuul20:21
pabelangerjeblair: clarkb: do you mind helping land https://review.openstack.org/#/c/399770/ to fix a race21:13
morgan_o/21:17
morgan_sorry. will be missing g the IRC meeting today21:17
morgan_going to be on a plane.21:17
morgan_I'll look for the notes.21:17
morgan_cheers and have a safe holidays (if you're in the us )21:18
clarkbme too, taking advantage of post dentist babysitting for afternonn without kids21:18
rcarrillocruzhah21:18
pabelangerabout to find the hotel bar, and prep for the meeting :)21:20
jeblairpabelanger: +321:24
jheskethMorning21:24
jeblairpabelanger: 642 looks good in spirit -- some minor nits inline, one of which should make it pass.21:28
jeblairphschwartz: maybe we can talk about the job dependency stuff after the meeting?21:28
rcarrillocruzpabelanger: mind looking at my devstack-gate ansible roles changes when you get a chance? topic zuulv3 as usual21:29
phschwartzJeblair: I will message you if I am able to after the meeting. If not I will ping you tomorrow morning.21:30
jeblairphschwartz: sounds good.21:30
phschwartzYeah. The 4 month old is a little cranky and the wife is very sick so it is interesting.21:31
jeblairphschwartz: understood.  take care.21:31
openstackgerritMerged openstack-infra/nodepool: Better protect for race condition in waitForBuildDeletion()  https://review.openstack.org/39977021:34
openstackgerritMerged openstack-infra/nodepool: Re-enable test: test_image_delete_invalid  https://review.openstack.org/39973921:38
jeblairpabelanger, Shrews: i'll start working on the change needed to make the diskimage delete happen as described in https://etherpad.openstack.org/p/um87VzudKq21:38
openstackgerritMerged openstack-infra/nodepool: Add test_image_delete test  https://review.openstack.org/39975721:39
openstackgerritMerged openstack-infra/nodepool: Remove image-update based tests  https://review.openstack.org/39976421:39
openstackgerritMerged openstack-infra/nodepool: Remove image-upload command and tests  https://review.openstack.org/39976621:39
Shrewsjeblair: ack21:41
openstackgerritJames E. Blair proposed openstack-infra/nodepool: Delete builds when diskimage removed from config  https://review.openstack.org/40042121:58
jeblairit's about time for our meeting in #openstack-meeting-alt21:59
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool: Re-enable tests: test_alien_*  https://review.openstack.org/40042222:01
*** willthames has joined #zuul22:13
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool: Re-enable test: test_alien_list_fail  https://review.openstack.org/40042222:30
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool: Re-enable test: test_alien_list_fail  https://review.openstack.org/40042222:32
Shrewsjeblair: hrm, think maybe we should leave the alien* tests disabled until the next nodepool phase?22:56
Shrewsthat one above will pass, but will break again once we start storing node info in ZK22:57
jeblairShrews: let's re-enable it, then re-disable it -- mostly because we will probably want to maintain the master branch, and having all tests running will help.22:57
Shrewsok. need to fix up the alien-image-list command. it's still pulling from the db, so it will pass, but it's a false positive22:59
jeblair++22:59
openstackgerritMerged openstack-infra/nodepool: Print all fields of ZK objects  https://review.openstack.org/40027023:13
openstackgerritMerged openstack-infra/nodepool: Sort existing builds properly in cleanup thread  https://review.openstack.org/40029223:14
openstackgerritMerged openstack-infra/nodepool: Fix upload recency table bug  https://review.openstack.org/40032023:14
*** openstack has joined #zuul23:46

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!