openstack/ironic
Code Issues Proposed changes

Add functionality for individual cleaning on nodes

Browse Source 
Create the object for automated clean and add the logic
in the conductor to be able to enable clean for specific
nodes, when general automated clean is disabled.

Story: #2002161
Task: #24579

Change-Id: If0130082e16d1205fdf65d083854ef9849754f8b
This commit is contained in:
Yolanda Robla 2018-07-26 17:47:15 +02:00
parent 36e87dc5b4
commit 869b86e0c3
10 changed files with 238 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
ironic/common/release_mappings.py
View File

@ -134,7 +134,7 @@ RELEASE_MAPPING = {
'api': '1.46',
'rpc': '1.47',
'objects': {
'Node': ['1.27'],
'Node': ['1.28'],
'Conductor': ['1.3'],
'Chassis': ['1.3'],
'Port': ['1.8'],

2
ironic/conductor/manager.py
View File

@ -1200,7 +1200,7 @@ class ConductorManager(base_manager.BaseConductorManager):
LOG.debug('Starting %(type)s cleaning for node %(node)s',
{'type': clean_type, 'node': node.uuid})
if not manual_clean and not CONF.conductor.automated_clean:
if not manual_clean and utils.skip_automated_cleaning(node):
# Skip cleaning, move to AVAILABLE.
node.clean_step = None
node.save()

8
ironic/conductor/utils.py
View File

@ -990,3 +990,11 @@ def notify_conductor_resume_clean(task):
def notify_conductor_resume_deploy(task):
_notify_conductor_resume_operation(task, 'deploying',
'continue_node_deploy')
def skip_automated_cleaning(node):
"""Checks if node cleaning needs to be skipped for an specific node.
:param node: the node to consider
"""
return not CONF.conductor.automated_clean and not node.automated_clean

10
ironic/drivers/modules/oneview/deploy.py
View File

@ -22,6 +22,7 @@ import six
from ironic.common import exception
from ironic.common import states
from ironic.conductor import utils
from ironic.conf import CONF
from ironic.drivers.modules import agent
from ironic.drivers.modules import iscsi_deploy
@ -241,7 +242,10 @@ class OneViewIscsiDeploy(iscsi_deploy.ISCSIDeploy, OneViewPeriodicTasks):
@METRICS.timer('OneViewIscsiDeploy.tear_down')
def tear_down(self, task):
if not CONF.conductor.automated_clean:
# teardown if automated clean is disabled on the node
# or if general automated clean is not enabled generally
# and not on the node as well
if utils.skip_automated_cleaning(task.node):
deploy_utils.tear_down(task)
return super(OneViewIscsiDeploy, self).tear_down(task)
@ -285,7 +289,9 @@ class OneViewAgentDeploy(agent.AgentDeploy, OneViewPeriodicTasks):
@METRICS.timer('OneViewAgentDeploy.tear_down')
def tear_down(self, task):
if not CONF.conductor.automated_clean:
# if node specifically has cleanup disabled, or general cleanup
# is disabled and node has not it enabled
if utils.skip_automated_cleaning(task.node):
deploy_utils.tear_down(task)
return super(OneViewAgentDeploy, self).tear_down(task)

73
ironic/objects/node.py
View File

@ -64,7 +64,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
# Version 1.25: Add fault field
# Version 1.26: Add deploy_step field
# Version 1.27: Add conductor_group field
VERSION = '1.27'
# Version 1.28: Add automated_clean field
VERSION = '1.28'
dbapi = db_api.get_instance()
@ -130,7 +131,7 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
'inspection_started_at': object_fields.DateTimeField(nullable=True),
'extra': object_fields.FlexibleDictField(nullable=True),
'automated_clean': objects.fields.BooleanField(nullable=True),
'bios_interface': object_fields.StringField(nullable=True),
'boot_interface': object_fields.StringField(nullable=True),
'console_interface': object_fields.StringField(nullable=True),
@ -529,6 +530,26 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
elif self.conductor_group:
self.conductor_group = ''
# NOTE (yolanda): new method created to avoid repeating code in
# _convert_to_version, and to avoid pep8 too complex error
def _adjust_field_to_version(self, field_name, field_default_value,
target_version, major, minor,
remove_unavailable_fields):
field_is_set = self.obj_attr_is_set(field_name)
if target_version >= (major, minor):
# target version supports the major/minor specified
if not field_is_set:
# set it to its default value if it is not set
setattr(self, field_name, field_default_value)
elif field_is_set:
# target version does not support the field, and it is set
if remove_unavailable_fields:
# (De)serialising: remove unavailable fields
delattr(self, field_name)
elif getattr(self, field_name) is not field_default_value:
# DB: set unavailable field to the default value
setattr(self, field_name, field_default_value)
def _convert_to_version(self, target_version,
remove_unavailable_fields=True):
"""Convert to the target version.
@ -552,6 +573,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
this, it should be removed.
Version 1.27: conductor_group field was added. For versions prior to
this, it should be removed.
Version 1.28: automated_clean was added. For versions prior to this, it
should be set to None (or removed).
:param target_version: the desired version of the object
:param remove_unavailable_fields: True to remove fields that are
@ -560,47 +583,13 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
values; set this to False for DB interactions.
"""
target_version = versionutils.convert_version_to_tuple(target_version)
# Convert the rescue_interface field.
rescue_iface_is_set = self.obj_attr_is_set('rescue_interface')
if target_version >= (1, 22):
# Target version supports rescue_interface.
if not rescue_iface_is_set:
# Set it to its default value if it is not set.
self.rescue_interface = None
elif rescue_iface_is_set:
# Target version does not support rescue_interface, and it is set.
if remove_unavailable_fields:
# (De)serialising: remove unavailable fields.
delattr(self, 'rescue_interface')
elif self.rescue_interface is not None:
# DB: set unavailable field to the default of None.
self.rescue_interface = None
traits_is_set = self.obj_attr_is_set('traits')
if target_version >= (1, 23):
# Target version supports traits.
if not traits_is_set:
self.traits = None
elif traits_is_set:
if remove_unavailable_fields:
delattr(self, 'traits')
elif self.traits is not None:
self.traits = None
bios_iface_is_set = self.obj_attr_is_set('bios_interface')
if target_version >= (1, 24):
# Target version supports bios_interface.
if not bios_iface_is_set:
# Set it to its default value if it is not set.
self.bios_interface = None
elif bios_iface_is_set:
# Target version does not support bios_interface, and it is set.
if remove_unavailable_fields:
# (De)serialising: remove unavailable fields.
delattr(self, 'bios_interface')
elif self.bios_interface is not None:
# DB: set unavailable field to the default of None.
self.bios_interface = None
# Convert the different fields depending on version
fields = [('rescue_interface', 22), ('traits', 23),
('bios_interface', 24), ('automated_clean', 28)]
for name, minor in fields:
self._adjust_field_to_version(name, None, target_version,
1, minor, remove_unavailable_fields)
self._convert_fault_field(target_version, remove_unavailable_fields)
self._convert_deploy_step_field(target_version,

2
ironic/tests/unit/api/utils.py
View File

@ -112,6 +112,8 @@ def node_post_data(**kw):
node.pop('resource_class')
if 'fault' not in kw:
node.pop('fault')
if 'automated_clean' not in kw:
node.pop('automated_clean')
internal = node_controller.NodePatchType.internal_attrs()
return remove_internal(node, internal)

122
ironic/tests/unit/conductor/test_manager.py
View File

@ -3304,6 +3304,128 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.assertNotIn('clean_steps', node.driver_internal_info)
self.assertNotIn('clean_step_index', node.driver_internal_info)
@mock.patch('ironic.drivers.modules.fake.FakePower.validate',
autospec=True)
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
autospec=True)
def test__do_node_clean_automated_disabled_individual_enabled(
self, mock_network, mock_validate):
self.config(automated_clean=False, group='conductor')
self._start_service()
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.CLEANING,
target_provision_state=states.AVAILABLE,
last_error=None, automated_clean=True)
with task_manager.acquire(
self.context, node.uuid, shared=False) as task:
self.service._do_node_clean(task)
self._stop_service()
node.refresh()
# Assert that the node clean was called
self.assertTrue(mock_validate.called)
self.assertIn('clean_steps', node.driver_internal_info)
@mock.patch('ironic.drivers.modules.fake.FakePower.validate',
autospec=True)
def test__do_node_clean_automated_disabled_individual_disabled(
self, mock_validate):
self.config(automated_clean=False, group='conductor')
self._start_service()
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.CLEANING,
target_provision_state=states.AVAILABLE,
last_error=None, automated_clean=False)
with task_manager.acquire(
self.context, node.uuid, shared=False) as task:
self.service._do_node_clean(task)
self._stop_service()
node.refresh()
# Assert that the node was moved to available without cleaning
self.assertFalse(mock_validate.called)
self.assertEqual(states.AVAILABLE, node.provision_state)
self.assertEqual(states.NOSTATE, node.target_provision_state)
self.assertEqual({}, node.clean_step)
self.assertNotIn('clean_steps', node.driver_internal_info)
self.assertNotIn('clean_step_index', node.driver_internal_info)
@mock.patch('ironic.drivers.modules.fake.FakePower.validate',
autospec=True)
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
autospec=True)
def test__do_node_clean_automated_enabled(self, mock_validate,
mock_network):
self.config(automated_clean=True, group='conductor')
self._start_service()
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.CLEANING,
target_provision_state=states.AVAILABLE,
last_error=None)
with task_manager.acquire(
self.context, node.uuid, shared=False) as task:
self.service._do_node_clean(task)
self._stop_service()
node.refresh()
# Assert that the node was cleaned
self.assertTrue(mock_validate.called)
self.assertIn('clean_steps', node.driver_internal_info)
@mock.patch('ironic.drivers.modules.fake.FakePower.validate',
autospec=True)
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
autospec=True)
def test__do_node_clean_automated_enabled_individual_enabled(
self, mock_network, mock_validate):
self.config(automated_clean=True, group='conductor')
self._start_service()
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.CLEANING,
target_provision_state=states.AVAILABLE,
last_error=None, automated_clean=True)
with task_manager.acquire(
self.context, node.uuid, shared=False) as task:
self.service._do_node_clean(task)
self._stop_service()
node.refresh()
# Assert that the node was cleaned
self.assertTrue(mock_validate.called)
self.assertIn('clean_steps', node.driver_internal_info)
@mock.patch('ironic.drivers.modules.fake.FakePower.validate',
autospec=True)
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
autospec=True)
def test__do_node_clean_automated_enabled_individual_none(
self, mock_validate, mock_network):
self.config(automated_clean=True, group='conductor')
self._start_service()
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.CLEANING,
target_provision_state=states.AVAILABLE,
last_error=None, automated_clean=None)
with task_manager.acquire(
self.context, node.uuid, shared=False) as task:
self.service._do_node_clean(task)
self._stop_service()
node.refresh()
# Assert that the node was cleaned
self.assertTrue(mock_validate.called)
self.assertIn('clean_steps', node.driver_internal_info)
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
autospec=True)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.prepare_cleaning',

