openstack/ironic
Code Issues Proposed changes

Merge "Add conductor_group field to config, node and conductor objects"

Browse Source
This commit is contained in:
Zuul 2018-07-19 19:42:26 +00:00 committed by Gerrit Code Review
commit 3b7e7fb3fc
14 changed files with 222 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
ironic/api/controllers/v1/node.py
View File

@ -172,6 +172,9 @@ def hide_fields_in_newer_versions(obj):
if not api_utils.allow_bios_interface():
obj.bios_interface = wsme.Unset
# TODO(jroll) add a microversion here
obj.conductor_group = wsme.Unset
def update_state_in_older_versions(obj):
"""Change provision state names for API backwards compatibility.
@ -1074,6 +1077,9 @@ class Node(base.APIBase):
bios_interface = wsme.wsattr(wtypes.text)
"""The bios interface to be used for this node"""
conductor_group = wsme.wsattr(wtypes.text)
"""The conductor group to manage this node"""
# NOTE(deva): "conductor_affinity" shouldn't be presented on the
# API because it's an internal value. Don't add it here.

4
ironic/common/exception.py
View File

@ -254,6 +254,10 @@ class InvalidName(Invalid):
_msg_fmt = _("Expected a logical name but received %(name)s.")
class InvalidConductorGroup(Invalid):
_msg_fmt = _("Expected a conductor group but received %(group)s.")
class InvalidIdentity(Invalid):
_msg_fmt = _("Expected a UUID or int but received %(identity)s.")

4
ironic/common/release_mappings.py
View File

@ -118,8 +118,8 @@ RELEASE_MAPPING = {
'api': '1.45',
'rpc': '1.46',
'objects': {
'Node': ['1.26'],
'Conductor': ['1.2'],
'Node': ['1.26', '1.27'],
'Conductor': ['1.3'],
'Chassis': ['1.3'],
'Port': ['1.8'],
'Portgroup': ['1.4'],

7
ironic/common/utils.py
View File

@ -547,3 +547,10 @@ def parse_instance_info_capabilities(node):
parse_error()
return capabilities
def validate_conductor_group(conductor_group):
if not isinstance(conductor_group, six.string_types):
raise exception.InvalidConductorGroup(group=conductor_group)
if not re.match(r'^[a-zA-Z0-9_\-\.]*$', conductor_group):
raise exception.InvalidConductorGroup(group=conductor_group)

5
ironic/conductor/base_manager.py
View File

@ -158,7 +158,8 @@ class BaseConductorManager(object):
try:
# Register this conductor with the cluster
self.conductor = objects.Conductor.register(
admin_context, self.host, hardware_type_names)
admin_context, self.host, hardware_type_names,
CONF.conductor.conductor_group)
except exception.ConductorAlreadyRegistered:
# This conductor was already registered and did not shut down
# properly, so log a warning and update the record.
@ -167,7 +168,7 @@ class BaseConductorManager(object):
{'hostname': self.host})
self.conductor = objects.Conductor.register(
admin_context, self.host, hardware_type_names,
update_existing=True)
CONF.conductor.conductor_group, update_existing=True)
# register hardware types and interfaces supported by this conductor
# and validate them against other conductors

6
ironic/conf/conductor.py
View File

@ -177,6 +177,12 @@ opts = [
'automatically moved out of maintenance mode once its '
'power state is retrieved successfully. Set to 0 to '
'disable this check.')),
cfg.StrOpt('conductor_group',
default='',
help=_('Name of the conductor group to join. Can be up to '
'255 characters and is case insensitive. This '
'conductor will only manage nodes with a matching '
'"conductor_group" field set on the node.')),
]

12
ironic/objects/conductor.py
View File

@ -17,6 +17,7 @@
from oslo_versionedobjects import base as object_base
from ironic.common.i18n import _
from ironic.common import utils
from ironic.db import api as db_api
from ironic.objects import base
from ironic.objects import fields as object_fields
@ -29,7 +30,8 @@ class Conductor(base.IronicObject, object_base.VersionedObjectDictCompat):
# to touch() optional.
# Version 1.2: Add register_hardware_interfaces() and
# unregister_all_hardware_interfaces()
VERSION = '1.2'
# Version 1.3: Add conductor_group field.
VERSION = '1.3'
dbapi = db_api.get_instance()
@ -37,6 +39,7 @@ class Conductor(base.IronicObject, object_base.VersionedObjectDictCompat):
'id': object_fields.IntegerField(),
'drivers': object_fields.ListOfStringsField(nullable=True),
'hostname': object_fields.StringField(),
'conductor_group': object_fields.StringField(),
}
# NOTE(xek): We don't want to enable RPC on this call just yet. Remotable
@ -95,13 +98,16 @@ class Conductor(base.IronicObject, object_base.VersionedObjectDictCompat):
# Implications of calling new remote procedures should be thought through.
# @object_base.remotable
@classmethod
def register(cls, context, hostname, drivers, update_existing=False):
def register(cls, context, hostname, drivers, conductor_group,
update_existing=False):
"""Register an active conductor with the cluster.
:param cls: the :class:`Conductor`
:param context: Security context
:param hostname: the hostname on which the conductor will run
:param drivers: the list of drivers enabled in the conductor
:param conductor_group: conductor group to join, used for
node:conductor affinity.
:param update_existing: When false, registration will raise an
exception when a conflicting online record
is found. When true, will overwrite the
@ -110,9 +116,11 @@ class Conductor(base.IronicObject, object_base.VersionedObjectDictCompat):
:returns: a :class:`Conductor` object.
"""
utils.validate_conductor_group(conductor_group)
db_cond = cls.dbapi.register_conductor(
{'hostname': hostname,
'drivers': drivers,
'conductor_group': conductor_group.lower(),
'version': cls.get_target_version()},
update_existing=update_existing)
return cls._from_db_object(context, cls(), db_cond)

36
ironic/objects/node.py
View File

@ -20,6 +20,7 @@ from oslo_versionedobjects import base as object_base
from ironic.common import exception
from ironic.common.i18n import _
from ironic.common import utils
from ironic.db import api as db_api
from ironic import objects
from ironic.objects import base
@ -62,7 +63,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
# Version 1.24: Add bios_interface field
# Version 1.25: Add fault field
# Version 1.26: Add deploy_step field
VERSION = '1.26'
# Version 1.27: Add conductor_group field
VERSION = '1.27'
dbapi = db_api.get_instance()
@ -98,6 +100,7 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
# that has most recently performed some action which could require
# local state to be maintained (eg, built a PXE config)
'conductor_affinity': object_fields.IntegerField(nullable=True),
'conductor_group': object_fields.StringField(nullable=True),
# One of states.POWER_ON|POWER_OFF|NOSTATE|ERROR
'power_state': object_fields.StringField(nullable=True),
@ -361,6 +364,7 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
values = self.do_version_changes_for_db()
self._validate_property_values(values.get('properties'))
self._validate_and_remove_traits(values)
self._validate_and_format_conductor_group(values)
db_node = self.dbapi.create_node(values)
self._from_db_object(self._context, self, db_node)
@ -408,6 +412,7 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
self.driver_internal_info = {}
updates = self.do_version_changes_for_db()
self._validate_and_remove_traits(updates)
self._validate_and_format_conductor_group(updates)
db_node = self.dbapi.update_node(self.uuid, updates)
self._from_db_object(self._context, self, db_node)
@ -431,6 +436,18 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
raise exception.BadRequest()
fields.pop('traits')
def _validate_and_format_conductor_group(self, fields):
"""Validate conductor_group and format it for our use.
Currently formatting is just lowercasing it.
:param fields: a dict of Node fields for create or update.
:raises: InvalidConductorGroup if validation fails.
"""
if 'conductor_group' in fields:
utils.validate_conductor_group(fields['conductor_group'])
fields['conductor_group'] = fields['conductor_group'].lower()
# NOTE(xek): We don't want to enable RPC on this call just yet. Remotable
# methods can be used in the future to replace current explicit RPC calls.
# Implications of calling new remote procedures should be thought through.
@ -499,6 +516,19 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
elif self.deploy_step:
self.deploy_step = {}
def _convert_conductor_group_field(self, target_version,
remove_unavailable_fields=True):
# NOTE(jroll): The default conductor_group is "", not None
is_set = self.obj_attr_is_set('conductor_group')
if target_version >= (1, 27):
if not is_set:
self.conductor_group = ''
elif is_set:
if remove_unavailable_fields:
delattr(self, 'conductor_group')
elif self.conductor_group:
self.conductor_group = ''
def _convert_to_version(self, target_version,
remove_unavailable_fields=True):
"""Convert to the target version.
@ -520,6 +550,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
this, it should be removed.
Version 1.26: deploy_step field was added. For versions prior to
this, it should be removed.
Version 1.27: conductor_group field was added. For versions prior to
this, it should be removed.
:param target_version: the desired version of the object
:param remove_unavailable_fields: True to remove fields that are
@ -573,6 +605,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
self._convert_fault_field(target_version, remove_unavailable_fields)
self._convert_deploy_step_field(target_version,
remove_unavailable_fields)
self._convert_conductor_group_field(target_version,
remove_unavailable_fields)
@base.IronicObjectRegistry.register

