From 5b70f32b741efdb819eda40ab626ae4fbd0984bf Mon Sep 17 00:00:00 2001 From: esberglu Date: Thu, 21 Jun 2018 10:02:18 -0500 Subject: [PATCH] DiskAdapter Parent Class In-tree Backports This contains backports for code changes made during in-tree driver DiskAdapter parent class development [1]. - Forces subclasses to implement capacity methods - Marks abstract methods/properties in the parent class - Introduces a fake disk adapter to allow testing with abstract methods and properties. [1] https://review.openstack.org/#/c/549053/ Change-Id: I1f8ebe64571c009020be7f3888167e3450d2fd4d --- .../tests/virt/powervm/disk/fake_adapter.py | 60 +++++++++++++++++++ .../tests/virt/powervm/disk/test_driver.py | 14 ++--- nova_powervm/virt/powervm/disk/driver.py | 34 ++++++----- nova_powervm/virt/powervm/disk/localdisk.py | 19 ++++++ nova_powervm/virt/powervm/disk/ssp.py | 9 +++ 5 files changed, 111 insertions(+), 25 deletions(-) create mode 100644 nova_powervm/tests/virt/powervm/disk/fake_adapter.py diff --git a/nova_powervm/tests/virt/powervm/disk/fake_adapter.py b/nova_powervm/tests/virt/powervm/disk/fake_adapter.py new file mode 100644 index 00000000..4e83315c --- /dev/null +++ b/nova_powervm/tests/virt/powervm/disk/fake_adapter.py @@ -0,0 +1,60 @@ +# Copyright IBM Corp. and contributors +# +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from nova_powervm.virt.powervm.disk import driver as disk_dvr + + +class FakeDiskAdapter(disk_dvr.DiskAdapter): + """A fake subclass of DiskAdapter. + + This is done so that the abstract methods/properties can be stubbed and the + class can be instantiated for testing. + """ + def vios_uuids(self): + pass + + def _disk_match_func(self, disk_type, instance): + pass + + def disconnect_disk_from_mgmt(self, vios_uuid, disk_name): + pass + + def capacity(self): + pass + + def capacity_used(self): + pass + + def disconnect_disk(self, instance): + pass + + def delete_disks(self, storage_elems): + pass + + def create_disk_from_image(self, context, instance, image_meta): + pass + + def connect_disk(self, instance, disk_info, stg_ftsk): + pass + + def extend_disk(self, instance, disk_info, size): + pass + + def check_instance_shared_storage_local(self, context, instance): + pass + + def check_instance_shared_storage_remote(self, context, data): + pass diff --git a/nova_powervm/tests/virt/powervm/disk/test_driver.py b/nova_powervm/tests/virt/powervm/disk/test_driver.py index a02eaba2..ced8f12a 100644 --- a/nova_powervm/tests/virt/powervm/disk/test_driver.py +++ b/nova_powervm/tests/virt/powervm/disk/test_driver.py @@ -1,4 +1,4 @@ -# Copyright 2015, 2017 IBM Corp. +# Copyright IBM Corp. and contributors # # All Rights Reserved. # @@ -19,8 +19,8 @@ import mock from nova import test from pypowervm import const as pvm_const +from nova_powervm.tests.virt.powervm.disk import fake_adapter from nova_powervm.tests.virt.powervm import fixtures as fx -from nova_powervm.virt.powervm.disk import driver as disk_dvr class TestDiskAdapter(test.NoDBTestCase): @@ -36,13 +36,9 @@ class TestDiskAdapter(test.NoDBTestCase): self.mgmt_uuid.return_value = 'mp_uuid' # The values (adapter and host uuid) are not used in the base. - # Default them to None. - self.st_adpt = disk_dvr.DiskAdapter(None, None) - - def test_capacity(self): - """These are arbitrary capacity numbers.""" - self.assertEqual(2097152, self.st_adpt.capacity) - self.assertEqual(0, self.st_adpt.capacity_used) + # Default them to None. We use the fake adapter here because we can't + # instantiate DiskAdapter which is an abstract base class. + self.st_adpt = fake_adapter.FakeDiskAdapter(None, None) def test_get_info(self): # Ensure the base method returns empty dict diff --git a/nova_powervm/virt/powervm/disk/driver.py b/nova_powervm/virt/powervm/disk/driver.py index 5092b9e1..0692b5fd 100644 --- a/nova_powervm/virt/powervm/disk/driver.py +++ b/nova_powervm/virt/powervm/disk/driver.py @@ -87,7 +87,7 @@ class DiskAdapter(object): self.host_uuid = host_uuid self.mp_uuid = mgmt.mgmt_uuid(self.adapter) - @property + @abc.abstractproperty def vios_uuids(self): """List the UUIDs of the Virtual I/O Servers hosting the storage.""" raise NotImplementedError() @@ -129,6 +129,7 @@ class DiskAdapter(object): return _('The configured disk driver does not support migration ' 'or resize.') + @abc.abstractmethod def _disk_match_func(self, disk_type, instance): """Return a matching function to locate the disk for an instance. @@ -214,6 +215,7 @@ class DiskAdapter(object): # We either didn't find the boot dev, or failed all attempts to map it. raise npvmex.InstanceDiskMappingFailed(instance_name=instance.name) + @abc.abstractmethod def disconnect_disk_from_mgmt(self, vios_uuid, disk_name): """Disconnect a disk from the management partition. @@ -223,21 +225,15 @@ class DiskAdapter(object): """ raise NotImplementedError() - @property + @abc.abstractproperty def capacity(self): - """Capacity of the storage in gigabytes + """Capacity of the storage in gigabytes.""" + raise NotImplementedError() - Default is to make the capacity arbitrarily large - """ - return 1 << 21 - - @property + @abc.abstractproperty def capacity_used(self): - """Capacity of the storage in gigabytes that is used - - Default is to say none of it is used. - """ - return 0 + """Capacity of the storage in gigabytes that is used.""" + raise NotImplementedError() @staticmethod def _get_disk_name(disk_type, instance, short=False): @@ -301,6 +297,7 @@ class DiskAdapter(object): disk_bytes = floor return disk_bytes + @abc.abstractmethod def disconnect_disk(self, instance, stg_ftsk=None, disk_type=None): """Disconnects the storage adapters from the image disk. @@ -316,8 +313,9 @@ class DiskAdapter(object): :return: A list of all the backing storage elements that were disconnected from the I/O Server and VM. """ - pass + raise NotImplementedError() + @abc.abstractmethod def delete_disks(self, storage_elems): """Removes the disks specified by the mappings. @@ -325,7 +323,7 @@ class DiskAdapter(object): deleted. Derived from the return value from disconnect_disk. """ - pass + raise NotImplementedError() def create_disk_from_image(self, context, instance, image_meta, image_type=DiskType.BOOT): @@ -370,6 +368,7 @@ class DiskAdapter(object): """ pass + @abc.abstractmethod def connect_disk(self, instance, disk_info, stg_ftsk=None): """Connects the disk image to the Virtual Machine. @@ -384,8 +383,9 @@ class DiskAdapter(object): the FeedTask is not provided, the updates will be run immediately when this method is executed. """ - pass + raise NotImplementedError() + @abc.abstractmethod def extend_disk(self, instance, disk_info, size): """Extends the disk. @@ -395,6 +395,7 @@ class DiskAdapter(object): """ raise NotImplementedError() + @abc.abstractmethod def check_instance_shared_storage_local(self, context, instance): """Check if instance files located on shared storage. @@ -406,6 +407,7 @@ class DiskAdapter(object): """ raise NotImplementedError() + @abc.abstractmethod def check_instance_shared_storage_remote(self, context, data): """Check if instance files located on shared storage. diff --git a/nova_powervm/virt/powervm/disk/localdisk.py b/nova_powervm/virt/powervm/disk/localdisk.py index cfbfb428..fb3e499a 100644 --- a/nova_powervm/virt/powervm/disk/localdisk.py +++ b/nova_powervm/virt/powervm/disk/localdisk.py @@ -339,6 +339,25 @@ class LocalStorage(disk_dvr.DiskAdapter): LOG.exception("PowerVM Error extending disk.", instance=instance) + def check_instance_shared_storage_local(self, context, instance): + """Check if instance files located on shared storage. + + This runs check on the destination host, and then calls + back to the source host to check the results. + + :param context: security context + :param instance: nova.objects.instance.Instance object + """ + raise NotImplementedError() + + def check_instance_shared_storage_remote(self, context, data): + """Check if instance files located on shared storage. + + :param context: security context + :param data: result of check_instance_shared_storage_local + """ + raise NotImplementedError() + def _get_vg_wrap(self): return pvm_stg.VG.get(self.adapter, uuid=self.vg_uuid, parent_type=pvm_vios.VIOS, diff --git a/nova_powervm/virt/powervm/disk/ssp.py b/nova_powervm/virt/powervm/disk/ssp.py index ea308c82..8fe5ad47 100644 --- a/nova_powervm/virt/powervm/disk/ssp.py +++ b/nova_powervm/virt/powervm/disk/ssp.py @@ -286,6 +286,15 @@ class SSPDiskAdapter(disk_drv.DiskAdapter): if stg_ftsk.name == 'ssp': stg_ftsk.execute() + def extend_disk(self, instance, disk_info, size): + """Extends the disk. + + :param instance: instance to extend the disk for. + :param disk_info: dictionary with disk info. + :param size: the new size in gb. + """ + raise NotImplementedError() + def check_instance_shared_storage_local(self, context, instance): """Check if instance files located on shared storage.