| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-05-30 | |||
| 16:25:57 | dansmith | and I don't think the cinder one (which is the most important) is being backported past xena | |
| 16:26:28 | dansmith | our part of the fix for that CVE is pretty minor and we could backport it, but without the FC part of the brick fix it's not complete | |
| 16:26:28 | bauzas | that's why I personnally too lean forward to EOLing stable/train | |
| 16:26:47 | dansmith | only debian spoke up about/agains EOLing right/ | |
| 16:26:51 | bauzas | correct | |
| 16:27:01 | bauzas | I can reply and explain the problem again | |
| 16:27:31 | dansmith | shrug | |
| 16:27:49 | dansmith | cinder does still have a train branch | |
| 16:27:56 | dansmith | and the last thing there was the VMDK fix | |
| 16:28:15 | dansmith | so does brick, but the last thing was last summer | |
| 16:28:30 | elodilles | i also stated that we could keep open train as long as the gate is working, though indeed it is unfortunate that CVE fixes doesn't get backported :/ | |
| 16:28:36 | dansmith | idk, almost seems like there's not enough people to care to even push us one way or the other :) | |
| 16:28:49 | bauzas | I would indeed be more inclined if some other project like cinder would make the same step than us quite at the same time | |
| 16:29:20 | bauzas | dansmith: elodilles: honestly I guess I'll propose a patch for -eol and people can -1 if they care | |
| 16:29:33 | bauzas | that's probably where we'll capture most of the concerns | |
| 16:29:42 | dansmith | ack | |
| 16:29:46 | bauzas | or we could round forever | |
| 16:29:48 | elodilles | ++ | |
| 16:30:16 | bauzas | #action bauzas to propose a gerrit change for tagging stein-eol so people could vote on it | |
| 16:30:30 | elodilles | you mean train-eol :) | |
| 16:30:35 | bauzas | damn | |
| 16:30:38 | bauzas | #undo | |
| 16:30:38 | opendevmeet | Removing item from minutes: #action bauzas to propose a gerrit change for tagging stein-eol so people could vote on it | |
| 16:30:45 | elodilles | stein-eol is long gone for nova ;) | |
| 16:30:46 | bauzas | #action bauzas to propose a gerrit change for tagging train-eol so people could vote on it | |
| 16:30:54 | bauzas | elodilles: my brain fscked | |
| 16:31:13 | bauzas | anyway, I guess we're done on this | |
| 16:31:22 | bauzas | #topic Open discussion | |
| 16:31:23 | elodilles | ++ | |
| 16:31:36 | bauzas | geguileo: you had a point :) | |
| 16:31:41 | bauzas | (geguileo) Change to os-brick's connect_volume idempotency | |
| 16:31:47 | geguileo | bauzas: yes, thanks | |
| 16:31:49 | bauzas | #link https://review.opendev.org/c/openstack/os-brick/+/882841 | |
| 16:31:52 | bauzas | #link https://bugs.launchpad.net/nova/+bug/2020699 | |
| 16:32:11 | geguileo | So with the latest CVE on SCSI volumes we decided in os-brick to make some changes | |
| 16:32:45 | geguileo | Specifically os-brick would remove any existing devices before starting the connect_volume code | |
| 16:32:55 | geguileo | and then proceeding with the actual attachment | |
| 16:33:09 | geguileo | this means that connect_volume will no longer be idempotent | |
| 16:33:17 | geguileo | (which is never something we promised would be) | |
| 16:33:26 | geguileo | that seems to break some Nova operations | |
| 16:33:57 | geguileo | particularly the rescue/unrescue operations | |
| 16:34:05 | dansmith | is there some way we can check to see if we need to run connect_volume()? | |
| 16:34:24 | bauzas | geguileo: thanks for spotting this in advance | |
| 16:34:30 | dansmith | because otherwise it's hard to know if we're restarting after a recovery or just a service restart | |
| 16:34:39 | dansmith | whether or not we should run that | |
| 16:35:08 | bauzas | yeah that's one of the problems | |
| 16:35:22 | geguileo | dansmith: that could be solved if Nova didn't stash and then unstash the instance config | |
| 16:35:33 | geguileo | and instead rebuilt the instance config | |
| 16:35:40 | dansmith | meaning the guest xml? | |
| 16:35:41 | geguileo | I believe sean-k-mooney[m] mentioned that | |
| 16:35:45 | geguileo | dansmith: yes | |
| 16:36:00 | bauzas | geguileo: we also recreate the XML on stop/start fwiw | |
| 16:36:11 | dansmith | okay, but if we're running on a readonly-root host or recovering from a disaster, none of that would be available after a restart for us | |
| 16:36:27 | dansmith | yeah, the guest XML isn't something we can assume hangs around long-term IMHO | |
| 16:36:41 | geguileo | dansmith: could nova know if it's attaching the volume for the recovery instance on the same host that was running the instance? | |
| 16:36:42 | dansmith | most of nova is designed to assume that it's tempoary | |
| 16:36:53 | bauzas | yeah and in general, we don't consider libvirt a source of truth for persisting some instance metadata | |
| 16:37:01 | dansmith | bauzas: right | |
| 16:38:00 | dansmith | geguileo: for the rescue case specifically? if the instance was already stopped and you go straight to rescue, we wouldn't really know when the last time the connect was run.. could have been weeks ago and multiple host reboots since | |
| 16:38:01 | bauzas | would the spec on dangling volumes helping the problem ? | |
| 16:38:17 | dansmith | bauzas: I don't think so | |
| 16:38:19 | bauzas | I mean, our BDM/volume reconciliation | |
| 16:38:33 | dansmith | that's not the problem, it's the underlying host devices | |
| 16:38:48 | bauzas | yeah I remember the context of the CVE :) | |
| 16:39:21 | bauzas | and I guess only brick knows whether there is a residue ? | |
| 16:39:29 | dansmith | geguileo: so let's say we reboot with one instance stopped that points to /dev/vdf, | |
| 16:39:49 | dansmith | then we reboot, and someone spawns a new instance, we call connect_volume() for that new instance, it gets /dev/vdf, | |
| 16:40:09 | dansmith | then the user starts the old was-stopped instance, we can't look at the instance xml config and know that the volume is wrong right? | |
| 16:40:20 | dansmith | because /dev/vdf exists, but it's no longer relevant for this instance | |
| 16:40:55 | geguileo | dansmith: well, Nova could check the ID of the volume and see if it matches the information returned by os-brick | |
| 16:41:07 | dansmith | point being, there could be multiple instances with disks that point to stale host devices at any given point.. | |
| 16:41:09 | geguileo | but I don't know if we want Nova to be on the business of doing those checks | |
| 16:41:17 | dansmith | yeah, ideally not | |
| 16:41:19 | geguileo | dansmith: yes, that could happen | |
| 16:42:05 | bauzas | could nova ask brick such thing ? | |
| 16:42:08 | dansmith | geguileo: maybe everywhere we currently do connect_volume() we do a disconnect first and ignore an error? that generates a lot of churn, but maybe that's safer? | |
| 16:42:29 | dansmith | bauzas: that's what I was wondering.. if there was some validate_volume() call or something we could run to know if we should run connect | |
| 16:42:41 | bauzas | like, before doing connect_volume(), do a check_vol() | |
| 16:42:43 | bauzas | ahah | |
| 16:42:54 | geguileo | dansmith: os-brick will be doing that already with optimized code | |
| 16:43:20 | geguileo | dansmith: that's the issue that I'm bringing, that the new code that cleans things up break the idempotency, so there's no need for nova to do that call you mention | |
| 16:43:37 | geguileo | os-brick cannot do a proper check_vol like we would want | |
| 16:43:48 | geguileo | because the volume can have changed in the background | |
| 16:44:06 | geguileo | for example change the size, point a different volume in the backend (like happened in the CVE) | |
| 16:44:26 | dansmith | okay, I'm just not sure how we can know what the right thing to do is | |
| 16:44:27 | geguileo | there are a bunch of messes that could happen, and I'm not sure we would want this to depend on nova calling a method | |
| 16:45:04 | geguileo | dansmith: well, if you call connect_volume all instances in Nova should use the device it returns from that moment onwards | |
| 16:45:10 | bauzas | geguileo: I'm confused, who's responsible for knowing that all connectors are present on the host ? | |
| 16:45:17 | bauzas | I thought it was brick | |
| 16:45:23 | dansmith | geguileo: okay I thought you didn't want us to do that | |
| 16:45:27 | geguileo | what do you mean by connectors? | |
| 16:45:48 | bauzas | wrong wording, my bad. | |
| 16:45:51 | dansmith | geguileo: or are you just saying any time we call connect_volume() we need to examine the result and make sure the instance is (changed, if necessary) to use that/ | |
| 16:46:16 | bauzas | geguileo: I'd say the device number | |
| 16:46:44 | geguileo | dansmith: what I don't want is nova to call connect_volume, get the value and stash it (/dev/sdc), then call connect_volume again (/dev/sda) to use it in the rescue instance, and then use the stashed value (/dev/sdc) that no longer exists | |
| 16:47:05 | geguileo | dansmith: yes, that's it | |
| 16:47:18 | dansmith | geguileo: okay | |
| 16:47:25 | geguileo | dansmith: if connect_volume is called, make sure that is used in all the instances that use the same cinder volume on that host | |
| 16:47:53 | dansmith | sounds like the solution is that we need to call connect_volume every time we're going to start an instance for any reason, and update the XML (rescue or otherwise) to use that value before we start | |
| 16:48:43 | geguileo | dansmith: that would be slower but safe | |