conductor: Cross-HV resize preflight checks#625
Conversation
Sanitize VMware-specific image properties directly on the canonical RequestSpec before scheduling, matching how request_spec.flavor is already handled in the cold migrate flow. The sanitizer mutates request_spec.image.properties in place and returns a dict of original values. After task.execute() succeeds, the conductor persists this into MigrationContext.old_image_properties (via instance.save()) before calling request_spec.save(). This ordering ensures the rollback copy is always available if request_spec has been rewritten. Changes: - nova/compute/utils.py: add sanitize_image_props_for_kvm() - nova/conductor/tasks/migrate.py: call sanitizer in _execute() - nova/conductor/manager.py: persist old_image_properties to MigrationContext before request_spec.save() Change-Id: I4f7035358a9a7f46d942bfad07d748d1d333f8c5
91eb9fa to
41d1d06
Compare
| self.context, self.instance.uuid) | ||
| volume_ids = {bdm.volume_id for bdm in bdms if bdm.volume_id} | ||
| if volume_ids: | ||
| snapshots = volume_api.get_all_snapshots(self.context) |
There was a problem hiding this comment.
This iterates through all the snapshots of a project as far as I can tell. Is there maybe a way to filter for the volume id below at the cinder API level already?
There was a problem hiding this comment.
https://docs.openstack.org/api-ref/block-storage/v3/index.html#volume-snapshots-snapshots
doesn't look like it :/
| self.context, self.instance.uuid) | ||
| volume_ids = {bdm.volume_id for bdm in bdms if bdm.volume_id} | ||
| if volume_ids: | ||
| snapshots = volume_api.get_all_snapshots(self.context) |
There was a problem hiding this comment.
I know it is still specified in the ticket to have this snapshot check. But since we are not doing volume retyping for BFV during the resize (anymore), it's probably not required (anymore).
There was a problem hiding this comment.
hm, right... I'll remove it altogether.
251f927 to
4116348
Compare
|
Added unit tests. |
4116348 to
78d8199
Compare
We allow it only if it's: - a BFV instance, that - is powered on. And only from VMware to CH, not backwards. The image properties are now sanitized and saved, only on vmware->ch hv transition, inside the new _prep_cross_hv_resize() step. Change-Id: I50dedca220e70f51811c0f9b9327ca3ae7ea9e12
78d8199 to
72706db
Compare
|
Integrated on top of #623 . |
| and extra_specs_ops.match("CH", dest_hv): | ||
| if not self.request_spec.is_bfv: | ||
| raise exception.InvalidCrossHvResizePrecondition( | ||
| 'Must be BFV instance') |
There was a problem hiding this comment.
| 'Must be BFV instance') | |
| reason='Must be BFV instance') |
| 'Must be BFV instance') | ||
| if self.instance.power_state != power_state.RUNNING: | ||
| raise exception.InvalidCrossHvResizePrecondition( | ||
| 'Instance must be running for cross-hypervisor resize. ' |
There was a problem hiding this comment.
| 'Instance must be running for cross-hypervisor resize. ' | |
| reason='Instance must be running for cross-hypervisor resize. ' |
| msg_fmt = _('Resize from hypervisor type %(src_hv_type)s to ' | ||
| '%(dest_hv_type)s is not allowed') | ||
|
|
||
| class InvalidCrossHvResizePrecondition(NovaException): |
There was a problem hiding this comment.
| class InvalidCrossHvResizePrecondition(NovaException): | |
| class InvalidCrossHvResizePrecondition(NovaException): |
to make PEP8 happy I guess.
| self.request_spec)) | ||
| src_hv = self._source_cn.hypervisor_type | ||
| dest_hv = self.flavor.extra_specs.get('capabilities:hypervisor_type') | ||
| if dest_hv and not extra_specs_ops.match(src_hv, dest_hv): |
There was a problem hiding this comment.
There's no test where dest_hv is set but is identical to src_hv (so both are VMware or both CH). It think it would be nice to have a test that we don't break the existing same-HV resize flow: It should confirm that _prep_cross_hv_resize is not called when source and destination are same-HV.
| exception.InvalidCrossHvResizePrecondition) as ex: | ||
| vm_state = instance.vm_state | ||
| if not vm_state: | ||
| vm_state = vm_states.ACTIVE |
There was a problem hiding this comment.
Adding a test for this branch would also be nice: Verify the instance state is properly restored (not set to ERROR) when the exception fires and resets to the ACTIVE fallback.
| self._old_image_properties = ( | ||
| compute_utils.sanitize_image_props_for_kvm( | ||
| self.request_spec)) | ||
| src_hv = self._source_cn.hypervisor_type |
There was a problem hiding this comment.
Could src_hv somehow also be None? It's properly handled for dest_hv below, just feels like we could do the same for src_hv?
a9d7726 to
6eb48d6
Compare
We allow it only if it's:
And only from VMware to CH, not backwards.
The image properties are now sanitized and saved, only on vmware->ch hv transition, inside the new _prep_cross_hv_resize() step.