Merge "VMware image clone strategy settings and overrides"

nova/tests/virt/vmwareapi/db_fakes.py

@@ -82,6 +82,7 @@ def stub_out_db_instance_api(stubs):
             'mac_addresses': [{'address': values['mac_address']}],
             'root_gb': type_data['root_gb'],
             'node': values['node'],
+            'metadata': []
             }
 
         return base_options

nova/tests/virt/vmwareapi/test_configdrive.py

@@ -75,7 +75,8 @@ class ConfigDriveTestCase(test.TestCase):
                               'reservation_id': 'r-3t8muvr0',
                               'id': 1,
                               'uuid': 'fake-uuid',
-                              'node': self.node_name}
+                              'node': self.node_name,
+                              'metadata': []}
 
         class FakeInstanceMetadata(object):
             def __init__(self, instance, content=None, extra_md=None):

nova/tests/virt/vmwareapi/test_vmwareapi_vmops.py

@@ -76,3 +76,30 @@ class VMwareVMOpsTestCase(test.TestCase):
         self.flags(network_api_class='nova.network.quantumv2.api.API')
         ops = vmops.VMwareVMOps(None, None, None)
         self.assertTrue(ops._is_neutron)
+
+    def test_use_linked_clone_override_nf(self):
+        value = vmops.VMwareVMOps.decide_linked_clone(None, False)
+        self.assertFalse(value, "No overrides present but still overridden!")
+
+    def test_use_linked_clone_override_nt(self):
+        value = vmops.VMwareVMOps.decide_linked_clone(None, True)
+        self.assertTrue(value, "No overrides present but still overridden!")
+
+    def test_use_linked_clone_override_ny(self):
+        value = vmops.VMwareVMOps.decide_linked_clone(None, "yes")
+        self.assertTrue(value, "No overrides present but still overridden!")
+
+    def test_use_linked_clone_override_ft(self):
+        value = vmops.VMwareVMOps.decide_linked_clone(False, True)
+        self.assertFalse(value,
+                        "image level metadata failed to override global")
+
+    def test_use_linked_clone_override_nt(self):
+        value = vmops.VMwareVMOps.decide_linked_clone("no", True)
+        self.assertFalse(value,
+                        "image level metadata failed to override global")
+
+    def test_use_linked_clone_override_yf(self):
+        value = vmops.VMwareVMOps.decide_linked_clone("yes", False)
+        self.assertTrue(value,
+                        "image level metadata failed to override global")

nova/utils.py

@@ -50,6 +50,7 @@ from nova.openstack.common import lockutils
 from nova.openstack.common import log as logging
 from nova.openstack.common import processutils
 from nova.openstack.common.rpc import common as rpc_common
+from nova.openstack.common import strutils
 from nova.openstack.common import timeutils
 
 notify_decorator = 'nova.notifications.notify_decorator'
@@ -1285,3 +1286,10 @@ def get_image_from_system_metadata(system_meta):
         image_meta['properties'] = properties
 
     return image_meta
+
+
+def get_boolean(value):
+    if isinstance(value, bool):
+        return value
+    else:
+        return strutils.bool_from_string(value)

nova/virt/vmwareapi/driver.py

@@ -159,6 +159,10 @@ class VMwareESXDriver(driver.ComputeDriver):
         self._host = host.Host(self._session)
         self._host_state = None
 
+        #TODO(hartsocks): back-off into a configuration test module.
+        if CONF.vmware.use_linked_clone is None:
+            raise error_util.UseLinkedCloneConfigurationFault()
+
     @property
     def host_state(self):
         if not self._host_state:

nova/virt/vmwareapi/error_util.py

@@ -18,6 +18,7 @@
 """
 Exception classes and SOAP response error checking module.
 """
+from nova import exception
 
 from nova.openstack.common.gettextutils import _
 
@@ -96,3 +97,25 @@ class FaultCheckers(object):
             raise VimFaultException(fault_list, Exception(_("Error(s) %s "
                     "occurred in the call to RetrievePropertiesEx") %
                     exc_msg_list))
+
+
+class VMwareDriverException(exception.NovaException):
+    """Base class for all exceptions raised by the VMware Driver.
+
+    All exceptions raised by the VMwareAPI drivers should raise
+    an exception descended from this class as a root. This will
+    allow the driver to potentially trap problems related to its
+    own internal configuration before halting the nova-compute
+    node.
+    """
+    msg_fmt = _("VMware Driver fault.")
+
+
+class VMwareDriverConfigurationException(VMwareDriverException):
+    """Base class for all configuration exceptions.
+    """
+    msg_fmt = _("VMware Driver configuration fault.")
+
+
+class UseLinkedCloneConfigurationFault(VMwareDriverConfigurationException):
+    msg_fmt = _("No default value for use_linked_clone found.")

