| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-01-31 | |||
| 13:27:29 | bauzas | when would you unmount the share ? | |
| 13:27:42 | Uggla | bauzas, it depends. :) | |
| 13:27:51 | bauzas | of the snow level ? | |
| 13:27:53 | bauzas | :) | |
| 13:28:06 | Uggla | bauzas, if it is not used by another VM yes. | |
| 13:28:23 | bauzas | so when deleting the instance ? | |
| 13:30:27 | Uggla | bauzas, hum not sure I get your point. But if you delete a VM with a share it goes to the same code and umount the share if it is not used elsewhere. | |
| 13:40:45 | gibi | Uggla: yeah, as we are calling other APIs I would make the detach async as well. But as you pointed out that will be faster as no polling there. | |
| 13:40:50 | bauzas | so yeah, it should be an async call | |
| 13:41:00 | bauzas | by design, we never ever stop a deletion | |
| 13:41:25 | bauzas | we first accept the deletion request and only after we try to delete the resource | |
| 13:42:43 | gibi | bauzas: having the unmount at detach means VM deletion does not need to talk to manila | |
| 13:42:57 | gibi | scratch that | |
| 13:43:16 | gibi | if we delete a VM with an attached share then the umount need to happen during VM deletion | |
| 13:43:37 | gibi | but VM delet is already async so I don't see an issue there | |
| 13:43:45 | Uggla | gibi, ++ | |
| 13:47:09 | bauzas | gibi: you're right on the fact that the delete is already async | |
| 13:47:46 | bauzas | when thinking about that, I also wonder why we need to unmount the share when deleting the resource in the compute | |
| 13:47:50 | Uggla | gibi, I would say the polling is more to ensure manila has applied the policy before mounting than really wait for manila. I set a timeout here mainly to avoir an infinite loop if manila is not working. | |
| 13:48:07 | bauzas | it could be a periodic | |
| 13:48:24 | bauzas | (if we have correct statuses) | |
| 13:49:39 | Uggla | bauzas, yep but it is better to clean the room yourself and not your mom (periodic). :) | |
| 13:49:51 | gibi | I see periodics as fallbacks when something bad happens during normal operators. | |
| 13:50:03 | bauzas | my only concern is the polling | |
| 13:50:13 | bauzas | if we don't need to call manila, sure | |
| 13:50:28 | gibi | we need to call manial to unlock the share | |
| 13:50:40 | bauzas | hence my concern | |
| 13:50:43 | gibi | but during delete we don't need to poll manila | |
| 13:50:49 | gibi | as unlock is sync | |
| 13:50:51 | gibi | I think | |
| 13:51:02 | bauzas | there are two stages | |
| 13:51:07 | bauzas | 1/ user asks to delete the instance | |
| 13:51:15 | bauzas | 2/ compute eventually deletes it | |
| 13:51:29 | bauzas | surely, for 1/, it's async | |
| 13:51:50 | bauzas | the compute could be not running | |
| 13:51:51 | bauzas | but for 2/ | |
| 13:52:04 | bauzas | once the compute gets the fact that it needs to delete a guest, then it runs | |
| 13:52:15 | bauzas | and that's my concern | |
| 13:52:53 | bauzas | I don't want this compute run to be waiting to manila to unlock the share | |
| 13:53:20 | gibi | but we do the same with port unbinding and probably for unreserving the volume in glance | |
| 13:53:23 | gibi | in cinder | |
| 13:54:08 | bauzas | hmm ok | |
| 13:54:37 | gibi | also we tend to say that nova compute works sort of with all disable periodics | |
| 13:56:08 | bauzas | wdym ? | |
| 13:57:06 | sean-k-mooney | im in a meeting right now but in genreal we shoudl be able to run nova without perodics enabeld modulo bugs | |
| 13:57:14 | gibi | :) | |
| 13:57:20 | sean-k-mooney | so fallback to perodic sure | |
| 13:57:21 | gibi | yeah I heard this from Sean | |
| 13:57:40 | gibi | bauzas: we do the volume detach in cinder API during terminate instance here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L3284 | |
| 13:58:24 | sean-k-mooney | the only "features" that need perodics to fucntion are things like local_delete | |
| 13:58:40 | sean-k-mooney | where we will process any delete that were done while the agent was stopped | |
| 13:58:50 | sean-k-mooney | and all of those run in init host | |
| 13:59:38 | bauzas | if people are okay with waiting for manila by polling, fine by me then | |
| 14:00:00 | bauzas | it will be simplier for Uggla :) | |
| 14:00:06 | sean-k-mooney | i dont think its safe to consider the isntace deleted without at least unmounting the share form the host | |
| 14:00:16 | sean-k-mooney | the unlock i think shoudl be done too | |
| 14:00:30 | sean-k-mooney | buti could see not failing the delete if tha tdoes nto happen | |
| 14:00:36 | sean-k-mooney | and trying again via a perodic | |
| 14:00:38 | bauzas | that's my concern | |
| 14:00:45 | sean-k-mooney | but doing only in perodic woudl not be ok in my view | |
| 14:00:54 | bauzas | I don't want the delete to have an exception if we are not able to call Manila | |
| 14:01:12 | gibi | what happens today when a volume cannot be detached during VM delete? | |
| 14:01:18 | bauzas | we should rather delete the guest and eventually try to unmount | |
| 14:01:35 | sean-k-mooney | no the unmount on the host has to happen as part of the delete | |
| 14:01:41 | sean-k-mooney | its a security isssue if it does not | |
| 14:01:49 | bauzas | why ? | |
| 14:01:51 | sean-k-mooney | the unlock of the share could be defered | |
| 14:02:06 | sean-k-mooney | bauzas: because you have a tenatns data volume present on the host | |
| 14:02:15 | bauzas | if we don't delete the share in nova, it's just a file | |
| 14:02:22 | bauzas | that none of other instances could use | |
| 14:02:56 | sean-k-mooney | its not just a file its an nfs/cephfs share that is mounted on the host | |
| 14:03:09 | sean-k-mooney | that anyoen with host access coudl read with the correct permissions | |
| 14:03:32 | bauzas | anyone with host access would have more possibilties than using a share | |
| 14:03:38 | sean-k-mooney | i.e. if you break out of a vm and get host access you can read the shares for any share mounted on the host | |
| 14:04:02 | sean-k-mooney | sure, to gibi's point we should mirror what we do for cinder volumes | |
| 14:04:12 | bauzas | I think we said that it's not a security issue if you need to ssh to the host | |
| 14:04:43 | bauzas | the problem we had with cinder is that we were able to get *that* bdm for other instances | |
| 14:04:52 | sean-k-mooney | this is very close but not the same as the cidner issue with deleting the atatchment in my view | |
| 14:05:09 | bauzas | we discussed that a bit this morning during our meeting | |
| 14:05:12 | sean-k-mooney | bauzas: not bdm, host block device | |
| 14:05:21 | bauzas | correct | |
| 14:05:36 | bauzas | anyway, I'm doing other things by now, I can't discuss it more now | |
| 14:05:42 | sean-k-mooney | ya so i agree you wont have sharing because we will be mounting the sahre under the instance dir right | |
| 14:05:44 | bauzas | Uggla: are you OK ? | |
| 14:06:09 | bauzas | no, we don't mount the share on the instance dir | |
| 14:06:31 | bauzas | but we have share mappings telling which share to use with what instance | |
| 14:06:33 | sean-k-mooney | it should be either configurabel or under the instace dir | |
| 14:06:45 | bauzas | there is a spec | |
| 14:06:53 | bauzas | and within this spec, this is explained | |
| 14:07:01 | Uggla | bauzas, yep for me it is ok. I have my answers | |
| 14:07:15 | sean-k-mooney | bauzas: im pretty sure the spec did not say its oke to defer the unmoutn to a perodic | |
| 14:08:31 | bauzas | that, indeed, I was just saying about the share path | |
| 14:08:53 | bauzas | anyway, /me going back to work given Uggla got his answers | |
| 14:09:33 | sean-k-mooney | Uggla: so your going to do the unmount when deleting the isntance as part of delete with a fallback to a perodic | |
| 14:10:08 | Uggla | sean-k-mooney, yep but atm I have not periodic. | |
| 14:10:08 | sean-k-mooney | and not have manilla block the delete if unlocking fails? | |
| 14:10:39 | sean-k-mooney | right so without that we would have to make the failure to unlock put the instnace in error | |
| 14:10:49 | sean-k-mooney | gibi: how do you feel about that ^ | |
| 14:12:48 | sean-k-mooney | i do not have the full context so im mostly relying on you adn bauzas to ensure the correct behavior. if we just ignroed the failure to unlock the manilla share with no recovery method i woudl think that deserves a -2 | |
| 14:14:59 | gibi | The cheaper way is to put the instance in ERROR during delete. I suggest to check what we do with cinder volumes during delete if cinder is unavailable | |