Merge "Allow accessIPv4 and accessIPv6 on rebuild action"

4 files changed, 113 insertions, 115 deletions

diff --git a/nova/api/openstack/v2/servers.py b/nova/api/openstack/v2/servers.py
index 51ccaadde..cb0ece1f0 100644
--- a/nova/api/openstack/v2/servers.py
+++ b/nova/api/openstack/v2/servers.py
@@ -16,10 +16,10 @@
 
 import base64
 import os
+from xml.dom import minidom
 
 from webob import exc
 import webob
-from xml.dom import minidom
 
 from nova.api.openstack import common
 from nova.api.openstack.v2 import ips
@@ -731,46 +731,68 @@ class Controller(wsgi.Controller):
         return self._resize(req, id, flavor_ref)
 
     def _action_rebuild(self, info, request, instance_id):
-        context = request.environ['nova.context']
-        instance = self._get_server(context, instance_id)
+        """Rebuild an instance with the given attributes"""
+        try:
+            body = info['rebuild']
+        except (KeyError, TypeError):
+            raise exc.HTTPBadRequest(_("Invalid request body"))
 
         try:
-            image_href = info["rebuild"]["imageRef"]
+            image_href = body["imageRef"]
         except (KeyError, TypeError):
             msg = _("Could not parse imageRef from request.")
-            LOG.debug(msg)
             raise exc.HTTPBadRequest(explanation=msg)
 
-        personality = info["rebuild"].get("personality", [])
-        injected_files = []
-        if personality:
-            injected_files = self._get_injected_files(personality)
+        try:
+            password = body['adminPass']
+        except (KeyError, TypeError):
+            password = utils.generate_password(FLAGS.password_length)
 
-        metadata = info["rebuild"].get("metadata")
-        name = info["rebuild"].get("name")
+        context = request.environ['nova.context']
+        instance = self._get_server(context, instance_id)
+
+        attr_map = {
+            'personality': 'files_to_inject',
+            'name': 'display_name',
+            'accessIPv4': 'access_ip_v4',
+            'accessIPv6': 'access_ip_v6',
+            'metadata': 'metadata',
+        }
 
-        if metadata:
-            self._validate_metadata(metadata)
+        kwargs = {}
 
-        if 'rebuild' in info and 'adminPass' in info['rebuild']:
-            password = info['rebuild']['adminPass']
-        else:
-            password = utils.generate_password(FLAGS.password_length)
+        for request_attribute, instance_attribute in attr_map.items():
+            try:
+                kwargs[instance_attribute] = body[request_attribute]
+            except (KeyError, TypeError):
+                pass
+
+        self._validate_metadata(kwargs.get('metadata', {}))
+
+        if 'files_to_inject' in kwargs:
+            personality = kwargs['files_to_inject']
+            kwargs['files_to_inject'] = self._get_injected_files(personality)
 
         try:
-            self.compute_api.rebuild(context, instance, image_href,
-                                     password, name=name, metadata=metadata,
-                                     files_to_inject=injected_files)
+            self.compute_api.rebuild(context,
+                                     instance,
+                                     image_href,
+                                     password,
+                                     **kwargs)
+
         except exception.RebuildRequiresActiveInstance:
-            msg = _("Instance %s must be active to rebuild.") % instance_id
+            msg = _("Instance must be active to rebuild.")
             raise exc.HTTPConflict(explanation=msg)
         except exception.InstanceNotFound:
-            msg = _("Instance %s could not be found") % instance_id
+            msg = _("Instance could not be found")
             raise exc.HTTPNotFound(explanation=msg)
 
         instance = self._get_server(context, instance_id)
+
         self._add_instance_faults(context, [instance])
         view = self._view_builder.show(request, instance)
+
+        # Add on the adminPass attribute since the view doesn't do it
         view['server']['adminPass'] = password
 
         return view
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 61394233a..70e9876ae 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -1194,25 +1194,22 @@ class API(base.Base):
     @check_instance_state(vm_state=[vm_states.ACTIVE],
                           task_state=[None, task_states.RESIZE_VERIFY])
     @scheduler_api.reroute_compute("rebuild")
