Tuesday, 2021-02-23

tristanCclarkb: in the config module, reset should only be called if a driver has been active00:01
clarkbheh not sure how I completely skipped over that file. I must've clicked on the second one00:02
openstackgerritTristan Cacqueray proposed zuul/nodepool master: drivers: only reset drivers that have been active  https://review.opendev.org/c/zuul/nodepool/+/77702400:03
clarkbis there another related issue here where you could have config for multiple k8s installations in the k8s config file but only use a subset of them in the nodepool providers?00:04
clarkbthat is probably more difficult to address because i bet config loading for k8s loads everything00:04
tristanCclarkb: and there was a typo. But again, there is surely a better way to do that00:04
*** tosky has quit IRC00:38
*** hamalq has quit IRC01:26
*** rlandy has quit IRC01:28
*** smyers has quit IRC02:44
*** smyers has joined #zuul02:47
*** ricolin has quit IRC03:15
*** ricolin has joined #zuul03:30
*** ykarel has joined #zuul04:36
*** saneax has joined #zuul04:51
*** saneax has quit IRC04:52
*** saneax has joined #zuul05:10
*** evrardjp has quit IRC05:33
*** evrardjp has joined #zuul05:33
*** jfoufas1 has joined #zuul05:48
*** sanjayu_ has joined #zuul06:00
*** saneax has quit IRC06:03
*** vishalmanchanda has joined #zuul06:25
*** hashar has joined #zuul07:12
*** piotrowskim has joined #zuul07:35
*** jcapitao has joined #zuul07:40
felixedelcorvus: No, I'm currently working on the refactoring of the builds API. So would be glad if you could do that. Regarding your comments on the change: I'm fine with that procedure and I will rebase my change once the tls changes are all merged. But I would like to keep it in it's current position in the stack. Otherwise I would have to do a lot more rebasing and I don't think it's worth.08:03
*** rpittau|afk is now known as rpittau08:28
*** ykarel_ has joined #zuul08:31
*** ykarel has quit IRC08:33
*** tosky has joined #zuul08:50
*** dry has joined #zuul08:56
*** msuszko has quit IRC08:58
*** jpena|off is now known as jpena08:58
*** nils has joined #zuul09:20
*** noonedeadpunk has quit IRC09:26
*** jonass_ has quit IRC09:29
*** noonedeadpunk has joined #zuul09:29
*** jonass has joined #zuul09:30
*** jfoufas1 has quit IRC09:41
*** jfoufas1 has joined #zuul09:46
*** jfoufas1 has quit IRC10:17
openstackgerritOleksandr Kozachenko proposed zuul/zuul-jobs master: Revert "Revert "Update upload-logs roles to support endpoint override""  https://review.opendev.org/c/zuul/zuul-jobs/+/77667710:18
*** ykarel_ is now known as ykarel10:20
openstackgerritOleksandr Kozachenko proposed zuul/zuul-jobs master: Revert "Revert "Update upload-logs roles to support endpoint override""  https://review.opendev.org/c/zuul/zuul-jobs/+/77667710:21
*** ajitha has joined #zuul10:35
iceyis it possible for a project to override a zuul job defined in project config?10:50
*** rpittau is now known as rpittau|bbl11:00
*** iurygregory_ has joined #zuul11:00
*** iurygregory has quit IRC11:01
*** iurygregory_ is now known as iurygregory11:06
*** jcapitao is now known as jcapitao_lunch11:13
*** ikhan has joined #zuul11:50
*** hashar is now known as hasharLunch11:57
*** rlandy has joined #zuul12:29
*** jpena is now known as jpena|lunch12:30
Open10K8SHi team. Is there any chance to check these PSs regarding zuul log upload? https://review.opendev.org/c/opendev/base-jobs/+/777087  https://review.opendev.org/c/zuul/zuul-jobs/+/77667712:36
avassneither include_tasks or include_role runs in parallel on multiple hosts :(12:39
avasshowever it looks like a bare 'include' does run in parallel12:41
avassI'm gonna do a bit of investigation around that later, but that could mean that some roles in zuul-jobs has a perfomance cost for multi-node jobs. (looking at bindep specifically atm)12:44
avassoh actually it could be that looping does not happen in parallel, and we're looping over a include_tasks to install packages in the bindep role12:49
*** jcapitao_lunch is now known as jcapitao12:50
*** rpittau|bbl is now known as rpittau13:01
openstackgerritTristan Cacqueray proposed zuul/nodepool master: drivers: only reset drivers that have been active  https://review.opendev.org/c/zuul/nodepool/+/77702413:27
*** jpena|lunch is now known as jpena13:33
openstackgerritTristan Cacqueray proposed zuul/nodepool master: kubernetes: refactor client creation to utils_k8s  https://review.opendev.org/c/zuul/nodepool/+/77702213:36
openstackgerritTristan Cacqueray proposed zuul/nodepool master: kubernetes: refactor client creation to utils_k8s  https://review.opendev.org/c/zuul/nodepool/+/77702213:52
openstackgerritBenedikt Löffler proposed zuul/zuul master: Add tenant to reconfiguration end log message  https://review.opendev.org/c/zuul/zuul/+/77710913:53
*** rlandy is now known as rlandy|training13:57
mordredavass: and sadly we're doing that to handle a case that is unlikely to happen most of the time. also - why dont' loops happen in parallel??14:25
*** harrymichal has joined #zuul14:25
*** vishalmanchanda has quit IRC14:33
openstackgerritTristan Cacqueray proposed zuul/nodepool master: kubernetes: refactor client creation to utils_k8s  https://review.opendev.org/c/zuul/nodepool/+/77702214:36
avassmordred: I can't think of a good reason why they don't. maybe we should update that role to handle that a bit better somehow14:40
avassI guess weird things could technically happen if you modify the inventory in a loot and run those in parallel on multiple host and get race conditions that way14:43
avasss/loot/loop/14:44
avassbut I don't see why ansible wouldn't allow loop to run in parallel at all because of that14:44
Shrewsmordred: avass: i would probably ask bcoca to confirm since that bit of code is a bit complex and I've only looked at it in bits and pieces, but I believe that since `include_*` are dynamic, the execution of that include is in parallel, but it gets expanded on the workers as a new task list which is then executed sequentially.14:58
mordrednod14:59
*** ykarel has quit IRC14:59
openstackgerritTristan Cacqueray proposed zuul/nodepool master: kubernetes: refactor client creation to utils_k8s  https://review.opendev.org/c/zuul/nodepool/+/77702215:01
*** jangutter has joined #zuul15:12
jangutterHi! We're bootstrapping a zuul install from scratch, and I'm trying to debug something weird in the web-ui. If I curl <zuul-web>/api/tenant/workday/builds, I get 404 "tenant workday doesn't exist"15:15
jangutterHowever, accessing jobs and projects works perfectly.15:15
fungijangutter: have you checked to see if zuul-web is logging any errors? maybe it's having trouble reaching the database?15:23
jangutterfungi: the logs are _really_ quiet, I'm not seeing any obvious errors. Status seems to be operating fine though....15:27
fungialso possible that's the benign result of zuul-web misinterpreting an empty query result because there have been no builds for that tenant yet?15:27
fungior at least none which belong to any reported buildset15:28
jangutterfungi: hah, and that situation occurs rather often on a clean install!15:29
jangutterdo we need a db reporter for this to work?15:30
fungiif you're installing zuul >=4.0.0 the mysql reporter is implicit on all pipelines now i believe15:31
* fungi double-checks release notes15:31
jangutterBwaha: 3.19.1.dev422 064042c715:31
corvusjangutter, fungi: i'd start with checking the scheduler15:33
corvusthat's actually what the zuul-web consults for determining if there's a tenant15:33
jangutterThanks! I can now explain my hairloss! I did not configure an sqlreporter.15:33
corvusoh, you only see the tenant doesn't exist with builds?  then yeah, lack of sql is probably the issue15:34
janguttercorvus: I _think_ the scheduler is running fine? I can see jobs, even stream the console of one live....15:34
* jangutter goes and adds sqlreporter to the base job.15:35
corvusjangutter: i'd go straight to 4.0 at this point, and don't configure a 'sql' connection or reporter, just configure a 'database' entry15:35
corvusjangutter: 4.0 is nearly the same as what you're running, just with different syntax around sql reporting, and requires zk tls15:35
corvusor.. maybe even exactly the same as what you're running15:36
corvus(not sure which commit 3.19.1.dev422 064042c7 actually maps to)15:36
jangutterGoogling that version leads me to a log that happened in 2020-11-0515:38
openstackgerritTristan Cacqueray proposed zuul/nodepool master: kubernetes: refactor client creation to utils_k8s  https://review.opendev.org/c/zuul/nodepool/+/77702215:39
corvusjangutter: but i think dev 422 is 422 commits after that, so probably more recent15:40
corvusthere were 522 commits between that version and now15:42
corvusso it's probably not from yesterday :)15:43
fungithough pbr also counts merge commits in that total so it might be half as many changes as that15:43
corvusyeah15:43
jangutterheh, we're using zuul from containers, but obviously our internal mirror is a bit... stale :-p15:45
corvusjangutter: yeah, we do the same thing on opendev, sometimes running from a random master commit.  you're in good company there, i think we just haven't quite solved the traceability issue.15:46
clarkbthe 064042c7 should map to a specific commit15:46
clarkbhowever not one in our upstream repo?15:47
fungiin theory yes, but it doesn't map to a commit in the history of zuul's master branch that i can find, so could be a fork15:47
clarkbya15:47
corvusclarkb: yeah, but possibly a speculative one15:47
clarkboh I see15:47
corvusdoesn't have to be a fork15:47
corvusthe tagged releases will map to known commits15:47
corvusbut the running :latest from master can be speculative commits15:48
corvusthat's the mapping issue we need to figure out :)15:48
fungiso it could actually just be an ephemeral fork we made in our gate pipeline ;)15:48
jangutteryep, we took from :latest - good to know that it's so new it might be in a future that hasn't happened yet!15:49
corvuswe could solve that either by encoding additional information about the actual commits, or zuul-push could address that by making the ephemeral tree reality15:49
corvusjangutter: yeah, time behaves differently for zuul :)15:49
clarkband even gerrit may not know about that due to speculative merges15:50
avassfungi: "there is no fork"15:58
avasstoo bad git doesn't have spoons :)15:59
fungiheh15:59
*** sshnaidm is now known as sshnaidm|afk16:04
avassit might be a good idea to merge a change in zuul/zuul to bump the docs so can I interest anyone in some easy reviews? :) https://review.opendev.org/c/zuul/zuul/+/775499 https://review.opendev.org/c/zuul/zuul/+/77249116:17
corvusavass: done!16:19
*** hasharLunch is now known as hashar16:22
jangutterAha, I see now: https://review.opendev.org/c/zuul/zuul/+/630472 (I've been shamelessly copying the config and pipelines from _current_ zuul, but using the containers built on old-and-busted zuul)16:25
fungiin opendev our scheduler is logging lots of "Exception: No job nodeset for some_job" tracebacks. probably benign, looks like it may happen if it tries to cancel a build which hasn't gotten a node assignment yet. is this a known behavior? investigating a (likely) separate problem and stumbled across these errors16:26
avassfungi: I think I've seen that before but didn't investigate why it happened. maybe we have something similar I can dig up16:30
corvusjangutter: ah yep, we removed our sql reporter defs immediately after landing that (the warnings are quite a motivator)16:30
corvusfungi: have a stacktrace handy?16:31
corvusit is likely benign but we raise that exception to catch bugs :)16:32
avassyep, got 22 events the last 24h16:32
fungicorvus: http://paste.openstack.org/show/802936/16:33
fungiwe got 85054 of those in the past 10 hours16:35
corvusfungi: likely for the same event/jobs over and over16:38
mordredcorvus: do we have a really old python or something on the nodepool nodes in gerrit's zuul?16:42
corvusmordred: oh maybe, i think they are stretch?16:42
mordredcorvus: https://ci.gerritcodereview.com/t/gerrit/build/9001d1fda6494c43bc5dc9306f660d07/console#1/0/20/testnode - that's what I get trying to read gerrit's .gitmodules - but if I read it locally (with python2, which seems to be what we're using) - it doesn't work16:42
mordredcorvus: yeah, I think so16:42
corvusmordred: it does or doesn't work locally?16:43
mordredcorvus: it works locally16:43
mordredwith both python2 and python316:43
mordredI did some googling and python added support for leading spaces in ini files for configparser like back in 2010 - so I'm really confused :(16:44
corvusheh shouldn't be that old16:44
mordred(I'm now trying spinning up some stretch for more debugging)16:45
mordredright?16:45
mordredok - yes - stretch python2 does not parse .gitmodules16:45
mordredstretch python3 does work16:46
mordredcorvus: should we maybe consider using python3 for ansible there? I could also obviously just read the file into a string, remove the leading spaces, then wrap it in a StringIO to pass to configparser16:47
mordredwhich should work everywhere16:48
corvusmordred: is py3 already installed?16:48
corvusif not, then we'll take a hit installing it16:48
mordredgood question16:48
mordredwe're not building our own images there are we?16:48
mordredI'm guessing no - since I dont think we're overriding the python setting, which means ansible is just finding python2 as python16:49
corvuscorrect16:50
mordredwhy don't I do StringIO for now - we can talk about python3, or changing the node type, as a follow16:50
corvusyeah, we should change the node type but there are too many irons in the fire16:50
mordred++16:50
corvusfungi, avass: it looks like the bug centers around change C which git-depends on change B which is failing and change B is queue-dependent on change A which is passing.  in that case, we can't shift change C out of the way any more, it just has to wait behind B, but since B is failing, we stop running jobs on C.  we just cancel the jobs over and over; we don't see that they're already canceled.16:58
corvusi can't dig into it any more than that right now, but i think that's enough to know that it probably really is benign other than the log/processing spam.  we should be able to make a test case for that and fix it.16:59
fungiyep, thanks. it's also looking increasingly unrelated to the problem i was initially trying to debug17:00
corvuswe should definitely fix it; there is a notable human and cpu cost to all that extra processing17:01
fungiyes, sounds like repeatedly retrying to do something that's already been done is certainly suboptimal17:03
*** harrymichal has quit IRC17:12
pabelangerclarkb: looping back to ready unlocked nodes, it would be interesting to also debug https://zuul.opendev.org/t/openstack/nodes I see a few nodes that are greater then 10hours / 18hours. 1 is even 2 years17:12
pabelangerI'm still trying to trace them on our side17:13
clarkbpabelanger: ya we are looking at them currently in #openstack-infra17:13
clarkbpabelanger: the vast majority are all in a single provider and originate during a similar time period17:13
pabelangerack17:14
clarkbthey belong to what appear to be completed node requests, nodepool is just not marking them as completed so zuul doesn't do anything with them17:14
clarkbwhich is slightly different than the behavior you describee17:14
tobiashready unlocked or ready locked?17:14
pabelangerclarkb: yah17:15
clarkbtobiash: ready locked17:15
pabelangertobiash: in our case, they are ready unlocked17:15
clarkbtobiash: and the node request is sitting pending17:15
tobiashyeah we see that as well, there is still a node leak to be fixed17:15
clarkbtobiash: it looks like the handler poll is simply not noticing the node is ready and can be unlocked and set the node request to fulfilled17:16
fungiwhich winds up blocking changes in a dependent queue from merging because the change currently at the front has a slew of queued builds corresponding to those node requests17:16
tobiashbut had no luck and time yet so we currently work arou d that by automatically delete those locks after 24h17:16
pabelangertobiash: how do you delete them?17:16
clarkbdeleting the lock won't cause the node request to be fulfilled though?17:16
fungijust deleting the znodes for them i guess?17:16
clarkbmaybe it will detect it lost the lock and then try again though17:16
pabelangerI guess, crontab and zuul client?17:16
tobiashwe delete the lock via zkcli17:17
pabelangerah17:17
fungigot it, yeah that's what i figured you meant17:17
corvuspabelanger: i thought your issue was ready unlocked and you were going to check on logs to see if nodepool got around to deleting them?17:17
tobiashbasically nodepool list | grepsedsomething | xargs zkcli delete17:17
tobiashpabelanger: do you use max ready age?17:18
pabelangercorvus: yah, that is the issue. nodepool eventually deletes them or they get used, but only have a few hours17:18
pabelangertobiash: yes17:18
corvuspabelanger: right, so if nodepool eventually gets around to them, then this isn't something that should be solved by deleting zk records17:18
pabelangerbut, I cannot see why they are ready unlocked for so long17:18
pabelangercorvus: agree17:18
pabelangerI can actually delete the unlocked node, which frees up quota, and get launched again17:19
pabelangerjust trouble tracking why the large delay17:19
clarkbin this case if we restart the launcher we should end up trying again because the locks would be removed right?17:20
clarkbit is almost like these ~12 node request handler threads all just died17:20
clarkbbecause there doesn't seem to be any of the logging you'd expect from the handler after the launcher finishes launching the node17:21
fungimaybe triggered by a cloud api outage?17:21
fungiwould a thread dump help?17:21
clarkbthread dump may help if we label the threads well enough to figure out which might be handling this request17:21
clarkbseems like we should start there17:21
pabelangerclarkb: is there enough quota for more nodes to be launched? are the node requests waiting for another node to comeonline to unlock?17:21
fungiat least see what those threads *think* they're doing17:21
clarkbpabelanger: its a single node request so no17:21
pabelangerk17:22
fungiyeah, a lot of these are for very simple (linter, unit test) jobs17:22
openstackgerritGuillaume Chauvel proposed zuul/zuul master: Gitlab: raise MergeFailure exception to retry a failing merge  https://review.opendev.org/c/zuul/zuul/+/77716917:23
pabelangerclarkb: yah, I would expect nodepool-launcher restart to clear the lock17:26
openstackgerritMerged zuul/zuul master: Add a note how to access zuul.items variable  https://review.opendev.org/c/zuul/zuul/+/77249117:32
openstackgerritMerged zuul/zuul master: Replace reset_repo_to_head(repo) with GitPython.  https://review.opendev.org/c/zuul/zuul/+/77549917:36
clarkbtobiash: pabelanger: we took sigusr2 thread dumps then restarted the service. All of the ready locked nodes seem to be getting used now17:36
clarkb(stopping the launcher would've unlocked the nodes then starting it again would've made nodepool realize it can fulfill requests with them I think)17:37
openstackgerritGuillaume Chauvel proposed zuul/zuul master: Gitlab: raise MergeFailure exception to retry a failing merge  https://review.opendev.org/c/zuul/zuul/+/77716917:37
clarkbfwiw looking at the thread dumps doesn't seem to show much. There is no launcher thread for the node id (implying its launchign was complete) and I think the handler polling happens in the main provider thread which appeared to be running just fine17:38
clarkbmaybe we lost a handler reference somewhere?17:38
*** rpittau is now known as rpittau|afk17:41
mnaserhas the circular dependencies seen progress other than the spec? i don't see anything right off the bat17:41
tobiashclarkb: have you any storenode related exceptions in the logs?17:41
tobiashI see them occasionally so could be related17:42
corvusmnaser: yes, there's an implementation in review.17:43
clarkbtobiash: there aren't any in the log file for the period of time when this node request was first handled. There are a couple in more recent blocks of time thuogh17:44
*** piotrowskim has quit IRC17:47
*** Shrews has left #zuul17:56
*** jpena is now known as jpena|off18:02
*** rlandy|training is now known as rlandy18:02
corvustristanC, tobiash: can you look at https://review.opendev.org/776566 real quick?18:39
avasscorvus: is there any specific reason zuuls dockerfiles doesn't set 'USER zuul' ?18:42
tobiashavass: the executor at least needs privileges due to bubblewrap18:44
tobiashas well as nocepool- builder18:44
avasstobiash: yeah but the rest should be able to do non-root by default18:44
tobiashcorvus: lgtm18:44
corvustobiash, avass: zuul-executor does not need root18:44
corvusopendev runs its executors as the zuul user18:45
clarkbyou do need the kernel support for the user namespacing though18:45
clarkband I think rhel/centos disable that by default18:45
clarkb(in those cases you need root aiui)18:46
corvustobiash, avass: opendev also runs its builders as non-root (the nodepool user)18:46
tobiashcorvus: right, it just needs sudo18:46
corvustobiash: yeah that does seem to be the case18:48
corvusthe only thing that needs to start as root is fingergw if (and only if) you want to run in on port 79.  but if you're using containers or behind a lb, you can run it as non-root and map the port.18:48
avassthat still doesn't really answer my question, I see no reason why zuul-web would need to run as root by default for example18:48
*** jcapitao has quit IRC18:48
corvusavass: i think we don't set USER to accomodate openshift18:48
avassoh18:49
corvushowever, openshift is changing how they're doing their user mapping, so it's worth checking back on that when we can assume openshift 4 everywhere18:49
tobiashunfortunately we're not there yet18:49
tobiashbut we won't switch to 4 but to plain k8s later this year18:50
avassI haven't really looked into openshift at all18:51
corvusi'd love to switch.  i think defaults should always be the correct value, and unfortunately we have a case here where if you use the defaults with k8s or docker, you're doing the wrong thing.18:52
corvusbut last we looked, specifying the user was the wrong thing in openshift.  so <shrug>18:52
corvusbut somehow we managed to thread the needle so at least it is possible to do the right thing in both systems18:53
avassyeah it's just a bit awkward to do runAsUser: 1000118:54
openstackgerritMerged zuul/zuul-storage-proxy master: Add zuul user to container  https://review.opendev.org/c/zuul/zuul-storage-proxy/+/77656618:54
*** jangutter has quit IRC19:06
*** hashar has quit IRC19:22
*** gmann is now known as gmann_lunch19:47
*** gmann_lunch is now known as gmann20:07
*** ikhan has quit IRC20:14
*** sanjayu_ has quit IRC20:16
mordredcorvus: so - I'm a little stumped. https://ci.gerritcodereview.com/t/gerrit/build/e5d5502511bb4732b0a53f87edbc8aee/console#1/0/20/testnode20:20
mordredshows a bunch of "no section" errors20:20
mordredbut when I run similar commands locally on the .gitmodules file from gerrit, it totally works20:21
mordredhttps://gerrit-review.googlesource.com/c/zuul/jobs/+/297442 - I just pushed up a few more debug lines to see if we can figure out what's different on the nodes there from locally20:25
*** hamalq has joined #zuul20:30
*** ikhan has joined #zuul20:33
mordredoh wow.20:37
mordredhttps://ci.gerritcodereview.com/t/gerrit/build/b3e05e06045143858fd1eed056136166/console#1/0/20/testnode <--20:37
mordredwe're sorting line numbers in the console output as strings not as numbers20:37
avasshah20:37
avassyou mean you don't count like that?20:38
mordredI mean, I do think that 2 should come between 19 and 2020:39
mordredbut others might find that confusing20:39
*** ikhan has quit IRC20:42
mordredavass: any idea where I should look for the code that is rendering those?20:43
avassmordred: I'm looking in web/src/containers/build/Console.jsx right now20:45
avassnot sure if that's it however20:46
corvusit's not us, it's an external component20:46
corvusreact-json-view20:47
corvusReactJson from ^20:47
mordredneat. so react-json-view is displauing lists wrong? :)20:49
corvusafaict20:50
corvusmaybe the 'sortKeys' option we set is affecting this20:50
avasschecking that right now20:50
corvushttps://github.com/mac-s-g/react-json-view/pull/26320:51
mordredoh - I betcha20:51
mordredhttps://github.com/mac-s-g/react-json-view/blob/825e8dc96b59a551c6167bef3aa72eee0d2045c7/src/js/components/DataTypes/Object.js#L25220:52
corvusmordred: i linked to the fix ^20:52
mordredah - you found the PR - it looks like master dtrt20:52
mordred++20:52
avasssortKeys={false} does it20:52
mordrednow - if only they tagged their repo it would be easy to see if that fix is in the latest release :)20:52
corvusi think so.  i'll push a zuul change.20:53
mordredcool20:53
mordredyeah - latest release 18 days ago20:53
avasswow that took a long time to merge20:54
fungiwho needs arrays with more than 10 values? i mean, really20:54
mordredgood point20:54
fungiif you have that many values, you should just use arrays of arrays20:55
mordredor just alphabetical names of your favorite lemurs20:55
openstackgerritJames E. Blair proposed zuul/zuul master: Update react-json-view  https://review.opendev.org/c/zuul/zuul/+/77722320:56
mordredcorvus: woot20:56
avasssurprisingly few depdencies that were updated20:57
avassat least it feels like you usually get another 100 packages for free :)20:57
fungithe fix was also mentioned in a subsequently merged pr "bump patch to 1.20.5" so looks like they don't use git tags for their release process20:58
mordredcorvus: figuring that out has not helped me figure out why the config parser in the job is working differently than the local configparser20:58
corvusso demanding20:59
mordredbasically - I'm now printing out the .gitmodules that we're loading with configparser (it's down at the bottom of the output because it's line 40) - and you can see the sections are there that are later tossing "No section:" errors20:59
mordredI've been trying to reproduce with a local stretch container - and it's perfectly happy loading that content :(21:00
*** harrymichal has joined #zuul21:04
corvusmordred: try outputing config.sections() to see what the section names are represented as internally21:05
corvusmaybe there's some subtle difference (bytes? quotes?)21:05
fungithis is still the "why is indentation breaking ini file parsing" investigation?21:05
avassyep21:07
mordredwell - no, we're working around the indentation - that turned out ot be "old python on stretch" - but now we have a new ini file parsing issue :)21:09
mordredcorvus: ++21:09
avassmordred: my python2 is able to read the .gitmodules file after removing indentation but the sections are empty21:14
mordredavass: oh? that's great - that's more reproducing than mine21:14
mordredif I remove the indentation my python2 can read it and sections are not empty21:14
avassthis is on python 2.7.1821:15
mordredmine is 2.7.1321:16
avassoh...21:17
pabelangershould database dependencies be installed by default now with zuul?  Right now pymysql is missing, if you setup dburi21:17
corvuspabelanger: i think we have them set as extras21:20
corvusso you pick the right extras package based on which db you want to use21:20
corvuswe could just include both i guess21:20
pabelangerYah, remember that now21:20
pabelangerwe don't document that in upgrade notes21:20
pabelangerwe should likely add it or maybe move them into default requirements now21:21
corvusi'd be fine with that21:23
mordred++21:23
mordredI don't think pymysql or the pg one are terribly onerous install requirements21:23
*** ajitha has quit IRC21:24
fungipymysql doesn't have, at least. it's a pure python implementation. not sure about psycopg221:26
avassyeah I get the same on 2.7.1321:26
avassjust running the reading, formatting and loading of .gitmodules in the interpreter however21:27
fungilooks like we use psycopg2-binary, which vendors in its precompiled c libs21:28
fungiso not quite as clean as pymysql, but dependency-wise still not bad21:29
avassmordred: ooooooh21:34
avassmordred: I think the tempfile could be removed before configparser gets to read it21:34
avassmordred: moving the config.read inside the 'with tempfile' works a lot better21:36
mordredavass: aha! which is actually what I was doing locally21:39
mordredavass: THANKYOU - I don't know how long I would have stared at things and still not seen that21:40
openstackgerritPaul Belanger proposed zuul/zuul master: Include database requirements by default  https://review.opendev.org/c/zuul/zuul/+/77724521:40
avasswho would have thought temporary files were temporary :)21:40
mordredavass: ikr?21:40
fungiaggressively so at times, apparently21:44
avasstbh configparser should complain at least a little bit about the file you're telling it to read does not actually exist21:45
avassmordred: \o/21:46
corvusheh, yes... the section is missing, but that's not really the big picture is it?  :)21:46
fungishoe not tied (leg missing)21:46
mordredcorvus, avass : WOOT - I think the new python code is actually working now22:26
mordredhttps://ci.gerritcodereview.com/t/gerrit/build/033c128e41e3416e9073a3aab396b1f9/console#1/0/20/testnode shows actions that I think we expect22:27
mordredso - I think next step is putting the jobs back on that patch and letting it do real builds22:27
corvus\o/ ++22:30
avassoh are your doing specualive submodules22:30
avass?22:30
avass(since you're doing a 'mv ..' instead of initializing the submodules)22:31
corvusyeah, it actually kinda works with gating if you're using gerrit because gerrit will update the submodule reference when a commit lands.  so the gating atomicity is still guaranteed.22:31
avassoh interesting22:31
mordredwe've been doing it in a job for now to learn edge cases22:33
corvus(if you did this without gerrit, then it's basically a recipe for a guaranteed broken tree)22:33
avassyeah we're using submodules but setting the remote to a local copy checked out with required-projects22:34
avassto get around needing secrets to initialize the submodules mainly22:35
mordredcorvus: think it's ok for me to DOS gerrit with that change?22:36
mordred(I'm fairly confident it'll work this time)22:36
avassI guess superproject subscriptions is what configures gerrit to bump submodules ?22:37
mordredyup22:37
avasscool that could be useful22:38
mordredso the code we have there will either use the required-projects version of the repo if there is one and it's set up to do subscriptions - or it will check out the ref in the repo if it's not configured for subproject subscriptions22:38
corvusmordred: yep22:38
mordredcorvus: bombs away22:39
*** nils has quit IRC22:44
mordredcorvus: you know - now that this logic is in python - we could probably get rid of the gerrit_project_mapping22:44
mordredbecause you should be able to calculate it with relative paths - it's just not reasonable to do that in ansible directly22:45
*** kgz has quit IRC22:46
mordredalso - should be able to get rid of gerrit_root and make it fully generic - because you could really just iterate over every repo on disk looking for .gitmodules files- then process relative paths from the url parameter22:47
mordrednot doing that today - just saying I think we coudl do that - and then it would be reasonable to move that role into zuul-jobs22:47
corvus++22:48
avassmordred: ++22:48
*** kgz has joined #zuul22:50
mordredcorvus: https://ci.gerritcodereview.com/t/gerrit/buildset/708abf94afe74c9490d0fae1566beca2 - all green23:04
*** ikhan has joined #zuul23:06
*** harrymichal has quit IRC23:15
fungiand as you've discovered, it's not easy being green23:23
openstackgerritJames E. Blair proposed zuul/zuul-storage-proxy master: Allow mounting somewhere other than /  https://review.opendev.org/c/zuul/zuul-storage-proxy/+/77726123:39
openstackgerritJames E. Blair proposed zuul/zuul-storage-proxy master: Add an integration test to the image build  https://review.opendev.org/c/zuul/zuul-storage-proxy/+/77726223:39
corvusmordred: ^ i don't know if that job is going to work on the first try, but i know the process works (i ran the playbook locally).  that should be our first automated test of something zuul related with a live swift.23:40
mordredcorvus: \o/23:58

Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!