opendevreview | Takashi Kajinami proposed openstack/governance master: Retire heat-cfnclient: Retire Repository from the Governance Repository https://review.opendev.org/c/openstack/governance/+/905821 | 04:01 |
---|---|---|
timburke | clarkb, thanks for raising the eventlet issue here! i was working on other things for the most part today, only had time to write up the fix :-) | 05:14 |
opendevreview | Takashi Kajinami proposed openstack/governance master: Retire heat-cfnclient: Retire Repository from the Governance Repository https://review.opendev.org/c/openstack/governance/+/905821 | 05:51 |
tkajinam | I also found a strange error in manila unit tests which seems to be related to eventlet bump. | 06:40 |
tkajinam | https://zuul.opendev.org/t/openstack/build/bfbeee8014254a61a94a3e002b37b09c | 06:40 |
tkajinam | I'm not sure I have time to dig into this but in case anyone has any idea then that'd be great | 06:41 |
tkajinam | ^^^ fyi gouthamr fungi clarkb JayF | 06:42 |
slaweq | tc-members: one question about Monasca because I'm not sure which path should we choose. Do we want to retire it completly (option 1)(https://docs.openstack.org/project-team-guide/repository.html#retiring-a-repository) so no stable branches in the openstack/ namespace anymore too, or do we want to deprecate master branch only (option 2)(https://docs.openstack.org/project-team-guide/repository.html#deprecating-a-repository). | 11:36 |
slaweq | ? | 11:36 |
frickler | slaweq: IMHO all should be retired, since there will be noone to support the stable branches either | 11:42 |
slaweq | frickler that's what I though but wanted to make sure :) | 12:30 |
rosmaita | slaweq: i agree with frickler, complete retirement makes sense for monasca | 14:13 |
fungi | so that would include eol'ing all open stable branches, even if they're not yet out of standard maintenance | 14:14 |
spotz[m] | How would that affect current users? | 14:43 |
spotz[m] | In RDO we plan on leaving the inactive projects in for 1 cycle as long as they're not breaking the CI so folks have time to move off | 14:44 |
spotz[m] | We are warning they're unmaintained | 14:44 |
*** blarnath is now known as d34dh0r53 | 14:56 | |
fungi | i think it just means current users would no longer be able to pull from those stable branches and would instead have to checkout the corresponding eol tags. also any bug fixes would have to be patched in downstream, if any | 14:59 |
frickler | the question is who would push bugfixes if there are no maintainers around? making the branch go away seems like a good tool to alert users that may not notice the inactive state otherwise | 15:05 |
frickler | maybe part of the retirement process could be adding a red warning reno to all stable branches and publishing those renos before removing the repo content? | 15:06 |
JayF | It is worth noting we are operating under a policy that is already written. As I understand it our options are retirement, remaining inactive, or making it active. | 15:07 |
JayF | It also could lead to significant confusion if there are two active sets of repos for this, one for stable branches and one for master | 15:07 |
JayF | I cannot think of any scenario where it would be positive for openstack to maintain stable branches for monasca. | 15:07 |
frickler | well users who deployed the 2023.2 release of it could come complaining "you promised to maintain this for 18 months" and they would be right, be I also see no way around that | 15:09 |
JayF | I don't disagree with you, but I would rather us be upfront with them today than imply that we're keeping that promise when we aren't. | 15:10 |
JayF | This is why one of the things I strongly consider when thinking about if a project should be reactivated is if they would be able to commit to an 18-month support cycle. | 15:10 |
frickler | yes, that's the issue. we can only try to learn from that and try to be more active in marking projects as inactive earlier | 15:11 |
ianychoi | frickler: I am not a tc member but can add about google analytics. Previously during Atlanta PTG I got access to the ga when i was i18n ptl. i could get some insights for i18n and docs and I shared via mailing list like: https://lists.openstack.org/pipermail/openstack-docs/2017-May/009976.html . Now I don’t have the access but i think it will be great if ga provides insights for docs & i18n + will be great if some available information | 15:50 |
ianychoi | is publicized for healthy community. | 15:50 |
frickler | hberaud: I think https://review.opendev.org/q/topic:%22bump-eventlet%22 would warrant some global discussion, since even a single project merging this will essentially enforce this onto all others at least for coinstallable setups. cc tc-members | 16:26 |
frickler | and I think timburke's objection in https://review.opendev.org/c/openstack/swift/+/905945 is valid | 16:26 |
JayF | There has been chatter about this in -ironic and -oslo already, was going to make an ML thread about it in a few hours when I'm free. We need better comms around that for sure | 16:26 |
frickler | ah, o.k., thx | 16:27 |
fungi | couldn't it use environment markers? | 16:29 |
fungi | so only bump eventlet when using python 3.12 or later? | 16:29 |
JayF | there is value in bumping beyond py3.12 | 16:29 |
JayF | that is the context missing from comms | 16:30 |
JayF | I cannot have this conversation now, am closing IRC to avoid distraction even though I want to have it now :D | 16:30 |
* frickler goes reading osle and ironic backlog | 16:40 | |
*** gthiemon1e is now known as gthiemonge | 16:41 | |
JayF | frickler: https://lists.openstack.org/archives/list/openstack-discuss@lists.openstack.org/thread/7PJZ4WYRS2B3PIZ5KYFL6V3JWYL4V5FQ/ thanks for waiting :D I am working with a PT grad student this year, getting him onboarded to OpenStack and this morning was our pairing session :). (If his proposed changes land, I think he'll be up to 5 changes :D). Gotta get him warmed up for helping with the excising of Eventlet :D | 17:40 |
fungi | excising but also exorcising! | 17:55 |
fungi | gotta drive out those eventlet d(a)emons! | 17:55 |
gmann | slaweq: I agree on retiring the monasca completely, As there is no maintainers, we cannot really keep stable branches. That is same we did in past for any project/deliverables got retired due to no maintainer | 18:00 |
frickler | so essentially the message is "we as openstack community think that eventlet <0.34 is so broken that no deployment should use it any longer and we feel so strongly about this that we want to essentially enforce this onto all distributions" | 18:04 |
frickler | I can kind of agree to that, except I think a bit more testing (like maybe two weeks) of the whole setup might be good, also possibly giving time to make another patch release to fix the issues that were seen with swift (and manila)? | 18:06 |
frickler | the one thing that concerns me is how this transfers to stable branches that are running with those older eventlet versions that we are now shunning | 18:06 |
dansmith | I definitely don't agree with that sentiment if it doesn't include an asterisk of "on python3.12" | 18:07 |
fungi | and if it's "on python3.12" specifically, environment markers can be used to limit it to that | 18:08 |
dansmith | right | 18:08 |
dansmith | and since 3.12 isn't even on the runtime list for 2024.1 I really don't see the problem with allowing it in u-c but avoiding the minimum change | 18:09 |
dansmith | if there is some reason we can't lift the limit, then I'd delay the bump personally | 18:09 |
frickler | since we don't officially support py3.12 yet, this would kind of make the whole patch almost useless, exactly | 18:09 |
dansmith | further, if the just-released eventlet isn't in 22.04 we'd be requiring something that can't be satisfied by the distros right? | 18:12 |
dansmith | we don't bump requirements for libvirt the day it's released upstream because nobody can actually use that yet | 18:12 |
JayF | I take the question posed by those patches as "projects should strongly consider using latest eventlet" not "we require latest eventlet for all openstack" but I guess that's a distinction without a difference when you consider coinstallability | 18:13 |
JayF | availability in supported platforms is not something I had thought about either, and would be a good thing to point to on that ML thread | 18:13 |
dansmith | bumping the u-c is the "strongly suggest".. bumping the minimum across the board is a different story entirely | 18:15 |
JayF | well, by projects I mean, e.g. Ironic/Nova/Glance/etc individually | 18:16 |
JayF | but I see the flaw in that independent thinking combined with coinstallability | 18:16 |
JayF | and am now trying to reconcile how anything ever makes sense with many projects having version locks in their requirements.txt | 18:17 |
fungi | was proposing the per-project changes an attempt to get better test coverage to see what jobs would fail, since the cross jobs from the requirements repo don't really have much coverage? | 18:17 |
* JayF suspects it's possible for two openstack projects to make things not-coinstallable with independent project changes, without breaking any CI | 18:17 | |
frickler | well uca does carry library updates when they are required by openstack advancing beyond what is in stable lts, and I think zigo handles debian similarly | 18:17 |
fungi | in which case, dnm changes with a depends-on to the proposed constraints change in requirements would probably be equally effective | 18:18 |
frickler | we do not have lower constraints at all in reqs | 18:18 |
JayF | in global-reqs, no. In individual project reqs ,yes | 18:18 |
JayF | well, constraints the general term, as written in requirements files | 18:19 |
fungi | though most projects have discarded the lower-constraints.txt files/jobs | 18:19 |
frickler | I assumed fungi was talking about g-r, as in openstack/requirements repo | 18:19 |
JayF | not constraints-files the python/pip concept | 18:19 |
fungi | i was talking about the proposed change to increase the eventlet version declared in upper-constraints.txt in openstack/requirements | 18:19 |
JayF | frickler: I'm just trying to think about the system as a whole now, because I'm confused what is keeping the house of cards up :D | 18:19 |
* JayF applies more gorilla glue to the seven of diamonds | 18:20 | |
frickler | lots of black magic and a bit of luck? ;) | 18:20 |
JayF | I suspect a *lot* of the latter combined with manual noticing and effort here and there | 18:20 |
JayF | which occassional blows up ala tooz/sqla issues | 18:20 |
dansmith | I think one pillar that keeps the house of cards up is "conservative updates and careful attention" | 18:21 |
dansmith | not "I released this yesterday, here is 100 changes to bump the minimum" | 18:21 |
JayF | Yeah; we failed at that one a bit by not paying careful attention to Eventlet and taking action before it was in such a bad state. | 18:21 |
JayF | So now we're left with a handful of undesirable choices. | 18:21 |
frickler | the other thing to talk about is that we have had previous examples of such mass patching and iirc it was always agreed that discussing them with the community before submitting them would have been a good idea. and I wonder what we as tc can do to improve this culture | 18:22 |
dansmith | I don't think we're at the undesirable choices stage yet, because we've got a working runtime for the next release, and eventlet is fixe for 3.12 well in advance of our next runtime buimp | 18:22 |
JayF | I think part of what we've done already: disparte conversations happened, they were noticed by TC members and brought into a larger discussion. This larger discussion is surfacing larger issues. | 18:22 |
JayF | dansmith: My confidence in eventlet has eroded significantly through seeing the march of frightening bugs and bugfixes through there in the last months. I think that is a strong influence on my take here. Perhaps too much, honestly. | 18:25 |
dansmith | I understand, but I'm less concerned about it in the short term than I am a hasty u-turn | 18:27 |
JayF | Yeah. I think there's no right answer here, but I am less keen on the kinda middle ground my mailing list post posits after this conversation. | 18:29 |
JayF | We should probably be all-in or all-out, and it's after C-2 :/ | 18:29 |
JayF | I'll be curious to see what Herve's perspective is. I wonder if he saw something specific that made him want to push this upgrade. | 18:30 |
fungi | it seems like both the prior and latest eventlet versions could go in upper-constraints.txt split by an environment marker for python <3.12 and >=3.12 so that non-voting 3.12 testing can proceed unhindered in preparation for the next cycle when it should become voting, while not introducing regressions for the current cycle | 18:33 |
fungi | alternatively, a wip change to update fully to latest eventlet in upper-constraints.txt (without environment markers) could be more thoroughly tested with dnm depends-on changes in relevant projects that rely on it | 18:34 |
frickler | well that change has already been merged, do you propose to revert it? | 18:34 |
frickler | https://review.opendev.org/c/openstack/requirements/+/904147 | 18:35 |
fungi | i don't see why not, it wouldn't be the first time we've reverted a constraints update | 18:35 |
frickler | I'm not sure yet why it should be reverted. we do regularly bump all kinds of libs to their latest pypi versions | 18:36 |
frickler | that's assuming the known issues with swift and manila can be fixed otherwise | 18:36 |
JayF | Swift is already fixed/patches up both in Eventlet for a perm-fix and in Swift for a now-fix | 18:37 |
fungi | sure, forging ahead and fixing regressions as they crop up is clearly also an option (and our usual default one), it just sounded like there was some concern about doing it in this specific case | 18:38 |
JayF | fungi: I think the concern raised was around *actual changes to requirements.txt* being pushed to all openstack repositories, changing them to require eventlet=>0.34.3 | 18:38 |
fungi | somewhat to do with a lack of good cross-project test coverage for constraints updates | 18:38 |
JayF | I've not heard concerns over the constraints other than the technical issues which appear to be being worked from a technical perspective. | 18:39 |
fungi | JayF: oh, well yes it seemed to me that was just a mistake | 18:39 |
fungi | i assumed we were already past that | 18:39 |
fungi | (specifically that the per-project requirements.txt changes will get abandoned as incorrect process) | 18:39 |
frickler | I think the reqs team is always open to add more cross testing if it makes sense, but I don't think running every job for every project is reasonable | 18:40 |
JayF | I don't want to assume we can get past anything in two hours while some of our community (including likely the person who authored the change) is asleep. | 18:40 |
clarkb | what is the motivation of updating requirements.txt? | 18:41 |
clarkb | if its merely that users will be better off then you can communicate that to them without enforcement via pip | 18:41 |
clarkb | also pip will eb default install the newest thing | 18:42 |
fungi | clarkb: theoretically? to indicate that those projects will in no way ever work correctly with any earlier version of eventlet, which seems patently untrue and why we don't normally do that | 18:42 |
clarkb | right | 18:42 |
JayF | I posted a whole thing to the mailing list about why I think it's a decent idea to do it | 18:42 |
JayF | but only Herve can say why *he actually did it* | 18:42 |
clarkb | so for users using pip they'll already get the new thing by default (even when using constraints). For everyone else the distros can decide if they want to update or not | 18:42 |
clarkb | oh sorry catching up on email now. I just made sure I can walk from my front door to the sidewalk without dying | 18:43 |
clarkb | much ice was chipped off the concrete | 18:43 |
fungi | distro package maintainers already can (and do) completely ignore the versions of things in requirements.txt files when they need to | 18:43 |
fungi | and in our central constraints list too | 18:44 |
frickler | so one question might be do we want to mass-abandon/-1/wip all those patches before unaware projects merge them and we later have to revert? kind of like what we had with the drop of py3.8 support earlier | 18:45 |
fungi | it's not my call, but seems like a prudent safeguard | 18:46 |
fungi | project maintainers are on the hook anyway to update requirements.txt files if they merge changes that they know will require a newer version of some dependency, which is how that's normally handled | 18:47 |
JayF | I replied to my list email: https://lists.openstack.org/archives/list/openstack-discuss@lists.openstack.org/message/TO7F5HJJP2RBVCUNTM2G55UPES2MLEME/ suggesting that projects hold off on merging these changes until a larger discussion happens. | 18:50 |
JayF | I do not personally intend on mass abandon/-1/wip'ing any patches outside of Ironic. | 18:50 |
fungi | it could certainly wait for hberaud to do that, and then if projects happen to merge any of them in the meantime, recommend that they merge a revert | 18:51 |
JayF | fungi: https://review.opendev.org/c/openstack/networking-generic-switch/+/905904 e.g.? :D | 18:52 |
fungi | indeed | 18:52 |
fungi | ot | 18:52 |
fungi | er | 18:52 |
fungi | it's too bad other changes don't get such quick turn-around on reviewing ;) | 18:53 |
JayF | Do you submit changes to Ironic frequently? | 18:53 |
JayF | We have extremely good review turnarounds. | 18:53 |
fungi | clearly not! | 18:53 |
fungi | it's more that the projects i'm on the hook for reviewing don't get nearly the attention from me that they deserve | 18:54 |
JayF | As a project, we prioritize reviewing changes -- it avoids rebase churn, helps ensure cores have a large amount of context on new code. It's really nice. Probably somewhat of a luxury based on us being in the "sweet spot" size wise. | 18:54 |
timburke | i'd just note that swift has no now-fix at the moment -- eventlet 0.34.3 broke our gate in a way that the cross-project testing missed. yesterday, i wrote a patch to get swift tests passing with tip-of-master eventlet, but there hasn't been a release that includes https://github.com/eventlet/eventlet/pull/890 yet | 18:54 |
timburke | so our gate will remain broken until there's a u-c change to take eventlet back to a working version, or forward to some new release | 18:54 |
JayF | timburke: oh, I thought there was a patch in swift side that monkey-patch fixed the broken code? | 18:55 |
timburke | there was for eventlet 0.34.2, but not 0.34.3. but at least now it's just some unit test failures, rather than segfaults! | 18:56 |
fungi | and we can in theory merge a revert of the constraints bump, however that's also going to break networking-generic-switch and any other projects that merge the proposed requirements.txt minimums between now and when the constraints revert lands (after which the requirements.txt changes will fail in the gate at least) | 18:56 |
frickler | so we should actually revert the u-c bump and that should implicitly block the requirements.txt updates | 18:56 |
frickler | plus what fungi says | 18:56 |
JayF | timburke: passing this ( https://github.com/eventlet/eventlet/pull/890#issuecomment-1896448730 ) on to itamar downstream as well | 18:56 |
timburke | thanks | 18:57 |
timburke | given how there were another three PRs up shortly after my fix, i figured another release would be in-bound before so very long, and so i wasn't planning on freaking out until next week or so :-) | 18:59 |
frickler | https://review.opendev.org/c/openstack/requirements/+/905905 ... I'm essentially eoding now, maybe someone else can review and approve. tonyb prometheanfire or some infra-root? | 19:00 |
fungi | i can pull my tact sig chair hat on more firmly, sure | 19:00 |
clarkb | not sure if this was mentioned but we have eventlet in zuul so you can depends on changes to it | 19:02 |
clarkb | what this means is that swift and manilla et al can depends on the fixes and confirm that their test suites are happy and not rely on the multi pass step through requirements first | 19:02 |
clarkb | we can also theoretically act as third party ci on eventlet but that requires the eventlet project in github to do things on their end | 19:03 |
clarkb | or maybe not. I guess we'd mostly need an ack from them that they want that to ahppen | 19:03 |
clarkb | we only need to configure the app if gating in github which we don't do | 19:03 |
clarkb | adding the app is a nice thing either way as it improves api rate limit quotas | 19:04 |
fungi | i think any jobs wanting to test cross-project dependencies on eventlet still need variants with the eventlet repo as a required-project though | 19:04 |
clarkb | fungi: yes and no. If you explicitly use depends on and it is a tox job then the siblings code path will install the correct version | 19:05 |
fungi | and should probably be set non-voting since they'd be testing latest eventlet default branch tip rather than the latest release from pypi | 19:05 |
clarkb | other jobs like devstack would need it as a required project and devstack would need to be updated to add eventlet to the install from source list | 19:06 |
clarkb | LIB_FROM_GIT or whatever it is called | 19:06 |
fungi | clarkb: wouldn't switching automatically from release to default branch create a potential for deadlock? i'm pretty sure job definitions have to opt into consuming a project from source | 19:07 |
clarkb | fungi: yes they do thats what I'm saying. You would need to modify devstack to support it and then have jobs that set eventlet in LIBS_FROM_GIT | 19:07 |
clarkb | but depends-on in tox jobs will work today without anything changing due to siblings | 19:07 |
fungi | the devstack job does auto-populate LIBS_FROM_GIT from required-projects | 19:07 |
clarkb | fungi: ya but LIBS_FROM_GIT has explicit handling for each lib I think | 19:08 |
clarkb | so its a noop until you explicitly add support | 19:08 |
fungi | probably worth moving this to the zuul matrix channel, but if your project has only unit test jobs and can merge a change with a depends-on to a merged (but unreleased) change in another project while normally installing that project from a release on pypi in the same job, that would break the ability to merge future changes until the next release | 19:09 |
clarkb | you cannot merge because of the depends on | 19:09 |
clarkb | oh I see what oyua re saying | 19:10 |
fungi | depends-on only blocks changes from merging while the depended-on change has not yet merged | 19:10 |
clarkb | ya so thats a potential gap in the tox sibling sstuff | 19:10 |
clarkb | but I don't think its a big enough concern when used deliberately | 19:10 |
clarkb | you would have a specific change to test that the upstream fixes work and not make any changes downstream | 19:10 |
fungi | my understanding is that jobs have to explicitly opt into consuming a dependency from source (via required-projects list), and then the siblings role uses that same list to decide what gets installed into the tox venv | 19:11 |
fungi | which is how zuul avoids the deadlock i described | 19:11 |
clarkb | fungi: yes but depends on creates an implicit entry in required-projects iirc | 19:12 |
fungi | i find that surprising, if so | 19:12 |
clarkb | pretty easy to test with a throwaway change | 19:12 |
clarkb | or look at old changes. /me starts there | 19:15 |
clarkb | fungi: https://zuul.opendev.org/t/openstack/build/5eda057f8cb74be99534739252758610/log/zuul-info/inventory.yaml#241-250 | 19:17 |
clarkb | that is a glance tox-py38 job with a depends on grenade. I'm fairly certain that job doesn't normal have grenade as a required project but it is in the project list | 19:17 |
fungi | clarkb: it checked out the repository, but did not add it to required-projects based on what tox-siblings installed: https://zuul.opendev.org/t/openstack/build/5eda057f8cb74be99534739252758610/console#4/0/10/ubuntu-focal | 19:24 |
clarkb | fungi: siblings won't install grenade because it isn't a dep of the project | 19:25 |
fungi | granted, i doubt a glance tox-py38 job would even want to install grenade, so this is probably an incomplete test | 19:25 |
clarkb | yes this is just showing that a depends-on not in your required-projects goes in the projects list | 19:25 |
clarkb | then if you read the siblings code it should install any project from source listed in the projects list | 19:26 |
clarkb | but onyl if that project is a dependency of your project | 19:26 |
clarkb | *a pip installed python dependency | 19:26 |
fungi | you're sure it gets reflected in required-projects, not just replicated to the node in case it's in required-projects? | 19:26 |
clarkb | fungi: zuul refines required-projects and depends on into that projects list. The siblings code doesn't really know about required-projects just the zuul.projects dict | 19:27 |
fungi | got it. still surprised and somewhat worried then, in that case | 19:27 |
clarkb | and looking at the inventory file I don't think zuul distinguishes where projects entries come from once running jobs | 19:28 |
fungi | definitely not a behavior i would want in a job | 19:28 |
clarkb | I think it is maybe less than ideal, hwoever the act of using a depends on is deliberate | 19:28 |
clarkb | and developers and reviewers should have some understanding of what the end result is | 19:29 |
clarkb | unlike say a wedge where two people acting independently without knowledge of one another can stop the assembly line | 19:29 |
fungi | while true, it sort of flies in the face of zuul's promise to not merge broken changes | 19:30 |
clarkb | zuul does prvent those. But this is a deliberate informed act | 19:30 |
fungi | i suppose this is less a gap in zuul itself, and more a gap in the tox abstract job (or tox-siblings role)? | 19:31 |
clarkb | ya its tricky because the change itself isn't broken if its dependencies merge. But this is a corner case where zuul doesn't actually know that the state of master is broken as a result | 19:31 |
clarkb | yes exactly. The job is saying all is well. That is the signal zuul uses to determine if a change is broken or not | 19:31 |
clarkb | its possible that improving zuul would allow for smarter siblings use, but off the top of my head I can't really think of a good way to address this without the current risk | 19:32 |
clarkb | you want to be abel to test if a library you normally consume from releases works from tip of master or from bug fixes | 19:32 |
fungi | right, in the past we did that by having separate non-voting jobs that used the project from source (by extending required-projects) | 19:36 |
clarkb | ya and in many cases people completely ignored the results. That said with sibligns I think few people realize its an easy option and don't use it to get the needed info either | 19:38 |
timburke | ...which is surely a thing i ought to do for swift+eventlet, now that i think about it... | 19:38 |
fungi | so just to circle back around after hashing this out in the zuul matrix channel, you do still need to add the project to required-projects in order to get the siblings install behavior, and so do need a separate (probably non-voting) job variant that puts the eventlet repo in required-projects for testing it from source and with cross-project dependencies | 19:55 |
clarkb | you wouldn't need a separate job if you were always installing the particular code from source rather than releaes | 19:56 |
clarkb | just to clarify since some projects may use siblings that way | 19:56 |
fungi | right, but i wouldn't recommend that except for situations like devstack installing from source for things in the integrated projects queue | 19:57 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!