From bf5853b2cd98f6b682a65d0ca0325a9d9cdc1568 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Fri, 1 Apr 2016 17:49:51 -0400 Subject: [PATCH] deployment vmedia ops should not be run when not deploying When a conductor fail-over occurs, the conductor _do_takeover method calls task.driver.deploy.prepare(). In some drivers, this results in task.driver.boot.prepare_ramdisk() being called which when dealing with virtual media, is inappropriate to do when not performing deployment operations as this can result in the deployment virtual media being attached to a running host. Change-Id: Ief8dc385ccb67432eed05813ca266dd1e316575c Closes-Bug: #1565104 --- ironic/drivers/modules/ilo/boot.py | 7 +++++++ ironic/drivers/modules/irmc/boot.py | 7 +++++++ .../unit/drivers/modules/ilo/test_boot.py | 20 +++++++++++++++++++ .../unit/drivers/modules/irmc/test_boot.py | 16 +++++++++++++++ 4 files changed, 50 insertions(+) diff --git a/ironic/drivers/modules/ilo/boot.py b/ironic/drivers/modules/ilo/boot.py index 524988f81a..481e3ebd33 100644 --- a/ironic/drivers/modules/ilo/boot.py +++ b/ironic/drivers/modules/ilo/boot.py @@ -32,6 +32,7 @@ from ironic.common.i18n import _LE from ironic.common.i18n import _LW from ironic.common import image_service from ironic.common import images +from ironic.common import states from ironic.common import swift from ironic.conductor import utils as manager_utils from ironic.drivers import base @@ -293,6 +294,12 @@ class IloVirtualMediaBoot(base.BootInterface): """ node = task.node + # NOTE(TheJulia): If this method is being called by something + # aside from a deployment, such as conductor takeover, we should + # treat this as a no-op and move on otherwise we would modify + # the state of the node due to virtual media operations. + if node.provision_state != states.DEPLOYING: + return # Clear ilo_boot_iso if it's a glance image to force recreate # another one again (or use existing one in glance). diff --git a/ironic/drivers/modules/irmc/boot.py b/ironic/drivers/modules/irmc/boot.py index e09e60db18..89c58a8d57 100644 --- a/ironic/drivers/modules/irmc/boot.py +++ b/ironic/drivers/modules/irmc/boot.py @@ -603,6 +603,13 @@ class IRMCVirtualMediaBoot(base.BootInterface): :raises: IRMCOperationError, if some operation on iRMC fails. """ + # NOTE(TheJulia): If this method is being called by something + # aside from a deployment, such as conductor takeover, we should + # treat this as a no-op and move on otherwise we would modify + # the state of the node due to virtual media operations. + if task.node.provision_state != states.DEPLOYING: + return + deploy_nic_mac = deploy_utils.get_single_nic_with_vif_port_id(task) ramdisk_params['BOOTIF'] = deploy_nic_mac diff --git a/ironic/tests/unit/drivers/modules/ilo/test_boot.py b/ironic/tests/unit/drivers/modules/ilo/test_boot.py index f1e3f79648..93ea84c4b2 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_boot.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_boot.py @@ -27,6 +27,7 @@ from ironic.common import exception from ironic.common.glance_service import service_utils from ironic.common import image_service from ironic.common import images +from ironic.common import states from ironic.common import swift from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils @@ -499,7 +500,24 @@ class IloVirtualMediaBootTestCase(db_base.DbTestCase): setup_vmedia_mock.assert_called_once_with(task, 'deploy-iso', expected_ramdisk_opts) + @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, + autospec=True) + def test_prepare_ramdisk_not_deploying(self, mock_is_image): + """Ensure ramdisk build operations are blocked when not deploying""" + + for state in states.STABLE_STATES: + mock_is_image.reset_mock() + self.node.provision_state = state + self.node.save() + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertIsNone( + task.driver.boot.prepare_ramdisk(task, None)) + self.assertFalse(mock_is_image.called) + def test_prepare_ramdisk_glance_image(self): + self.node.provision_state = states.DEPLOYING + self.node.save() self._test_prepare_ramdisk( ilo_boot_iso='swift:abcdef', image_source='6b2f0c0c-79e8-4db6-842e-43c9764204af') @@ -507,6 +525,8 @@ class IloVirtualMediaBootTestCase(db_base.DbTestCase): self.assertNotIn('ilo_boot_iso', self.node.instance_info) def test_prepare_ramdisk_not_a_glance_image(self): + self.node.provision_state = states.DEPLOYING + self.node.save() self._test_prepare_ramdisk( ilo_boot_iso='http://mybootiso', image_source='http://myimage') diff --git a/ironic/tests/unit/drivers/modules/irmc/test_boot.py b/ironic/tests/unit/drivers/modules/irmc/test_boot.py index c018f16c18..a607282ead 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_boot.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_boot.py @@ -907,6 +907,7 @@ class IRMCVirtualMediaBootTestCase(db_base.DbTestCase): instance_info['irmc_boot_iso'] = 'glance://abcdef' instance_info['image_source'] = '6b2f0c0c-79e8-4db6-842e-43c9764204af' self.node.instance_info = instance_info + self.node.provision_state = states.DEPLOYING self.node.save() ramdisk_params = {'a': 'b'} @@ -923,6 +924,21 @@ class IRMCVirtualMediaBootTestCase(db_base.DbTestCase): self.assertEqual('glance://abcdef', self.node.instance_info['irmc_boot_iso']) + @mock.patch.object(irmc_boot, '_setup_deploy_iso', spec_set=True, + autospec=True) + def test_prepare_ramdisk_not_deploying(self, mock_is_image): + """Ensure ramdisk build operations are blocked when not deploying""" + + for state in states.STABLE_STATES: + mock_is_image.reset_mock() + self.node.provision_state = state + self.node.save() + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertIsNone( + task.driver.boot.prepare_ramdisk(task, None)) + self.assertFalse(mock_is_image.called) + @mock.patch.object(irmc_boot, '_cleanup_vmedia_boot', spec_set=True, autospec=True) def test_clean_up_ramdisk(self, _cleanup_vmedia_boot_mock):