-    def rebuild(self, context, instance, image_href, admin_password,
-                name=None, metadata=None, files_to_inject=None):
-        """Rebuild the given instance with the provided metadata."""
-        name = name or instance["display_name"]
-
-        files_to_inject = files_to_inject or []
-        metadata = metadata or {}
+    def rebuild(self, context, instance, image_href, admin_password, **kwargs):
+        """Rebuild the given instance with the provided attributes."""
 
+        files_to_inject = kwargs.pop('files_to_inject', [])
         self._check_injected_file_quota(context, files_to_inject)
+
+        metadata = kwargs.get('metadata', {})
         self._check_metadata_properties_quota(context, metadata)
 
         self.update(context,
                     instance,
-                    metadata=metadata,
-                    display_name=name,
                     image_ref=image_href,
                     vm_state=vm_states.REBUILDING,
                     task_state=None,
-                    progress=0)
+                    progress=0,
+                    **kwargs)
 
         rebuild_params = {
             "new_pass": admin_password,
diff --git a/nova/tests/api/openstack/v2/test_server_actions.py b/nova/tests/api/openstack/v2/test_server_actions.py
index 1c1c2b986..9a96c7337 100644
--- a/nova/tests/api/openstack/v2/test_server_actions.py
+++ b/nova/tests/api/openstack/v2/test_server_actions.py
@@ -16,6 +16,7 @@
 import base64
 import datetime
 
+import mox
 import stubout
 import webob
 
@@ -69,7 +70,8 @@ def return_server_with_uuid_and_state(vm_state, task_state=None):
 
 
 def stub_instance(id, metadata=None, image_ref="10", flavor_id="1",
-                  name=None, vm_state=None, task_state=None, uuid=None):
+                  name=None, vm_state=None, task_state=None, uuid=None,
+                  access_ip_v4="", access_ip_v6=""):
     if metadata is not None:
         metadata_items = [{'key':k, 'value':v} for k, v in metadata.items()]
     else:
@@ -113,8 +115,8 @@ def stub_instance(id, metadata=None, image_ref="10", flavor_id="1",
         "display_description": "",
         "locked": False,
         "metadata": metadata_items,
-        "access_ip_v4": "",
-        "access_ip_v6": "",
+        "access_ip_v4": access_ip_v4,
+        "access_ip_v6": access_ip_v6,
         "uuid": uuid,
         "virtual_interfaces": [],
         "progress": 0,
@@ -253,7 +255,7 @@ class ServerActionsControllerTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPNotFound, self.controller.action,
                           req, str(utils.gen_uuid()), body)
 
-    def test_server_rebuild_accepted_minimum(self):
+    def test_rebuild_accepted_minimum(self):
         new_return_server = return_server_with_attributes(image_ref='2')
         self.stubs.Set(nova.db, 'instance_get', new_return_server)
 
@@ -270,7 +272,7 @@ class ServerActionsControllerTest(test.TestCase):
         self.assertEqual(len(body['server']['adminPass']),
                          FLAGS.password_length)
 
