| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-01-31 | |||
| 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 | sean-k-mooney | and not have manilla block the delete if unlocking fails? | |
| 14:10:08 | Uggla | sean-k-mooney, yep but atm I have not periodic. | |
| 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 | |
| 14:15:08 | sean-k-mooney | we need to be careful of two things: 1.) we are not locking the share indefintly if we delete the isntnce and the manial call fails, that has billing implciations. 2.) we are not exporting the sahre to host it shoudl not be on, the has security implcitations, and possible regelarotry complicne issues. | |
| 14:15:34 | gibi | I agree about these goals ^^ | |
| 14:16:21 | sean-k-mooney | i would have been ok with puting the instnace in error | |
| 14:16:28 | sean-k-mooney | so you can just try again | |
| 14:16:33 | sean-k-mooney | when manilla is fixed | |
| 14:17:07 | sean-k-mooney | and i suspect that is what would happen for a BFV guest with delete on terminate but im not sure | |
| 14:18:08 | sean-k-mooney | it looks like in terminate we do self._cleanup_volumes(context, instance, bdms, | |
| 14:18:10 | sean-k-mooney | raise_exc=False, detach=False) | |
| 14:18:59 | sean-k-mooney | that imples we allwo the delete to proceed | |
| 14:19:18 | bauzas | I also agree with the two bullet points (just lurking) | |
| 14:20:30 | bauzas | tbh, I don't really like us copying the cinder workflow | |
| 14:20:36 | sean-k-mooney | i am about to enter 3 hours of meetings in 3.5 hours | |
| 14:20:45 | bauzas | recent issues led me think there are some corner cases I'd like to avoid :) | |
| 14:20:47 | sean-k-mooney | so ill also be "steppign way shortly" | |
| 14:21:18 | bauzas | but agreed on the | |
| 14:21:23 | sean-k-mooney | right i think gibi and i wourl prefer to be stircter and fail the delte | |
| 14:21:25 | bauzas | DONTs and the DOES | |
| 14:21:50 | sean-k-mooney | but lets loop back to this after Uggla has updated things | |
| 14:21:53 | sean-k-mooney | and we have more time | |
| 14:22:05 | bauzas | I think we said we should never try to fail on an instance delete | |
| 14:22:15 | bauzas | as there are also some capacity and billing implications | |
| 14:22:25 | sean-k-mooney | we can fail on isntance delete for other reasons im pretty sure | |
| 14:22:45 | sean-k-mooney | but you are correct on the capsity and billing implciations if we do too | |
| 14:22:55 | gibi | it is a trade off if we let the VM delete to happen but leak resources in the meantime you still get billing issues :D | |
| 14:23:12 | bauzas | as an operator, I'd be more angry if my instance would continue to exist because of a cheap open file not deleted | |
| 14:23:37 | bauzas | but I could be wrong | |
| 14:23:50 | gibi | bauzas: e.g. we delete your VM happily but fail to unlock the share in manila. So you are billed for a share you cannot use :) | |
| 14:24:03 | bauzas | hence the periodic :) | |
| 14:24:27 | gibi | which can be turned off : | |
| 14:24:28 | gibi | :) | |
| 14:24:34 | bauzas | I'm not saying we should leave the pipe leaking | |
| 14:24:53 | bauzas | what I'm saying is that there are bigger things to do than fixing the pipe | |
| 14:24:54 | sean-k-mooney | bauzas: would that same operator be ok if custoemr data was leaked via presitent memroy, or if vgpus were never made avaiable again or pci devices | |
| 14:24:58 | gibi | at least if my VM is in ERROR and I cannot delete it I will notice and raise a ticket | |
| 14:25:18 | sean-k-mooney | so there is a way to fix this | |
| 14:25:40 | sean-k-mooney | we coudl ask manilla to aloow the share delteion if the nova instnace nolonger exists | |
| 14:25:56 | sean-k-mooney | that coudl be a followup enhacnment on there side | |
| 14:26:24 | sean-k-mooney | so if its locked by nova but nova says the isntnace is gone aloow the user to delelete it even with the lock | |
| 14:26:31 | bauzas | gibi: sean-k-mooney: standing as a user for a sec, | |
| 14:26:42 | bauzas | if I delete my instance and it goes into ERROR | |
| 14:26:53 | bauzas | the second after, I'm gonna try to delete again | |
| 14:27:01 | sean-k-mooney | yep which might work | |
| 14:27:01 | bauzas | so I could be okay | |
| 14:27:04 | gibi | :) | |
| 14:27:21 | bauzas | provided we ensure that the instance deletion path is idempotent | |
| 14:27:38 | sean-k-mooney | yes but we already need to do that | |
| 14:28:05 | sean-k-mooney | i belive one path where instance delete ends with an instance in error | |
| 14:28:10 | sean-k-mooney | is if the connection to libvirt is down | |
| 14:29:23 | sean-k-mooney | or ironic/hyperv/vmware api is not accable | |
| 14:29:45 | sean-k-mooney | assuming the comptue agent is fine it will have to put it in error when it cant talk to the hypervior | |
| 14:30:00 | sean-k-mooney | well that or leave the instnace in active | |
| 14:30:05 | sean-k-mooney | and put the instnace action in errror | |