21
ironic/tests/unit/common/test_utils.py
View File

@ -593,3 +593,24 @@ class JinjaTemplatingTestCase(base.TestCase):
utils.render_template(path,
self.params))
jinja_fsl_mock.assert_called_once_with('/path/to')
class ValidateConductorGroupTestCase(base.TestCase):
def test_validate_conductor_group_success(self):
self.assertIsNone(utils.validate_conductor_group('foo'))
self.assertIsNone(utils.validate_conductor_group('group1'))
self.assertIsNone(utils.validate_conductor_group('group1.with.dot'))
self.assertIsNone(utils.validate_conductor_group('group1_with_under'))
self.assertIsNone(utils.validate_conductor_group('group1-with-dash'))
def test_validate_conductor_group_fail(self):
self.assertRaises(exception.InvalidConductorGroup,
utils.validate_conductor_group, 'foo:bar')
self.assertRaises(exception.InvalidConductorGroup,
utils.validate_conductor_group, 'foo*bar')
self.assertRaises(exception.InvalidConductorGroup,
utils.validate_conductor_group, 'foo$bar')
self.assertRaises(exception.InvalidConductorGroup,
utils.validate_conductor_group, object())
self.assertRaises(exception.InvalidConductorGroup,
utils.validate_conductor_group, None)

