| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-05-24 | |||
| 16:38:27 | elodilles | gibi: see melwitt's etherpad | |
| 16:38:28 | gibi | that is bad :/ | |
| 16:38:46 | elodilles | though probably they are some known issues to fix | |
| 16:39:09 | gibi | I agree to add some periodic there then | |
| 16:39:25 | elodilles | gibi: ack, i can backport the patch that added the periodic | |
| 16:39:39 | elodilles | * periodic-stable | |
| 16:40:18 | bauzas | gibi: agreed too | |
| 16:41:04 | bauzas | moving on ? | |
| 16:41:14 | elodilles | bauzas: ++ | |
| 16:41:24 | bauzas | #topic Open discussion | |
| 16:41:29 | bauzas | (whoami-rajat) Discuss regarding the design of rebuild volume backed instance feature | |
| 16:41:35 | whoami-rajat | Hi | |
| 16:41:35 | bauzas | whoami-rajat: your turn | |
| 16:41:39 | whoami-rajat | thanks | |
| 16:41:52 | whoami-rajat | #link https://review.opendev.org/c/openstack/nova-specs/+/840155 | |
| 16:42:20 | whoami-rajat | So I started working on this feature in yoga (this was proposed/reproposed several times before) and the spec got approved | |
| 16:42:41 | whoami-rajat | now while reproposing it, sean-k-mooney has some concerns regarding the new parameter we are introducing ``reimage_boot_volume`` | |
| 16:43:01 | whoami-rajat | it's a request parameter to tell the API, we are performing rebuild on a volume backed instance and not an ephemeral disk | |
| 16:43:13 | sean-k-mooney | yep | |
| 16:43:28 | whoami-rajat | initially the idea was not to have feature parity between both workflows but later there were many concerns with this operation being destructive | |
| 16:43:38 | whoami-rajat | even if you follow past specs, the concern has been discussed | |
| 16:44:15 | whoami-rajat | so lyarwood suggested to add this parameter ``reimage_boot_volume`` so any user who would like to opt in for this (as it has data loss risk) would only be able to do it | |
| 16:44:22 | sean-k-mooney | i really think that havign feature partiy btween bfv=True|false is imporant | |
| 16:44:38 | sean-k-mooney | i dont think the data loss argument holds | |
| 16:44:57 | sean-k-mooney | my reason is tha thtis is a deliberate instance action to rebuild the root disk | |
| 16:45:06 | gibi | rebuild is destructive for image bases instances too | |
| 16:45:11 | sean-k-mooney | yep | |
| 16:45:30 | sean-k-mooney | and rebuild is not the same as evacuate | |
| 16:45:31 | whoami-rajat | yes but the destructive operation is performed by cinder in this case where the volume resides on the cinder side | |
| 16:45:45 | sean-k-mooney | for evacuate we shoudl preserve the data | |
| 16:45:48 | bauzas | that's the whole purpose of this spec | |
| 16:45:58 | sean-k-mooney | for rebuild via the api we shoudl reimage the root volume | |
| 16:46:01 | bauzas | rebuild on BFV wasn't destructive, right? | |
| 16:46:14 | sean-k-mooney | rebuild was rejected | |
| 16:46:16 | sean-k-mooney | for bfv | |
| 16:46:17 | whoami-rajat | we didn't support rebuild on BFV | |
| 16:46:34 | sean-k-mooney | so the wole point is to allow rebuild with bfv | |
| 16:46:50 | bauzas | if so, there is a clear implication of what rebuild means for the root disk | |
| 16:47:13 | bauzas | we blocked because we were unable to rebuild the root disk if bfv | |
| 16:47:19 | sean-k-mooney | and technialy extra ephmeral disks | |
| 16:47:28 | sean-k-mooney | bauzas: correct | |
| 16:48:05 | bauzas | then, I don't see a need for differenciating BFV and non-BFV from an API pov | |
| 16:48:20 | bauzas | both will be destructive for the root disk | |
| 16:48:33 | sean-k-mooney | if so we also do not need an api microversion correct | |
| 16:48:39 | sean-k-mooney | and no api change at all | |
| 16:48:44 | sean-k-mooney | we just remove the block | |
| 16:48:50 | whoami-rajat | the destructive nature of this operation was the concern from many folks, I can't name everyone but this was approved in yoga so you can see | |
| 16:48:51 | bauzas | good question | |
| 16:48:53 | sean-k-mooney | when cinder is new enough | |
| 16:49:10 | whoami-rajat | dansmith, has been actively reviewing the changes I proposed last cycle so maybe he can weigh in | |
| 16:49:49 | bauzas | whoami-rajat: frankly, if we were about adding some parameter, it would be more for *not* recreating the volume | |
| 16:50:10 | dansmith | bauzas: the point of the spec/effort is to rebuild the root volume | |
| 16:50:17 | dansmith | i.e. to reimage it, but let cinder do the reimaging | |
| 16:50:29 | bauzas | dansmith: that's what I understand | |
| 16:50:38 | bauzas | so... | |
| 16:51:37 | bauzas | tbc, I don't see a need for an API param that'd say "yes, I want to rebuild by reimaging" | |
| 16:51:56 | bauzas | which would imply that the default would be "rebuild by not reimaging" | |
| 16:52:25 | sean-k-mooney | bauzas: no default would reject | |
| 16:52:49 | sean-k-mooney | bauzas: that was the behavior that i think lee suggested but i dont think i reviewd the previous iteration | |
| 16:52:57 | dansmith | I think user-initiated rebuild where we don't reimage root is pointless right? | |
| 16:53:06 | sean-k-mooney | correct | |
| 16:53:06 | dansmith | as long as we don't rebuild on evacuate then we're good, | |
| 16:53:13 | bauzas | I agree | |
| 16:53:13 | sean-k-mooney | ya | |
| 16:53:19 | dansmith | but this is specifically to make BFV behave like regular instances | |
| 16:53:48 | sean-k-mooney | right so evacuate shoudl continue to preseve the root disk if its on shared storage | |
| 16:53:50 | bauzas | correct me if I'm wrong, but I feel we are on the same page | |
| 16:53:57 | bauzas | evacuate should differ | |
| 16:54:06 | whoami-rajat | Since the main destruction is performed on the cinder side, I know a lot of folks on cinder side that won't agree to the idea of not adding this additional precautionary measure to avoid it | |
| 16:54:06 | sean-k-mooney | and rebuidl will always reimage it provided cinder i new enough | |
| 16:54:13 | bauzas | but rebuild should behave like regular instance, ie. reimage | |
| 16:54:17 | dansmith | bauzas: okay I guess I thought you were arguing for a special param | |
| 16:54:20 | whoami-rajat | as where the initial concern started ^ | |
| 16:54:35 | bauzas | dansmith: I was absolutely on the other direction, see above :) | |
| 16:54:36 | sean-k-mooney | i realy dont liek the idea of make bfv special in the nova api | |
| 16:54:45 | bauzas | me too | |
| 16:54:51 | dansmith | bauzas: ack, sorry, I'm double-meeting-ing | |
| 16:55:04 | bauzas | from an API point of view, this is clear | |
| 16:55:11 | sean-k-mooney | whoami-rajat: if we want to prevent this form the cidner side | |
| 16:55:21 | sean-k-mooney | i think cinder need a way to block the reimage not nova | |
| 16:55:31 | bauzas | of course, since we share the same internal methods for evacuate and rebuild, we should make them differ based on some conditional | |
| 16:55:32 | sean-k-mooney | like locking the volume or similar | |
| 16:55:47 | bauzas | but this conditional doesn't have to be exposed at the API level | |
| 16:55:57 | sean-k-mooney | bauzas: i think we pass a flag to rebudil to signal if its an evacuate right | |
| 16:55:58 | dansmith | bauzas: we already have a flag to pass, | |
| 16:56:04 | dansmith | bauzas: because we have to honor the old microversion, | |
| 16:56:11 | dansmith | so we can just make sure it's ==false for the evac case | |
| 16:56:16 | bauzas | dansmith: yeah, I know, that's the conditional I thought | |
| 16:56:30 | dansmith | conditional at the rpc layer, but the only conditional in the api is "old or new microversion" | |
| 16:56:44 | dansmith | the only conditional *should* be version, I mean | |
| 16:56:46 | bauzas | dansmith: correct, that being said, there was an open question | |
| 16:56:50 | sean-k-mooney | dansmith: well do we need a microverion | |
| 16:56:57 | bauzas | about even whether we would need a microversion | |
| 16:57:01 | sean-k-mooney | there is not api request change | |
| 16:57:08 | bauzas | if we just unblock | |
| 16:57:10 | dansmith | I think we absolutely do, | |
| 16:57:17 | sean-k-mooney | i realy think we should not | |
| 16:57:19 | dansmith | because right now rebuild does not destroy data and after this, it would | |
| 16:57:30 | sean-k-mooney | right now it rejects the request | |
| 16:57:35 | whoami-rajat | sean-k-mooney, if the operation is initiated from nova side, I'm not sure how from cinder side we can provide a user input to block this | |