Fix deprecated network api

The rpc-only network api is deprecated in Grizzly, but it should still
be able to function.

Change summary:
* Remove update_cache kwyword argument to get_instance_network_info
* Add conductor_api keyword argument to get_instance_network_info
* Add blanket **kwargs for future compat to get_instance_network_info
* Pass instance['uuid'] as instance_id kwarg in _get_instance_nw_info
* Add conductor_api keyword argument to remove_fixed_ip_from_instance
  and add_fixed_ip_to_instance
* Add blanket **kwargs for future compat to remove_fixed_ip_from_instance
  and add_fixed_ip_to_instance
* Pass empty rxtx_factor in remove_fixed_ip_from_instance and
  add_fixed_ip_to_instance for rpcapi compat
* Add blanket **kwargs for future compat to deallocate_for_instance
* Add conductor_api and security_groups to allocate_for_instance
* Add blanket **kwargs for future compat to allocate_for_instance
* Pass instance['uuid'] as instance_id kwarg in allocate_for_instance

Fix Bug 1134512

Change-Id: I6791df1bf1ac2456c5e83ec5ae47b0226409cf95

2 files changed, 120 insertions, 81 deletions

diff --git a/nova/network/api_deprecated.py b/nova/network/api_deprecated.py
index b84a08a6d..734a990e3 100644
--- a/nova/network/api_deprecated.py
+++ b/nova/network/api_deprecated.py
@@ -22,82 +22,20 @@ This version of the api is deprecated in Grizzly and will be removed.
 It is provided just in case a third party manager is in use.
 """
 
-import functools
-import inspect
-
 from nova.db import base
 from nova import exception
+from nova.network import api as shiny_api
 from nova.network import model as network_model
 from nova.network import rpcapi as network_rpcapi
 from nova.openstack.common import log as logging
-from nova import policy
 
 LOG = logging.getLogger(__name__)
 
 
-def refresh_cache(f):
-    """
-    Decorator to update the instance_info_cache
-
-    Requires context and instance as function args
-    """
-    argspec = inspect.getargspec(f)
-
-    @functools.wraps(f)
-    def wrapper(self, context, *args, **kwargs):
-        res = f(self, context, *args, **kwargs)
-
-        try:
-            # get the instance from arguments (or raise ValueError)
-            instance = kwargs.get('instance')
-            if not instance:
-                instance = args[argspec.args.index('instance') - 2]
-        except ValueError:
-            msg = _('instance is a required argument to use @refresh_cache')
-            raise Exception(msg)
-
-        update_instance_cache_with_nw_info(self, context, instance,
-                                           nw_info=res)
-
-        # return the original function's return value
-        return res
-    return wrapper
-
-
-def update_instance_cache_with_nw_info(api, context, instance,
-                                        nw_info=None):
-
-    try:
-        if not isinstance(nw_info, network_model.NetworkInfo):
-            nw_info = None
-        if not nw_info:
-            nw_info = api._get_instance_nw_info(context, instance)
-        # update cache
-        cache = {'network_info': nw_info.json()}
-        api.db.instance_info_cache_update(context, instance['uuid'], cache)
-    except Exception:
-        LOG.exception(_('Failed storing info cache'), instance=instance)
-
-
-def wrap_check_policy(func):
-    """Check policy corresponding to the wrapped methods prior to execution."""
-
-    @functools.wraps(func)
-    def wrapped(self, context, *args, **kwargs):
-        action = func.__name__
-        check_policy(context, action)
-        return func(self, context, *args, **kwargs)
-
-    return wrapped
-
-
-def check_policy(context, action):
-    target = {
-        'project_id': context.project_id,
-        'user_id': context.user_id,
-    }
-    _action = 'network:%s' % action
-    policy.enforce(context, _action, target)
+refresh_cache = shiny_api.refresh_cache
+_update_instance_cache = shiny_api.update_instance_cache_with_nw_info
+update_instance_cache_with_nw_info = _update_instance_cache
+wrap_check_policy = shiny_api.wrap_check_policy
 
 
 class API(base.Base):
@@ -232,7 +170,9 @@ class API(base.Base):
     @wrap_check_policy
     @refresh_cache
     def allocate_for_instance(self, context, instance, vpn,
-                              requested_networks, macs=None):
+                              requested_networks, macs=None,
+                              conductor_api=None, security_groups=None,
+                              **kwargs):
         """Allocates all network structures for an instance.
 
         TODO(someone): document the rest of these parameters.
@@ -246,8 +186,7 @@ class API(base.Base):
         args = {}
         args['vpn'] = vpn
         args['requested_networks'] = requested_networks
-        args['instance_id'] = instance['id']
-        args['instance_uuid'] = instance['uuid']
+        args['instance_id'] = instance['uuid']
         args['project_id'] = instance['project_id']
         args['host'] = instance['host']
         args['rxtx_factor'] = instance['instance_type']['rxtx_factor']
@@ -256,7 +195,7 @@ class API(base.Base):
         return network_model.NetworkInfo.hydrate(nw_info)
 
     @wrap_check_policy
-    def deallocate_for_instance(self, context, instance):
+    def deallocate_for_instance(self, context, instance, **kwargs):
         """Deallocates all network structures related to instance."""
 
         args = {}
@@ -267,21 +206,25 @@ class API(base.Base):
 
     @wrap_check_policy
     @refresh_cache