1
ironic/tests/unit/db/utils.py
View File

@ -214,6 +214,7 @@ def get_test_node(**kw):
'tags': kw.get('tags', []),
'resource_class': kw.get('resource_class'),
'traits': kw.get('traits', []),
'automated_clean': kw.get('automated_clean', None),
}
for iface in drivers_base.ALL_INTERFACES:

63
ironic/tests/unit/objects/test_node.py
View File

@ -706,6 +706,69 @@ class TestConvertToVersion(db_base.DbTestCase):
self.assertEqual('', node.conductor_group)
self.assertEqual({'conductor_group': ''}, node.obj_get_changes())
def test_automated_clean_supported_missing(self):
# automated_clean_interface not set, should be set to default.
node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
delattr(node, 'automated_clean')
node.obj_reset_changes()
node._convert_to_version("1.28")
self.assertIsNone(node.automated_clean)
self.assertEqual({'automated_clean': None},
node.obj_get_changes())
def test_automated_clean_supported_set(self):
# automated_clean set, no change required.
node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
node.automated_clean = True
node.obj_reset_changes()
node._convert_to_version("1.28")
self.assertEqual(True, node.automated_clean)
self.assertEqual({}, node.obj_get_changes())
def test_automated_clean_unsupported_missing(self):
# automated_clean not set, no change required.
node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
delattr(node, 'automated_clean')
node.obj_reset_changes()
node._convert_to_version("1.27")
self.assertNotIn('automated_clean', node)
self.assertEqual({}, node.obj_get_changes())
def test_automated_clean_unsupported_set_remove(self):
# automated_clean set, should be removed.
node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
node.automated_clean = True
node.obj_reset_changes()
node._convert_to_version("1.27")
self.assertNotIn('automated_clean', node)
self.assertEqual({}, node.obj_get_changes())
def test_automated_clean_unsupported_set_no_remove_non_default(self):
# automated_clean set, should be set to default.
node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
node.automated_clean = True
node.obj_reset_changes()
node._convert_to_version("1.27", False)
self.assertIsNone(node.automated_clean)
self.assertEqual({'automated_clean': None},
node.obj_get_changes())
def test_automated_clean_unsupported_set_no_remove_default(self):
# automated_clean set, no change required.
node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
node.automated_clean = None
node.obj_reset_changes()
node._convert_to_version("1.27", False)
self.assertIsNone(node.automated_clean)
self.assertEqual({}, node.obj_get_changes())
class TestNodePayloads(db_base.DbTestCase):

2
ironic/tests/unit/objects/test_objects.py
View File

@ -677,7 +677,7 @@ class TestObject(_LocalTest, _TestObject):
# version bump. It is an MD5 hash of the object fields and remotable methods.
# The fingerprint values should only be changed if there is a version bump.
expected_object_fingerprints = {
'Node': '1.27-129323d486c03a99de27053503b2cae3',
'Node': '1.28-d4aba1f583774326903f7366fbaae752',
'MyObj': '1.5-9459d30d6954bffc7a9afd347a807ca6',
'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905',
'Port': '1.8-898a47921f4a1f53fcdddd4eeb179e0b',