| opendevreview | Michael Still proposed openstack/nova-specs master: Propose a new API microversion for a spice-direct console. https://review.opendev.org/c/openstack/nova-specs/+/915190 | 09:16 |
|---|---|---|
| opendevreview | Takashi Kajinami proposed openstack/nova master: Remove old excludes https://review.opendev.org/c/openstack/nova/+/917594 | 09:21 |
| opendevreview | ribaudr proposed openstack/nova-specs master: Update and Re-propose "Allow Manila shares to be directly attached to an instance when using libvirt" for Dalmatian https://review.opendev.org/c/openstack/nova-specs/+/913997 | 10:06 |
| opendevreview | Takashi Kajinami proposed openstack/placement master: Remove old excludes https://review.opendev.org/c/openstack/placement/+/917608 | 10:15 |
| opendevreview | Takashi Kajinami proposed openstack/os-vif master: Remove old excludes https://review.opendev.org/c/openstack/os-vif/+/917611 | 10:17 |
| opendevreview | Takashi Kajinami proposed openstack/os-traits master: Remove old excludes https://review.opendev.org/c/openstack/os-traits/+/917613 | 10:20 |
| opendevreview | Takashi Kajinami proposed openstack/os-resource-classes master: Remove old excludes https://review.opendev.org/c/openstack/os-resource-classes/+/917614 | 10:21 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Add regression test case for bug 1978573 https://review.opendev.org/c/openstack/nova/+/914653 | 11:30 |
| opendevreview | Merged openstack/os-traits master: Remove old excludes https://review.opendev.org/c/openstack/os-traits/+/917613 | 11:53 |
| opendevreview | Takashi Kajinami proposed openstack/os-vif master: Remove old excludes https://review.opendev.org/c/openstack/os-vif/+/917611 | 12:13 |
| opendevreview | Takashi Kajinami proposed openstack/python-novaclient master: Remove old excludes https://review.opendev.org/c/openstack/python-novaclient/+/917685 | 12:44 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Fix KeyError on assisted snapshot call https://review.opendev.org/c/openstack/nova/+/900783 | 12:48 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Add regression test case for bug 1978573 https://review.opendev.org/c/openstack/nova/+/914653 | 12:54 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Restore original AZ when unshelve fails https://review.opendev.org/c/openstack/nova/+/911108 | 12:54 |
| opendevreview | Takashi Kajinami proposed openstack/os-vif master: DNM: Run requirement-check jobs https://review.opendev.org/c/openstack/os-vif/+/917707 | 13:52 |
| tkajinam | ^^^ sean-k-mooney I think this shows how the requirement-check job fails with the current u-c.txt file which was cleaned up. | 13:53 |
| sean-k-mooney | tkajinam: im not sayign we shoudl not update them | 13:58 |
| sean-k-mooney | im saying that the update you propoes is not correct as it allows know broken version | 13:58 |
| sean-k-mooney | so we either need to fix the requiremetn repo | 13:58 |
| sean-k-mooney | or you need to increase the min version above the blocked version | 13:59 |
| tkajinam | I can bump the min version but I doubt the current list excludes all of known bad versions really | 13:59 |
| sean-k-mooney | sure but allowing know broken version i would consider to be a medium severity bug | 14:00 |
| sean-k-mooney | it would be a high severity bug if pip tied to install a previously blocked version and it cuase dissues | 14:01 |
| tkajinam | I understand that | 14:01 |
| tkajinam | my expectation is that pip would install the "latest" good version so it may not attempt to install such ancient versions | 14:01 |
| sean-k-mooney | sure and my epecation is project do not allow pip to install know bad versions | 14:02 |
| tkajinam | so this is more like a policy decision. I'll leave it for a while in case someone else chime in with a different thought | 14:02 |
| sean-k-mooney | by default it wont but it should not be possibel even if you pass a constratigs file | 14:02 |
| tkajinam | yeah | 14:03 |
| tkajinam | but will update it to bump min unless I get additional feedback | 14:03 |
| tkajinam | Stephen merged patches in a few other nova repos so I can push min bumps as follow-up if we agree with the direction | 14:04 |
| sean-k-mooney | or we revert them | 14:04 |
| tkajinam | we can't revert because requiremet-check job fails on reverts | 14:04 |
| tkajinam | unless we "fix" the upper-constraint file | 14:05 |
| sean-k-mooney | ok well lets fix them quickly then | 14:05 |
| sean-k-mooney | and let not merge any more | 14:05 |
| sean-k-mooney | i think os-traits is the only one where it merged | 14:09 |
| opendevreview | Takashi Kajinami proposed openstack/os-vif master: Remove old excludes https://review.opendev.org/c/openstack/os-vif/+/917611 | 14:10 |
| opendevreview | Takashi Kajinami proposed openstack/os-traits master: Bump min versions to exclude known bad versions https://review.opendev.org/c/openstack/os-traits/+/917713 | 14:12 |
| tkajinam | ^^^ this is the only nova repo which gets that change merged | 14:13 |
| tkajinam | I'll update the other open ones | 14:13 |
| tkajinam | maybe I can bring the topic to the meeting today before I update all | 14:14 |
| sean-k-mooney | sure but it likely should also be disucssed with the requirements project | 14:29 |
| sean-k-mooney | im +2 on both by the way | 14:29 |
| tkajinam | let's see how others think. but I hope we can move on with project decisions now and follow up with the global policy discussion, because the job failure may block any change which updates requirements | 14:37 |
| opendevreview | OpenStack Release Bot proposed openstack/nova master: reno: Update master for unmaintained/zed https://review.opendev.org/c/openstack/nova/+/917741 | 15:00 |
| opendevreview | OpenStack Release Bot proposed openstack/os-vif master: reno: Update master for unmaintained/zed https://review.opendev.org/c/openstack/os-vif/+/917743 | 15:01 |
| opendevreview | OpenStack Release Bot proposed openstack/osc-placement master: reno: Update master for unmaintained/zed https://review.opendev.org/c/openstack/osc-placement/+/917745 | 15:01 |
| opendevreview | OpenStack Release Bot proposed openstack/placement master: reno: Update master for unmaintained/zed https://review.opendev.org/c/openstack/placement/+/917747 | 15:02 |
| opendevreview | OpenStack Release Bot proposed openstack/python-novaclient master: reno: Update master for unmaintained/zed https://review.opendev.org/c/openstack/python-novaclient/+/917749 | 15:02 |
| elodilles | if any nova core wants easy-win patch reviews then these bot-proposed, generated patches need to merge ASAP ^^^ o:) | 15:27 |
| gibi | on it | 15:37 |
| gibi | I think I hit all of them | 15:39 |
| elodilles | thx o/ | 15:40 |
| gibi | thanks for eoling zed :) | 15:40 |
| gibi | I know onlu unmaintained it, but for me that is eol :D | 15:41 |
| elodilles | :D yeah, i remember :) | 15:43 |
| bauzas | cool thanks | 15:47 |
| bauzas | sorry guys, I was working hard on a devstack issue, eventually I'm all done \o/ | 15:48 |
| bauzas | fwiw, centos9 has some iptables rule that doesn't accept to forward any IP address :) | 15:48 |
| bauzas | folks, nova meeting now in 12 mins | 15:48 |
| elodilles | no worries, gibi +2+W'd everything ;) | 15:49 |
| opendevreview | Merged openstack/os-vif master: reno: Update master for unmaintained/zed https://review.opendev.org/c/openstack/os-vif/+/917743 | 15:49 |
| opendevreview | Merged openstack/python-novaclient master: reno: Update master for unmaintained/zed https://review.opendev.org/c/openstack/python-novaclient/+/917749 | 15:50 |
| bauzas | #startmeeting nova | 16:00 |
| opendevmeet | Meeting started Tue Apr 30 16:00:12 2024 UTC and is due to finish in 60 minutes. The chair is bauzas. Information about MeetBot at http://wiki.debian.org/MeetBot. | 16:00 |
| opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 16:00 |
| opendevmeet | The meeting name has been set to 'nova' | 16:00 |
| bauzas | hey folks | 16:00 |
| bauzas | #link https://wiki.openstack.org/wiki/Meetings/Nova#Agenda_for_next_meeting | 16:00 |
| Uggla_ | o/ | 16:00 |
| tkajinam | o/ | 16:00 |
| gibi | o/ | 16:00 |
| auniyal | o/ | 16:00 |
| elodilles | o/ | 16:00 |
| fwiesel | \o | 16:00 |
| bauzas | cool, let's start | 16:00 |
| bauzas | #topic Bugs (stuck/critical) | 16:01 |
| bauzas | #info No Critical bug | 16:01 |
| bauzas | #info Add yourself in the team bug roster if you want to help https://etherpad.opendev.org/p/nova-bug-triage-roster | 16:01 |
| bauzas | any bug to report ? | 16:01 |
| bauzas | looks not | 16:01 |
| bauzas | moving on? | 16:01 |
| bauzas | #topic Gate status | 16:02 |
| bauzas | #link https://bugs.launchpad.net/nova/+bugs?field.tag=gate-failure Nova gate bugs | 16:02 |
| bauzas | #link https://etherpad.opendev.org/p/nova-ci-failures-minimal | 16:02 |
| opendevreview | Merged openstack/nova master: reno: Update master for unmaintained/zed https://review.opendev.org/c/openstack/nova/+/917741 | 16:02 |
| bauzas | #link https://zuul.openstack.org/builds?project=openstack%2Fnova&project=openstack%2Fplacement&pipeline=periodic-weekly Nova&Placement periodic jobs status | 16:02 |
| opendevreview | Merged openstack/placement master: reno: Update master for unmaintained/zed https://review.opendev.org/c/openstack/placement/+/917747 | 16:02 |
| bauzas | all greens | 16:02 |
| bauzas | #info Please look at the gate failures and file a bug report with the gate-failure tag. | 16:02 |
| bauzas | #info Please try to provide meaningful comment when you recheck | 16:03 |
| bauzas | anything about some gate failure you just found ? | 16:03 |
| tkajinam | I noticed that requirements-check job is broken now but I'll talk about that later when we talk about review priorities | 16:03 |
| bauzas | ack | 16:03 |
| bauzas | #topic Release Planning | 16:04 |
| bauzas | #link https://releases.openstack.org/dalmatian/schedule.html | 16:04 |
| bauzas | #info Dalmatian-1 in 2 weeks | 16:04 |
| bauzas | #action bauzas to add the nova deadlines in the Dalmatian schedule page | 16:04 |
| bauzas | I'll do this tomorrow (as I'm done with my devstack problems :) ) | 16:04 |
| bauzas | #topic Review priorities | 16:04 |
| bauzas | #link https://etherpad.opendev.org/p/nova-dalmatian-status | 16:04 |
| bauzas | tkajinam: shoot | 16:05 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Fix KeyError on assisted snapshot call https://review.opendev.org/c/openstack/nova/+/900783 | 16:05 |
| tkajinam | I've added summary to L52 | 16:05 |
| bauzas | I see you added it indeed | 16:05 |
| tkajinam | requirements-check job is currently broken because old excludes were cleaned up in global requirements file | 16:05 |
| tkajinam | because these excludes are no longer maintained in the global requirements repo, I think it makes sense to remove these from individual repo | 16:06 |
| sean-k-mooney | yep it sound like we are missing somethign on the requirement repo to have caught that breakage | 16:06 |
| bauzas | correct | 16:06 |
| bauzas | and I'm afraid to bump our minimums just because of that | 16:07 |
| sean-k-mooney | so i think its a bug if we do nto bump | 16:07 |
| bauzas | we don't have jobs testing our minimums, right? | 16:07 |
| bauzas | we had that before but we removed it AFAICR | 16:08 |
| sean-k-mooney | im not really happy about removing know broken verison and not bumping to a version above them as our minium | 16:08 |
| sean-k-mooney | correct | 16:08 |
| sean-k-mooney | we dont test our miniums | 16:08 |
| sean-k-mooney | so i would like to rasied them above any version that was listed as know to have issues | 16:08 |
| bauzas | so there is no clear guarantee about the minimums | 16:08 |
| sean-k-mooney | we will still use latest in the jobs | 16:08 |
| bauzas | the problem is simple : we are on a non-SLURP release | 16:09 |
| sean-k-mooney | correct they may be older then we actully require | 16:09 |
| sean-k-mooney | why would htat matter | 16:09 |
| sean-k-mooney | we are allowed to raise minium in non slurps | 16:09 |
| bauzas | oh maybe I was wrong | 16:09 |
| tkajinam | +1 to sean, otherwise we can't implement anything which rely on new os-traits thing during this cycle | 16:10 |
| opendevreview | Merged openstack/osc-placement master: reno: Update master for unmaintained/zed https://review.opendev.org/c/openstack/osc-placement/+/917745 | 16:10 |
| sean-k-mooney | or new release of oslo libs ectra | 16:10 |
| tkajinam | yeah | 16:10 |
| bauzas | if operators skip this release, they still need to upgrade their requirements in the next slurp | 16:10 |
| tkajinam | so... stepping back to help people with following the discussion there are a few options we have | 16:10 |
| sean-k-mooney | right but that needed anyway | 16:10 |
| tkajinam | 1. just remove old excludes | 16:11 |
| sean-k-mooney | i.e. what ever release theyre upgrading too then need ot install its deps | 16:11 |
| tkajinam | 2. remove old excludes and bump minimum to effectively excludes these bad versions | 16:11 |
| tkajinam | 3. ask requirements to revert the change | 16:11 |
| tkajinam | 4. loose the validation to ignore mismatch in excludes | 16:11 |
| bauzas | looks like we went to a corner now we no longer have min reqs | 16:12 |
| sean-k-mooney | i asked tkajinam to do 2 | 16:12 |
| bauzas | the problem is that we only test on gate our latest releases in pip | 16:12 |
| sean-k-mooney | bauzas: we still have min requirement in our repo we just dont test them | 16:12 |
| sean-k-mooney | so i think min version testing is out of scope for right now | 16:13 |
| gibi | the fact that we are not testing the minimum is true even without the bump, we just don't know that the current minimums are better than the new ones | 16:13 |
| sean-k-mooney | and we should folcus on the 4 optiosn tkajinam has presented | 16:13 |
| bauzas | that's the point, we have minimums for flexibility to make sure that operators don't need to update every dependency to latest everytime, right? | 16:13 |
| sean-k-mooney | kind of | 16:13 |
| bauzas | our support envelope is really "latest" | 16:13 |
| elodilles | we have this comment in the req.txt: https://opendev.org/openstack/nova/src/branch/master/requirements.txt#L1-L3 | 16:14 |
| bauzas | which can change during a release cycle | 16:14 |
| sean-k-mooney | its mainly to document to packagers what we thing shoudl work | 16:14 |
| gibi | are there any major bumps in the list? minor bumps should be compatible | 16:14 |
| bauzas | there are | 16:14 |
| bauzas | if we go with bumping | 16:14 |
| sean-k-mooney | gibi: we were already crossing major bondaries | 16:14 |
| bauzas | https://review.opendev.org/c/openstack/nova/+/917594/1/requirements.txt#b54 | 16:14 |
| tkajinam | yeah there are | 16:15 |
| tkajinam | bauzas, yes, but note that the versions removed from excludes are all 2+ years old | 16:15 |
| bauzas | we would be changing our cinderclient min to 4.x | 16:15 |
| bauzas | sure | 16:15 |
| bauzas | but I don't want zigo to kill me at milestone-3 :) | 16:15 |
| gibi | so the only thing that I can foresee if we bump our minor from major to major+1 but our latest we test with is major+2 | 16:15 |
| sean-k-mooney | right we are currently testign with cinder client 9.5.0 | 16:15 |
| gibi | tell zigo at m-1 | 16:15 |
| gibi | :) | 16:16 |
| sean-k-mooney | so bumping to 4.x is prablly better then saying it actully works with 3.3.0 | 16:16 |
| sean-k-mooney | it might but we have not tested that in years | 16:16 |
| bauzas | he's highlighted now :D | 16:16 |
| sean-k-mooney | well for cinder client we test major +6 now | 16:16 |
| zigo | Even buster has 1:4.0.1-2 ... :P | 16:16 |
| bauzas | honestly, I don't know how much of that matters | 16:16 |
| bauzas | it's more a distro thing | 16:17 |
| zigo | bauzas: Are you sure we wont break Folsom ? :P | 16:17 |
| * zigo reads the backlog, what are we talking about exactly ? requirements ? | 16:17 | |
| bauzas | zigo: now you're here (glad you continue to listen to us :) ), are you okay with nova bumping their minimums straighly ? | 16:17 |
| sean-k-mooney | does anyone object ot bumping to the first release after the last knnow bad relsese ? | 16:17 |
| bauzas | after all that discussion, I have to admit I have no objection now | 16:18 |
| gibi | based on the current discussion we are affraid to bump the mins, but we are also not doing anything to be less affraid. In the other hand project arond us are moving forward. Either we need to be less affraid or start doing things that makes us less affraid | 16:18 |
| bauzas | distro owners seem to be smart folks who actually don't really use our requirements for shipping their own packages | 16:18 |
| sean-k-mooney | i personally am not that concerned with bumping the mins on master | 16:19 |
| sean-k-mooney | as long as we dont just require latest | 16:19 |
| gibi | I have no objection to bump to the next | 16:19 |
| zigo | bauzas: I don't mind, and I don't really care the SLURP thingy, because it's 1/ not alligned with Debian in terms of dates 2/ still a 1 year thingy instead of 2. | 16:19 |
| sean-k-mooney | we obviously cant do that on stable | 16:19 |
| tkajinam | If I hear no immediate objections here then I'll update all patches based on that strategy | 16:19 |
| bauzas | and given our minimums are quite (very) old, I think I'm OK with bumpinig | 16:19 |
| tkajinam | if you find any concern with specific bumps then we can discuss that in detail | 16:19 |
| zigo | Once we switch to a 1 year release and SLURP, effectively supporting debian stable -> stable +1 upgrades, then I'll start to care. | 16:19 |
| sean-k-mooney | +2 | 16:19 |
| bauzas | tkajinam: you know what ? do it, and we'll see if people yell | 16:19 |
| tkajinam | most of my patches are based on option 1 (without bump) so have to be updated for option 2. | 16:20 |
| tkajinam | bauzas, yup :-) that's why I created these changes, actually | 16:20 |
| zigo | Mostly in distros: we don't need to care about excludes, we just need to *not* package them. :P | 16:21 |
| bauzas | #agreed it seems our requirement minimums are actually very old so we all accept to raise the versions of the packages that were having excluded versions to the N+1 release | 16:21 |
| zigo | (which only very rarely happen) | 16:21 |
| bauzas | zigo: it only happens when the latest version has to be excluded: ) | 16:21 |
| bauzas | :) | 16:21 |
| zigo | Yeah, we're talking about buster to trixie (ie: debian 9 to 13) here ... :P | 16:21 |
| fwiesel | Stupid question: are there really people out there that mix and match releases of individual packages in the same python environment? Is that even allowed? | 16:21 |
| bauzas | when we talk of 5-yo excluded versions, I think nobody cares | 16:22 |
| zigo | fwiesel: It depends what service, for some it's possible. | 16:22 |
| bauzas | that's why we have global requirements | 16:22 |
| zigo | fwiesel: For example, if you're doing multi-region with a single keystone, it's important that older Keystone is supported by newer rest-of-openstack. | 16:22 |
| zigo | Keystone is probably a special case... | 16:22 |
| fwiesel | Sure, but that is kind of guaranteed by the microversions. | 16:23 |
| tkajinam | I'll update these patches so it'd be nice these can be review with priority. as I said requirements-check job is broken and we need these to merge any changes which update reqs now | 16:23 |
| fwiesel | At least we upgrade keystone and other services independently. and use the uppser-constraints of the respective releases of the service. | 16:23 |
| bauzas | bumping our minimums puts some stress to some other projects that may also need to upgrade their own minimums, but here we're talking of very old versions | 16:23 |
| bauzas | tkajinam: noted, I'll pay my duty | 16:23 |
| tkajinam | I've also left the other bug there and that's also nice-to-have thing to unblock our testing glance run by httpd+mod_wsgi in puppet jobs | 16:23 |
| tkajinam | but that's not as much urgent now | 16:23 |
| bauzas | I was frankly off upstream those weeks but I'm now back | 16:24 |
| bauzas | moving on I guess | 16:24 |
| tkajinam | that's all from my side | 16:24 |
| tkajinam | yup | 16:24 |
| bauzas | tkajinam: thanks for having reported it and worked on it | 16:24 |
| bauzas | #topic Stable Branches | 16:25 |
| bauzas | elodilles: you're next ! | 16:25 |
| elodilles | o7 | 16:25 |
| elodilles | #info no recent merges, but stable/202*.* branches' gates should be OK | 16:25 |
| elodilles | #info final release for stable zed is out (26.3.0) | 16:25 |
| bauzas | shhhh | 16:25 |
| elodilles | #info stable/zed transitioned to Unmaintained | 16:25 |
| bauzas | cool | 16:25 |
| elodilles | #info open stable/zed patches were abandoned, cherry pick them to unmaintained/zed if needed | 16:25 |
| bauzas | ++ | 16:26 |
| elodilles | note: as with the previous newly opened unmaintained branches, unmaintained/zed gate might need some fixes before we can merge anything there | 16:26 |
| elodilles | last but not least: | 16:26 |
| elodilles | #info stable branch status / gate failures tracking etherpad: https://etherpad.opendev.org/p/nova-stable-branch-ci | 16:26 |
| bauzas | which means that now all our supported releases follow the SLURP cadence, huzzah | 16:26 |
| opendevreview | Takashi Kajinami proposed openstack/placement master: Remove old excludes https://review.opendev.org/c/openstack/placement/+/917608 | 16:26 |
| sean-k-mooney | are we expecting the default brances for grenade to be updated on stable/antelope centrally | 16:26 |
| zigo | zed is what's in bookworm ... :/ | 16:26 |
| elodilles | bauzas: yepp | 16:27 |
| * zigo goes in the corner and cries ... | 16:27 | |
| * bauzas gives tissues to zigo | 16:27 | |
| opendevreview | Takashi Kajinami proposed openstack/placement master: Remove old excludes https://review.opendev.org/c/openstack/placement/+/917608 | 16:27 |
| bauzas | thanks elodilles | 16:27 |
| bauzas | anything else on the stable side ? | 16:27 |
| elodilles | sean-k-mooney: we had some patches, but i don't know whether that is among them :X | 16:27 |
| elodilles | bauzas: np, nothing else from me | 16:28 |
| sean-k-mooney | ack so that will cause grenade to fail then since the base barnch wont exist | 16:28 |
| sean-k-mooney | but its also a simple enough change once all the branch movign is done | 16:28 |
| elodilles | ++ | 16:28 |
| bauzas | I think grenade has some homework anytime we delete a branch | 16:28 |
| bauzas | but let's move on | 16:29 |
| bauzas | #topic vmwareapi 3rd-party CI efforts Highlights | 16:29 |
| bauzas | fwiesel: wanting to say something ? | 16:29 |
| fwiesel | Not really. | 16:29 |
| fwiesel | https://review.opendev.org/c/openstack/nova/+/909474 has been reviewed, so that's great. Thanks to everyone. | 16:30 |
| fwiesel | Any questions? | 16:30 |
| sean-k-mooney | am i saw the chagnes that dan asked for | 16:30 |
| bauzas | cool, I'll see if I can look at it | 16:30 |
| bauzas | but dan and sean already did their work | 16:30 |
| sean-k-mooney | i have not had time to re review but ill try and get to it today | 16:30 |
| sean-k-mooney | after the meeting | 16:30 |
| fwiesel | Thanks | 16:30 |
| bauzas | cool so | 16:30 |
| fwiesel | #info Nothing to report. | 16:30 |
| bauzas | cool | 16:30 |
| bauzas | last topic then | 16:30 |
| bauzas | let's be short | 16:30 |
| sean-k-mooney | asuming that merges can you add the next one to the list for review | 16:31 |
| bauzas | #topic Open discussion | 16:31 |
| bauzas | I have one thing to raise | 16:31 |
| gibi | I have one too | 16:31 |
| fwiesel | sean-k-mooney: Of course, I'll add that one | 16:31 |
| bauzas | (bauzas) I won't be able to run the next nova meeting | 16:31 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Remove old excludes https://review.opendev.org/c/openstack/nova/+/917594 | 16:31 |
| bauzas | (I'll take a few days off starting Wed, but I'll need to travel on Tues by 5pm my time) | 16:32 |
| bauzas | so, who can run the meeting in my absence ? | 16:32 |
| dansmith | or just cancel? | 16:32 |
| bauzas | if nobody volunteers, I can cancel | 16:32 |
| bauzas | yeah that | 16:32 |
| gibi | I can run it if needed | 16:32 |
| bauzas | thanks gibi | 16:33 |
| gibi | no problemo | 16:33 |
| bauzas | #action gibi to run next nova meeting | 16:33 |
| bauzas | gibi: shoot your own point | 16:33 |
| gibi | just a heads up | 16:33 |
| gibi | we started a small standup with Uggla_ about his manial series 30 mins twice a week. So far it is Uggla_ and me but eventually we need a second code for the series | 16:34 |
| bauzas | count me on it | 16:34 |
| gibi | and such core could benefit joining to these meetings | 16:34 |
| gibi | bauzas: thanks, I will make sure you will have the invite | 16:34 |
| bauzas | I already reviewed the series so I'd be happy to continue to shepherd it | 16:34 |
| bauzas | gibi: cool, timezones will then match hopefully :) | 16:35 |
| bauzas | (it was a joke) | 16:35 |
| gibi | it will :) | 16:35 |
| gibi | that is all | 16:35 |
| bauzas | cool | 16:35 |
| bauzas | and then that's all for me too | 16:35 |
| bauzas | thanks all | 16:35 |
| opendevreview | Takashi Kajinami proposed openstack/python-novaclient master: Remove old excludes https://review.opendev.org/c/openstack/python-novaclient/+/917685 | 16:35 |
| bauzas | was a productive meeting | 16:35 |
| bauzas | see you :) | 16:35 |
| bauzas | #endmeeting | 16:35 |
| opendevmeet | Meeting ended Tue Apr 30 16:35:52 2024 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 16:35 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/nova/2024/nova.2024-04-30-16.00.html | 16:35 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/nova/2024/nova.2024-04-30-16.00.txt | 16:35 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/nova/2024/nova.2024-04-30-16.00.log.html | 16:35 |
| Uggla_ | bauzas, I will send you the invites | 16:35 |
| bauzas | np | 16:36 |
| elodilles | thanks o/ | 16:36 |
| tkajinam | thank you :-D | 16:36 |
| sean-k-mooney | just a note f you want ot send a message to the list to invite other to join if they want to dont send the ical | 16:36 |
| sean-k-mooney | if you send the ical to the list is will effectivly invite the list :) | 16:36 |
| gibi | I think this will not go to the list :) | 16:37 |
| gibi | we want t keep it focused | 16:37 |
| opendevreview | Takashi Kajinami proposed openstack/os-resource-classes master: Remove old excludes https://review.opendev.org/c/openstack/os-resource-classes/+/917614 | 16:37 |
| fwiesel | thanks | 16:37 |
| sean-k-mooney | which is fine. i likely wont join those meeting but if you want me to attend as a one off for a specific patch or topic ping | 16:37 |
| gibi | sure, thanks | 16:39 |
| gibi | btw if we already discussing this series I'm wondering if we are OK to have a sqlite specific db migration codepath https://review.opendev.org/c/openstack/nova/+/912518/1/nova/db/main/migrations/versions/d60bddf7a903_add_constraint_instance_share_avoid_.py | 16:40 |
| dansmith | wow, really? | 16:40 |
| gibi | our migraton testing fail without it | 16:41 |
| gibi | so we either skip testing this migration or have an sqlite specific path | 16:41 |
| dansmith | and it's a table drop/recreate? that seems incredibly unfortunate to me | 16:41 |
| gibi | it is for sqlite | 16:41 |
| dansmith | what is the actual problem? | 16:42 |
| gibi | sqlite does not support altering constarint on a table without recreate | 16:42 |
| sean-k-mooney | we should not need that | 16:42 |
| sean-k-mooney | although i know ironic does some sqlite specific tweaks | 16:42 |
| dansmith | the create_table includes mysql-specific stuff too | 16:42 |
| sean-k-mooney | gibi: as in adding a foringkey | 16:42 |
| dansmith | is this because we merged the migration before the implementation and then decided that we needed something different? | 16:43 |
| sean-k-mooney | so this is the share mapping table | 16:43 |
| sean-k-mooney | which does not exist right | 16:43 |
| sean-k-mooney | so why do we need this drop table and recreate | 16:44 |
| sean-k-mooney | did that merge last cycle? | 16:44 |
| gibi | dansmith: we needed an exta constraint to prevent creating two sharemapping with the same share id and instance id | 16:44 |
| gibi | sean-k-mooney: we merged the db code | 16:44 |
| dansmith | ugh, that's very unfortunate | 16:44 |
| sean-k-mooney | ... ok this partly why i didint want to do that | 16:44 |
| dansmith | yeah seriously | 16:44 |
| dansmith | but, if it's not used currently, can we just drop and recreate it for all backends? should be very non-impactful for an empty mysql table I think | 16:45 |
| sean-k-mooney | ya i think i prefer ^ | 16:45 |
| gibi | dansmith: correct, the api code is not landed, so the table is empty | 16:45 |
| dansmith | yeah, I'd prefer that, but.. also prefer not landing db code before implementation is ready for exactly this reason :( | 16:46 |
| gibi | OK | 16:46 |
| sean-k-mooney | we dont alwasy do this but my prerfered ordering is "driver -> object -> rpc ->db -> api -> docs" | 16:46 |
| gibi | just a side note that in the other hand people are not happy with 30 patches in a series. | 16:47 |
| gibi | driver uses object code I think | 16:47 |
| gibi | object used db backing | 16:47 |
| sean-k-mooney | it didnt need too but lets not rehash that | 16:48 |
| gibi | anyhow thanks for the feedback, then this migration will be a drop / recreate for all backends | 16:48 |
| gibi | Uggla_: ^^ | 16:48 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Fix KeyError on assisted snapshot call https://review.opendev.org/c/openstack/nova/+/900783 | 16:48 |
| gibi | I need to drop now. see you on thrusday | 16:49 |
| sean-k-mooney | enjoy o/ | 16:49 |
| Uggla_ | gibi, seen | 16:49 |
| Uggla_ | I will update to drop / recreate the table | 16:49 |
| frct1 | Hello, could anybody from nova team review change to nova-api ? https://review.opendev.org/c/openstack/nova/+/917276 this fix #cloud-config provide over vendor_data2.json which is broken for now. | 17:20 |
| sean-k-mooney | frct1: so any change shoudl really have test coverage to validate it | 17:25 |
| sean-k-mooney | also this seams like a new feature | 17:26 |
| sean-k-mooney | adding cloud init supprot to dymic veder data | 17:26 |
| frct1 | Not a new feature, since its support declared by cloud-init (https://cloudinit.readthedocs.io/en/latest/reference/datasources/openstack.html#vendor-data), there is proposal to fix https://blueprints.launchpad.net/nova/+spec/dynamicjson-vendordata-cloud-config | 17:27 |
| sean-k-mooney | in json format | 17:27 |
| frct1 | Current DynamicJSON implementation strongly limit developers to return JSON encoded response from provider | 17:28 |
| sean-k-mooney | right | 17:28 |
| sean-k-mooney | we dont supprot any other format | 17:28 |
| sean-k-mooney | only json | 17:28 |
| sean-k-mooney | so that is the new feature your addign | 17:28 |
| frct1 | I have to implement another vendor data provider ? | 17:28 |
| sean-k-mooney | allowign the raw respocne to be passed | 17:28 |
| sean-k-mooney | which also changes the returnd formate form a dict to a string | 17:29 |
| sean-k-mooney | so the vendor data feature only supprot talking to rest api that return a json responce | 17:29 |
| bauzas | folks, I'll be off tomorrow, see you on Thur | 17:30 |
| frct1 | What are ways to solve it ? New vendor data provider ? It is possible is over-engineering to implement new one because cloud-init is only thing that expects cloud-init key in vendor_data2.json | 17:31 |
| sean-k-mooney | frct1: the first step woudl really be to create a repoducer test case for the issue | 17:44 |
| sean-k-mooney | do you have an example fo a valid respocne that has a cloud-init key? | 17:45 |
| frct1 | yep: https://paste.openstack.org/show/bNmVjYg2Fmq3wpxtny9y/ | 17:52 |
| sean-k-mooney | ok well that not a valid respocne the content of the data return by the service has to be a valid json object | 17:53 |
| frct1 | Yes, but in this case cloud-init will ignore cloud-config if it is presented in nested json even if cloud-init key present. There is discrepancy between cloud-init and nova implementation, because cloud-init support to run cloud-config provided in vendor data (https://cloudinit.readthedocs.io/en/latest/reference/datasources/openstack.html#vendor-data) | 17:54 |
| sean-k-mooney | it should be something like this https://paste.openstack.org/show/bua9OEI0FpYb24kTIjHj/ | 17:54 |
| sean-k-mooney | frct1: right and nova does not supprot that so adding supprot would be a new feature | 17:55 |
| frct1 | I've found that there is some issues reported to cloud-init, (https://github.com/canonical/cloud-init/issues/3433 and https://github.com/canonical/cloud-init/pull/777) that has been ignored | 17:55 |
| sean-k-mooney | sure the cloud init doc you linked is incorrect | 17:56 |
| sean-k-mooney | that is not supproted | 17:56 |
| frct1 | I've tried to modify nova-api code to return plain response if service_name equals to "cloud-init" and it actually works | 17:56 |
| sean-k-mooney | ok but that is not valid | 17:57 |
| sean-k-mooney | we could enable that | 17:58 |
| sean-k-mooney | but its not somethign that was orgianlly uspported when we added this feature | 17:58 |
| frct1 | Exactly. That;s why i started to investigate on how to solve it. If it is will be implemented as new feature, should it have it's own configuration ? | 17:58 |
| sean-k-mooney | is the comment in the respocne | 17:59 |
| sean-k-mooney | https://paste.openstack.org/show/bNmVjYg2Fmq3wpxtny9y/ | 17:59 |
| frct1 | No, there is no any comments in response, it was added as note | 18:00 |
| sean-k-mooney | https://paste.openstack.org/show/bb6tjYB8z4Zc2xrcVBXy/ | 18:00 |
| sean-k-mooney | ok this ^ would be valid json but the other poast is not | 18:00 |
| sean-k-mooney | so the expecation is that content would be put into a file called somethign like openstack/2016-10-06/vendor_data2.json | 18:02 |
| frct1 | Yes, Or openstack/latest/vendor_data2.json, not sure what path cloud-init looks for, but it checks that `cloud-init` key is presented and is string type and then process it | 18:03 |
| sean-k-mooney | ok but it should be presented like that today | 18:03 |
| frct1 | If there is cloud-config provided in vendor_data2.json and user-data is provided then cloud-init will merge both of them (user-data will override config options presented in vendor_data cloud-config) | 18:04 |
| frct1 | How do you think what is more preferred way to implement it ? Adding cloud-init as whitelisted is easiest workaround but probably violates idea of DynamicJSON provider | 18:05 |
| sean-k-mooney | personally i dont want to see use special case at all | 18:06 |
| sean-k-mooney | im trying to reconsile what cloud-init expect with what openstack supprots | 18:07 |
| frct1 | Since DynamicJSON combine responses from all providers. May be we can use some sort of suffix to vendor data provider name like "testing@10.10.10.10:1337,cloud-init:string@10.10.13.37:8000" to make it more declarative to let provider know that response should be string. | 18:12 |
| sean-k-mooney | frct1: so our functional tests assert that each backend is a nested resouce contianing a dict repsponce | 18:12 |
| sean-k-mooney | https://github.com/openstack/nova/blob/fa678870b24d450df6d5abce57db2e6eb83703c5/nova/tests/functional/test_metadata.py#L97-L119 | 18:12 |
| sean-k-mooney | so based on that this is a bug in cloud init | 18:13 |
| sean-k-mooney | it shouldl be loopting over the keys and looking for a backend with a nested "cloud-init" key | 18:13 |
| sean-k-mooney | frct1: have you condierd filing a bug with cloud-init for this | 18:14 |
| frct1 | I did it couple days ago: https://github.com/canonical/cloud-init/issues/5221 | 18:14 |
| frct1 | What if multiple vendor data provider responses will have cloud-iniit key ? It is big question on how to solve it | 18:16 |
| sean-k-mooney | we explcitly say that that is not supproted in our docs | 18:16 |
| sean-k-mooney | at lest in our olde rdocs | 18:17 |
| sean-k-mooney | cloud init woudl be free to merge the section si guess if that was the case | 18:17 |
| frct1 | ¯\_(ツ)_/¯ Re-implementing at cloud-init side would require from operators building brand new images with new cloud-init version that contain fixes that could take months to propagate over repos. | 18:21 |
| frct1 | When it is fixed on nova side it could be easily update and all "relatively" recent images will work as expected with new feature | 18:22 |
| sean-k-mooney | but its not broke on the nova side | 18:23 |
| sean-k-mooney | to do what you want would actully have sideffect | 18:23 |
| sean-k-mooney | the first sideefect is obvioulsy that cloud-init | 18:23 |
| sean-k-mooney | is currenlty a valid backend name | 18:23 |
| sean-k-mooney | i.e vendordata_dynamic_targets=['cloud-init@http://127.0.0.1:123'] | 18:24 |
| sean-k-mooney | is currently valid | 18:24 |
| sean-k-mooney | second this is a breaking change to the format provided by openstack | 18:24 |
| frct1 | What do you think on implementing new vendor data provider that operator will be able to configure how response type should look ? For example vendordata_dynamic_targets="cloud-init:striing@http://127.0.0.1:123,testing:json@http://127.0.0.1:124" ? | 18:25 |
| sean-k-mooney | meaning this could only be done in a new version and we woudl have to kept it for all the old version | 18:25 |
| sean-k-mooney | frct1: thats more palitable but we likely need to have a nova-spec for that | 18:26 |
| sean-k-mooney | to ensure there are no upgrade impacts to this | 18:26 |
| frct1 | So old DynamicJSON provider will continue to work as before, but if operator would like to provide some sort of "common" cloud-config he can enable new implementation that covers old DynamicJSON and new one that allows response type explicitly defined | 18:26 |
| sean-k-mooney | we would likely need a new version | 18:26 |
| sean-k-mooney | and only include the string providers in the latest version | 18:26 |
| sean-k-mooney | right now clinet of the vendor data file should expect the fiile to be {<provider>:<data>} string to dict | 18:28 |
| sean-k-mooney | but in the new format they would have to support parsing string to (dict or string) | 18:28 |
| frct1 | So yeah, this is really not a bad option to explicitly define response type. Is there any other options for types to be supported like number or string ? Is it really needed ? | 18:30 |
| sean-k-mooney | we only supprot json today as you saw | 18:30 |
| sean-k-mooney | so each provider is expected to provide a json dict | 18:30 |
| sean-k-mooney | but the content of that dict can technially be any valid json so the nest dict can have strings more dicts number ectra | 18:31 |
| frct1 | I'm talking not about current implementation, but new one provider with options to explicitly define provider response type (json or string) | 18:35 |
| frct1 | I will able to describe proposal for new provider as well as new vendor data provider code covered with tests | 18:36 |
| sean-k-mooney | i dont think this would be a new provider | 18:39 |
| sean-k-mooney | well it depend on what you mean by provider | 18:40 |
| sean-k-mooney | it would not be a new VendorDataDriver but an enhacment to the existing VendorDataDriver | 18:40 |
| frct1 | Now i'm not sure how it should look like... Do you have possible example for configuring it ? | 18:41 |
| sean-k-mooney | maybe somethign like this vendordata_dynamic_targets=['cloud-init:string@http://127.0.0.1:123'] which is what you were suggesting | 18:43 |
| sean-k-mooney | can you try somethign else first | 18:43 |
| sean-k-mooney | technially https://paste.openstack.org/show/bYyFYXCJ5GCPrW45iYEC/ is valid json | 18:43 |
| sean-k-mooney | if you neamed the provider "vendordata_dynamic_targets=['cloud-init@http://127.0.0.1:123']" today | 18:44 |
| sean-k-mooney | and it provide jus the quoted string as the respocne | 18:44 |
| sean-k-mooney | i think it would work | 18:44 |
| frct1 | Nope, just tried one more time. https://paste.openstack.org/show/bRonCys8hzLUKA6FyQr0/ | 18:47 |
| frct1 | https://opendev.org/openstack/nova/src/branch/master/nova/api/metadata/vendordata_dynamic.py#L98 this line raise exception | 18:47 |
| sean-k-mooney | this should be jsonutils.loads('"#cloud-config\nhostname: ocdvworks"') | 18:48 |
| frct1 | Nope https://paste.openstack.org/show/bQYbqkIHPTeoDaJtuXge/ | 18:49 |
| sean-k-mooney | its not happy with the \n | 18:49 |
| frct1 | Uhm, with double slashes it works | 18:50 |
| frct1 | >>> jsonutils.loads('"#cloud-config\\nhostname: ocdvworks"') '#cloud-config\nhostname: ocdvworks' | 18:50 |
| sean-k-mooney | ok ya you need to secape it so that it actlly \n in the data | 18:50 |
| sean-k-mooney | but thats because of how you are typing it | 18:51 |
| sean-k-mooney | i have commented here https://github.com/canonical/cloud-init/issues/5221#issuecomment-2086560281 | 18:51 |
| frct1 | So response would be like https://paste.openstack.org/show/byDvpUrAI61jMVAUMRfY/ ? | 18:53 |
| frct1 | nah, ""#cloud-config\\nhostname: ocdvworks"" not works at all, raise exception, somehow it have to be escaped | 18:54 |
| sean-k-mooney | i do not think it will render as that but rther the file will contain https://paste.openstack.org/show/bXFAMO5uOlegBy4KJmbP/ | 18:54 |
| frct1 | If provider will return "#cloud-config\nhostname: ocdvworks" it will raise the same JSON exception, i would not dig in it if it would work | 18:56 |
| sean-k-mooney | from where | 18:58 |
| sean-k-mooney | https://paste.openstack.org/show/bcayBBbJaZjiagsjcVdu/ | 18:58 |
| sean-k-mooney | im using a raw sting in that version to avoid any escaping | 18:58 |
| sean-k-mooney | i guess you mean when this is used by nova | 18:59 |
| frct1 | So somehow provider have to return escaped string, it is ? | 19:01 |
| sean-k-mooney | the provider need to return a vaild json sring containing something that can be parsed by the client | 19:01 |
| sean-k-mooney | cloud init is documeitnign that it shoudl have a srting with newlines excaped as \n | 19:02 |
| sean-k-mooney | https://cloudinit.readthedocs.io/en/latest/reference/datasources/openstack.html#vendor-data | 19:02 |
| sean-k-mooney | so the server litrally need to return "#cloud-config\npackage_upgrade: True\npackages:\n - htop" | 19:03 |
| sean-k-mooney | including the " | 19:03 |
| sean-k-mooney | my guess is internaly cloud-init is reading the content of that string directly adn parsing it in text mode | 19:04 |
| sean-k-mooney | that or writign it to a file first and then reading it back | 19:06 |
| sean-k-mooney | i think its doign this here ? https://github.com/canonical/cloud-init/blob/main/cloudinit/sources/DataSourceOpenStack.py#L193-L195 | 19:07 |
| sean-k-mooney | so we are going to take this path https://github.com/canonical/cloud-init/blob/main/cloudinit/sources/__init__.py#L1154 | 19:08 |
| sean-k-mooney | im not familar with this code but i think tis hten used here https://github.com/canonical/cloud-init/blob/7eb5a1df0769fb88bbe3fc357ab44a378770043b/cloudinit/stages.py#L562-L564 | 19:10 |
| sean-k-mooney | which calls json_dumps on the string contaiing the cloud config | 19:11 |
| sean-k-mooney | https://github.com/canonical/cloud-init/blob/7eb5a1df0769fb88bbe3fc357ab44a378770043b/cloudinit/stages.py#L604-L605 | 19:11 |
| sean-k-mooney | and it finaly write it to a file | 19:11 |
| sean-k-mooney | https://github.com/canonical/cloud-init/blob/7eb5a1df0769fb88bbe3fc357ab44a378770043b/cloudinit/stages.py#L590-L594 | 19:11 |
| sean-k-mooney | which has mode wb https://github.com/canonical/cloud-init/blob/7eb5a1df0769fb88bbe3fc357ab44a378770043b/cloudinit/util.py#L2249 | 19:12 |
| sean-k-mooney | so its doign a binary write of the raw data to a file | 19:12 |
| sean-k-mooney | frct1: so the respocne form the backend woudl have to be a raw binary sring | 19:13 |
| sean-k-mooney | contiing the cloud-init file content | 19:13 |
| frct1 | Yep, i'm trying to implement it somehow | 19:14 |
| frct1 | The main thing is to make vendor data provider return escaped data | 19:14 |
| frct1 | Uhm, it works, lol https://paste.openstack.org/show/bShqKlZLG9hjmix2wCmc/ | 19:28 |
| frct1 | https://paste.openstack.org/show/bV1JTGhzjKuRdsHGGS5B/, i will try to launch cloud-init shortly | 19:29 |
| sean-k-mooney | so two thing. one your obviously in unsupproted terrirotry using implemenation specific knowldage | 19:33 |
| sean-k-mooney | two if you get it work ing we have a few optiosn | 19:33 |
| sean-k-mooney | first we could docuemnt and add test coverage for this hack | 19:33 |
| sean-k-mooney | second we could still consider the new feature to do this properly | 19:34 |
| frct1 | Actually i'm really inclined to add support for DynamicJSON provider's response typings like mentioned earlier "cloud-init:string@10.10.13.37:123,testing@10.10.13.37:124", so cloud-init key will be explicitly encoded as string, but testing will be presented as json | 19:37 |
| frct1 | I will specs to provide details about observed hack to docs | 19:37 |
| frct1 | I will add* | 19:37 |
| frct1 | Wrapping everything up to update vendor data provider used with Consul and then move to compose docs update | 19:41 |
| frct1 | What we have to add to test coverage for this case ? Something like check that string is probably valid escaped JSON string, i cant find any RFC that covers that | 19:42 |
| melwitt | gmann: dunno if you may get a moment to take a look at this small API controllers tracking change https://review.opendev.org/c/openstack/nova/+/915732 I don't really understand it but I'm sure you will :) | 20:39 |
| sean-k-mooney[m] | escaping is not really the correct way to thikn about it cloud-init treats the string as a binary sting | 20:46 |
| sean-k-mooney[m] | python3 and json by defualt is generally parsed as utf8 | 20:47 |
| sean-k-mooney[m] | what would likely be better then have cloud-init:string is perhaps cloud-init:base64 and then render the decoded content in | 20:48 |
| sean-k-mooney[m] | the vendor data | 20:48 |
| frct1 | You mean base64 for any service or especially for cloud-init ? What if other provider will return json encoded in base64 ? In what type we should decode it before response ? | 20:52 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!