Wednesday, 2018-06-06

openstackgerritMohammed Naser proposed openstack-infra/zuul master: Hide queue headers for empty queues when filtering  https://review.openstack.org/57258800:02
mnasermordred: ^ this one is for you00:03
mnaserthis is what the result looks like https://usercontent.irccloud-cdn.com/file/9vii67C7/Screen%20Shot%202018-06-05%20at%208.03.28%20PM.png00:04
mnaseryay for uncluttered status pages?00:04
mnaser(to test locally, `yarn install` and `npm start` in your terminal)00:05
openstackgerritClark Boylan proposed openstack-infra/zuul master: Fix race in slow start test  https://review.openstack.org/57258400:16
pabelangermnaser: sweat! now do a button / toggle to switch from https:// to finger:// please for console logs00:20
clarkbI don't see the difference00:21
clarkbbut I've been fixing races in the tests so my eyes are probably not working proeprly00:21
pabelangermnaser: oh, another one is getting estimate time back when you hover over status bar, not sure where we lost that00:24
pabelangerbut is a nice thing to have00:24
*** weshay_ has joined #zuul00:28
*** mhu` has joined #zuul00:28
*** pabelanger_ has joined #zuul00:28
*** _ari__ has joined #zuul00:29
*** mhu has quit IRC00:30
*** myoung|off has quit IRC00:31
*** pabelanger has quit IRC00:31
*** _ari_ has quit IRC00:32
*** weshay has quit IRC00:32
*** myoung has joined #zuul00:33
mnaserclarkb: here is nova filtered on what is deployed live https://usercontent.irccloud-cdn.com/file/bZx5bxNJ/Screen%20Shot%202018-06-05%20at%208.34.41%20PM.png00:35
mnasernote the lack of useless Queue: ..00:35
mnaserpabelanger_: those are easy quick ones to do00:35
* mnaser will hack on this a bit later today00:35
clarkboh gotcha00:37
*** acozine1 has joined #zuul00:41
*** mhu has joined #zuul00:42
*** pabelanger has joined #zuul00:42
*** mhu` has quit IRC00:42
*** _ari_ has joined #zuul00:43
*** pabelanger_ has quit IRC00:43
*** myoung_ has joined #zuul00:43
*** weshay has joined #zuul00:43
*** weshay_ has quit IRC00:44
openstackgerritClark Boylan proposed openstack-infra/zuul master: Fix race in slow start test  https://review.openstack.org/57258400:44
*** _ari__ has quit IRC00:44
*** myoung has quit IRC00:44
clarkbI fixed one race but I Think that just exposed another ... I'll have to pick that up tomorrow probably00:46
*** harlowja has quit IRC01:10
*** xinliang has quit IRC01:12
*** threestrands_ has joined #zuul01:33
*** threestrands has quit IRC01:36
*** nguyenhai93 has quit IRC01:47
*** nguyenhai93 has joined #zuul01:48
*** yolanda_ has joined #zuul01:56
*** yolanda_ has quit IRC01:57
*** yolanda_ has joined #zuul01:58
*** yolanda has quit IRC01:59
openstackgerritClark Boylan proposed openstack-infra/zuul master: Fix race in slow start test  https://review.openstack.org/57258403:00
clarkbthat deserves careful review but I believe I may have reached a non racy test_slow_start state03:01
clarkbcommit message tries to go into much detail about what I have done03:01
*** josefwells has joined #zuul03:39
josefwellshey, I was on earlier with some issues with github setup03:41
josefwellsI just pushed my docker setup for zookeeper+nodepool+zuul03:41
josefwellshttps://github.com/josefwells/zuul-docker03:42
clarkbjosefwells: let me take a quick look03:42
josefwellsI'm pretty new at docker, but I think it is pretty good03:42
josefwellshey clarkb, thanks!03:42
josefwellsI had been following the zuul from scratch and github instructions here: https://zuul-ci.org/docs/zuul/admin/github_setup.html03:43
josefwellsso some of the repository assumes some of that has been done03:43
josefwellssetting up webhook, getting zuul.pem03:44
clarkbjosefwells: have the error message you get in the logs?03:44
josefwellsfunny enough it tends to not be reproducible, but it complains about the zuul-test-config yaml file, I see it when I start zuul-scheduler03:46
clarkbjosefwells: reading that config you need to have a zuul-git connection03:46
clarkbthe zuul from scratch document explains this (we should maybe make that more explciit in the github config doc since its used there)03:47
josefwells extra keys not allowed @ data['trigger']['github']03:47
josefwellsthat is one03:47
josefwellsoh, you MUST have zuul-git?03:47
josefwellsmy stuff is behind some painful firewalls03:47
clarkbyou don't have to have it but you configured it in main.yaml but there is no corresponding connection in zuul.conf03:48
josefwellsoh, I did eventually remove that03:48
clarkbas for the github key I think the issue there is you have named the github connection "my_github" not "github"03:48
clarkbso they don't match03:48
josefwellssorry, I did some sanitzation to remove some internal info03:48
clarkbthat is fine, you just need to make sure that the connection name for github in zuul.conf is what you use in your pipeline trigger config03:49
clarkbgithub isn't a generic there, it is referring to a specific named connection03:49
josefwellsoh, the trigger itself is referring to the specific connection? ok03:51
clarkbyup03:51
clarkbits saying trigger when connection foo has these events. And you need to match that to a connection confi03:51
*** bhavik1 has joined #zuul03:51
clarkbthis allows you to talk to github.com and github enterprise install locally if necesary03:51
clarkbor multiple github installs locally, etc03:53
josefwellsmakes perfect sense, just didn't realize03:54
josefwellslooks like zuul-scheduler likes it now!03:54
josefwellsprogress!03:54
josefwellsthink any of the docker-goo would be helpful to you guys?03:55
clarkbjosefwells: I know mordred has plans to try and automate some of the image building on a simple base then bind mount in configs or similar iirc03:55
clarkbbut I'm not full up to date on all those details. pbrx is the name of the tool being built iirc03:56
josefwellshmm, still no web interface visible03:57
josefwellsshould just be able to load it over port 9000 right?03:58
clarkbjosefwells: if the zuul-web process is running yes I would expect so03:58
clarkbI think it may listen on locahost only by default though03:58
josefwells2018-06-05 18:04:50,353 INFO zuul.WebServer: Zuul Web Server starting03:58
clarkbpossible that it isn't listening on an ip that you can hit?03:58
josefwellsit is listening on 0.0.0.0, should be all interfaces03:59
clarkbdo you get anything if you curl the root?03:59
josefwellsERR_EMPTY_RESPONSE03:59
clarkbwhat about /api/info04:00
josefwells{"info": {"capabilities": {"job_history": false}, "stats": {"url": null, "prefix": null, "type": "graphite"}, "websocket_url": null}}04:01
clarkbok so it is there and working04:01
josefwellsyeah, I never set up statsd04:01
josefwellsnot sure if it is required04:01
clarkbno it shouldn't be04:01
josefwellsI haven't pushed anything to zuul-test, just zuul-test-config04:01
josefwellsalso the zuul.conf in zuul-test-config only has pipeline, no jobs or other stuff, again, just following the simple setup stuff I found04:02
clarkblooks like the status page lives at status.html not index.html or similar04:03
clarkbtry getting /status.html and see if that igves you a status page04:03
clarkb(we rewrite / to status.html in our webserver)04:03
clarkbcorvus: ^ we might want to add a route for / to status.html by default in cherrypy04:03
josefwellsplain text: 404: Not Found04:04
clarkbhrm /tenants.html is the other root path I would expect towork, is that one any better?04:05
clarkbthe pretending to not be multitenant while being multitenant has made some of this confusing04:06
josefwellstried that, also tried puting my tenant name in the url localhost:9000/github-test/status.html04:07
clarkbok if that isn't working my best guess is that the static path isn't right or we aren't installing the build artifacts for the client side of zuul-web04:08
clarkbanything in the zuul-web logs related to trying to fetch the html files?04:08
josefwellsno, just that zuul-web started04:08
josefwellsgonna kill/restart it04:08
josefwellssame deal, only stopping/started added to log04:09
josefwellsodd that the base url returns empty, while any specific page gives me a 404 page04:10
clarkbyou aren't setting a static_path under [web] in zuul.conf so it should look at $zuul_install_path/zuul/web/static/ for those files04:10
*** rlandy|rover|bbl is now known as rlandy|rover04:10
clarkbjosefwells: I think that is due to how the routes are setup in zuul-web a specific page will end up matching the route for file lookups but the root doesn't have one04:10
josefwellsperhaps my pip -u installs are breaking the lookup04:11
josefwellsI don't see anything in ~/.local/lib/python3.5/site-packages/zuul/web just handler.py _init_.py and _pycache_04:12
clarkbhttps://zuul-ci.org/docs/zuul/developer/javascript.html that may be helpful in figuring out how to bootstrap those static files04:13
josefwellsodd that api/info works04:14
clarkbjosefwells: the api data is coming out of zuul itself and none of it relies on prerendering static assets04:15
josefwellsah, ok04:15
josefwellsstill, I was stuck on that github trigger business for a while today, thanks so much for your help04:15
josefwellsI'll dig into this web stuff tomorrow04:16
josefwellsI gotta roll, when I get it going I'll feedback some info for you04:17
*** harlowja has joined #zuul04:17
clarkblooks like we've got apache set up to direclty serve the static assets on our install so I'm less help04:19
tobiashclarkb: looks like we're missing the yarn stuff from zfs docs04:20
clarkbtobiash: I want to say that the wheels are supposed to have it installed so maybe a problem with that04:20
tobiashclarkb: the wheels are missing the static stuff yet04:21
clarkbah ok, then ya we need to have that in the install doc likely04:21
tobiashclarkb: we may want to fix the wheels and switch zfs to not install from source04:22
tobiashAnd maybe a fatal installation error if yarn is missing during installation04:23
tobiashmordred: thoughts?04:24
*** bhavik1 has quit IRC04:24
tobiashNot sure if that's possible without breaking tox04:24
tobiashAt least we may want to check for the static stuff during zuul-web startup04:25
clarkbtobiash: well you may offload serving of those files like openstack does. It is a tricky problem04:26
clarkbmaybe a warning04:26
*** josefwells has quit IRC04:26
tobiashhrm, I don't like warnings if I don't care in one case and want an error in the other case04:29
tobiashmaybe a fatal check that can be disabled in config04:29
*** harlowja has quit IRC04:45
clarkbtobiash: I put up a change ti fix test_slow_start if you are wanting to review things and updated the log stteaming change04:49
clarkbee test for ^04:49
tobiashclarkb: I already saw it. I'll look at it later @work04:50
openstackgerritMerged openstack-infra/zuul-jobs master: Add a job variable to control javascript link copying  https://review.openstack.org/57254205:23
*** sshnaidm|brq has joined #zuul05:42
*** sshnaidm|brq has quit IRC05:47
*** pcaruana has joined #zuul06:44
openstackgerritMerged openstack-infra/zuul-jobs master: Fix deprecation warning of ansible-2.5  https://review.openstack.org/57199606:47
*** openstackgerrit has quit IRC07:19
*** sshnaidm|brq has joined #zuul07:21
*** hashar has joined #zuul07:41
*** jpena|off is now known as jpena07:52
*** threestrands_ has quit IRC08:03
*** gtema has joined #zuul08:17
*** electrofelix has joined #zuul08:49
*** hashar is now known as hasharAway09:50
*** openstackgerrit has joined #zuul09:56
openstackgerritFabien Boucher proposed openstack-infra/zuul master: Add a loading_errors info to {tenant}/info endpoint  https://review.openstack.org/55387309:56
openstackgerritMarkos Chandras (hwoarang) proposed openstack-infra/nodepool master: Add opensuse-150 to nodepool dsvm  https://review.openstack.org/57241610:13
*** yolanda_ has quit IRC11:00
*** yolanda_ has joined #zuul11:00
andreaftobiash ping11:03
tobiashandreaf: hi11:05
andreafI'm interested in setting up zuul (for demo purposes) on k8s - clarkb mentioned yesterday that you do that today11:06
*** jpena is now known as jpena|lunch11:06
andreafso I was wondering if you have docker images and/or helm charts that you could share11:06
tobiashI'm running zuul on openshift (without helm)11:08
gtemathat's interesting. Was going to start thinking about that next week.11:09
gtemaare you using buildah or s2i for building zuul images?11:10
tobiashpabelanger also asked me about my dockerfiles a few days ago: http://eavesdrop.openstack.org/irclogs/%23zuul/%23zuul.2018-05-31.log.html#t2018-05-31T17:50:3211:10
tobiashgtema: I'm using traditional dockerfiles11:10
openstackgerritFabien Boucher proposed openstack-infra/zuul master: Add a loading_errors info to {tenant}/info endpoint  https://review.openstack.org/55387311:11
tobiashI currently have a zuul-base image and some layers on top which are special to my deployment (e.g. generation of main.yaml from a repo)11:11
gtematobiash: I see. I want however to try buildah or s2i - seems promising11:11
tobiashgtema: there is also interest in providing official zuul docker images11:12
andreafgtema: this may be interesting as well https://github.com/GoogleContainerTools/kaniko11:12
tobiashgtema: there is a zuul container spec11:12
gtematobiash: are you using templates to describe whole installation?11:13
andreaftobiash: yeah I would be interested in building a chart as well11:13
andreaftobiash: if we could get a zuul chart into the incubator repo it would be great11:13
tobiashgtema: yes, we use templates (and a small script that assembles the template from a folder structure)11:13
tobiashthe container spec: https://review.openstack.org/56013611:14
andreaftobiash thanks for sharing the zuul docker image - do you have something similar for nodepool?11:14
tobiashyes11:14
tobiashjust a sec11:14
gtemaandreaf: agree. Chart, OpenShift template, and probably OpenShift ADB seems nice possibilities to get this done11:14
tobiashandreaf: nodepool dockerfile: http://paste.openstack.org/show/722790/11:16
gtemawould be probably nice to integrate it into windmill or create a separate project for that11:16
tobiashandreaf: but beware that needs some updates and currently needs to be in place in contrast to the zuul dockerfile11:16
tobiashmy nodepool dockerfile got less love in the last few months ;)11:18
andreaftobiash thanks, I'll keep that in mind11:18
gtematobiash: is it your "production" installation, or simply sandbox?11:18
andreaftobiash: thanks for sharing again11:18
tobiashproduction11:19
andreaftobiash: do you have manifests as well? something pruned of any prod value that you could share?11:19
gtemagreat. Any problems/notes on running?11:19
tobiashmanifests?11:19
andreafk8s manifests? like deployments, services, ingresses, configmaps etc11:20
andreafI never used openshift, but at the end it's built around k8s right?11:21
tobiashandreaf: openshift adds further stuff like build configs, image streams which we currently use to build the images directly in openshift11:21
gtemaI guess openshift template is a combination of manifests (kind of)11:21
tobiashyes, together with a simple parameter substitution11:22
gtematobiash: would you, please, also share template?11:22
tobiashhave to check first what I need to strip from11:23
gtemasure11:23
andreaftobiash ok thanks11:23
andreaftobiash do you have any problem if I end up putting something built on those in a public repo under apache 2.0 license?11:24
andreafs/problem/concern11:24
tobiashandreaf: I'll strip out all special stuff from us and share it as a zuul review (which never will merge). Once it's there it's apache 2 as zuul... ;)11:40
andreafcool thanks!11:40
gtemathanks a lot Tobias11:42
*** jluk has joined #zuul11:55
jlukhi, any idea why periodic pipeline triggered from zuul CLI reports "Executing jobs for change <Branch 0x7fd deletes refs/heads/master from 000000" while a genuine timer trigger produces "<Branch 0x7fd refs/heads/master updated None..None>"?12:06
*** GonZo2000 has joined #zuul12:07
*** GonZo2000 has quit IRC12:07
*** GonZo2000 has joined #zuul12:07
openstackgerritTobias Henkel proposed openstack-infra/zuul master: DNM: Add OpenShift templates  https://review.openstack.org/57274612:08
tobiashandreaf, gtema: ^12:08
tobiashandreaf, gtema: that12:08
tobiashthat won't run out of the box but should give you a starting point12:09
*** gtema has quit IRC12:09
andreaftobiash: that's really really useful, thank you!12:09
tobiashhad to strip out a lot of our special stuff (e.g. on the fly config generation, master key generation)12:09
andreafsure12:10
*** jpena|lunch is now known as jpena12:10
*** GonZo2000 has quit IRC12:19
tobiashpabelanger: you might be interested in that too ^12:25
*** jluk has quit IRC12:31
*** hasharAway is now known as hashar12:32
*** gtema has joined #zuul12:50
fungitobiash: clarkb: catching up, if the wheels get fixed then a `pip install .` in the source tree should install those static assets because modern pip builds a wheel from the source and installs that behind the scenes13:14
*** myoung_ is now known as myoung13:21
tobiashShrews: our nodepool now starts building qcow2 images and raw images while we only have raw image clouds defined14:06
tobiashit may be caused by the driver refactoring, but have to analyse first14:06
gtematobiash: I think it may be from me: https://review.openstack.org/#/c/566437/14:07
Shrewstobiash: api refactor doesn't touch builder code. check I27f8f45ba62e5ee0cca0ea7cd27893092b726ebd14:07
tobiashgtema, Shrews: thanks for the hint14:08
gtemashouldn't be, as it was set only if nothing else was present. But who knows14:08
Shrewstobiash: i never reviewed that, but if you could identify and fix, it would be appreciated14:08
tobiashShrews: I think I see the error14:08
tobiashI'm currently relying on the auto-detection via clouds.yaml and don't define the formats in the nodepool config14:09
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool master: Create a base Driver class  https://review.openstack.org/57158514:09
*** hashar is now known as hasharAway14:09
tobiashso auto-detection detects raw and the default adds a further qcow214:09
tobiashso I can workaround that in the config and afterwards try to fix that14:10
gtemaok14:11
Shrewscorvus: https://review.openstack.org/571585 test failures are confusing me if you get a chance to look14:12
Shrewscorvus: TypeError: getProviderConfig() missing 1 required positional argument: 'provider'14:12
gtematobiash: I think in that case the solution might be to set a default in build.py:_buildImage as I wanted initially and not to set in in config reading14:14
tobiashgtema: yes probably14:15
tobiashok, the config workaround works14:16
tobiashjust noticed that the builder doesn't seem to reload the config in contrast to the launcher14:17
gtemawas also suspecting this14:17
Shrewstobiash: it's supposed to14:21
tobiashhrm, updated the config and it still built both, after restarting the builder it built the correct formats14:22
tobiashhave to check that again when I have time14:22
tobiashShrews: it's supposed to reload or not reload the config?14:23
Shrewsthat's a bug then, b/c it is getting reloaded14:23
Shrewsboth BuildWorker and UploadWorker threads reload the config14:23
Shrewsmaybe something needs reset during the reload14:24
tobiashmaybe14:24
mnaserif anyone is around, i made a thing to make things pretty -- https://review.openstack.org/#/c/572588/14:26
*** pcaruana has quit IRC15:23
openstackgerritMonty Taylor proposed openstack-infra/zuul master: Upgrade from angularjs (v1) to angular (v6)  https://review.openstack.org/55198915:29
mordredtristanC: ok. I think that ^^ is finaly ready15:30
mordredmnaser: ^^ you too, since you grok web things15:30
mordredmnaser: also - YAY on your patch15:30
openstackgerritMonty Taylor proposed openstack-infra/zuul master: Translate zuulStartStream into typescript  https://review.openstack.org/55861815:35
openstackgerritMonty Taylor proposed openstack-infra/zuul master: Shift log streaming code into StreamComponent  https://review.openstack.org/55861915:35
clarkbif people are doing zuul stuff this morning, I've still got a handful of changes up from yesterday related to fixing some of the brokeness we found after restarting services on Monday15:36
clarkbI think I even have a fix for test_slow_start which seems to fail 1/2 job runs15:37
*** sshnaidm|brq has quit IRC15:37
corvusclarkb: yeah, i'm about 30 minutes into reviewing that one now15:45
clarkbcorvus: cool let me know if you have questions, it started by fixing the easy race with build1.waiting then running into the other stuff needing to be updated because we were running in a bit more lockstep15:46
clarkbI tried to capture what and why I did things in the commit message15:47
mnaserthats a terrifying big patch but i'll try to dig through it haha15:57
clarkbmnaser: no kidding15:58
mnasermordred: going through it, yay, actual routes16:00
openstackgerritMichael Johnson proposed openstack-infra/zuul-jobs master: Collect the coverage report for npm test jobs  https://review.openstack.org/57026016:00
mordredmnaser: yah - sorry it's so big - but hopefully it sets up enough things for us to be able to make big improvements16:00
*** hasharAway is now known as hashar16:01
mnaser"<zuul-dashboard></zuul-dashboard>"16:01
mnaserlovely16:01
mordredmnaser: also - http://logs.openstack.org/89/551989/38/check/zuul-build-dashboard/53d282b/npm/html/status.html and http://logs.openstack.org/89/551989/38/check/zuul-build-dashboard-multi-tenant/769ae05/npm/html/tenants.html should both fully work16:03
mnaseroh i was about to start it locally16:03
mnaserbut that works too16:03
mnaser:P16:03
mnaserthats a relaly cool job16:04
mnaserhelps merge changes easier for reviewers16:04
mordredyah - I think otherwise sometimes it's too hard16:04
clarkbmay want fungi to sanity check that http://logs.openstack.org/89/551989/38/check/zuul-build-dashboard/53d282b/npm/html/status.html doesnt perform worse on his browser16:05
clarkbseems to work fine in mine16:05
gtemain Chrome on the Builds page the "refresh" button is bigger, that search input boxes. In FF - same size (of the button). In FF rounded corners, while in Chrome - not. Also in this place there is no padding/margin at all - would be nicer16:10
clarkbgtema: this is with the link above? or already merged status page?16:12
fungimy browser is firefox 60.0.1 with a handful of extensions (tridactyl, eff privacy badger, duckduckgo privacy essentials, multi-account containers)16:12
fungiseems to render that draft fine16:12
gtemaclarkb: http://logs.openstack.org/89/551989/38/check/zuul-build-dashboard-multi-tenant/769ae05/npm/html/t/openstack.org/builds.html16:13
fungiit's slow-ish for me, but my browser is somewhat overloaded at the moment anyway16:13
mnaserfungi: i think that with time, we'll be able to improve things significantly16:13
fungistill performs better than the "old" status page from, say, a week or two ago16:13
mnaserthe infrastructure mordred is putting down means that we don't have to redraw the world16:13
AJaegertobiash, clarkb, mnaser, I see a change by clarkb that has two +2s - but was not approved: https://review.openstack.org/#/c/562042/ . Should we merge this finally? Or is there a reason that nobody gave the +A?16:14
mnaserat some point only the things that have to be updated will be updated16:14
mnaserAJaeger: i left a +2 but not +A because i wanted other cores to be aware of that change because it affects $world16:14
mnasershould have probably clarified16:14
mnaseri assume that was tobiash intention too16:14
clarkbAJaeger: that one became less important, hwoever I think the consistency will be nice for readers16:15
tobiashAJaeger, mnaser: yupp ;)16:15
mordredmnaser: yes- I look forward to being able to make that status page more directly reactive - I think it should decrease the browser cpu usage significantly16:16
tobiashone caveat is that the job info is on one line then16:16
AJaegerso, anybody wants to +A - or what shall we do with it?16:18
clarkbAJaeger: I think we should consider what people find more readable. We fixed the logstash issue externally. If they find the new version better lets merge it, but I am happy to abandon if that is not the case16:19
gtemaand overall this filter block on Build page was "part of" navbar, while now it is an additional rowm which takes place16:19
AJaegerclarkb: how do you want to reach that consensus?16:19
* AJaeger has to step out for a bit now...16:20
clarkbAJaeger: maybe we just ask the existing reviewers, tobiash mnaser on https://review.openstack.org/#/c/562042/2 do you think the new content is more readable or prefer the old one?16:20
tobiashActually looking again onto the result:16:20
tobiashhttp://logs.openstack.org/42/562042/2/check/openstack-infra-base-integration-centos-7/c07855f/job-output.txt.gz#_2018-05-24_04_20_58_06243916:20
tobiashI find it quite hard to visually separate the three pieces of information on that single line16:21
tobiashoh actually four pieces of information...16:22
tobiashwhile this looks ugly but the information is better readable:16:23
tobiashhttp://logs.openstack.org/42/572342/1/check/tox-cover/bcb3341/job-output.txt.gz#_2018-06-05_11_12_48_11494016:23
tobiashI guess making that a localhost shell task would be visually best but limit it to trusted jobs16:24
*** elyezer has quit IRC16:24
tobiashclarkb: we may also be able to fix that in the ansible callback?16:25
clarkbtobiash: so that it adds timestamps to each line? possibly16:26
mordredI really need to make some progress on the in-broswer rendering of the json ...16:26
tobiashclarkb: yes, that's the idea16:26
tobiashclarkb: I think that would ideal16:27
tobiashthat would solve the problem while staying nicely readable16:27
clarkbok why don't I abandon the existing change then16:27
tobiash++16:28
mordredmnaser: thanks for those reviews - they're super helpful - and yes, I do think we need the routerlinkactive on the li element - lemme try updating it16:30
*** elyezer has joined #zuul16:31
mnasermordred: cool, yup, done too much bootstrap in my life time16:31
clarkbtobiash: what code path does that go through? it isn't the regular callback as that is just for shell and command streaming from a remote right?16:33
mordredmnaser: woot! I fixed the active thing. now for the container thing16:34
mnasershould probably look into making the leap to bs4 at some point too16:35
mordredmnaser: yes. I may leave that patch to you ;)16:35
tobiashclarkb: I thought it's the same callback as shell and branches off to command streaming for shell and command16:36
mnaserif i have spare time i'd probably focus that on making things more reactive cause thats probably far more important16:36
tobiashbut have to look again at that16:36
clarkboh _process_result_for_localhost is what it is I think16:36
openstackgerritArtem Goncharov proposed openstack-infra/nodepool master: retire shade in favor of openstacksdk  https://review.openstack.org/57282916:37
mnaseri am pretty excited im sure people will tell the difference with their status pages being cleaner :)16:37
mnasermordred: i wonder if i can borrow your job for my change to make it easier to get reviews.. unless you aim to merge your stuff soon16:37
tobiashclarkb: check out v2_runner_on_ok16:40
tobiashline 37516:41
tobiashthat's handling debug results16:41
gtemamordred: please look at the "builds" page. The row coloring is missing: <tr class="undefined". Another is that the filter section does not look nice. At least adding padding-right:5px for label and input there will be better16:42
clarkbtobiash: ya, but what calls that? is that part of the ansible api? I think for some reason I assumed it was zuul v216:42
clarkbgrep in ansible tree says it is ansible api16:42
tobiashclarkb: that's a callback called by ansible itself16:42
tobiashclarkb: so actually _log would need to split 'msg'16:43
clarkbtobiash: ya _log_message already does the splitting so I think we just need to stop treating debug: msg as special there16:46
clarkbI'm reading _log_message to make sure I understand it16:46
corvusclarkb: did you actually encounter any races after you added the build1.waiting/sleep loop?16:46
mnaseris there a known bug right now in console logs16:46
clarkbcorvus: the test consistently failed after that because we were accepting jobs at that point.16:47
tobiashclarkb: _log_message adds the hostname and calls _log16:47
clarkbcorvus: the race was older code could be not accepting (which is what we were asserting) I think. Then waiting there prevented that race from happening and I had to go step by step16:47
clarkbtobiash: it also sets the result dict output16:48
clarkbcorvus: I think what the waiting check did was expose that we were racing on the subsequent assertFalse(accepting) and that could never be the case without tweaking the min and max values16:49
clarkb* could never be the case now that we were sequencing things a bit more rigidly16:49
clarkbtobiash: I'll do the easy update of iterating over msg if it is multiple lines16:50
*** myoung is now known as myoung|lunch16:50
tobiash++16:50
corvusclarkb: i'm confused.  if i add only the build1.waiting/sleep loop in, i can't get the test to fail, nor do i see how it could.  let me push up a change.16:51
clarkbcorvus: we set max accepting to 3 but then only start 2 jobs so we should be accepting from that point forward unless we change the number down to 216:52
clarkbat least that was my reading of it16:52
openstackgerritMonty Taylor proposed openstack-infra/zuul master: Upgrade from angularjs (v1) to angular (v6)  https://review.openstack.org/55198916:52
mordredmnaser: ^^ that fixes oth things - thank you, I'd forgotten that both of those regressions were bothering me :)16:53
openstackgerritTobias Henkel proposed openstack-infra/nodepool master: Fix adding qcow2 format without need  https://review.openstack.org/57283616:53
tobiashShrews, gtema: I think that should fix the image format issue ^16:54
tobiashbut I should add a test case ;)16:54
gtematobiash: yes. Looks nice16:54
mordredmnaser: I would like to merge this soon - it's been lurking out there WAY too long - maybe just rebase yours on top of mine and you'll get the jobs?16:54
mordredgtema: oh - hrm. wheredid the row coloring go ...16:55
clarkbcorvus: similarly because min accepting was set to 1 we would transition to accepting on the next manageLoad() which made it harder to assert that state as I had to accomodate each manageLoad() I simplified by setting min to 0 so it wouldn't flip unless jobs finished or max was increased16:55
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Fix race in slow start test  https://review.openstack.org/57284217:12
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Make test_slow_start more robust  https://review.openstack.org/57284317:12
corvusclarkb: ^ okay, i wrote up why i think the test should work more-or-less as-written (but with your initial fix).17:13
corvusclarkb: setting the min to 0 is kind of a red flag for me -- that's not possible in production use17:14
*** gtema has quit IRC17:14
clarkbcorvus: https://review.openstack.org/#/c/572843/1/zuul/executor/server.py is another way to approach that17:15
clarkb(I'm fine with that but still need to finish reviewing it)17:15
corvusclarkb: i think a key nuance is that the test actually relies on manageLoad *not* being called too often.  if it is called too often, it will notice that jobs are no longer "starting".17:15
clarkbya17:15
corvus(by the time a job hits the "hold_jobs_in_build" wait, it's no longer starting, it's running)17:16
clarkbcorvus: I think it would be nice to keep the raise in waitForExecutorBuild but I can add that back in in a followup17:16
clarkbit just makes that case of error easier to understand when reading the test fail log17:16
corvuswe could add a new "hold jobs in start" feature, and i got halfway through doing that, but it's a lot of code for basically just this test17:16
clarkbcorvus: that was why I went with min accepting 0 :)17:16
corvusclarkb: agreed re: exception17:16
corvusclarkb: i'm just really surprised you hit a race with just 572842; i've run it a lot locally with no failures -- the races that 572843 are really pathological cases (which, to be fair, we get sometimes around here, but usually not reliably)17:19
corvusclarkb: actually, i think waitForBuildCompletion can just be replaced with another iterate_timeout call17:19
corvus(it's only a single check)17:20
clarkbto check the starting attribute17:20
corvussorry, waitForWorkerCompletion17:20
corvusit just checks whether build.uuid is in job_workers17:21
clarkbcorvus: https://review.openstack.org/#/c/572842/1/tests/unit/test_executor.py line 550 of the new side of the diff is what gave me much trouble. That won't be reliable until the child change update to ordering of manageLoad() and starting work. Do we want to combine the two changes for that reason?17:21
clarkbcorvus: ah17:21
openstackgerritMonty Taylor proposed openstack-infra/zuul master: Upgrade from angularjs (v1) to angular (v6)  https://review.openstack.org/55198917:22
mordredmnaser: ^^ that should fix the thing gtema pointed out (missing color bars on builds page)17:22
mordredmnaser: I'm going to need some help to figure out how to get the filter boxes up into the nav bar17:23
mordredmnaser: since the navbar is a component now - we can probably do that as a followup though17:23
mordredmnaser: oh - interesting. the header works differently in produciton for status and for builds/jobs17:24
corvusclarkb: python has to essentially only run the job thread all the way until it stops to hold in build and then switch back to the executor thread to call manageLoad, and it has to do that *twice* (one for each job) in order for that race to fail the test.  i'm happy to squash the change, but also, you should buy a lottery ticket.  :)17:24
mordredcorvus: if you have a sec, could you look at the navbar header on https://zuul.openstack.org/builds.html vs https://zuul.openstack.org/status.html17:24
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Fix race in slow start test  https://review.openstack.org/57284217:26
mordredcorvus: one spans the entire page, one doesn't - in the angular patch it becomes the same component so will be consistent, but we should pick which one we want to be consistent to17:27
corvusmordred: i agree they look different.  builds has a margin17:27
mordredyah17:27
mordredI *think* I prefer full page width - but I don't have a strong opinion17:27
corvusmordred: i think i like status (w/o margin).  but it's almost a toss up.17:27
corvusmordred: i think we said the same thing :)17:27
mordredcorvus: :)17:28
mordredthat's also what mnaser suggested in an earlier review, so it's what the patch does now - so yay!17:28
clarkbcorvus: +2 I will do a followup to add the raise. Also I think the child cahnge can be abandoned?17:30
corvusdone17:31
corvusclarkb: i was suggesting instead of adding the raise, replace that entire method with an iterate_timeout17:31
*** bhavik1 has joined #zuul17:32
openstackgerritClark Boylan proposed openstack-infra/zuul master: Add timestamps to multiline debug message logs  https://review.openstack.org/57284517:33
clarkbcorvus: oh gotch17:33
clarkbtobiash: ^ that is the fix for logging with test update too17:33
mnasermordred: I can provide brain cycles on how to best wire this up17:34
mnaserBut I can’t commit to time to figuring that part out because I’m a little overcommitted right now17:34
openstackgerritClark Boylan proposed openstack-infra/zuul master: Treat ::1 as localhost in addition to 127.0.0.1  https://review.openstack.org/57284717:34
clarkbthat was somethign I noticed when fixing the log message thing above. Not sure if strictly necessary17:35
*** gtema has joined #zuul17:35
corvusShrews: i'm looking at 57158517:36
Shrewscorvus: relooking too now that i'm back from lunch. i suspect it has something to do with the class loading aspect17:37
Shrewsmaybe the driver needs instantiation?17:37
openstackgerritArtem Goncharov proposed openstack-infra/nodepool master: retire shade in favor of openstacksdk  https://review.openstack.org/57282917:38
Shrewsget_provider_config() used to do the instantiation. doesn't do that now17:40
*** jpena is now known as jpena|off17:40
openstackgerritTobias Henkel proposed openstack-infra/nodepool master: Fix adding qcow2 format without need  https://review.openstack.org/57283617:40
tobiashShrews, gtema: with test now ^17:40
tobiashthe test was helpful because PS1 was actually broken :)17:41
gtematobiash: great17:41
Shrewsyay tests17:41
tobiashthat adds test for gtema's use case as well as a test for my regression17:42
corvusShrews: yeah, i think you're right.  left comments on the change.17:44
openstackgerritClark Boylan proposed openstack-infra/zuul master: Remove unnecessary method in test_slow_start  https://review.openstack.org/57285017:46
clarkbcorvus: something like that I Guess17:46
*** bhavik1 has quit IRC17:47
*** gtema has quit IRC17:48
corvusclarkb: ya17:52
openstackgerritTobias Henkel proposed openstack-infra/nodepool master: Fix adding qcow2 format without need  https://review.openstack.org/57283617:52
mordredmnaser: no worries - I don't think it's a huge issue we need to solve today - and I found some web blog things with suggestions17:54
mordredmnaser: they involve words I don't understand yet of course :)17:54
mnasermordred: i can help with translating things :P18:05
mnaserbut yeah moving the filter into the navbar is nice but from an angular pov i don't think it's right because we're taking a component all together and splicing it out of it18:05
mnaserimagine someone taking the dashboard downstream and wanting to use particular components18:06
mnaserlike say i load the libraries and just want to add a `<zuul-status server="zuul.openstack.org" />` using the libraries18:06
mnaserwe're pretty far away from that but the idea of creating these components is letting (hopefully) downstream users easily integrate things like this18:06
mnaserunless we find a way to wire it up to the search like <zuul><zuul-filter /><zuul-status .../></zuul> that way its all in there together18:07
mnaserit's a fun discussion we can have18:07
mnaser</irc-blog-post>18:07
mordredmnaser: ++18:09
*** electrofelix has quit IRC18:10
openstackgerritMerged openstack-infra/zuul master: Fix race in slow start test  https://review.openstack.org/57284218:11
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool master: Create a base Driver class  https://review.openstack.org/57158518:11
Shrewscorvus: ^^ passes locally now18:12
*** myoung|lunch is now known as myoung18:18
openstackgerritMerged openstack-infra/zuul master: Remove unnecessary method in test_slow_start  https://review.openstack.org/57285018:19
openstackgerritTobias Henkel proposed openstack-infra/nodepool master: Move QuotaInformation to driver utils  https://review.openstack.org/57286018:23
*** robled has quit IRC18:32
*** robled has joined #zuul18:36
*** robled has quit IRC18:36
*** robled has joined #zuul18:36
openstackgerritMonty Taylor proposed openstack-infra/zuul master: Translate zuulStartStream into typescript  https://review.openstack.org/55861818:58
openstackgerritMonty Taylor proposed openstack-infra/zuul master: Shift log streaming code into StreamComponent  https://review.openstack.org/55861918:58
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool master: Have Drivers create Providers  https://review.openstack.org/57158618:58
mordredcorvus, tristanC, clarkb: https://review.openstack.org/#/c/551989 https://review.openstack.org/#/c/558618 and https://review.openstack.org/#/c/558619/ should all be good to go now19:01
mordredShrews: I like how that patch has a net-zero line impact19:03
Shrewsmordred: a well balanced nodepool19:04
kmallocwhoa, i didn't know nodepool used the old "class X:" form. threw me for a loop having not seen that in a while /embarassing :P19:13
kmalloc(embarassing for me not recognizing it as valid python)19:13
Shrewskmalloc: yeah I noticed a couple of those today. probably should clean those up19:14
kmallocShrews: ++ thankfully that is super easy19:15
tobiashmordred: comment on https://review.openstack.org/55861919:20
tobiashmordred: oh actually that's already broken in 55198919:23
mordredtobiash: ah - cool. thanks19:41
tobiashShrews: is the driver class ready to go?19:44
Shrewstobiash: seems to be19:45
Shrewstobiash: maybe we should add a release note for your qcow2 fix?19:47
clarkbqcow2 fix?19:47
tobiashclarkb: https://review.openstack.org/57283619:48
tobiashShrews: was the bug introduced in a previous release?19:48
clarkbI want to say that the autodetection from clouds.yaml is relatively new19:49
clarkbbug has probably been in there since we added that functionality19:49
Shrewstobiash: oh, maybe not19:49
Shrewsclarkb: no, i think this was a recent-ish change19:49
tobiashclarkb: I use the autodetection from clouds.yaml since the beginning19:49
tobiashShrews: I don't see a tag after the bug was introduced19:50
clarkbah19:50
Shrewstobiash: was there a release note on the breaking change?19:50
tobiashnope19:50
Shrewsk19:50
Shrewsprobably should have been19:50
openstackgerritMerged openstack-infra/zuul-jobs master: Include hostnames in /etc/hosts  https://review.openstack.org/57231319:51
Shrewswe need to be more diligent with that19:51
tobiashShrews: do you want a release not for fallback qcow2 feature?19:51
tobiashs/not/note19:51
Shrewstobiash: seems worth mentioning, yeah?19:52
tobiashI'll add one19:52
Shrewsi'm going to have to add one for all the driver api changes, i think19:54
Shrewsthough we always said that wasn't set in stone yet19:54
openstackgerritMerged openstack-infra/nodepool master: Create a base Driver class  https://review.openstack.org/57158519:54
corvusShrews: yeah, i don't think we've made any user-visible changes yet.  except that when it's all done, we should be able to add a note about how 'nodepool list' will show static ready nodes.19:55
openstackgerritMerged openstack-infra/nodepool master: Have Drivers create Providers  https://review.openstack.org/57158619:55
Shrewscorvus: so we don't want to use reno for driver api changes?19:56
Shrewsjust user visible ones?19:57
corvusShrews: i think so.  at least until we're ready to consider it a public api.19:57
Shrewsi guess most nodepool users don't care about the apit19:57
corvus(but it's still an internal api right now)19:58
Shrewss/apit/api19:58
Shrewsk19:58
corvusdevelopers can read the git log :)19:58
Shrewslol19:58
openstackgerritTobias Henkel proposed openstack-infra/nodepool master: Fix adding qcow2 format without need  https://review.openstack.org/57283619:58
Shrews"code is self documenting!"19:58
corvusi just wrote a bunch of documentation for it, so that seems to be working ;)19:59
Shrewsindeed19:59
tobiashShrews: now with release note ^19:59
tobiashcorvus: are you running 572210 on your instance currentlz?20:05
tobiashI thought about upgrading our instance and I'm not sure if it will break without 57221020:06
corvustobiash: yes i think it's needed20:06
tobiashk, thx20:10
*** acozine1 has quit IRC20:38
corvusalso, i think there's an issue with websocket streaming still, working on that now21:00
*** myoung is now known as myoung|off21:06
openstackgerritMerged openstack-infra/nodepool master: Fix adding qcow2 format without need  https://review.openstack.org/57283621:18
*** dims has left #zuul21:31
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Handle more than one websocket streamer at a time  https://review.openstack.org/57302322:25
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Handle more than one websocket streamer at a time  https://review.openstack.org/57302322:28
corvustobiash: ^ that's also necessary22:30
corvusShrews, mordred: ^ may be of interest22:31
*** rlandy|rover is now known as rlandy|rover|bbl22:37
*** threestrands has joined #zuul22:51
*** eandersson_ is now known as eandersson22:59
*** hashar has quit IRC23:00
clarkbfungi: I reworkflowed it as I'm satisfied with corvus' response23:01
clarkbI'll need to rebase my change to that test once it goes in23:01
fungiwfm23:02
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Handle websocket client hangups better  https://review.openstack.org/57303423:06
corvusclarkb, fungi: that's a non-urgent improvement, but you might want to go ahead and review it while it's fresh23:07
corvus(the current behavior is okay, but results in things hanging around too long and spurious exceptions in the log)23:07
openstackgerritMerged openstack-infra/zuul master: Handle more than one websocket streamer at a time  https://review.openstack.org/57302323:12
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Fix zuul-web sql connections  https://review.openstack.org/57221023:13

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