From 79c2d134b15b2d682466d8390d38e70a47fe1f22 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Tue, 19 Jul 2016 12:25:02 +0100 Subject: [PATCH] Add "erase_devices_metadata_priority" config option This patch is adding a new configuration option called "erase_devices_metadata_priority" to allow users to configure the priority (and enabling/disabling) of the "erase_devices_metadata" cleaning step. The documentation will be done in a subsequent patch. Partial-Bug: #1603411 Change-Id: I110008b3d738de0b5d2add68c9d54a4a147fc007 --- etc/ironic/ironic.conf.sample | 7 +++++++ ironic/conf/deploy.py | 7 +++++++ ironic/drivers/modules/agent.py | 2 ++ ironic/drivers/modules/ilo/deploy.py | 2 ++ ironic/drivers/modules/iscsi_deploy.py | 4 +++- ironic/tests/unit/drivers/modules/ilo/test_deploy.py | 11 ++++++++--- ironic/tests/unit/drivers/modules/test_agent.py | 7 +++++-- .../tests/unit/drivers/modules/test_iscsi_deploy.py | 4 +++- ...ase-devices-metadata-config-f39b6ca415a87757.yaml | 12 ++++++++++++ 9 files changed, 49 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/erase-devices-metadata-config-f39b6ca415a87757.yaml diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 0f0a02926d..df778417d4 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -951,6 +951,13 @@ # set to 0, will not run during cleaning. (integer value) #erase_devices_priority = +# Priority to run in-band clean step that erases metadata from +# devices, via the Ironic Python Agent ramdisk. If unset, will +# use the priority set in the ramdisk (defaults to 99 for the +# GenericHardwareManager). If set to 0, will not run during +# cleaning. (integer value) +#erase_devices_metadata_priority = + # During shred, overwrite all block devices N times with # random data. This is only used if a device could not be ATA # Secure Erased. Defaults to 1. (integer value) diff --git a/ironic/conf/deploy.py b/ironic/conf/deploy.py index d033ea23d5..05a74bc754 100644 --- a/ironic/conf/deploy.py +++ b/ironic/conf/deploy.py @@ -32,6 +32,13 @@ opts = [ 'set in the ramdisk (defaults to 10 for the ' 'GenericHardwareManager). If set to 0, will not run ' 'during cleaning.')), + cfg.IntOpt('erase_devices_metadata_priority', + help=_('Priority to run in-band clean step that erases ' + 'metadata from devices, via the Ironic Python Agent ' + 'ramdisk. If unset, will use the priority set in the ' + 'ramdisk (defaults to 99 for the ' + 'GenericHardwareManager). If set to 0, will not run ' + 'during cleaning.')), # TODO(mmitchell): Remove the deprecated name/group during Ocata cycle. cfg.IntOpt('shred_random_overwrite_iterations', deprecated_name='erase_devices_iterations', diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 16a7fdcfcb..ecbf451cf7 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -517,6 +517,8 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface): """ new_priorities = { 'erase_devices': CONF.deploy.erase_devices_priority, + 'erase_devices_metadata': + CONF.deploy.erase_devices_metadata_priority, } return deploy_utils.agent_get_clean_steps( task, interface='deploy', diff --git a/ironic/drivers/modules/ilo/deploy.py b/ironic/drivers/modules/ilo/deploy.py index cd4fa4c482..3e6213abd2 100644 --- a/ironic/drivers/modules/ilo/deploy.py +++ b/ironic/drivers/modules/ilo/deploy.py @@ -299,6 +299,8 @@ class IloVirtualMediaAgentDeploy(agent.AgentDeploy): new_priorities = { 'erase_devices': priority, + 'erase_devices_metadata': + CONF.deploy.erase_devices_metadata_priority, } return deploy_utils.agent_get_clean_steps( task, interface='deploy', diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 756e20bf5a..880805eb09 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -557,7 +557,9 @@ class ISCSIDeploy(AgentDeployMixin, base.DeployInterface): steps = deploy_utils.agent_get_clean_steps( task, interface='deploy', override_priorities={ - 'erase_devices': CONF.deploy.erase_devices_priority}) + 'erase_devices': CONF.deploy.erase_devices_priority, + 'erase_devices_metadata': + CONF.deploy.erase_devices_metadata_priority}) return steps @METRICS.timer('ISCSIDeploy.execute_clean_step') diff --git a/ironic/tests/unit/drivers/modules/ilo/test_deploy.py b/ironic/tests/unit/drivers/modules/ilo/test_deploy.py index 877b1ccfdb..b83ce61304 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_deploy.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_deploy.py @@ -495,6 +495,7 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase): autospec=True) def test_get_clean_steps_with_conf_option(self, get_clean_step_mock): self.config(clean_priority_erase_devices=20, group='ilo') + self.config(erase_devices_metadata_priority=10, group='deploy') get_clean_step_mock.return_value = [{ 'step': 'erase_devices', 'priority': 10, @@ -506,12 +507,14 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase): task.driver.deploy.get_clean_steps(task) get_clean_step_mock.assert_called_once_with( task, interface='deploy', - override_priorities={'erase_devices': 20}) + override_priorities={'erase_devices': 20, + 'erase_devices_metadata': 10}) @mock.patch.object(deploy_utils, 'agent_get_clean_steps', spec_set=True, autospec=True) def test_get_clean_steps_erase_devices_disable(self, get_clean_step_mock): self.config(clean_priority_erase_devices=0, group='ilo') + self.config(erase_devices_metadata_priority=0, group='deploy') get_clean_step_mock.return_value = [{ 'step': 'erase_devices', 'priority': 10, @@ -523,7 +526,8 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase): task.driver.deploy.get_clean_steps(task) get_clean_step_mock.assert_called_once_with( task, interface='deploy', - override_priorities={'erase_devices': 0}) + override_priorities={'erase_devices': 0, + 'erase_devices_metadata': 0}) @mock.patch.object(deploy_utils, 'agent_get_clean_steps', spec_set=True, autospec=True) @@ -539,7 +543,8 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase): task.driver.deploy.get_clean_steps(task) get_clean_step_mock.assert_called_once_with( task, interface='deploy', - override_priorities={'erase_devices': None}) + override_priorities={'erase_devices': None, + 'erase_devices_metadata': None}) @mock.patch.object(agent.AgentDeploy, 'prepare_cleaning', spec_set=True, autospec=True) diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 8b639f3b80..0e3dad2dbd 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -572,7 +572,8 @@ class TestAgentDeploy(db_base.DbTestCase): steps = self.driver.get_clean_steps(task) mock_get_clean_steps.assert_called_once_with( task, interface='deploy', - override_priorities={'erase_devices': None}) + override_priorities={'erase_devices': None, + 'erase_devices_metadata': None}) self.assertEqual(mock_steps, steps) @mock.patch('ironic.drivers.modules.deploy_utils.agent_get_clean_steps', @@ -581,6 +582,7 @@ class TestAgentDeploy(db_base.DbTestCase): # Test that we can override the priority of get clean steps # Use 0 because it is an edge case (false-y) and used in devstack self.config(erase_devices_priority=0, group='deploy') + self.config(erase_devices_metadata_priority=0, group='deploy') mock_steps = [{'priority': 10, 'interface': 'deploy', 'step': 'erase_devices'}] mock_get_clean_steps.return_value = mock_steps @@ -588,7 +590,8 @@ class TestAgentDeploy(db_base.DbTestCase): self.driver.get_clean_steps(task) mock_get_clean_steps.assert_called_once_with( task, interface='deploy', - override_priorities={'erase_devices': 0}) + override_priorities={'erase_devices': 0, + 'erase_devices_metadata': 0}) @mock.patch.object(deploy_utils, 'prepare_inband_cleaning', autospec=True) def test_prepare_cleaning(self, prepare_inband_cleaning_mock): diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index 4defcb9ba3..423839c979 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -638,6 +638,7 @@ class ISCSIDeployTestCase(db_base.DbTestCase): def test_get_clean_steps(self, mock_get_clean_steps): # Test getting clean steps self.config(group='deploy', erase_devices_priority=10) + self.config(group='deploy', erase_devices_metadata_priority=5) mock_steps = [{'priority': 10, 'interface': 'deploy', 'step': 'erase_devices'}] self.node.driver_internal_info = {'agent_url': 'foo'} @@ -648,7 +649,8 @@ class ISCSIDeployTestCase(db_base.DbTestCase): mock_get_clean_steps.assert_called_once_with( task, interface='deploy', override_priorities={ - 'erase_devices': 10}) + 'erase_devices': 10, + 'erase_devices_metadata': 5}) self.assertEqual(mock_steps, steps) @mock.patch.object(deploy_utils, 'agent_execute_clean_step', autospec=True) diff --git a/releasenotes/notes/erase-devices-metadata-config-f39b6ca415a87757.yaml b/releasenotes/notes/erase-devices-metadata-config-f39b6ca415a87757.yaml new file mode 100644 index 0000000000..d063d19725 --- /dev/null +++ b/releasenotes/notes/erase-devices-metadata-config-f39b6ca415a87757.yaml @@ -0,0 +1,12 @@ +--- +features: + - Adds a new ``[deploy]/erase_devices_metadata_priority`` configuration + option to allow operators to configure the priority (or disable) of the + "erase_devices_metadata" cleaning step. +upgrade: + - The new "erase_devices_metadata" cleaning step is enabled by + default (if available) in the ironic-python-agent project (priority + 99). Wiping the devices metadata is usually very fast and shouldn't + add much time (if any) to the overall cleaning process. Operators + wanting to disable this cleaning step can do it by setting the + ``[deploy]/erase_devices_metadata_priority`` configuration option to 0.