Merge "Enable cinder storage interface for generic hardware"

3 files changed, 136 insertions, 10 deletions

diff --git a/ironic/tests/unit/api/v1/test_drivers.py b/ironic/tests/unit/api/v1/test_drivers.py
index 7c753d40e..61795ece3 100644
--- a/ironic/tests/unit/api/v1/test_drivers.py
+++ b/ironic/tests/unit/api/v1/test_drivers.py
@@ -48,7 +48,7 @@ class TestListDrivers(base.BaseApiTest):
             self.dbapi.register_conductor_hardware_interfaces(
                 c.id, self.d3, 'deploy', ['iscsi', 'direct'], 'direct')
 
-    def _test_drivers(self, use_dynamic, detail=False):
+    def _test_drivers(self, use_dynamic, detail=False, storage_if=False):
         self.register_fake_conductors()
         headers = {}
         expected = [
@@ -58,7 +58,10 @@ class TestListDrivers(base.BaseApiTest):
         ]
         expected = sorted(expected, key=lambda d: d['name'])
         if use_dynamic:
-            headers[api_base.Version.string] = '1.30'
+            if storage_if:
+                headers[api_base.Version.string] = '1.33'
+            else:
+                headers[api_base.Version.string] = '1.30'
 
         path = '/drivers'
         if detail:
@@ -83,6 +86,12 @@ class TestListDrivers(base.BaseApiTest):
             # as this case can't actually happen.
             if detail:
                 self.assertIn('default_deploy_interface', d)
+                if storage_if:
+                    self.assertIn('default_storage_interface', d)
+                    self.assertIn('enabled_storage_interfaces', d)
+                else:
+                    self.assertNotIn('default_storage_interface', d)
+                    self.assertNotIn('enabled_storage_interfaces', d)
             else:
                 # ensure we don't spill these fields into driver listing
                 # one should be enough
@@ -94,7 +103,7 @@ class TestListDrivers(base.BaseApiTest):
     def test_drivers_with_dynamic(self):
         self._test_drivers(True)
 
-    def test_drivers_with_dynamic_detailed(self):
+    def _test_drivers_with_dynamic_detailed(self, storage_if=False):
         with mock.patch.object(self.dbapi, 'list_hardware_type_interfaces',
                                autospec=True) as mock_hw:
             mock_hw.return_value = [
@@ -112,7 +121,13 @@ class TestListDrivers(base.BaseApiTest):
                 },
             ]
 
-            self._test_drivers(True, detail=True)
+            self._test_drivers(True, detail=True, storage_if=storage_if)
+
+    def test_drivers_with_dynamic_detailed(self):
+        self._test_drivers_with_dynamic_detailed()
+
+    def test_drivers_with_dynamic_detailed_storage_interface(self):
+        self._test_drivers_with_dynamic_detailed(storage_if=True)
 
     def _test_drivers_type_filter(self, requested_type):
         self.register_fake_conductors()
@@ -163,7 +178,8 @@ class TestListDrivers(base.BaseApiTest):
         self.assertEqual([], data['drivers'])
 
     @mock.patch.object(rpcapi.ConductorAPI, 'get_driver_properties')
-    def _test_drivers_get_one_ok(self, use_dynamic, mock_driver_properties):
+    def _test_drivers_get_one_ok(self, use_dynamic, mock_driver_properties,
+                                 storage_if=False):
         # get_driver_properties mock is required by validate_link()
         self.register_fake_conductors()
 
@@ -176,8 +192,14 @@ class TestListDrivers(base.BaseApiTest):
             driver_type = 'classic'
             hosts = [self.h1]
 
+        headers = {}
+        if storage_if:
+            headers[api_base.Version.string] = '1.33'
+        else:
+            headers[api_base.Version.string] = '1.30'
+
         data = self.get_json('/drivers/%s' % driver,
-                             headers={api_base.Version.string: '1.30'})
+                             headers=headers)
 
         self.assertEqual(driver, data['name'])
         self.assertEqual(sorted(hosts), sorted(data['hosts']))
@@ -186,8 +208,7 @@ class TestListDrivers(base.BaseApiTest):
 
         if use_dynamic:
             for iface in driver_base.ALL_INTERFACES:
-                # NOTE(jroll) we don't expose storage interface yet
-                if iface != 'storage':
+                if storage_if or iface != 'storage':
                     self.assertIn('default_%s_interface' % iface, data)
                     self.assertIn('enabled_%s_interfaces' % iface, data)
             self.assertIsNotNone(data['default_deploy_interface'])
