Merge "More fixes for anaconda deploy interface"

ironic/common/pxe_utils.py

@@ -669,42 +669,52 @@ def get_instance_image_info(task, ipxe_enabled=False):
         return image_info
 
     labels = ('kernel', 'ramdisk')
+    image_properties = None
     d_info = deploy_utils.get_image_instance_info(node)
     if not (i_info.get('kernel') and i_info.get('ramdisk')):
+        # NOTE(rloo): If both are not specified in instance_info
+        # we won't use any of them. We'll use the values specified
+        # with the image, which we assume have been set.
         glance_service = service.GlanceImageService(context=ctx)
-        iproperties = glance_service.show(d_info['image_source'])['properties']
+        image_properties = glance_service.show(
+            d_info['image_source'])['properties']
         for label in labels:
-            i_info[label] = str(iproperties[label + '_id'])
+            i_info[label] = str(image_properties[label + '_id'])
         node.instance_info = i_info
         node.save()
 
     anaconda_labels = ()
     if deploy_utils.get_boot_option(node) == 'kickstart':
-        # stage2  - Installer stage2 squashfs image
+        # stage2: installer stage2 squashfs image
-        # ks_template - Anaconda kickstart template
+        # ks_template: anaconda kickstart template
         # ks_cfg - rendered ks_template
         anaconda_labels = ('stage2', 'ks_template', 'ks_cfg')
-        if not (i_info.get('stage2') and i_info.get('ks_template')):
+        if not i_info.get('stage2') or not i_info.get('ks_template'):
-            iproperties = glance_service.show(
+            if not image_properties:
-                d_info['image_source']
+                glance_service = service.GlanceImageService(context=ctx)
-            )['properties']
+                image_properties = glance_service.show(
-            for label in anaconda_labels:
+                    d_info['image_source'])['properties']
+            if not i_info.get('ks_template'):
                 # ks_template is an optional property on the image
-                if (label == 'ks_template'
+                if 'ks_template' not in image_properties:
-                        and not iproperties.get('ks_template')):
+                    i_info['ks_template'] = CONF.anaconda.default_ks_template
-                    i_info[label] = CONF.anaconda.default_ks_template
+                else:
-                elif label == 'ks_cfg':
+                    i_info['ks_template'] = str(
-                    i_info[label] = ''
+                        image_properties['ks_template'])
-                elif label == 'stage2' and 'stage2_id' not in iproperties:
+            if not i_info.get('stage2'):
-                    msg = ("stage2_id property missing on the image. "
+                if 'stage2_id' not in image_properties:
-                           "The anaconda deploy interface requires stage2_id "
+                    msg = ("'stage2_id' property is missing from the OS image "
-                           "property to be associated with the os image. ")
+                           "%s. The anaconda deploy interface requires this "
+                           "to be set with the OS image or in instance_info. "
+                           % d_info['image_source'])
                     raise exception.ImageUnacceptable(msg)
                 else:
-                    i_info[label] = str(iproperties['stage2_id'])
+                    i_info['stage2'] = str(image_properties['stage2_id'])
+        # NOTE(rloo): This is internally generated; cannot be specified.
+        i_info['ks_cfg'] = ''
 
-            node.instance_info = i_info
+        node.instance_info = i_info
-            node.save()
+        node.save()
 
     for label in labels + anaconda_labels:
         image_info[label] = (
@@ -1096,16 +1106,17 @@ def validate_kickstart_file(ks_cfg):
         return
 
     with tempfile.NamedTemporaryFile(
-            dir=CONF.tempdir, suffix='.cfg') as ks_file:
+            dir=CONF.tempdir, suffix='.cfg', mode='wt') as ks_file:
-        ks_file.writelines(ks_cfg)
+        ks_file.write(ks_cfg)
+        ks_file.flush()
         try:
-            result = utils.execute(
+            utils.execute(
                 'ksvalidator', ks_file.name, check_on_exit=[0], attempts=1
             )
-        except processutils.ProcessExecutionError:
+        except processutils.ProcessExecutionError as e:
             msg = _(("The kickstart file generated does not pass validation. "
-                     "The ksvalidator tool returned following error(s): %s") %
+                     "The ksvalidator tool returned the following error: %s") %
-                    (result))
+                    (e))
             raise exception.InvalidKickstartFile(msg)
 
 
@@ -1203,17 +1214,14 @@ def cache_ramdisk_kernel(task, pxe_info, ipxe_enabled=False):
     else:
         path = os.path.join(CONF.pxe.tftp_root, node.uuid)
     ensure_tree(path)
-    # anconda deploy will have 'stage2' as one of the labels in pxe_info dict
+    # anaconda deploy will have 'stage2' as one of the labels in pxe_info dict
     if 'stage2' in pxe_info.keys():
-        # stage2  will be stored in ipxe http directory. So make sure they
+        # stage2 will be stored in ipxe http directory so make sure the
-        # exist.
+        # directory exists.
-        ensure_tree(
+        file_path = get_file_path_from_label(node.uuid,
-            get_file_path_from_label(
+                                             CONF.deploy.http_root,
-                node.uuid,
+                                             'stage2')
-                CONF.deploy.http_root,
+        ensure_tree(os.path.dirname(file_path))
-                'stage2'
-            )
-        )
         # ks_cfg is rendered later by the driver using ks_template. It cannot
         # be fetched and cached.
         t_pxe_info.pop('ks_cfg')

ironic/drivers/modules/ks.cfg.template

@@ -18,7 +18,7 @@ autopart
 # Downloading and installing OS image using liveimg section is mandatory
 liveimg --url {{ ks_options.liveimg_url }}
 
