Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-05-24
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
16:57:44 dansmith sean-k-mooney: only if the image is different
16:58:00 sean-k-mooney dansmith: no its rejected always i tought
16:58:06 dansmith sean-k-mooney: if the image is the same, it allows it
16:58:13 dansmith whoami-rajat: right?
16:58:13 sean-k-mooney ...
16:58:15 whoami-rajat but maybe I'm the only one defending the proposal
16:58:44 whoami-rajat dansmith, yes, for same image it does allow the rebuild
16:58:54 dansmith sean-k-mooney: ^
16:59:14 bauzas hah
16:59:18 sean-k-mooney that seams like a bug
16:59:23 sean-k-mooney since tha talso destorys data
16:59:34 bauzas (18:46:01) bauzas: rebuild on BFV wasn't destructive, right?
16:59:45 sean-k-mooney there is no differne form a data perspective fi you use the same image or differnt one
16:59:47 bauzas damn, we're about the end of time
16:59:49 dansmith sean-k-mooney: it doesn't on BFV but does on regular instances
17:00:13 dansmith sean-k-mooney: on BFV if the image is the same, it will just rebuild the ports or whatever, but no change to the disk
17:00:20 dansmith but will destroy the disk with the same image on a regular instance
17:00:23 bauzas I'll close this meeting, but I beg the people here to continue discussing this topic after
17:00:24 sean-k-mooney that the same as a hard reboot
17:00:37 dansmith sean-k-mooney: alas, it's api behavior we have had for YEARS
17:00:44 sean-k-mooney rebuild is not a move op
17:00:49 dansmith so changing it to now destroy data is a Bad Plan (tm)
17:00:51 sean-k-mooney and it should not really update the port either
17:00:55 dansmith well understood :)
17:00:59 bauzas thanks all, and for people interested in this bfv resize discuss, please stay around
17:01:04 opendevmeet Log: https://meetings.opendev.org/meetings/nova/2022/nova.2022-05-24-16.00.log.html
17:01:04 opendevmeet Minutes (text): https://meetings.opendev.org/meetings/nova/2022/nova.2022-05-24-16.00.txt
17:01:04 opendevmeet Minutes: https://meetings.opendev.org/meetings/nova/2022/nova.2022-05-24-16.00.html
17:01:04 opendevmeet Meeting ended Tue May 24 17:01:04 2022 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
17:01:04 bauzas #endmeeting
17:01:29 bauzas ok, so, lemme clarify
17:01:30 sean-k-mooney dansmith: it feel pretty bad to have to opt in to have it do what we document
17:01:44 dansmith sean-k-mooney: https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3617
17:01:47 bauzas 1/ resize wasn't destructive on bfv if you pass the same image
17:01:48 sean-k-mooney the current docs https://docs.openstack.org/api-ref/compute/?expanded=rebuild-server-rebuild-action-detail#rebuild-server-rebuild-action= link to https://bugs.launchpad.net/nova/+bug/1482040

Earlier   Later