@@ -204,7 +225,7 @@ class TestListDrivers(base.BaseApiTest):
     def test_drivers_get_one_ok_classic(self):
         self._test_drivers_get_one_ok(False)
 
-    def test_drivers_get_one_ok_dynamic(self):
+    def _test_drivers_get_one_ok_dynamic(self, storage_if=False):
         with mock.patch.object(self.dbapi, 'list_hardware_type_interfaces',
                                autospec=True) as mock_hw:
             mock_hw.return_value = [
@@ -222,9 +243,15 @@ class TestListDrivers(base.BaseApiTest):
                 },
             ]
 
-            self._test_drivers_get_one_ok(True)
+            self._test_drivers_get_one_ok(True, storage_if=storage_if)
             mock_hw.assert_called_once_with([self.d3])
 
+    def test_drivers_get_one_ok_dynamic(self):
+        self._test_drivers_get_one_ok_dynamic()
+
+    def test_drivers_get_one_ok_dynamic_storage_interface(self):
+        self._test_drivers_get_one_ok_dynamic(storage_if=True)
+
     def test_driver_properties_hidden_in_lower_version(self):
         self.register_fake_conductors()
         data = self.get_json('/drivers/%s' % self.d1,
diff --git a/ironic/tests/unit/api/v1/test_nodes.py b/ironic/tests/unit/api/v1/test_nodes.py
index bcedaa781..02c2be9ba 100644
--- a/ironic/tests/unit/api/v1/test_nodes.py
+++ b/ironic/tests/unit/api/v1/test_nodes.py
@@ -116,6 +116,7 @@ class TestListNodes(test_api_base.BaseApiTest):
         self.assertNotIn('resource_class', data['nodes'][0])
         for field in api_utils.V31_FIELDS:
             self.assertNotIn(field, data['nodes'][0])
+        self.assertNotIn('storage_interface', data['nodes'][0])
         # never expose the chassis_id
         self.assertNotIn('chassis_id', data['nodes'][0])
 
@@ -149,6 +150,7 @@ class TestListNodes(test_api_base.BaseApiTest):
         self.assertIn('resource_class', data)
         for field in api_utils.V31_FIELDS:
             self.assertIn(field, data)
+        self.assertIn('storage_interface', data)
         # never expose the chassis_id
         self.assertNotIn('chassis_id', data)
 
@@ -168,6 +170,14 @@ class TestListNodes(test_api_base.BaseApiTest):
         for field in api_utils.V31_FIELDS:
             self.assertNotIn(field, data)
 
+    def test_node_storage_interface_hidden_in_lower_version(self):
+        node = obj_utils.create_test_node(self.context,
+                                          storage_interface='cinder')
+        data = self.get_json(
+            '/nodes/%s' % node.uuid,
+            headers={api_base.Version.string: '1.32'})
+        self.assertNotIn('storage_interface', data)
+
     def test_get_one_custom_fields(self):
         node = obj_utils.create_test_node(self.context,
                                           chassis_id=self.chassis.id)
@@ -267,6 +277,25 @@ class TestListNodes(test_api_base.BaseApiTest):
         for field in api_utils.V31_FIELDS:
             self.assertIn(field, response)
 
+    def test_get_storage_interface_fields_invalid_api_version(self):
+        node = obj_utils.create_test_node(self.context,
+                                          chassis_id=self.chassis.id)
+        fields = 'storage_interface'
+        response = self.get_json(
+            '/nodes/%s?fields=%s' % (node.uuid, fields),
+            headers={api_base.Version.string: str(api_v1.MIN_VER)},
+            expect_errors=True)
+        self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
+
+    def test_get_storage_interface_fields(self):
+        node = obj_utils.create_test_node(self.context,
+                                          chassis_id=self.chassis.id)
+        fields = 'storage_interface'
+        response = self.get_json(
+            '/nodes/%s?fields=%s' % (node.uuid, fields),
+            headers={api_base.Version.string: str(api_v1.MAX_VER)})
+        self.assertIn('storage_interface', response)
+
     def test_detail(self):
         node = obj_utils.create_test_node(self.context,
                                           chassis_id=self.chassis.id)
@@ -294,6 +323,7 @@ class TestListNodes(test_api_base.BaseApiTest):
         self.assertIn('resource_class', data['nodes'][0])
         for field in api_utils.V31_FIELDS:
             self.assertIn(field, data['nodes'][0])
+        self.assertIn('storage_interface', data['nodes'][0])
         # never expose the chassis_id
         self.assertNotIn('chassis_id', data['nodes'][0])
 
@@ -413,6 +443,17 @@ class TestListNodes(test_api_base.BaseApiTest):
                              headers={api_base.Version.string: "1.32"})
         self.assertIn('volume', data)
 