-    def add_fixed_ip_to_instance(self, context, instance, network_id):
+    def add_fixed_ip_to_instance(self, context, instance, network_id,
+                                 conductor_api=None, **kwargs):
         """Adds a fixed ip to instance from specified network."""
         args = {'instance_id': instance['uuid'],
                 'host': instance['host'],
-                'network_id': network_id}
+                'network_id': network_id,
+                'rxtx_factor': None}
         self.network_rpcapi.add_fixed_ip_to_instance(context, **args)
 
     @wrap_check_policy
     @refresh_cache
-    def remove_fixed_ip_from_instance(self, context, instance, address):
+    def remove_fixed_ip_from_instance(self, context, instance, address,
+                                      conductor=None, **kwargs):
         """Removes a fixed ip from instance from specified network."""
 
         args = {'instance_id': instance['uuid'],
                 'host': instance['host'],
-                'address': address}
+                'address': address,
+                'rxtx_factor': None}
         self.network_rpcapi.remove_fixed_ip_from_instance(context, **args)
 
     @wrap_check_policy
@@ -302,18 +245,17 @@ class API(base.Base):
         self.network_rpcapi.associate(context, network_uuid, associations)
 
     @wrap_check_policy
-    def get_instance_nw_info(self, context, instance, update_cache=True):
+    def get_instance_nw_info(self, context, instance, conductor_api=None,
+                             **kwargs):
         """Returns all network info related to an instance."""
         result = self._get_instance_nw_info(context, instance)
-        if update_cache:
-            update_instance_cache_with_nw_info(self, context, instance,
-                                                result)
+        update_instance_cache_with_nw_info(self, context, instance,
+                                           result, conductor_api)
         return result
 
     def _get_instance_nw_info(self, context, instance):
         """Returns all network info related to an instance."""
-        args = {'instance_id': instance['id'],
-                'instance_uuid': instance['uuid'],
+        args = {'instance_id': instance['uuid'],
                 'rxtx_factor': instance['instance_type']['rxtx_factor'],
                 'host': instance['host'],
                 'project_id': instance['project_id']}
@@ -463,3 +405,20 @@ class API(base.Base):
             args['host'] = migration['dest_compute']
 
         self.network_rpcapi.migrate_instance_finish(context, **args)
+
+    # NOTE(jkoelker) These functions where added to the api after
+    #                deprecation. Stubs provided for support documentation
+    def allocate_port_for_instance(self, context, instance, port_id,
+                                   network_id=None, requested_ip=None,
+                                   conductor_api=None):
+        raise NotImplementedError()
+
+    def deallocate_port_for_instance(self, context, instance, port_id,
+                                     conductor_api=None):
+        raise NotImplementedError()
+
+    def list_ports(self, *args, **kwargs):
+        raise NotImplementedError()
+
+    def show_port(self, *args, **kwargs):
+        raise NotImplementedError()
diff --git a/nova/tests/network/test_deprecated_api.py b/nova/tests/network/test_deprecated_api.py
new file mode 100644
index 000000000..48a7e4d6d
--- /dev/null
+++ b/nova/tests/network/test_deprecated_api.py
@@ -0,0 +1,80 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2013 Openstack Foundation
+# 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.
+
+"""Tests for the deprecated network API."""
+
+import inspect
+
+from nova.network import api
+from nova.network import api_deprecated
+from nova import test
+
+# NOTE(jkoelker) These tests require that decorators in the apis
+#                "do the right thing" and set __name__ properly
+#                they should all be using functools.wraps or similar
+#                functionality.
+
+
+def isapimethod(obj):
+    if inspect.ismethod(obj) and not obj.__name__.startswith('_'):
+        return True
+    return False
+
+
+def discover_real_method(name, method):
+    if method.func_closure:
+        for closure in method.func_closure:
+            if closure.cell_contents.__name__ == name:
+                return closure.cell_contents
+    return method
+
+
+class DeprecatedApiTestCase(test.TestCase):
+    def setUp(self):
+        super(DeprecatedApiTestCase, self).setUp()
+        self.api = api.API()
+        self.api_deprecated = api_deprecated.API()
+
+        self.api_methods = inspect.getmembers(self.api, isapimethod)
+
+    def test_api_compat(self):
+        methods = [m[0] for m in self.api_methods]
+        deprecated_methods = [getattr(self.api_deprecated, n, None)
+                              for n in methods]
+        missing = [m[0] for m in zip(methods, deprecated_methods)
+                   if m[1] is None]
+
+        self.assertFalse(missing,
+                         'Deprecated api needs methods: %s' % missing)
+
+    def test_method_signatures(self):
+        for name, method in self.api_methods:
+            deprecated_method = getattr(self.api_deprecated, name, None)
+            self.assertIsNotNone(deprecated_method,
+                                 'Deprecated api has no method %s' % name)
+
+            method = discover_real_method(name, method)
+            deprecated_method = discover_real_method(name,
+                                                     deprecated_method)
+
+            api_argspec = inspect.getargspec(method)
+            deprecated_argspec = inspect.getargspec(deprecated_method)
+
+            # NOTE/TODO(jkoelker) Should probably handle the case where
+            #                     varargs/keywords are used.
+            self.assertEqual(api_argspec.args, deprecated_argspec.args,
+                             "API method %s arguments differ" % name)