6
ironic/tests/unit/db/test_api.py
View File

@ -10,6 +10,8 @@
# License for the specific language governing permissions and limitations
# under the License.
import random
import mock
from oslo_db.sqlalchemy import utils as db_utils
from oslo_utils import uuidutils
@ -61,8 +63,10 @@ class UpgradingTestCase(base.DbTestCase):
def test_check_versions_conductor(self):
for v in self.object_versions['Conductor']:
# NOTE(jroll) conductor model doesn't have a uuid :(
conductor = utils.create_test_conductor(
uuid=uuidutils.generate_uuid(), version=v)
hostname=uuidutils.generate_uuid(), version=v,
id=random.randint(1, 1000000))
conductor = self.dbapi.get_conductor(conductor.hostname)
self.assertEqual(v, conductor.version)
self.assertTrue(self.dbapi.check_versions())

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

@ -182,6 +182,7 @@ def get_test_node(**kw):
'uuid': kw.get('uuid', '1be26c0b-03f2-4d2e-ae87-c02d7f33c123'),
'chassis_id': kw.get('chassis_id', None),
'conductor_affinity': kw.get('conductor_affinity', None),
'conductor_group': kw.get('conductor_group', ''),
'power_state': kw.get('power_state', states.NOSTATE),
'target_power_state': kw.get('target_power_state', states.NOSTATE),
'provision_state': kw.get('provision_state', states.AVAILABLE),
@ -381,6 +382,7 @@ def get_test_conductor(**kw):
'version': kw.get('version', conductor.Conductor.VERSION),
'hostname': kw.get('hostname', 'test-conductor-node'),
'drivers': kw.get('drivers', ['fake-driver', 'null-driver']),
'conductor_group': kw.get('conductor_group', ''),
'created_at': kw.get('created_at', timeutils.utcnow()),
'updated_at': kw.get('updated_at', timeutils.utcnow()),
}

