virt: Remove 'change_instance_metadata' API

This was used to propagate the metadata changes to the hypervisor. For
all non-XenAPI drivers, we still allow updating instance metadata via
the API (i.e. '/servers/{server_id}/metadata') but this simply changes
what is exposed via the metadata API.

Change-Id: Ibd0ffd9906e7d7f22a9233539091d450e8023f07
Signed-off-by: Stephen Finucane <[email protected]>

nova/compute/api.py

@@ -267,23 +267,6 @@ def inner(self, context, instance, *args, **kw):
     return outer
 
 
-def _diff_dict(orig, new):
-    """Return a dict describing how to change orig to new.  The keys
-    correspond to values that have changed; the value will be a list
-    of one or two elements.  The first element of the list will be
-    either '+' or '-', indicating whether the key was updated or
-    deleted; if the key was updated, the list will contain a second
-    element, giving the updated value.
-    """
-    # Figure out what keys went away
-    result = {k: ['-'] for k in set(orig.keys()) - set(new.keys())}
-    # Compute the updates
-    for key, value in new.items():
-        if key not in orig or value != orig[key]:
-            result[key] = ['+', value]
-    return result
-
-
 def load_cells():
     global CELLS
     if not CELLS:
@@ -5004,9 +4987,6 @@ def get_instance_metadata(self, context, instance):
     def delete_instance_metadata(self, context, instance, key):
         """Delete the given metadata item from an instance."""
         instance.delete_metadata_key(key)
-        self.compute_rpcapi.change_instance_metadata(context,
-                                                     instance=instance,
-                                                     diff={key: ['-']})
 
     @check_instance_lock
     @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.PAUSED,