-    def test_server_rebuild_rejected_when_building(self):
+    def test_rebuild_rejected_when_building(self):
         body = {
             "rebuild": {
                 "imageRef": "http://localhost/images/2",
@@ -286,7 +288,7 @@ class ServerActionsControllerTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPConflict,
                           self.controller.action, req, FAKE_UUID, body)
 
-    def test_server_rebuild_accepted_with_metadata(self):
+    def test_rebuild_accepted_with_metadata(self):
         metadata = {'new': 'metadata'}
 
         new_return_server = return_server_with_attributes(metadata=metadata)
@@ -304,7 +306,7 @@ class ServerActionsControllerTest(test.TestCase):
 
         self.assertEqual(body['server']['metadata'], metadata)
 
-    def test_server_rebuild_accepted_with_bad_metadata(self):
+    def test_rebuild_accepted_with_bad_metadata(self):
         body = {
             "rebuild": {
                 "imageRef": "http://localhost/images/2",
@@ -316,7 +318,7 @@ class ServerActionsControllerTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest,
                           self.controller.action, req, FAKE_UUID, body)
 
-    def test_server_rebuild_bad_entity(self):
+    def test_rebuild_bad_entity(self):
         body = {
             "rebuild": {
                 "imageId": 2,
@@ -327,7 +329,7 @@ class ServerActionsControllerTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest,
                           self.controller.action, req, FAKE_UUID, body)
 
-    def test_server_rebuild_bad_personality(self):
+    def test_rebuild_bad_personality(self):
         body = {
             "rebuild": {
                 "imageRef": "http://localhost/images/2",
@@ -342,7 +344,7 @@ class ServerActionsControllerTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest,
                           self.controller.action, req, FAKE_UUID, body)
 
-    def test_server_rebuild_personality(self):
+    def test_rebuild_personality(self):
         body = {
             "rebuild": {
                 "imageRef": "http://localhost/images/2",
@@ -358,7 +360,7 @@ class ServerActionsControllerTest(test.TestCase):
 
         self.assertTrue('personality' not in body['server'])
 
-    def test_server_rebuild_admin_pass(self):
+    def test_rebuild_admin_pass(self):
         new_return_server = return_server_with_attributes(image_ref='2')
         self.stubs.Set(nova.db, 'instance_get', new_return_server)
 
@@ -375,7 +377,7 @@ class ServerActionsControllerTest(test.TestCase):
         self.assertEqual(body['server']['image']['id'], '2')
         self.assertEqual(body['server']['adminPass'], 'asdf')
 
-    def test_server_rebuild_server_not_found(self):
+    def test_rebuild_server_not_found(self):
         def server_not_found(self, instance_id):
             raise exception.InstanceNotFound(instance_id=instance_id)
         self.stubs.Set(nova.db, 'instance_get', server_not_found)
@@ -390,6 +392,33 @@ class ServerActionsControllerTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPNotFound,
                           self.controller.action, req, FAKE_UUID, body)
 
+    def test_rebuild_accessIP(self):
+        attributes = {
+            'access_ip_v4': '172.19.0.1',
+            'access_ip_v6': 'fe80::1',
+        }
+
+        body = {
+            "rebuild": {
+                "imageRef": "http://localhost/images/2",
+                "accessIPv4": "172.19.0.1",
+                "accessIPv6": "fe80::1",
+            },
+        }
+
+        update = self.mox.CreateMockAnything()
+        self.stubs.Set(nova.compute.API, 'update', update)
+        req = fakes.HTTPRequest.blank(self.url)
+        context = req.environ['nova.context']
+        update(context, mox.IgnoreArg(),
+                image_ref='http://localhost/images/2',
+                vm_state=vm_states.REBUILDING,
+                task_state=None, progress=0, **attributes).AndReturn(None)
+        self.mox.ReplayAll()
+
+        self.controller.action(req, FAKE_UUID, body)
+        self.mox.VerifyAll()
+
     def test_resize_server(self):
 
         body = dict(resize=dict(flavorRef="http://localhost/3"))
diff --git a/nova/tests/integrated/test_servers.py b/nova/tests/integrated/test_servers.py
index 556b74abe..42deee413 100644
--- a/nova/tests/integrated/test_servers.py
+++ b/nova/tests/integrated/test_servers.py
@@ -312,47 +312,19 @@ class ServersTest(integrated_helpers._IntegratedTestBase):
         self._delete_server(created_server_id)
 
     def test_create_and_rebuild_server(self):
-        """Rebuild a server."""
+        """Rebuild a server with metadata."""
         self.flags(stub_network=True)
 
         # create a server with initially has no metadata
         server = self._build_minimal_create_server_request()
         server_post = {'server': server}
-        created_server = self.api.post_server(server_post)
-        LOG.debug("created_server: %s" % created_server)
-        self.assertTrue(created_server['id'])
-        created_server_id = created_server['id']
-
-        created_server = self._wait_for_state_change(created_server, 'BUILD')
-
-        # rebuild the server with metadata
-        post = {}
-        post['rebuild'] = {
-            "imageRef": "c905cedb-7281-47e4-8a62-f26bc5fc4c77",
-            "name": "blah",
-        }
-
-        self.api.post_server_action(created_server_id, post)
-        LOG.debug("rebuilt server: %s" % created_server)
-        self.assertTrue(created_server['id'])
-
-        found_server = self.api.get_server(created_server_id)
-        self.assertEqual(created_server_id, found_server['id'])
-        self.assertEqual({}, found_server.get('metadata'))
-        self.assertEqual('blah', found_server.get('name'))
-        self.assertEqual(post['rebuild']['imageRef'],
-                         found_server.get('image')['id'])
 
-        # Cleanup
-        self._delete_server(created_server_id)
+        metadata = {}
+        for i in range(30):
+            metadata['key_%s' % i] = 'value_%s' % i
 
-    def test_create_and_rebuild_server_with_metadata(self):
-        """Rebuild a server with metadata."""
-        self.flags(stub_network=True)
+        server_post['server']['metadata'] = metadata
 
-        # create a server with initially has no metadata
-        server = self._build_minimal_create_server_request()
-        server_post = {'server': server}
         created_server = self.api.post_server(server_post)
         LOG.debug("created_server: %s" % created_server)
         self.assertTrue(created_server['id'])
@@ -360,70 +332,48 @@ class ServersTest(integrated_helpers._IntegratedTestBase):
 
         created_server = self._wait_for_state_change(created_server, 'BUILD')
 
-        # rebuild the server with metadata
+        # rebuild the server with metadata and other server attributes
         post = {}
         post['rebuild'] = {
             "imageRef": "76fa36fc-c930-4bf3-8c8a-ea2a2420deb6",
             "name": "blah",
+            "accessIPv4": "172.19.0.2",
+            "accessIPv6": "fe80::2",
+            "metadata": {'some': 'thing'},
         }
 
-        metadata = {}
-        for i in range(30):
-            metadata['key_%s' % i] = 'value_%s' % i
-
-        post['rebuild']['metadata'] = metadata
-
         self.api.post_server_action(created_server_id, post)
         LOG.debug("rebuilt server: %s" % created_server)
         self.assertTrue(created_server['id'])
 
         found_server = self.api.get_server(created_server_id)
         self.assertEqual(created_server_id, found_server['id'])
-        self.assertEqual(metadata, found_server.get('metadata'))
+        self.assertEqual({'some': 'thing'}, found_server.get('metadata'))
         self.assertEqual('blah', found_server.get('name'))
+        self.assertEqual(post['rebuild']['imageRef'],
+                         found_server.get('image')['id'])
+        self.assertEqual('172.19.0.2', found_server['accessIPv4'])
+        self.assertEqual('fe80::2', found_server['accessIPv6'])
 
-        # Cleanup
-        self._delete_server(created_server_id)
-
-    def test_create_and_rebuild_server_with_metadata_removal(self):
-        """Rebuild a server with metadata."""
-        self.flags(stub_network=True)
-
-        # create a server with initially has no metadata
-        server = self._build_minimal_create_server_request()
-        server_post = {'server': server}
-
-        metadata = {}
-        for i in range(30):
-            metadata['key_%s' % i] = 'value_%s' % i
-
-        server_post['server']['metadata'] = metadata
-
-        created_server = self.api.post_server(server_post)
-        LOG.debug("created_server: %s" % created_server)
-        self.assertTrue(created_server['id'])
-        created_server_id = created_server['id']
-
-        created_server = self._wait_for_state_change(created_server, 'BUILD')
-
-        # rebuild the server with metadata
+        # rebuild the server with empty metadata and nothing else
         post = {}
         post['rebuild'] = {
             "imageRef": "76fa36fc-c930-4bf3-8c8a-ea2a2420deb6",
-            "name": "blah",
+            "metadata": {},
         }
 
-        metadata = {}
-        post['rebuild']['metadata'] = metadata
-
         self.api.post_server_action(created_server_id, post)
         LOG.debug("rebuilt server: %s" % created_server)
         self.assertTrue(created_server['id'])
 
         found_server = self.api.get_server(created_server_id)
         self.assertEqual(created_server_id, found_server['id'])
-        self.assertEqual(metadata, found_server.get('metadata'))
+        self.assertEqual({}, found_server.get('metadata'))
         self.assertEqual('blah', found_server.get('name'))
+        self.assertEqual(post['rebuild']['imageRef'],
+                         found_server.get('image')['id'])
+        self.assertEqual('172.19.0.2', found_server['accessIPv4'])
+        self.assertEqual('fe80::2', found_server['accessIPv6'])
 
         # Cleanup
         self._delete_server(created_server_id)