25
ironic/tests/unit/objects/test_conductor.py
View File

@ -20,6 +20,7 @@ import types
import mock
from oslo_utils import timeutils
from ironic.common import exception
from ironic import objects
from ironic.objects import base
from ironic.objects import fields
@ -90,7 +91,8 @@ class TestConductorObject(db_base.DbTestCase):
@mock.patch.object(base.IronicObject, 'get_target_version',
spec_set=types.FunctionType)
def _test_register(self, mock_target_version, update_existing=False):
def _test_register(self, mock_target_version, update_existing=False,
conductor_group=''):
mock_target_version.return_value = '1.5'
host = self.fake_conductor['hostname']
drivers = self.fake_conductor['drivers']
@ -98,12 +100,14 @@ class TestConductorObject(db_base.DbTestCase):
autospec=True) as mock_register_cdr:
mock_register_cdr.return_value = self.fake_conductor
c = objects.Conductor.register(self.context, host, drivers,
conductor_group,
update_existing=update_existing)
self.assertIsInstance(c, objects.Conductor)
mock_register_cdr.assert_called_once_with(
{'drivers': drivers,
'hostname': host,
'conductor_group': conductor_group.lower(),
'version': '1.5'},
update_existing=update_existing)
@ -113,6 +117,25 @@ class TestConductorObject(db_base.DbTestCase):
def test_register_update_existing_true(self):
self._test_register(update_existing=True)
def test_register_into_group(self):
self._test_register(conductor_group='dc1')
def test_register_into_group_uppercased(self):
self._test_register(conductor_group='DC1')
def test_register_into_group_with_update(self):
self._test_register(conductor_group='dc1', update_existing=True)
@mock.patch.object(base.IronicObject, 'get_target_version',
spec_set=types.FunctionType)
def test_register_with_invalid_group(self, mock_target_version):
mock_target_version.return_value = '1.5'
host = self.fake_conductor['hostname']
drivers = self.fake_conductor['drivers']
self.assertRaises(exception.InvalidConductorGroup,
objects.Conductor.register,
self.context, host, drivers, 'invalid:group')
@mock.patch.object(objects.Conductor, 'unregister_all_hardware_interfaces',
autospec=True)
def test_unregister(self, mock_unreg_ifaces):

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