@@ -5020,7 +5000,6 @@ def update_instance_metadata(self, context, instance,
         `metadata` argument will be deleted.
 
         """
-        orig = dict(instance.metadata)
         if delete:
             _metadata = metadata
         else:
@@ -5030,10 +5009,7 @@ def update_instance_metadata(self, context, instance,
         self._check_metadata_properties_quota(context, _metadata)
         instance.metadata = _metadata
         instance.save()
-        diff = _diff_dict(orig, instance.metadata)
-        self.compute_rpcapi.change_instance_metadata(context,
-                                                     instance=instance,
-                                                     diff=diff)
+
         return _metadata
 
     @block_accelerators()

nova/compute/manager.py

@@ -4228,13 +4228,11 @@ def unrescue_instance(self, context, instance):
             self.host, action=fields.NotificationAction.UNRESCUE,
             phase=fields.NotificationPhase.END)
 
+    # TODO(stephenfin): Remove this once we bump the compute API to v6.0
     @wrap_exception()
     @wrap_instance_fault
     def change_instance_metadata(self, context, diff, instance):
-        """Update the metadata published to the instance."""
-        LOG.debug("Changing instance metadata according to %r",
-                  diff, instance=instance)
-        self.driver.change_instance_metadata(context, instance, diff)
+        raise NotImplementedError()
 
     @wrap_exception()
     @wrap_instance_event(prefix='compute')

nova/compute/rpcapi.py

@@ -554,13 +554,6 @@ def attach_volume(self, ctxt, instance, bdm):
                 server=_compute_host(None, instance), version=version)
         cctxt.cast(ctxt, 'attach_volume', instance=instance, bdm=bdm)
 
-    def change_instance_metadata(self, ctxt, instance, diff):
-        version = '5.0'
-        cctxt = self.router.client(ctxt).prepare(
-                server=_compute_host(None, instance), version=version)
-        cctxt.cast(ctxt, 'change_instance_metadata',
-                   instance=instance, diff=diff)
-
     def check_can_live_migrate_destination(self, ctxt, instance, destination,
                                            block_migration, disk_over_commit,
                                            migration, limits):

nova/tests/unit/api/openstack/compute/test_server_metadata.py

@@ -82,10 +82,6 @@ def return_server_nonexistent(context, server_id,
     raise exception.InstanceNotFound(instance_id=server_id)
 
 
-def fake_change_instance_metadata(self, context, instance, diff):
-    pass
-
-
 class ServerMetaDataTestV21(test.TestCase):
     validation_ex = exception.ValidationError
     validation_ex_large = validation_ex
@@ -104,9 +100,6 @@ def setUp(self):
         self.stub_out('nova.db.api.instance_metadata_get',
                       return_server_metadata)
 
-        self.stub_out(
-            'nova.compute.rpcapi.ComputeAPI.change_instance_metadata',
-            fake_change_instance_metadata)
         self._set_up_resources()
 
     def _set_up_resources(self):
@@ -655,9 +648,6 @@ def setUp(self):
         super(BadStateServerMetaDataTestV21, self).setUp()
         self.stub_out('nova.db.api.instance_metadata_get',
                       return_server_metadata)
-        self.stub_out(
-            'nova.compute.rpcapi.ComputeAPI.change_instance_metadata',
-            fake_change_instance_metadata)
         self.stub_out('nova.compute.api.API.get',
                       fakes.fake_compute_get(
                           **{'uuid': '0cc3346e-9fef-4445-abe6-5d2b2690ec64',

nova/tests/unit/compute/test_api.py

@@ -7634,35 +7634,3 @@ def test__check_compute_service_for_mixed_instance(self, mock_ver):
             exception.MixedInstanceNotSupportByComputeService,
             self.compute_api._check_compute_service_for_mixed_instance,
             fake_numa_topo)
-
-
-class DiffDictTestCase(test.NoDBTestCase):
-    """Unit tests for _diff_dict()."""
-
-    def test_no_change(self):
-        old = dict(a=1, b=2, c=3)
-        new = dict(a=1, b=2, c=3)
-        diff = compute_api._diff_dict(old, new)
-
-        self.assertEqual(diff, {})
-
-    def test_new_key(self):
-        old = dict(a=1, b=2, c=3)
-        new = dict(a=1, b=2, c=3, d=4)
-        diff = compute_api._diff_dict(old, new)
-
-        self.assertEqual(diff, dict(d=['+', 4]))
-
-    def test_changed_key(self):
-        old = dict(a=1, b=2, c=3)
-        new = dict(a=1, b=4, c=3)
-        diff = compute_api._diff_dict(old, new)
-
-        self.assertEqual(diff, dict(b=['+', 4]))
-
-    def test_removed_key(self):
-        old = dict(a=1, b=2, c=3)
-        new = dict(a=1, c=3)
-        diff = compute_api._diff_dict(old, new)
-
-        self.assertEqual(diff, dict(b=['-']))

nova/tests/unit/compute/test_compute.py

@@ -9457,16 +9457,8 @@ def test_get_all_by_state(self):
         self.assertEqual(len(instances), 3)
 
     def test_instance_metadata(self):
-        meta_changes = [None]
         self.flags(notify_on_state_change='vm_state', group='notifications')
 
-        def fake_change_instance_metadata(inst, ctxt, diff, instance=None,
-                                          instance_uuid=None):
-            meta_changes[0] = diff
-        self.stub_out('nova.compute.rpcapi.ComputeAPI.'
-                      'change_instance_metadata',
-                      fake_change_instance_metadata)
-
         _context = context.get_admin_context()
         instance = self._create_fake_instance_obj({'metadata':
                                                        {'key1': 'value1'}})
@@ -9478,7 +9470,6 @@ def fake_change_instance_metadata(inst, ctxt, diff, instance=None,
                                                   {'key2': 'value2'})
         metadata = self.compute_api.get_instance_metadata(_context, instance)
         self.assertEqual(metadata, {'key1': 'value1', 'key2': 'value2'})
-        self.assertEqual(meta_changes, [{'key2': ['+', 'value2']}])
 
         self.assertEqual(len(fake_notifier.NOTIFICATIONS), 1)
         msg = fake_notifier.NOTIFICATIONS[0]
@@ -9487,15 +9478,10 @@ def fake_change_instance_metadata(inst, ctxt, diff, instance=None,
         self.assertEqual(payload['metadata'], metadata)
 
         new_metadata = {'key2': 'bah', 'key3': 'value3'}
-        self.compute_api.update_instance_metadata(_context, instance,
-                                                  new_metadata, delete=True)
+        self.compute_api.update_instance_metadata(
+            _context, instance, new_metadata, delete=True)
         metadata = self.compute_api.get_instance_metadata(_context, instance)
         self.assertEqual(metadata, new_metadata)
-        self.assertEqual(meta_changes, [{
-                    'key1': ['-'],
-                    'key2': ['+', 'bah'],
-                    'key3': ['+', 'value3'],
-                    }])
 
         self.assertEqual(len(fake_notifier.NOTIFICATIONS), 2)
         msg = fake_notifier.NOTIFICATIONS[1]
@@ -9506,7 +9492,6 @@ def fake_change_instance_metadata(inst, ctxt, diff, instance=None,
         self.compute_api.delete_instance_metadata(_context, instance, 'key2')
         metadata = self.compute_api.get_instance_metadata(_context, instance)
         self.assertEqual(metadata, {'key3': 'value3'})
-        self.assertEqual(meta_changes, [{'key2': ['-']}])
 
         self.assertEqual(len(fake_notifier.NOTIFICATIONS), 3)
         msg = fake_notifier.NOTIFICATIONS[2]
@@ -9515,13 +9500,6 @@ def fake_change_instance_metadata(inst, ctxt, diff, instance=None,
         self.assertEqual(payload['metadata'], {'key3': 'value3'})
 
     def test_disallow_metadata_changes_during_building(self):
-        def fake_change_instance_metadata(inst, ctxt, diff, instance=None,
-                                          instance_uuid=None):
-            pass
-        self.stub_out('nova.compute.rpcapi.ComputeAPI.'
-                      'change_instance_metadata',
-                       fake_change_instance_metadata)
-
         instance = self._create_fake_instance_obj(
             {'vm_state': vm_states.BUILDING})
 

nova/tests/unit/compute/test_rpcapi.py

@@ -213,10 +213,6 @@ def test_attach_volume(self):
                 instance=self.fake_instance_obj, bdm=self.fake_volume_bdm,
                 version='5.0')
 
-    def test_change_instance_metadata(self):
-        self._test_compute_api('change_instance_metadata', 'cast',
-                instance=self.fake_instance_obj, diff={}, version='5.0')
-
     def test_check_instance_shared_storage(self):
         self._test_compute_api('check_instance_shared_storage', 'call',
                 instance=self.fake_instance_obj, data='foo',

nova/virt/driver.py

@@ -1362,21 +1362,6 @@ def set_admin_password(self, instance, new_pass):
         """
         raise NotImplementedError()
 
-    # TODO(stephenfin): This was only implemented (properly) for XenAPI and
-    # should be removed.
-    def change_instance_metadata(self, context, instance, diff):
-        """Applies a diff to the instance metadata.
-
-        This is an optional driver method which is used to publish
-        changes to the instance's metadata to the hypervisor.  If the
-        hypervisor has no means of publishing the instance metadata to
-        the instance, then this method should not be implemented.
-
-        :param context: security context
-        :param instance: nova.objects.instance.Instance
-        """
-        pass
-
     def inject_network_info(self, instance, nw_info):
         """inject network info for specified instance."""
         # TODO(Vek): Need to pass context in for access to auth_token