| opendevreview | Ian Wienand proposed opendev/glean master: testing: remove centos7 and 8 https://review.opendev.org/c/opendev/glean/+/921911 | 06:29 |
|---|---|---|
| opendevreview | Ian Wienand proposed openstack/diskimage-builder master: Disabling CentOS Stream 8 jobs https://review.opendev.org/c/openstack/diskimage-builder/+/921823 | 06:42 |
| opendevreview | Artem Goncharov proposed openstack/project-config master: Add publish openapi specs job https://review.opendev.org/c/openstack/project-config/+/921934 | 10:17 |
| frickler | infra-root: seems that gerrit is no longer setting the "attention-needed" flag on voting, which is kind of breaking my workflow | 12:26 |
| *** tosky_ is now known as tosky | 12:28 | |
| fungi | frickler: any idea when that changed? i guess between 3.8 and 3.9? | 12:36 |
| frickler | must be pretty recent I think, might be related to the weird attention set UI look update that corvus mentioned yesterday. I can try to go through some changes in order to get some better bounds | 12:59 |
| fungi | okay, so it could be a bug in the 3.9 branch which came in with a more recent rebuild too | 13:00 |
| frickler | hmm, actually I find that I'm not so sure anymore, might be 3.8 or even older. at least this is documented https://gerrit-review.googlesource.com/Documentation/user-attention-set.html#bots and the corresponding change I've found is from 2020, so it may have been a bug somehow. one thing I'm not sure about for example is how zuul gets identified as bot (or does it even?) | 13:27 |
| frickler | ah, it is classified as "members of the "Service Users" group", which zuul indirectly is | 13:31 |
| fungi | oh, is your observation that zuul's votes don't set attention-needed? | 13:31 |
| rpittau | mmm the dib-functests job is failing with a 404 in the fedora section but it looks like the image is there, is it a known issue? | 13:38 |
| fungi | rpittau: i don't see a recent mention of anything in #openstack-dib at least, but i'll take a look at a failure and see if anything jumps out at me | 13:40 |
| rpittau | thanks fungi :) | 13:40 |
| fungi | https://zuul.opendev.org/t/openstack/build/53f75080df214f559b5b9f5d4177f5d7/log/logs/fedora_build-succeeds.FAIL.log#300 | 13:42 |
| fungi | there's an example | 13:42 |
| fungi | it's attempting to download https://download.fedoraproject.org/pub/fedora/linux/releases/37/Cloud/x86_64/images/Fedora-Cloud-Base-37-.x86_64.qcow2 | 13:42 |
| fungi | https://na.edge.kernel.org/fedora/releases/37/README says "The contents of this directory have been moved to our archives" | 13:43 |
| rpittau | yay | 13:44 |
| fungi | looks like 39 is the oldest release still served | 13:44 |
| fungi | so probably dib needs to increase the minimum fedora version it's testing with | 13:44 |
| rpittau | remove F37 alongside CS8 then ? | 13:44 |
| fungi | yeah, that's what i'd do anyway | 13:44 |
| rpittau | ok, I'll update my patch | 13:45 |
| rpittau | thanks again :) | 13:45 |
| fungi | yw | 13:45 |
| frickler | fungi: (re: attention set) yes. I was pretty sure this did still happen some months ago at least, now I'm not so sure anymore | 13:45 |
| frickler | like my workflow was to trigger a recheck and then wait for the patch to show up again in the "needs attention" section once zuul has returned with fresh results | 13:46 |
| fungi | got it. i have finely-tuned mail filters for that purpose, so had never tried the gerrit feature | 13:46 |
| frickler | when the attention-set feature came up, I was first annoyed by it, too. but finally I switched completely to using that and stopping getting any emails from gerrit | 13:47 |
| frickler | but now it seems gerrit maintainers have once again decided that this is not what they want me to do | 13:48 |
| opendevreview | Riccardo Pittau proposed openstack/diskimage-builder master: Disabling CentOS Stream 8 and Fedora 37 jobs https://review.opendev.org/c/openstack/diskimage-builder/+/921823 | 13:50 |
| frickler | o.k., weird, now for https://review.opendev.org/c/openstack/requirements/+/921656 I was just added to the attention-set again, and the only update was from zuul | 14:34 |
| frickler | and horizon just dropped py3.8 support, breaking reqs testing :-/ | 14:38 |
| fungi | probably a discussion for #openstack-tc but i thought previous experiences had indicated that decoupling the removal of testing and the introduction of requires_python bumps was necessary to avoid undue impact to integration testing | 14:53 |
| fungi | and to hold changes for requires_python until closer to the end of the release cycle | 14:54 |
| * JayF would bet all of the money that whoever made that change was completely unaware of that conversation re: removing requires_python ever happening | 14:55 | |
| hashar | frickler: Zuul voted with a negative score and in this case the owner of the change (or the uploader if different than owner) is added back to the attention set. I guess cause CI failing requires your attention ;) | 14:58 |
| hashar | frickler: ref with code: https://phabricator.wikimedia.org/T337648#8888627 | 14:58 |
| hashar | that task was about Gerrit adding everyone to the attention set (back in Gerrit 3.7 maybe??) | 14:59 |
| hashar | unrelated, I have a patch to enable git to colorize the message banner emitted by Gerrit when one sends a patch for review. | 15:06 |
| hashar | https://review.opendev.org/c/opendev/git-review/+/914000 :) | 15:07 |
| hashar | https://phabricator.wikimedia.org/T359981 has a screenshot of the current state (no color) and a comment below with a screenshot showing `SUCCESS` being colorized by the git client | 15:07 |
| frickler | hashar: oh, so this really changed with our update to 3.9 and I was not misremembering, thx for the pointer. now we can start discussing whether we might want to revert that "fix" or make if configurable for our deployment | 15:13 |
| clarkb | I don't think we should carry local patches (I stuck to that with the local docs build stuff too which is why it didn't happen for us). THe ideal would be to upstream somethign to make it configurable or maybe even configurable by user? | 15:15 |
| hashar | frickler: and potentially open a discussion with upstream. Though myabe that has changed again with 3.10 or a later version. It seems they keep iterating based on the feedback of their internal teams | 15:15 |
| fungi | a user-specific option would probably be the best outcome | 15:15 |
| fungi | and yes, gerrit upstream seems to feel that they're developing a necessarily opinionated workflow tool rather than a bunch of workflow options for users to implement custom workflows on top of | 15:16 |
| hashar | pretty much yes | 15:19 |
| hashar | I can not blame them for enforcing a single workflow, that makes things easier in the long run | 15:20 |
| fungi | yeah, i'm not saying it's a good or bad thing, it's simply their choice as the maintainers of the software | 15:24 |
| opendevreview | Tony Breeds proposed opendev/zone-opendev.org master: Add new openmetal mirror https://review.opendev.org/c/opendev/zone-opendev.org/+/921895 | 15:54 |
| opendevreview | Merged opendev/zone-opendev.org master: Add new openmetal mirror https://review.opendev.org/c/opendev/zone-opendev.org/+/921895 | 16:02 |
| opendevreview | Tony Breeds proposed opendev/system-config master: DNM: Initial dump or mediawiki role and config https://review.opendev.org/c/opendev/system-config/+/921322 | 17:38 |
| tonyb | I'm not going to be able to verify 921896: Add openmetal mirror node to the inventory | https://review.opendev.org/c/opendev/system-config/+/921896 after the deploy/prod runs | 17:44 |
| fungi | i can check it | 17:45 |
| opendevreview | Merged opendev/system-config master: Add openmetal mirror node to the inventory https://review.opendev.org/c/opendev/system-config/+/921896 | 17:49 |
| corvus | hi git-review people; in looking at the test results for https://6465d641e8d90881111c-e9283d1668aa96fc6a81b85ada542fce.ssl.cf5.rackcdn.com/921879/1/check/nox-py311-gerrit-default/f492a5f/testr_results.html which is from https://review.opendev.org/921879 i see that we have a test that verifies that the workflow "make a patch on master; download it; make a backport to stable; download it" is executed and the two download locations go to different | 18:15 |
| corvus | local branches that don't conflict | 18:15 |
| corvus | the thing about that test is that the branch collision is avoided because by default the backport patch would get a different topic, and the topic is included in the local branch name for the download | 18:16 |
| fungi | yeah, i can see where that's problematic logic | 18:16 |
| corvus | it strikes me that is encoding a lot of workflow particulars in those local branch names. and of course, it doesn't work if you have "notopic" set (and that change is attempting to make that the standard behavior) | 18:17 |
| fungi | but also maybe the underlying intent of that test was to make sure git-review is creating branches named after the topics | 18:17 |
| corvus | let me get the local branch names real quick... | 18:17 |
| corvus | the first download goes to review/administrator/I28fe02a53b773bcad5c6937a41182fc4e9544ab1 because it's the master branch and we don't set the topic on master branch changes; the second one goes to review/administrator/mybackport because the topic was mybackport. | 18:18 |
| clarkb | I dunno how much of that is workflow vs trying to avoid us needing to explain the reflog to people and relying on all the info present to create uniquish entries that also make sense in a git branch -a situation | 18:18 |
| Guest8493 | I do think that was the underlying intent - even though that may not longer be a super helpful intent. I'd also argue the the non-topic behavior of just making a branch named after the change should be a fine behavior to do all the time - rather than sometimes change/XXX and sometimes topic | 18:19 |
| clarkb | given that statement maybe we should just put the 7digit sha prefix in there | 18:19 |
| Guest8493 | wait - it's creating "I28fe02a53b773bcad5c6937a41182fc4e9544ab1" ? | 18:19 |
| corvus | fungi: it could be -- i sort of interpreted as wanting to ensure that a user could download both the master and backport branches of a change without having to delete local branches | 18:19 |
| Guest8493 | when I do git review -d I get review/monty_taylor/921605 | 18:19 |
| Guest8493 | which totally makes sense | 18:20 |
| JayF | Monty you know your nick is Guest8493, right? :D | 18:20 |
| fungi | might depend on the gerrit version | 18:20 |
| clarkb | Guest8493: oh ya that may be better than the changeid or sha prefix | 18:20 |
| clarkb | since it is unique per change and ties back to something easily found | 18:20 |
| fungi | i think gerrit recently changed how it reports back change identifiers? | 18:20 |
| Guest8493 | JayF: I am, in fact, curious where that monty_taylor is coming from | 18:20 |
| corvus | mordred: you're hitting the same code path; this test uses the gerrit change-id | 18:20 |
| corvus | mordred: so if you did `git-review -d I7112c8c58b3de204eb251cccb74642442ae053ee` you'd get what the test gets | 18:21 |
| clarkb | anyway my suggestion to address this would be to always use the change number for git review -d branch creation since we know those are unique ish (if you ahve two gerrit upstreams you may collide but otherwise it should be safe) | 18:21 |
| corvus | (i also assume this test was written when everyone thought the numbers would go away) | 18:21 |
| Guest8493 | fascinating. I have literally never imagined using the Change-Id in git review -d :) | 18:21 |
| Guest8493 | ah - yes. that makes sense | 18:21 |
| fungi | anyway, all this has got me thinking that the autotopic from branch name functionality sort of went hand-in-hand with the functionality to name local branches after the topics of retrieved changes | 18:21 |
| corvus | mordred: yeah, and if you do use the gerrit change id, you have to give the extra branch argument; so the second time this test runs the command it's `git-review -d I7112c8c58b3de204eb251cccb74642442ae053ee testbranch` | 18:22 |
| fungi | and that if we're turning off one we should probably turn off both | 18:22 |
| Guest8493 | corvus: that seems harder. but - hey, TIL | 18:23 |
| corvus | so review/921605/01 ? review/monty_taylor/921605/01 ? | 18:23 |
| Guest8493 | I never get a local branch with a /01 - just 921605 | 18:23 |
| corvus | yeah sorry, lemme splain | 18:23 |
| fungi | including the revision number in branch names is likely to cause confusion if those branches continue to get reused | 18:24 |
| corvus | the current behavior will do review/author/something for the current patchset; and review/author/something-patch1 if you do git-review -d 921605,1 | 18:24 |
| corvus | so i was also jumping ahead to try to make that consistent | 18:25 |
| corvus | but we can just leave the current behavior the way it is | 18:25 |
| fungi | a common workflow for me is to git review -d some change, commit some edits, then git review the result. or even -d a base change and then -x to cherry-pick multiple other changes to stack on top of it (so at that point even the current topic-less behavior of using the change number gets muddled... guess i'm out of good ideas) | 18:25 |
| corvus | okay, so always including the patch number would be weird; we should just leave that the way it is for now | 18:27 |
| fungi | i'm noticing that we also lack a feature to tell git-review -d what branch name we want it to create | 18:27 |
| fungi | so if you want to start a new topic branch from a change in gerrit, i guess the solution is to -d the change and then branch -m (or -M) it to your desired topic branch name | 18:28 |
| fungi | i suppose that's straightforward enough that it doesn't need to be wrapped as a convenience in git-review | 18:29 |
| Guest8493 | nod. gotit - I forgot about -d 921605,1 | 18:30 |
| Guest8493 | I agree - I think keeping that behavior when the person explicitly is doing a revision seems sane | 18:30 |
| Guest8493 | but I'm all about scrapping auto-topic - and branchname-from-topic | 18:30 |
| Guest8493 | also - I am now no longer sure why author is in the branch name - fwiw | 18:30 |
| Guest8493 | which I'm mostly noting is weird because I really don't know why monty_taylor is my author slug :) | 18:31 |
| corvus | yeah that wouldn't have been my guess :) | 18:31 |
| Guest8493 | I vote for review/921605/01 in the new world order | 18:32 |
| Guest8493 | and review/921605 | 18:32 |
| corvus | i like that too, i'll propose that | 18:33 |
| fungi | agreed, i think if -d is given an explicit revision number then including it in the branch name is fine | 18:34 |
| fungi | if it's given an unqualified change number only, then including the revision number could be weird | 18:34 |
| corvus | real bikeshed here: /01 or /1 ? :) | 18:35 |
| corvus | for the patchset | 18:35 |
| fungi | /1 because there's no effective limit on the number of revisions in a change | 18:35 |
| corvus | probably /1 i would think | 18:35 |
| corvus | yeah | 18:35 |
| corvus | fatal: cannot lock ref 'refs/heads/review/1/2': 'refs/heads/review/1' exists; cannot create 'refs/heads/review/1/2' | 18:36 |
| fungi | otherwise you have /01,..,/09,/10,...,/99,/100,... | 18:36 |
| tonyb | yes please '/1' | 18:36 |
| corvus | i was not expecting that error | 18:37 |
| corvus | i guess that's why it was '-patch1' before | 18:37 |
| fungi | and now we know why we included the author? | 18:37 |
| corvus | no i think it was using '-patch1' for other patches that did that | 18:37 |
| fungi | ohhh | 18:37 |
| corvus | i guess we should still do that | 18:37 |
| fungi | yeah never mind, "review" is not the name of a remote | 18:38 |
| fungi | i was confused | 18:38 |
| corvus | yeah.... git-review is a little loose with the change/review terminology | 18:38 |
| fungi | i have a feeling gerrit was too back when we made it | 18:39 |
| corvus | these tests make my computer warm! | 18:43 |
| corvus | https://imgur.com/screenshot-1rDvs9r | 18:43 |
| corvus | that is really doing a good job maxing all the cores | 18:44 |
| opendevreview | James E. Blair proposed opendev/git-review master: Stop setting a default topic on new changes https://review.opendev.org/c/opendev/git-review/+/921879 | 18:45 |
| corvus | that passes tests locally ^ | 18:45 |
| fungi | yes, i was not a fan of the approach of "unit" testing git-review by spinning up a bunch of gerrits on your machine, but since i didn't have time to write the test harness for it, we got what was contributed by someone who did | 18:58 |
| fungi | which is still an improvement over the manual exercising we used to do with it, at least | 18:58 |
| corvus | i kinda dig it. :) | 19:14 |
| AlbinVass[m]1 | I thought git review set the topic deliberately so you could easily merge a branch with submitwholetopic. Now I've never wanted it to do that so I use -Ta lot | 19:43 |
| fungi | AlbinVass[m]1: git-review's autotopic behavior pre-dates gerrit's submitwholetopic feature by about a decade | 19:45 |
| fungi | tonyb: 921896 deployed successfully about 15 minutes ago | 19:46 |
| fungi | and http://mirror.iad3.openmetal.opendev.org/ has the expected content | 19:47 |
| fungi | https is working fine too | 19:47 |
| fungi | [and there was much rejoicing] | 19:48 |
| fungi | do we have a nodepool launcher config addition change up yet? | 19:49 |
| fungi | not immediately spotting any | 19:49 |
| tonyb | no we don't but feel free to create one | 19:50 |
| tonyb | fungi: thanks for checking | 19:51 |
| fungi | will work on that in a sec | 19:51 |
| tonyb | cool beans | 19:52 |
| clarkb | fungi: we need images first but ya not sure if we've got any changes for that yet | 19:55 |
| fungi | oh, right need to add them to builder configs too for upload | 20:09 |
| fungi | is there any reason to split that into more than one config? | 20:09 |
| clarkb | fungi: I think you can do change 1 with both buildres and launchers and set max-servers to 0. Then maybe followup changes to actually put the provider into use | 20:09 |
| clarkb | that way we can spot check things by manually booting images first and ensure glean works in that cloud | 20:10 |
| fungi | for launchers, should i associate it with nl02 like was done for the old "inmotion" environment? | 20:11 |
| fungi | looks like nl02 is currently providerless | 20:11 |
| clarkb | fungi: yes that launcher has no providers in it so we should add it there | 20:11 |
| fungi | cool | 20:12 |
| opendevreview | Jeremy Stanley proposed openstack/project-config master: Add OnMetal to Nodepool and Grafana https://review.opendev.org/c/openstack/project-config/+/921987 | 20:29 |
| fungi | i think that's ^ got it | 20:29 |
| fungi | nope, wrong cloud name | 20:30 |
| * fungi sighs | 20:31 | |
| opendevreview | Jeremy Stanley proposed openstack/project-config master: Add OpenMetal to Nodepool and Grafana https://review.opendev.org/c/openstack/project-config/+/921987 | 20:34 |
| fungi | that should be better | 20:34 |
| clarkb | that looks good to me. One thing we should check while testing node boots is that we can direct attach the nodes to the external network as described in the clouds.yaml. I think this may happen automatically if we test node boots from a nodepool node and use the clouds.yaml there | 20:37 |
| fungi | yeah, i think the sdk is supposed to just figure that out from the config | 20:52 |
| clarkb | yup, but if we have the names wrong or if the cloud network stack has shifted and doesn't support it it will fail so we should check it before trying to launch nodes. Unless we just want nodepool to go for it and debug from there | 20:53 |
| clarkb | fungi: do we want to go aehad and land 921987 or wait for another review? My ability to keep an eye on stuff is quickly fading though | 21:33 |
| Guest8493 | ohwow - I was just looking through other git-review patches and learned about git-worktree | 21:51 |
| Guest8493 | and by wow - I mean - wow, git now supports the bzr workflow! | 21:51 |
| corvus | that could be really handy for, say, someone who does a lot of running of unit tests on development branches and master :) | 22:01 |
| fungi | clarkb: i can self-approve and check in on it after it deploys to make sure images upload, though may not test booting a node tonight | 22:14 |
| clarkb | fungi: ya I think image uploads may be slow too as we do them serially now iirc | 22:15 |
| fungi | so just as well then | 22:15 |
| clarkb | corvus: I've skimmed the zin stack really quickly and one thing that occurs to me (which may or may not be covered by the spec I can't recall) is wondering if cleaning out old nodepool images/labels will be more difficult with zuul's config checks | 22:26 |
| clarkb | today we can remoev stuff from nodepool and then let jobs fail when the ndoe doesn't exist. Are we going to not be able to do that without a force merge if jobs still rely on labels? | 22:27 |
| corvus | clarkb: i have 2 thoughts on that (and i think there's flexibility here): | 22:29 |
| corvus | 1) i'm pretty sure the most strict path we could take would still let you define a label object but not use it in any provider. so we can still remove labels from providers, but keep `label` entries until zero jobs reference it. but if those jobs try to run, then they get a node error | 22:30 |
| corvus | 2) a more lenient path would be not to syntax check that job labels exist in the system; that would be current behavior | 22:31 |
| corvus | i think neither one of those approaches removes our ability to remove images from providers | 22:31 |
| clarkb | ok that makes sense and I agree that continues to enable us to clean up the important bits that consume many cpu cycles and gigabytes of disk | 22:32 |
| corvus | option 1 sounds like more work, but it would pretty much make "label" objects behave like "nodeset" objects, which sort of makes sense. | 22:32 |
| corvus | (since that's how we use labels and nodesets in opendev; pretty much 1:1) | 22:32 |
| clarkb | ya | 22:32 |
| opendevreview | Merged openstack/project-config master: Add OpenMetal to Nodepool and Grafana https://review.opendev.org/c/openstack/project-config/+/921987 | 22:33 |
| clarkb | mostly I'm happy if we can somewhat decouple the lack of maintenance in projects from the maintenance of the CI system | 22:34 |
| corvus | ++ | 22:34 |
| clarkb | fungi: region iad3 is not valid needs to be IAD3 | 23:06 |
| fungi | d'oh! so it did change... | 23:06 |
| clarkb | sorry I shoukld've cauight that as frickler pointed it out on earlier changes for clouds.yaml | 23:06 |
| * fungi sighs | 23:06 | |
| clarkb | you can see the exceptions in the builder log if you are curiouis | 23:06 |
| clarkb | and I really can't type this afternoon | 23:06 |
| fungi | no worries, and thanks for spotting it! | 23:06 |
| opendevreview | Jeremy Stanley proposed openstack/project-config master: Correct OpenMetal region from iad3 to IAD3 https://review.opendev.org/c/openstack/project-config/+/921991 | 23:09 |
| fungi | clarkb: tonyb: ^ | 23:09 |
| fungi | if you have time | 23:09 |
| clarkb | done | 23:10 |
| fungi | thanks! | 23:10 |
| opendevreview | Merged openstack/project-config master: Correct OpenMetal region from iad3 to IAD3 https://review.opendev.org/c/openstack/project-config/+/921991 | 23:55 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!