Skip to content

Commit

Permalink
SAP: Use is_instance_storage_share instead of ssh
Browse files Browse the repository at this point in the history
The libvirt-driver implements is_instance_storage_share twice:
Once on driver-internally as `_is_path_shared_with` needing an ssh
trust, and via the driver interface `is_instance_storage_share`,
relying on RPC.

By moving exposing `is_instance_storage_share` in ComputeVirtAPI,
the driver can use this instead of the internal method removing
the need for an SSH key in case of having a shared storage.

But since it requires a change of the driver internal API, it is
questionable if we can upstream the change as it is.

Change-Id: Icddad1ab000ea07dea7af3aa5f7c18974f4eb8fb
  • Loading branch information
fwiesel committed Dec 9, 2024
1 parent 66f29b0 commit 58c98db
Show file tree
Hide file tree
Showing 12 changed files with 131 additions and 79 deletions.
6 changes: 5 additions & 1 deletion nova/compute/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,10 @@ def update_compute_provider_status(self, context, rp_uuid, enabled):
self.reportclient.set_traits_for_provider(
context, rp_uuid, new_traits, generation=trait_info.generation)

def is_instance_storage_shared(self, context, instance, compute_host):
return self._compute._is_instance_storage_shared(context, instance,
compute_host)


class ComputeManager(manager.Manager):
"""Manages the running instances from creation to destruction."""
Expand Down Expand Up @@ -5629,7 +5633,7 @@ def _resize_instance(
timeout, retry_interval = self._get_power_off_values(
instance, clean_shutdown)
disk_info = self.driver.migrate_disk_and_power_off(
context, instance, migration.dest_host,
context, instance, migration,
flavor, network_info,
block_device_info,
timeout, retry_interval)
Expand Down
23 changes: 23 additions & 0 deletions nova/tests/unit/compute/test_virtapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ def test_update_compute_provider_status(self):
nova_context.get_admin_context(), uuids.rp_uuid,
enabled=False)

def test_is_instance_storage_shared(self):
self.assertExpected('is_instance_storage_shared',
nova_context.get_admin_context(),
mock.sentinel.instance,
mock.sentinel.host)


class FakeVirtAPITest(VirtAPIBaseTest):

Expand All @@ -74,6 +80,8 @@ def assertExpected(self, method, *args, **kwargs):
self.virtapi.exit_wait_early(*args, **kwargs)
elif method == 'update_compute_provider_status':
self.virtapi.update_compute_provider_status(*args, **kwargs)
elif method == 'is_instance_storage_shared':
self.virtapi.is_instance_storage_shared(*args, **kwargs)
else:
self.fail("Unhandled FakeVirtAPI method: %s" % method)

Expand Down Expand Up @@ -116,6 +124,9 @@ def _prepare_for_instance_event(self, instance, name, tag):
self._events.append(m)
return m

def _is_instance_storage_shared(self, context, instance, host):
return


class ComputeVirtAPITest(VirtAPIBaseTest):

Expand Down Expand Up @@ -230,3 +241,15 @@ def test_update_compute_provider_status(self):
self.virtapi.update_compute_provider_status(
ctxt, uuids.rp_uuid, enabled=True)
self.assertEqual(set(), self.compute.provider_traits[uuids.rp_uuid])

def test_is_instance_storage_shared(self):
ctxt = nova_context.get_admin_context()

with mock.patch.object(self.compute,
'_is_instance_storage_shared') as mock_shared:
self.virtapi.is_instance_storage_shared(ctxt,
mock.sentinel.instance,
mock.sentinel.host)
mock_shared.assert_called_once_with(ctxt,
mock.sentinel.instance,
mock.sentinel.host)
7 changes: 5 additions & 2 deletions nova/tests/unit/virt/hyperv/test_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from os_win import exceptions as os_win_exc

from nova import exception
from nova import objects
from nova import safe_utils
from nova.tests.unit import fake_instance
from nova.tests.unit.virt.hyperv import test_base
Expand Down Expand Up @@ -387,15 +388,17 @@ def test_unplug_vifs(self):
mock.sentinel.instance, mock.sentinel.network_info)

def test_migrate_disk_and_power_off(self):
migration = objects.Migration(dest_host='10.0.0.1',
dest_compute='mock-compute')
self.driver.migrate_disk_and_power_off(
mock.sentinel.context, mock.sentinel.instance, mock.sentinel.dest,
mock.sentinel.context, mock.sentinel.instance, migration,
mock.sentinel.flavor, mock.sentinel.network_info,
mock.sentinel.block_device_info, mock.sentinel.timeout,
mock.sentinel.retry_interval)

migr_power_off = self.driver._migrationops.migrate_disk_and_power_off
migr_power_off.assert_called_once_with(
mock.sentinel.context, mock.sentinel.instance, mock.sentinel.dest,
mock.sentinel.context, mock.sentinel.instance, migration.dest_host,
mock.sentinel.flavor, mock.sentinel.network_info,
mock.sentinel.block_device_info, mock.sentinel.timeout,
mock.sentinel.retry_interval)
Expand Down
Loading

0 comments on commit 58c98db

Please sign in to comment.