@ -216,6 +216,51 @@ class TestNodeObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn):
self.assertRaises(exception.BadRequest, n.save)
self.assertFalse(mock_update_node.called)
def test_save_with_conductor_group(self):
uuid = self.fake_node['uuid']
with mock.patch.object(self.dbapi, 'get_node_by_uuid',
autospec=True) as mock_get_node:
mock_get_node.return_value = self.fake_node
with mock.patch.object(self.dbapi, 'update_node',
autospec=True) as mock_update_node:
mock_update_node.return_value = (
db_utils.get_test_node(conductor_group='group1'))
n = objects.Node.get(self.context, uuid)
n.conductor_group = 'group1'
n.save()
self.assertTrue(mock_update_node.called)
mock_update_node.assert_called_once_with(
uuid, {'conductor_group': 'group1',
'version': objects.Node.VERSION})
def test_save_with_conductor_group_uppercase(self):
uuid = self.fake_node['uuid']
with mock.patch.object(self.dbapi, 'get_node_by_uuid',
autospec=True) as mock_get_node:
mock_get_node.return_value = self.fake_node
with mock.patch.object(self.dbapi, 'update_node',
autospec=True) as mock_update_node:
mock_update_node.return_value = (
db_utils.get_test_node(conductor_group='group1'))
n = objects.Node.get(self.context, uuid)
n.conductor_group = 'GROUP1'
n.save()
mock_update_node.assert_called_once_with(
uuid, {'conductor_group': 'group1',
'version': objects.Node.VERSION})
def test_save_with_conductor_group_fail(self):
uuid = self.fake_node['uuid']
with mock.patch.object(self.dbapi, 'get_node_by_uuid',
autospec=True) as mock_get_node:
mock_get_node.return_value = self.fake_node
with mock.patch.object(self.dbapi, 'update_node',
autospec=True) as mock_update_node:
n = objects.Node.get(self.context, uuid)
n.conductor_group = 'group:1'
self.assertRaises(exception.InvalidConductorGroup, n.save)
self.assertFalse(mock_update_node.called)
def test_refresh(self):
uuid = self.fake_node['uuid']
returns = [dict(self.fake_node, properties={"fake": "first"}),
@ -611,6 +656,56 @@ class TestConvertToVersion(db_base.DbTestCase):
self.assertIsNone(node.fault)
self.assertEqual({'fault': None}, node.obj_get_changes())
def test_conductor_group_supported_set(self):
node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
node.conductor_group = 'group1'
node.obj_reset_changes()
node._convert_to_version('1.27')
self.assertEqual('group1', node.conductor_group)
self.assertEqual({}, node.obj_get_changes())
def test_conductor_group_supported_unset(self):
node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
delattr(node, 'conductor_group')
node.obj_reset_changes()
node._convert_to_version('1.27')
self.assertEqual('', node.conductor_group)
self.assertEqual({'conductor_group': ''}, node.obj_get_changes())
def test_conductor_group_unsupported_set(self):
node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
node.conductor_group = 'group1'
node.obj_reset_changes()
node._convert_to_version('1.26')
self.assertNotIn('conductor_group', node)
self.assertEqual({}, node.obj_get_changes())
def test_conductor_group_unsupported_unset(self):
node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
delattr(node, 'conductor_group')
node.obj_reset_changes()
node._convert_to_version('1.26')
self.assertNotIn('conductor_group', node)
self.assertEqual({}, node.obj_get_changes())
def test_conductor_group_unsupported_set_no_remove(self):
node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
node.conductor_group = 'group1'
node.obj_reset_changes()
node._convert_to_version('1.26', remove_unavailable_fields=False)
self.assertEqual('', node.conductor_group)
self.assertEqual({'conductor_group': ''}, node.obj_get_changes())
class TestNodePayloads(db_base.DbTestCase):

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

@ -664,12 +664,12 @@ 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.26-31732244b5bc3f8c334f77c03449f4c6',
'Node': '1.27-129323d486c03a99de27053503b2cae3',
'MyObj': '1.5-9459d30d6954bffc7a9afd347a807ca6',
'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905',
'Port': '1.8-898a47921f4a1f53fcdddd4eeb179e0b',
'Portgroup': '1.4-71923a81a86743b313b190f5c675e258',
'Conductor': '1.2-5091f249719d4a465062a1b3dc7f860d',
'Conductor': '1.3-d3f53e853b4d58cae5bfbd9a8341af4a',
'EventType': '1.1-aa2ba1afd38553e3880c267404e8d370',
'NotificationPublisher': '1.0-51a09397d6c0687771fb5be9a999605d',
'NodePayload': '1.9-c0aa5dd602adca3a28f091ca7848a41b',