-# Following %pre, %onerror and %trackback sections are mandatory
+# Following %pre and %onerror sections are mandatory
 %pre
 /usr/bin/curl -X POST -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'X-OpenStack-Ironic-API-Version: 1.72' -d '{"callback_url": "", "agent_token": "{{ ks_options.agent_token }}", "agent_status": "start", "agent_status_message": "Deployment starting. Running pre-installation scripts."}' {{ ks_options.heartbeat_url }}
 %end
@@ -27,10 +27,6 @@ liveimg --url {{ ks_options.liveimg_url }}
 /usr/bin/curl -X POST -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'X-OpenStack-Ironic-API-Version: 1.72' -d '{"callback_url": "", "agent_token": "{{ ks_options.agent_token }}", "agent_status": "error", "agent_status_message": "Error: Deploying using anaconda. Check console for more information."}' {{ ks_options.heartbeat_url }}
 %end
 
-%traceback
-/usr/bin/curl -X POST -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'X-OpenStack-Ironic-API-Version: 1.72' -d '{"callback_url": "", "agent_token": "{{ ks_options.agent_token }}", "agent_status": "error", "agent_status_message": "Error: Installer crashed unexpectedly."}' {{ ks_options.heartbeat_url }}
-%end
-
 # Sending callback after the installation is mandatory
 %post
 /usr/bin/curl -X POST -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'X-OpenStack-Ironic-API-Version: 1.72' -d '{"callback_url": "", "agent_token": "{{ ks_options.agent_token }}", "agent_status": "end", "agent_status_message": "Deployment completed successfully."}' {{ ks_options.heartbeat_url }}

ironic/drivers/modules/pxe.py

@@ -85,6 +85,9 @@ class PXEAnacondaDeploy(agent_base.AgentBaseMixin, agent_base.HeartbeatMixin,
             # NOTE(TheJulia): If this was any other interface, we would
             # unconfigure tenant networks, add provisioning networks, etc.
             task.driver.storage.attach_volumes(task)
+            node.instance_info = deploy_utils.build_instance_info_for_deploy(
+                task)
+            node.save()
         if node.provision_state in (states.ACTIVE, states.UNRESCUING):
             # In the event of takeover or unrescue.
             task.driver.boot.prepare_instance(task)
@@ -123,13 +126,13 @@ class PXEAnacondaDeploy(agent_base.AgentBaseMixin, agent_base.HeartbeatMixin,
             agent_base.log_and_raise_deployment_error(task, msg)
 
         try:
+            task.process_event('resume')
             self.clean_up(task)
             manager_utils.node_power_action(task, states.POWER_OFF)
             task.driver.network.remove_provisioning_network(task)
             task.driver.network.configure_tenant_networks(task)
             manager_utils.node_power_action(task, states.POWER_ON)
-            node.provision_state = states.ACTIVE
+            task.process_event('done')
-            node.save()
         except Exception as e:
             msg = (_('Error rebooting node %(node)s after deploy. '
                      'Error: %(error)s') %

ironic/tests/unit/drivers/modules/test_pxe.py

@@ -1022,15 +1022,23 @@ class PXEAnacondaDeployTestCase(db_base.DbTestCase):
             mock_prepare_ks_config.assert_called_once_with(task, image_info,
                                                            anaconda_boot=True)
 
+    @mock.patch.object(deploy_utils, 'build_instance_info_for_deploy',
+                       autospec=True)
     @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True)
-    def test_prepare(self, mock_prepare_instance):
+    def test_prepare(self, mock_prepare_instance, mock_build_instance):
+
         node = self.node
         node.provision_state = states.DEPLOYING
         node.instance_info = {}
         node.save()
+        updated_instance_info = {'image_url': 'foo'}
+        mock_build_instance.return_value = updated_instance_info
         with task_manager.acquire(self.context, node.uuid) as task:
             task.driver.deploy.prepare(task)
             self.assertFalse(mock_prepare_instance.called)
+            mock_build_instance.assert_called_once_with(task)
+            node.refresh()
+            self.assertEqual(updated_instance_info, node.instance_info)
 
     @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True)
     def test_prepare_active(self, mock_prepare_instance):

releasenotes/notes/anaconda-deploy-more-fixes-58d996c7031c8c4b.yaml

@@ -0,0 +1,33 @@
+---
+fixes:
+  - |
+    Fixes the logic for the anaconda deploy interface. If the
+    ironic node's instance_info doesn't have both 'stage2' and
+    'ks_template' specified, we weren't using the instance_info
+    at all. This has been fixed to use the instance_info if it
+    was specified. Otherwise, 'stage2' is taken from the
+    image's properties (assumed that it is set there).
+    'ks_template' value is from the image properties if specified
+    there (since it is optional); else we use the config setting
+    '[anaconda] default_ks_template'.
+  - |
+    For the anaconda deploy interface, the 'stage2' directory was
+    incorrectly being created using the full path of the stage2 file;
+    this has been fixed.
+  - |
+    The anaconda deploy interface expects the node's instance_info
+    to be populated with the 'image_url'; this is now populated
+    (via PXEAnacondaDeploy's prepare() method).
+  - |
+    For the anaconda deploy interface, when the deploy was finished
+    and the bm node was being rebooted, the node's provision state was
+    incorrectly being set to 'active' -- the provisioning state-machine
+    mechanism now handles that.
+  - |
+    For the anaconda deploy interface, the code that was doing the
+    validation of the kickstart file was incorrect and resulted in
+    errors; this has been addressed.
+  - |
+    For the anaconda deploy interface, the '%traceback' section in the
+    packaged 'ks.cfg.template' file is deprecated and fails validation,
+    so it has been removed.