nova/virt/vmwareapi/vmops.py

@@ -73,6 +73,7 @@ VMWARE_POWER_STATES = {
                     'suspended': power_state.SUSPENDED}
 VMWARE_PREFIX = 'vmware'
 
+VMWARE_LINKED_CLONE = 'vmware_linked_clone'
 
 RESIZE_TOTAL_STEPS = 4
 
@@ -156,6 +157,10 @@ class VMwareVMOps(object):
         data_store_ref = ds[0]
         data_store_name = ds[1]
 
+        #TODO(hartsocks): this pattern is confusing, reimplement as methods
+        # The use of nested functions in this file makes for a confusing and
+        # hard to maintain file. At some future date, refactor this method to
+        # be a full-fledged method. This will also make unit testing easier.
         def _get_image_properties():
             """
             Get the Size of the flat vmdk file that is there on the storage
@@ -173,11 +178,18 @@ class VMwareVMOps(object):
                                              "preallocated")
             # Get the network card type from the image properties.
             vif_model = image_properties.get("hw_vif_model", "VirtualE1000")
+
+            # Fetch the image_linked_clone data here. It is retrieved
+            # with the above network based API call. To retrieve it
+            # later will necessitate additional network calls using the
+            # identical method. Consider this a cache.
+            image_linked_clone = image_properties.get(VMWARE_LINKED_CLONE)
+
             return (vmdk_file_size_in_kb, os_type, adapter_type, disk_type,
-                vif_model)
+                vif_model, image_linked_clone)
 
         (vmdk_file_size_in_kb, os_type, adapter_type,
-            disk_type, vif_model) = _get_image_properties()
+            disk_type, vif_model, image_linked_clone) = _get_image_properties()
 
         vm_folder_ref = self._get_vmfolder_ref()
         node_mo_id = vm_util.get_mo_id_from_instance(instance)
@@ -362,7 +374,13 @@ class VMwareVMOps(object):
                 self._default_root_device, block_device_info)
 
         if not ebs_root:
-            linked_clone = CONF.vmware.use_linked_clone
+            # this logic allows for instances or images to decide
+            # for themselves which strategy is best for them.
+
+            linked_clone = VMwareVMOps.decide_linked_clone(
+                image_linked_clone,
+                CONF.vmware.use_linked_clone
+            )
             if linked_clone:
                 upload_folder = self._instance_path_base
                 upload_name = instance['image_ref']
@@ -516,6 +534,52 @@ class VMwareVMOps(object):
                     "cdrom %(file_path)s"),
                   {'instance_name': instance_name, 'file_path': file_path})
 
+    @staticmethod
+    def decide_linked_clone(image_linked_clone, global_linked_clone):
+        """Explicit decision logic: whether to use linked clone on a vmdk.
+
+        This is *override* logic not boolean logic.
+
+        1. let the image over-ride if set at all
+        2. default to the global setting
+
+        In math terms, I need to allow:
+        glance image to override global config.
+
+        That is g vs c. "g" for glance. "c" for Config.
+
+        So, I need  g=True vs c=False to be True.
+        And, I need g=False vs c=True to be False.
+        And, I need g=None vs c=True to be True.
+
+        Some images maybe independently best tuned for use_linked_clone=True
+        saving datastorage space. Alternatively a whole OpenStack install may
+        be tuned to performance use_linked_clone=False but a single image
+        in this environment may be best configured to save storage space and
+        set use_linked_clone=True only for itself.
+
+        The point is: let each layer of control override the layer beneath it.
+
+        rationale:
+        For technical discussion on the clone strategies and their trade-offs
+        see: https://www.vmware.com/support/ws5/doc/ws_clone_typeofclone.html
+
+        :param image_linked_clone: boolean or string or None
+        :param global_linked_clone: boolean or string or None
+        :return: Boolean
+        """
+
+        value = None
+
+        # Consider the values in order of override.
+        if image_linked_clone is not None:
+            value = image_linked_clone
+        else:
+            # this will never be not-set by this point.
+            value = global_linked_clone
+
+        return utils.get_boolean(value)
+
     def snapshot(self, context, instance, snapshot_name, update_task_state):
         """Create snapshot from a running VM instance.