| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-02-24 | |||
| 17:06:52 | bauzas | but we agreed 5 mins ago with gmann to not merge it | |
| 17:07:25 | bauzas | that said, I'll have to leave in 10 mins | |
| 17:07:28 | dansmith | bauzas: yeah, I really wish we shouldn't do those sorts of things | |
| 17:07:30 | dansmith | but regardles | |
| 17:07:54 | dansmith | all I'm trying to figure out is whether you actually wanted *two* microversions, one for the event and one for the rebuild change | |
| 17:08:10 | bauzas | two REST microversions ? | |
| 17:08:14 | dansmith | or if you meant to split the plumbing underneath (i.e. the actual event, the rebuild code) from the api changes | |
| 17:08:24 | bauzas | no, I didn't ask for it | |
| 17:08:36 | bauzas | I asked for splitting the plumbing as you speak | |
| 17:08:40 | dansmith | I forgot that we started microversions for the events (which I'm not sure why we started doing that), but.. | |
| 17:08:41 | dansmith | right okay | |
| 17:08:59 | bauzas | let's call it event versioning, if you prefer :) | |
| 17:09:02 | dansmith | cool cool, my mistake for forgetting that initially so I was on a slightly wrong track | |
| 17:09:19 | bauzas | but gibi knows more than me on it | |
| 17:09:46 | bauzas | dansmith: again, that said, I won't be able to review it honestly today | |
| 17:09:52 | dansmith | bauzas: that's fine | |
| 17:09:57 | bauzas | as I have an hard stop in 5 mins | |
| 17:09:58 | dansmith | whoami-rajat: are you cool with me trying to split your patch accordingly or do you want to do it? | |
| 17:11:31 | gibi | bauzas: I think you are conflating versioning notifications emitted by nova and microversion bump for the external_events REST API. | |
| 17:11:49 | whoami-rajat | dansmith, either way is fine with me, maybe better if you do it because I still don't have full idea of the ask here or if guided i can | |
| 17:12:07 | dansmith | whoami-rajat: ack, partially my fault anyway :D | |
| 17:13:00 | whoami-rajat | dansmith, i think it's because the codebase is fairly new to me :) | |
| 17:13:20 | dansmith | whoami-rajat: you should seize the opportunity to blame me ;P | |
| 17:13:32 | bauzas | gibi: I'm not conflating, but I wrote a comment which was unclear :) | |
| 17:13:53 | gibi | OK. I don't have the full context sorry :) | |
| 17:14:03 | bauzas | https://review.opendev.org/c/openstack/nova/+/820368/9#message-f4b92b3310dee2af9598f108dc761f7fabafd34a | |
| 17:14:21 | whoami-rajat | :D | |
| 17:14:27 | bauzas | "event one" was unclear | |
| 17:14:34 | gibi | OK. | |
| 17:14:34 | bauzas | I was referring to the patch | |
| 17:14:39 | gibi | I see now | |
| 17:14:48 | bauzas | not to any other microversion | |
| 17:17:23 | bauzas | other cores, I trust you | |
| 17:17:33 | bauzas | if you wanna continue to review, fine by me | |
| 17:19:17 | ralonsoh | gibi, https://review.opendev.org/c/openstack/neutron/+/829247 just a heads-up, if you didn't see my reply | |
| 17:22:34 | gibi | ralonsoh: thank, I haven't seen it yet. I will check tomorrow or next week, as we are in feature freeze crazy :) | |
| 17:22:43 | ralonsoh | sure, no rush | |
| 17:22:50 | ralonsoh | ping me whenever you can | |
| 17:28:27 | opendevreview | Jonathan Race proposed openstack/nova master: object/notification for Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/828369 | |
| 17:28:28 | opendevreview | Jonathan Race proposed openstack/nova master: zuul-job for Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/828372 | |
| 17:28:28 | opendevreview | Jonathan Race proposed openstack/nova master: driver/secheduler/docs for Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053 | |
| 17:29:04 | gibi | ralonsoh: thanks | |
| 17:29:43 | chateaulav | sean-k-mooney: that should do, sorry it took a litle bit the rebase got wonked so started fresh. | |
| 17:29:48 | chateaulav | gibi: thanks as well | |
| 17:30:23 | gibi | chateaulav: rebaswe looks clear | |
| 17:30:35 | gibi | so I'm +2 | |
| 17:30:41 | gibi | but now I have to stop for today | |
| 17:30:47 | gibi | see you all tomorrow o/ | |
| 17:31:02 | chateaulav | thanks | |
| 17:41:34 | opendevreview | Merged openstack/nova master: Allow per-context rule in error messages https://review.opendev.org/c/openstack/nova/+/816865 | |
| 17:45:32 | dansmith | whoami-rajat: so, functional tests fail without the api change in place, which I think means that we're not properly honoring the old microversion behavior | |
| 17:45:36 | dansmith | which is a good reason for this split | |
| 17:53:03 | whoami-rajat | dansmith, ack | |
| 17:59:35 | opendevreview | Dan Smith proposed openstack/nova master: Add support for volume backed server rebuild https://review.opendev.org/c/openstack/nova/+/820368 | |
| 17:59:36 | opendevreview | Dan Smith proposed openstack/nova master: Add API support for rebuilding BFV instances https://review.opendev.org/c/openstack/nova/+/830883 | |
| 17:59:36 | opendevreview | Dan Smith proposed openstack/nova master: Add volume-rebuild support to cinder module https://review.opendev.org/c/openstack/nova/+/830882 | |
| 17:59:48 | dansmith | whoami-rajat: ^ | |
| 18:00:04 | dansmith | whoami-rajat: assuming that first mini patch is good, we should be able to get that merged ASAP and cut down what remains | |
| 18:00:47 | dansmith | hmm, did I lose the reno? | |
| 18:01:22 | opendevreview | Dan Smith proposed openstack/nova master: Add API support for rebuilding BFV instances https://review.opendev.org/c/openstack/nova/+/830883 | |
| 18:01:28 | dansmith | there we go | |
| 18:01:51 | whoami-rajat | ack, thanks for splitting it up | |
| 18:02:05 | dansmith | whoami-rajat: so I think we need to get the rebuild flag down to the compute manager so it knows whether or not to trigger the new behavior | |
| 18:05:13 | whoami-rajat | dansmith, i agree with the usage of an old version of API but if they don't pass the flag, it will never reach the compute manager and should fail at API level | |
| 18:05:32 | whoami-rajat | but we need to still consider the old API new compute case | |
| 18:05:34 | dansmith | whoami-rajat: see the test failure in that middle patch :) | |
| 18:05:43 | whoami-rajat | will look into that | |
| 18:05:45 | dansmith | whoami-rajat: if the image ref does not change, we should rebuild the instance but not the volume, | |
| 18:05:56 | dansmith | but your code makes it still call rebuild, which will erase the root volume | |
| 18:06:48 | whoami-rajat | Ah ok, we support rebuilding volume backed instances having same image as the provided one | |
| 18:07:18 | dansmith | right, the image can't change, but we do that because we don't destroy the root disk, so we require the image to be the same | |
| 18:07:18 | whoami-rajat | so reimage_boot_volume should be checked in the manager, got it | |
| 18:07:32 | dansmith | yep | |
| 18:07:41 | whoami-rajat | ack, will work on that | |
| 18:07:42 | dansmith | also, | |
| 18:08:00 | dansmith | your api will only check that field if the image-ref changes, so even with the new version of the API, you can trigger the accidental root volume erase | |
| 18:08:16 | dansmith | so something needs to change there to make sure we never ever erase the root disk unless they specifically ask for it | |
| 18:08:33 | dansmith | lemme comment on that to record that | |
| 18:10:45 | whoami-rajat | if we don't trigger the rebuild_volume_backed_instance code in compute, something like, if reimage_boot_volume and is_volume_backed: , then we're safe from the same image case right? | |
| 18:11:02 | whoami-rajat | but if you want the operation to get blocked at the API layer only then we can change that | |
| 18:11:30 | dansmith | we have to support the old and new behavior in manager | |
| 18:11:41 | dansmith | we need to make sure the api properly communicates what should happen to the manager | |
| 18:11:56 | dansmith | the api may enforce certain behaviors of the api client, but we have to make sure manager knows what the right thing to do is | |
| 18:12:42 | whoami-rajat | ok | |
| 18:14:31 | dansmith | whoami-rajat: be sure to put your co-authored-by on those new patches when you revise.. I meant to do that, sorry | |
| 18:15:21 | dansmith | I don't want to look like I took credit for your work | |
| 18:15:33 | whoami-rajat | dansmith, np, will do it and doesn't matter much :) | |
| 18:15:37 | whoami-rajat | :D | |
| 18:15:46 | whoami-rajat | doesn't look like that at all | |
| 18:15:49 | dansmith | it matters A LOT :) | |
| 18:16:13 | whoami-rajat | :) | |
| 18:17:49 | opendevreview | Merged openstack/nova master: Revert project-specific APIs for servers https://review.opendev.org/c/openstack/nova/+/816206 | |
| 18:17:56 | opendevreview | Merged openstack/nova master: Test PROJECT_ADMIN APIs with no legacy rule case https://review.opendev.org/c/openstack/nova/+/824845 | |
| 18:18:03 | opendevreview | Merged openstack/nova master: Move rule_if_system() method to base test class https://review.opendev.org/c/openstack/nova/+/824475 | |
| 18:19:07 | opendevreview | Rajat Dhasmana proposed openstack/nova master: Add volume-rebuild support to cinder module https://review.opendev.org/c/openstack/nova/+/830882 | |
| 18:21:32 | whoami-rajat | dansmith, so does the first patch looks good or anything to change there? (just added the co-authored-by to commit msg ^) | |
| 18:22:01 | dansmith | whoami-rajat: I didn't actually look, but it's probably straightforward | |
| 18:22:45 | whoami-rajat | yeah just sending the request to cinderclient | |
| 18:23:17 | dansmith | yep, +2d | |
| 18:23:33 | whoami-rajat | great thanks | |