Merge "Ansible module: fix deployment for private and/or shared images" into stable/rocky

ironic/drivers/modules/ansible/deploy.py

@@ -266,7 +266,6 @@ def _prepare_variables(task):
     for i_key, i_value in i_info.items():
         if i_key.startswith('image_'):
             image[i_key[6:]] = i_value
-    image['mem_req'] = _calculate_memory_req(task)
 
     checksum = image.get('checksum')
     if checksum:
@@ -432,9 +431,24 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface):
     @task_manager.require_exclusive_lock
     def deploy(self, task):
         """Perform a deployment to a node."""
+        self._required_image_info(task)
         manager_utils.node_power_action(task, states.REBOOT)
         return states.DEPLOYWAIT
 
+    @staticmethod
+    def _required_image_info(task):
+        """Gather and save needed image info while the context is good.
+
+        Gather image info that will be needed later, during the
+        continue_deploy execution, where the context won't be the same
+        anymore, since coming from the server's heartbeat.
+        """
+        node = task.node
+        i_info = node.instance_info
+        i_info['image_mem_req'] = _calculate_memory_req(task)
+        node.instance_info = i_info
+        node.save()
+
     @METRICS.timer('AnsibleDeploy.tear_down')
     @task_manager.require_exclusive_lock
     def tear_down(self, task):

ironic/tests/unit/drivers/modules/ansible/test_deploy.py

@@ -307,22 +307,25 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase):
                 self.assertIn(six.text_type(key), six.text_type(exc))
                 self.assertIn(six.text_type(value), six.text_type(exc))
 
-    @mock.patch.object(ansible_deploy, '_calculate_memory_req', autospec=True,
-                       return_value=2000)
-    def test__prepare_variables(self, mem_req_mock):
+    def test__prepare_variables(self):
+        i_info = self.node.instance_info
+        i_info['image_mem_req'] = 3000
+        i_info['image_whatever'] = 'hello'
+        self.node.instance_info = i_info
+        self.node.save()
+
         expected = {"image": {"url": "http://image",
                               "validate_certs": "yes",
                               "source": "fake-image",
-                              "mem_req": 2000,
+                              "mem_req": 3000,
                               "disk_format": "qcow2",
-                              "checksum": "md5:checksum"}}
+                              "checksum": "md5:checksum",
+                              "whatever": "hello"}}
         with task_manager.acquire(self.context, self.node.uuid) as task:
             self.assertEqual(expected,
                              ansible_deploy._prepare_variables(task))
 
-    @mock.patch.object(ansible_deploy, '_calculate_memory_req', autospec=True,
-                       return_value=2000)
-    def test__prepare_variables_root_device_hints(self, mem_req_mock):
+    def test__prepare_variables_root_device_hints(self):
         props = self.node.properties
         props['root_device'] = {"wwn": "fake-wwn"}
         self.node.properties = props
@@ -330,7 +333,6 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase):
         expected = {"image": {"url": "http://image",
                               "validate_certs": "yes",
                               "source": "fake-image",
-                              "mem_req": 2000,
                               "disk_format": "qcow2",
                               "checksum": "md5:checksum"},
                     "root_device_hints": {"wwn": "fake-wwn"}}
@@ -338,9 +340,7 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase):
             self.assertEqual(expected,
                              ansible_deploy._prepare_variables(task))
 
-    @mock.patch.object(ansible_deploy, '_calculate_memory_req', autospec=True,
-                       return_value=2000)
-    def test__prepare_variables_noglance(self, mem_req_mock):
+    def test__prepare_variables_insecure_activated(self):
         self.config(image_store_insecure=True, group='ansible')
         i_info = self.node.instance_info
         i_info['image_checksum'] = 'sha256:checksum'
@@ -349,16 +349,13 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase):
         expected = {"image": {"url": "http://image",
                               "validate_certs": "no",
                               "source": "fake-image",
-                              "mem_req": 2000,
                               "disk_format": "qcow2",
                               "checksum": "sha256:checksum"}}
         with task_manager.acquire(self.context, self.node.uuid) as task:
             self.assertEqual(expected,
                              ansible_deploy._prepare_variables(task))
 
-    @mock.patch.object(ansible_deploy, '_calculate_memory_req', autospec=True,
-                       return_value=2000)
-    def test__prepare_variables_configdrive_url(self, mem_req_mock):
+    def test__prepare_variables_configdrive_url(self):
         i_info = self.node.instance_info
         i_info['configdrive'] = 'http://configdrive_url'
         self.node.instance_info = i_info
@@ -366,7 +363,6 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase):
         expected = {"image": {"url": "http://image",
                               "validate_certs": "yes",
                               "source": "fake-image",
-                              "mem_req": 2000,
                               "disk_format": "qcow2",
                               "checksum": "md5:checksum"},
                     'configdrive': {'type': 'url',
@@ -375,9 +371,7 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase):
             self.assertEqual(expected,
                              ansible_deploy._prepare_variables(task))
 
-    @mock.patch.object(ansible_deploy, '_calculate_memory_req', autospec=True,
-                       return_value=2000)
-    def test__prepare_variables_configdrive_file(self, mem_req_mock):
+    def test__prepare_variables_configdrive_file(self):
         i_info = self.node.instance_info
         i_info['configdrive'] = 'fake-content'
         self.node.instance_info = i_info
@@ -388,7 +382,6 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase):
         expected = {"image": {"url": "http://image",
                               "validate_certs": "yes",
                               "source": "fake-image",
-                              "mem_req": 2000,
                               "disk_format": "qcow2",
                               "checksum": "md5:checksum"},
                     'configdrive': {'type': 'file',
@@ -519,13 +512,18 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase):
                 task.driver.boot, task)
             get_boot_mock.assert_called_once_with(task.node)
 
+    @mock.patch.object(ansible_deploy, '_calculate_memory_req', autospec=True,
+                       return_value=2000)
     @mock.patch.object(utils, 'node_power_action', autospec=True)
-    def test_deploy(self, power_mock):
+    def test_deploy(self, power_mock, mem_req_mock):
         with task_manager.acquire(
                 self.context, self.node['uuid'], shared=False) as task:
             driver_return = self.driver.deploy(task)
             self.assertEqual(driver_return, states.DEPLOYWAIT)
             power_mock.assert_called_once_with(task, states.REBOOT)
+            mem_req_mock.assert_called_once_with(task)
+            i_info = task.node.instance_info
+            self.assertEqual(i_info['image_mem_req'], 2000)
 
     @mock.patch.object(utils, 'node_power_action', autospec=True)
     def test_tear_down(self, power_mock):

releasenotes/notes/bug-35702-25da234580ca0c31.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+      - |
+          Fixes an issue regarding the ``ansible`` deploy interface. Node
+          deployment was broken for any image that was not public because
+          the original request context was not available anymore at the time
+          some image information was fetched.