| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-02 | |||
| 17:19:45 | bauzas | dansmith: but of course, still mandatory on the server until 6.0 | |
| 17:19:56 | artom | Genuinely, we could go either way and I'll sleep perfectly well that night | |
| 17:20:10 | gibi | OK, let's go with the proposed implementation. as I don't see it as a dealbreaker | |
| 17:20:26 | gibi | I'm upgrading my vote on the patch | |
| 17:21:12 | dansmith | gibi: it shouldn't even be optional on the client for 6.0 right? | |
| 17:21:21 | stephenfin | cool, I'll take a look tomorrow | |
| 17:21:26 | gibi | stephenfin: thanks | |
| 17:21:27 | stephenfin | still working through the VNC fun | |
| 17:21:38 | stephenfin | it's a pain in the a*** :) | |
| 17:22:47 | gibi | dansmith: logically accel_uuids are a part of the client code, we do gather them from the flavor and from cyborg. So I think it should be mandatory on the client side as well | |
| 17:22:48 | dansmith | oops s/gibi/bauzas/ above | |
| 17:23:04 | dansmith | gibi: that comment was about request_spec, not accel_uuids | |
| 17:23:08 | gibi | aah | |
| 17:23:11 | gibi | then ignore me | |
| 17:23:16 | dansmith | gibi: I confused you by replying to you instead of bauzas | |
| 17:23:17 | gibi | sorry | |
| 17:23:25 | bauzas | dansmith: right | |
| 17:23:44 | bauzas | OK, looks like I have work to do | |
| 17:28:34 | bauzas | honestly, since this request_spec argument isn't deprecated yet in some 5.x API, removing it now would make 6.0 semantically different from 5.13 | |
| 17:28:55 | bauzas | so I won't do it, unless someone steps up and write a 5.x version for making it deprecated | |
| 17:28:59 | bauzas | stephenfin: ^ | |
| 17:29:23 | bauzas | we lost some opportunity here | |
| 17:30:07 | stephenfin | I haven't been following, I'm afraid :( We're passing an unnecessary argument through to some RPC API? | |
| 17:34:46 | sean-k-mooney | bauzas: how would we get the request_spec if we dont pass it? via the instance? | |
| 17:35:02 | sean-k-mooney | i dont think we want to lazy load that if its currently used | |
| 17:35:19 | bauzas | sean-k-mooney: stephenfin's point is that we don't use this parameter even if we pass it thru the wire | |
| 17:35:37 | sean-k-mooney | for which api call | |
| 17:35:47 | bauzas | and we have a few other methods that have some parameters passed thru the wire that we don't use | |
| 17:36:05 | sean-k-mooney | right so those whould have to be removed in 6.0 | |
| 17:36:06 | bauzas | you know what ? I'm changing the docstring to be 7.0 | |
| 17:36:29 | bauzas | and we know we have to write some 6.x API version that would deprecate those parameters | |
| 17:37:02 | sean-k-mooney | do we have to do deprecations like that for internal rpc apis? | |
| 17:37:07 | bauzas | honestly, now I wrote a 6.0 bump, I feel brave enough for writing a 7.0 one | |
| 17:37:38 | bauzas | but this will have to be at least for X, ideally Y | |
| 17:37:41 | sean-k-mooney | bauzas: well we likely wont do a 7.0 bump for another couple of releases | |
| 17:37:49 | bauzas | yes | |
| 17:37:50 | bauzas | I know | |
| 17:38:01 | sean-k-mooney | proably not before Z | |
| 17:38:22 | sean-k-mooney | i mean there is not strict rule but we have mostly wated 4-6 release between major bumps | |
| 17:38:23 | bauzas | sure, but at least we should stop passing those over the wire if we don't need them | |
| 17:38:29 | bauzas | no | |
| 17:38:42 | bauzas | sean-k-mooney: we released major RPC versions more often in the past | |
| 17:39:13 | bauzas | Kilo, Mitaka, Queens | |
| 17:39:21 | bauzas | and now Wallaby | |
| 17:39:53 | sean-k-mooney | right which is why i said there was no rule that we cant do it more often | |
| 17:40:09 | sean-k-mooney | but we avoid it unless it buys use a lot | |
| 17:40:16 | bauzas | sure, but my concern is to make sure that if we don't need them, we should stop passing them by the client | |
| 17:40:32 | bauzas | hence a 6.x release for stopping this | |
| 17:40:42 | bauzas | and later, a 7.0 for removing | |
| 17:40:46 | sean-k-mooney | with that said not that we enforce the min compute version on startup there is less utility in keeping a major version for a long time | |
| 17:42:44 | bauzas | sean-k-mooney: again, stop thinking about the next major RPC release | |
| 17:43:11 | bauzas | sean-k-mooney: just a single minor version for stopping to emit this param and I'll be happy | |
| 17:43:34 | sean-k-mooney | sure but to do that you need to make them optional parmaters right | |
| 17:43:41 | sean-k-mooney | and then just stop setting them | |
| 17:44:26 | sean-k-mooney | so you need to prepare for it in the 6.0 version | |
| 17:45:01 | sean-k-mooney | unless you were planning to keep them mandatoy and have the clinet hardcode None or something? | |
| 17:47:25 | sean-k-mooney | i dont think i have review a patch were we actully droped parmaters. i know matt did it before but im not sure what the process is | |
| 17:54:19 | sean-k-mooney | https://github.com/openstack/nova/commit/a761e57368280b6d3e931831ecd393fd5787b3ef dropped the compat code for 4.x but the parmaters i think were doped in teh 5.0 bump that added the 4.x proxy | |
| 17:54:21 | sean-k-mooney | https://github.com/openstack/nova/commit/eae37a27caa5ca8b0ca50187928bde81f28a24e1#diff-91f79786d7e3744c39926c88bbafe3b727630fa4eb48e845686d7f12f876d067L531 | |
| 18:05:02 | openstackgerrit | Merged openstack/nova master: Docs: Correct ``Password injection using the dashboard`` Explanation https://review.opendev.org/c/openstack/nova/+/775084 | |
| 18:09:53 | dansmith | sean-k-mooney: we only ever drop params in a major bump, and all such bumps have dropped parameters, AFAIR | |
| 18:10:31 | dansmith | sean-k-mooney: you need to continue to honor them for N-1.x compatibility in that shim, but the N.0 client and server can assume everything is always passed and expected at the new signatures you want going forward | |
| 18:11:05 | sean-k-mooney | dansmith: yes that is what i understood too | |
| 18:11:09 | dansmith | ack | |
| 18:15:44 | dansmith | bauzas: to be clear, we do not need to do any sort of "deprecate in 6.x and remove in 7.0" dance for _anything_. We're the only consumer of this API, so we only need to make sure we honor our own rules.. | |
| 18:16:08 | dansmith | The entire 5.x lineage is adding things to the API as optional, to be made mandatory in 6.0, and ignoring things we plan to drop in 6.0 | |
| 18:16:52 | dansmith | so all you need to do is support 5.max and 6.0 in both the client and server, and the only thing that you can't do is break 5.max.. 6.0 could be anything else you want if you're willing to make the changes | |
| 18:19:06 | bauzas | dansmith: ok, I'll see what I can do | |
| 18:19:20 | bauzas | the 6.0 bump already has a long list of changed things | |
| 18:19:28 | bauzas | and I'm just about cleaning it more | |
| 18:19:42 | bauzas | I'm the janitor and it's a mess | |
| 18:19:59 | dansmith | that's the whole point of a major bump though | |
| 18:20:09 | bauzas | I don't disagree | |
| 18:20:14 | dansmith | it's both why we should maybe do it more frequently, and also why we don't :) | |
| 18:20:41 | bauzas | I'm just afraid by the timings here | |
| 18:20:45 | bauzas | and the review steam | |
| 18:21:00 | dansmith | yup, it's hard, and why we've been talking about it for a cycle :) | |
| 18:21:38 | bauzas | Zuul eventually gave me its blessing after a long period of doubt | |
| 18:22:04 | bauzas | and I'm just removing extra other bits | |
| 18:24:00 | bauzas | but OK, I can try | |
| 18:43:59 | openstackgerrit | Stephen Finucane proposed openstack/python-novaclient master: WIP: Add support for setting VNC password https://review.opendev.org/c/openstack/python-novaclient/+/778229 | |
| 18:45:39 | stephenfin | melwitt: More comments on https://review.opendev.org/c/openstack/nova/+/622336 | |
| 18:45:49 | stephenfin | I'm so confused about what's doing what | |
| 18:48:04 | stephenfin | melwitt: At present, it seems totally broken but there's a good chance I'm misunderstanding something. If so, or if I'm saying daft things, please let me know :) | |
| 18:55:55 | bauzas | dansmith: around ? I'm facing some concern about https://review.opendev.org/c/openstack/nova/+/761452/9/nova/tests/functional/libvirt/test_numa_live_migration.py | |
| 18:56:19 | bauzas | since we upgraded to 6.0, now the version_cap is defaulted to 6.0 | |
| 18:56:42 | bauzas | and then can_send_version() can only support 6.0 | |
| 18:57:01 | bauzas | while previously with 5.13, can_support_version was accepting 5.3 | |
| 18:57:13 | bauzas | even if the version cap was 5.13 | |
| 18:57:48 | bauzas | what should I do ? stick with what I wrote, or stub the RPC API for a 5.13 version cap ? | |
| 18:58:00 | bauzas | (even if we don't pin) | |
| 18:58:39 | bauzas | this whole testclass is so tangled with RPC versioning that we should consider axing a lot of them later on | |
| 18:58:48 | bauzas | but I just want to pass the bar here | |
| 18:59:09 | bauzas | and not do unnecessary scrubbing | |
| 19:02:46 | bauzas | actually, I need to eat by now | |
| 19:16:35 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Bump the Compute RPC API to version 6.0 https://review.opendev.org/c/openstack/nova/+/761452 | |
| 19:41:38 | openstackgerrit | Merged openstack/nova master: libvirt: allow querying devices from the persistent domain https://review.opendev.org/c/openstack/nova/+/772383 | |
| 20:18:18 | dansmith | bauzas: I think those tests were doing exactly what they needed, and I remember all the work artom did to make them validate all the upgrade concerns | |
| 20:19:02 | dansmith | bauzas: I think a lot of the concern there goes away with 6.0, but it might be best to pin them to their original versions (i.e. pretend they're running in an upgrade scenario) until we can drop 5.x | |
| 20:19:48 | bauzas | so, which option would you prefer ? | |