| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-02-21 | |||
| 14:10:22 | sean-k-mooney | the active status of a share mappign shoudl not depend on the vm status (active vs powered off) | |
| 14:10:41 | bauzas | that's written in the spec | |
| 14:10:42 | sean-k-mooney | these are two diffenet logical resouces in the api | |
| 14:11:36 | gibi | bauzas: it is not about the instance action but about the state of the sharemapping. We can keep it inactive or we can put it in error. | |
| 14:12:37 | bauzas | gibi: I got that, lemme summarize the exception handling case | |
| 14:12:59 | bauzas | user stops a VM, share mapping goes inactive | |
| 14:13:12 | sean-k-mooney | looking at the Share mapping status diagram in the spec | |
| 14:13:21 | bauzas | when it stops the VM, some internal call is doing two actions : | |
| 14:13:34 | bauzas | 1/ detach the share from the VM | |
| 14:13:43 | sean-k-mooney | i can see where you were going with it but it does not follow the same conventiosn as cinder or neutron ports | |
| 14:13:47 | bauzas | 2/ umount the share if not used elsewhere | |
| 14:14:04 | gibi | bauzas: OK so you suggest to do the unmount during stop, not during the detach REST API call | |
| 14:14:20 | sean-k-mooney | well the spec says we do the unmount during stop | |
| 14:14:21 | bauzas | nah, sorry | |
| 14:14:28 | bauzas | I'm waaaay confused | |
| 14:14:39 | bauzas | user has to do two actions then | |
| 14:14:44 | bauzas | stop then detach | |
| 14:14:50 | bauzas | right? | |
| 14:14:56 | Uggla | yep | |
| 14:15:00 | bauzas | and detach does two things | |
| 14:15:04 | bauzas | (internally) | |
| 14:15:16 | bauzas | one is updating the guest XML | |
| 14:15:23 | gibi | nope | |
| 14:15:25 | bauzas | the other is to umount the share | |
| 14:15:32 | gibi | the vm is stopped no need to update the XML | |
| 14:15:41 | Uggla | gibi ++ | |
| 14:15:57 | bauzas | my head is full of vGPU crap for now | |
| 14:16:03 | gibi | the XML is generated during start based on the share_mappings | |
| 14:16:05 | bauzas | I have to reload that context | |
| 14:16:12 | sean-k-mooney | the vm is stopped meaning the share is also unmoutned already | |
| 14:16:17 | gibi | not | |
| 14:16:21 | gibi | today | |
| 14:16:31 | sean-k-mooney | in the spec that what the diagram states | |
| 14:16:39 | gibi | today stop is not unmounting the share from the host | |
| 14:16:50 | gibi | OK, then that is a gap between the spec and the code | |
| 14:16:51 | sean-k-mooney | well that is not in line with the spec then | |
| 14:16:54 | sean-k-mooney | yep | |
| 14:17:09 | sean-k-mooney | what in the spec i think aligns with what i would like to see | |
| 14:17:09 | gibi | do we want to move the host unmount to stop? | |
| 14:17:25 | sean-k-mooney | if we do that then detach cant fail in this edge case | |
| 14:17:35 | gibi | and do we want to fail the stop action if unmount fails? | |
| 14:18:02 | bauzas | do we all agree on the fact that unmount is not strictly required ? | |
| 14:18:15 | sean-k-mooney | and i belive unmounting the share (if no other vm is using it) in stop is also more secure | |
| 14:18:18 | gibi | bauzas: do you mean, is it OK to leak mounts? | |
| 14:18:25 | sean-k-mooney | bauzas: no | |
| 14:18:57 | gibi | I'm not sure what happens with a mount if the export on the manila side is removed | |
| 14:18:59 | bauzas | umount is not strictly required if we still have a share mapping, right ? | |
| 14:19:21 | gibi | bauzas: if you mean we have a record of the mount so we can do the unmount later, then yes | |
| 14:19:21 | sean-k-mooney | as far as im concerned the attach and detach rps shoudl not try to mount/unmount the share | |
| 14:19:55 | bauzas | gibi: I'm going back to my original statement : I don't want to have share leaks | |
| 14:20:26 | bauzas | gibi: if we're not able to guarantee that some share mapping state will continue to handle shares that are sitll residues on the compute, then we need to error the detach call | |
| 14:20:27 | sean-k-mooney | form the spec | |
| 14:20:30 | sean-k-mooney | """Share attachment/detachment can only be done if the VM state is STOPPED or ERROR. These are operations only on the database, and no RPC calls will be required to the compute API. This is an intentional design for this spec. As a result, this could lead to situation where the VM start operation fails as an underlying share attach fails.""" | |
| 14:20:41 | sean-k-mooney | that ^ is what the code should be doing | |
| 14:20:41 | gibi | bauzas: I agree | |
| 14:21:21 | bauzas | the phi-state in the diagram actually needs to be explained more | |
| 14:21:27 | gibi | OK. so If we follow that statement from the spec then both the mount and unmount RPC is unnecessary | |
| 14:21:45 | sean-k-mooney | correct | |
| 14:21:51 | sean-k-mooney | its just a cell db update | |
| 14:21:58 | bauzas | I have a vgpu meeting in 8 mins so I'll need to bail out shortly | |
| 14:21:59 | sean-k-mooney | to create a row or delete it | |
| 14:22:04 | gibi | this also means we will host mount during start and unmount during stop / delete | |
| 14:22:15 | sean-k-mooney | yep | |
| 14:22:35 | sean-k-mooney | if start fails to mount we go to error wheere you can either retry or detach the share | |
| 14:22:38 | gibi | so if the mount / umount fails start or stop action can fail and put the VM state in ERROR | |
| 14:22:54 | sean-k-mooney | yep | |
| 14:22:58 | sean-k-mooney | thats what we approved | |
| 14:23:22 | sean-k-mooney | anb all the later assumtiosn of the spec are based on that | |
| 14:23:36 | gibi | Uggla: do you have a reasoning why we diverged from the spec in this regard? Was there any late findings during implementation? | |
| 14:25:11 | gibi | or in other words, what issues do you forsee if we do what the spec says and move the host mount / unmount? | |
| 14:27:46 | Uggla | gibi, yes but I do not remember well. I guess the reason was simpler error mgmt and simplify later online attach. | |
| 14:28:18 | sean-k-mooney | well this discussion shows that the error magament is more complex | |
| 14:28:41 | gibi | online attach is a good topic. When we have that in libvirt we will need an RPC during the attach REST API call. | |
| 14:29:18 | sean-k-mooney | yes, although i do not think that is relvent here | |
| 14:29:28 | bauzas | I think the intent was to let the user restart its instance if it was erroring due to transient issues | |
| 14:29:48 | sean-k-mooney | yes we will need an rpc but that rpc would only be successful if the share was both mounted and attached to the vm | |
| 14:30:06 | sean-k-mooney | it shoudl likely revert the share mounting on the host if the libvirt attach failed | |
| 14:30:13 | sean-k-mooney | so you can try attach again | |
| 14:30:13 | bauzas | that said, there are billing implications on a stop action that would fail due to some internal problem | |
| 14:30:34 | sean-k-mooney | we explcitly allow detach in ERROR | |
| 14:30:40 | sean-k-mooney | to make sure that is not a porblem | |
| 14:30:48 | bauzas | I don't think our public cloud providers woud like their users to be continued to be charged because they have some network issues | |
| 14:30:58 | gibi | I think bauzas refer to stop failing not detach failing | |
| 14:30:58 | sean-k-mooney | if stop failes you can detach and then stop again | |
| 14:31:04 | bauzas | gibi: correct | |
| 14:31:05 | gibi | ahh | |
| 14:31:24 | gibi | you cannot detach if it is not stopped | |
| 14:31:39 | sean-k-mooney | the spec say you can do it in the error state | |
| 14:31:58 | sean-k-mooney | also from the ordering the vm would be stopped then the detach woudl fail | |
| 14:32:09 | sean-k-mooney | *then the unount woudl fail | |
| 14:32:42 | gibi | so stop fails, puts the VM in ERROR, we allow detach in ERROR, so user does the detach, detach deletes the mapping, we lost the mapping while the unmount never happened | |
| 14:32:44 | sean-k-mooney | so it should be safe to allow the detach to update the db. | |
| 14:33:05 | sean-k-mooney | yep and that where a perodic would come in | |
| 14:33:10 | sean-k-mooney | to allow that to be healed | |
| 14:34:07 | sean-k-mooney | same | |
| 14:34:16 | gibi | then lets park this for after the meeting | |
| 15:05:05 | opendevreview | Takashi Kajinami proposed openstack/os-vif master: Drop wrong stacklevel https://review.opendev.org/c/openstack/os-vif/+/909682 | |
| 15:23:25 | tkajinam | ^^^ I noticed I've made this mistake in a few places... it'd be nice if this can be merged and included in the final release for 2024.1 ... | |
| 15:29:20 | opendevreview | Rajesh Tailor proposed openstack/nova master: Add support for showing requested az in output https://review.opendev.org/c/openstack/nova/+/904568 | |
| 18:06:22 | opendevreview | Stephen Finucane proposed openstack/nova-specs master: Add openapi spec https://review.opendev.org/c/openstack/nova-specs/+/909448 | |