+    def test_hide_fields_in_newer_versions_storage_interface(self):
+        node = obj_utils.create_test_node(self.context,
+                                          storage_interface='cinder')
+        data = self.get_json(
+            '/nodes/detail', headers={api_base.Version.string: '1.32'})
+        self.assertNotIn('storage_interface', data['nodes'][0])
+        new_data = self.get_json(
+            '/nodes/detail', headers={api_base.Version.string: '1.33'})
+        self.assertEqual(node.storage_interface,
+                         new_data['nodes'][0]["storage_interface"])
+
     def test_many(self):
         nodes = []
         for id in range(5):
@@ -2013,6 +2054,35 @@ class TestPatch(test_api_base.BaseApiTest):
             self.assertEqual(http_client.BAD_REQUEST, response.status_int)
             self.assertEqual('application/json', response.content_type)
 
+    def test_update_storage_interface(self):
+        node = obj_utils.create_test_node(self.context,
+                                          uuid=uuidutils.generate_uuid())
+        self.mock_update_node.return_value = node
+        storage_interface = 'cinder'
+        headers = {api_base.Version.string: str(api_v1.MAX_VER)}
+        response = self.patch_json('/nodes/%s' % node.uuid,
+                                   [{'path': '/storage_interface',
+                                     'value': storage_interface,
+                                     'op': 'add'}],
+                                   headers=headers)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.OK, response.status_code)
+
+    def test_update_storage_interface_old_api(self):
+        node = obj_utils.create_test_node(self.context,
+                                          uuid=uuidutils.generate_uuid())
+        self.mock_update_node.return_value = node
+        storage_interface = 'cinder'
+        headers = {api_base.Version.string: '1.32'}
+        response = self.patch_json('/nodes/%s' % node.uuid,
+                                   [{'path': '/storage_interface',
+                                     'value': storage_interface,
+                                     'op': 'add'}],
+                                   headers=headers,
+                                   expect_errors=True)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code)
+
 
 def _create_node_locally(node):
     driver_factory.check_and_update_node_interfaces(node)
@@ -2122,6 +2192,12 @@ class TestPost(test_api_base.BaseApiTest):
             self.assertEqual('application/json', response.content_type)
             self.assertTrue(response.json['error_message'])
 
+    def test_create_node_explicit_storage_interface(self):
+        headers = {api_base.Version.string: '1.33'}
+        result = self._test_create_node(headers=headers,
+                                        storage_interface='cinder')
+        self.assertEqual('cinder', result['storage_interface'])
+
     def test_create_node_name_empty_invalid(self):
         ndict = test_api_utils.post_get_test_node(name='')
         response = self.post_json('/nodes', ndict,
@@ -2508,6 +2584,22 @@ class TestPost(test_api_base.BaseApiTest):
         self.assertEqual('application/json', response.content_type)
         self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
 
+    def test_create_node_storage_interface_old_api_version(self):
+        headers = {api_base.Version.string: '1.32'}
+        ndict = test_api_utils.post_get_test_node(storage_interface='cinder')
+        response = self.post_json('/nodes', ndict, headers=headers,
+                                  expect_errors=True)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
+
+    def test_create_node_invalid_storage_interface(self):
+        ndict = test_api_utils.post_get_test_node(storage_interface='foo')
+        response = self.post_json('/nodes', ndict, expect_errors=True,
+                                  headers={api_base.Version.string:
+                                           str(api_v1.MAX_VER)})
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.BAD_REQUEST, response.status_int)
+
 
 class TestDelete(test_api_base.BaseApiTest):
 
diff --git a/ironic/tests/unit/api/v1/test_utils.py b/ironic/tests/unit/api/v1/test_utils.py
index c0a4f7740..ae551fd03 100644
--- a/ironic/tests/unit/api/v1/test_utils.py
+++ b/ironic/tests/unit/api/v1/test_utils.py
@@ -419,6 +419,13 @@ class TestApiUtils(base.TestCase):
         mock_request.version.minor = 31
         self.assertFalse(utils.allow_volume())
 
+    @mock.patch.object(pecan, 'request', spec_set=['version'])
+    def test_allow_storage_interface(self, mock_request):
+        mock_request.version.minor = 33
+        self.assertTrue(utils.allow_storage_interface())
+        mock_request.version.minor = 32
+        self.assertFalse(utils.allow_storage_interface())
+
 
 class TestNodeIdent(base.TestCase):