diff --git a/ironic/api/controllers/v1/__init__.py b/ironic/api/controllers/v1/__init__.py index 81081aad65..8871b29ec7 100644 --- a/ironic/api/controllers/v1/__init__.py +++ b/ironic/api/controllers/v1/__init__.py @@ -38,7 +38,7 @@ MIN_VER = 0 -MAX_VER = 0 +MAX_VER = 1 class MediaType(base.APIBase): diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 0e4359708a..bd40551bef 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -53,6 +53,13 @@ _VENDOR_METHODS = {} +def assert_juno_provision_state_name(obj): + # if requested version is < 1.1, convert AVAILABLE to the old NOSTATE + if (pecan.request.version.minor < 1 and + obj.provision_state == ir_states.AVAILABLE): + obj.provision_state = ir_states.NOSTATE + + class NodePatchType(types.JsonPatchType): @staticmethod @@ -240,6 +247,7 @@ def convert(rpc_node): states = NodeStates() for attr in attr_list: setattr(states, attr, getattr(rpc_node, attr)) + assert_juno_provision_state_name(states) return states @classmethod @@ -520,6 +528,7 @@ def _convert_with_links(node, url, expand=True): @classmethod def convert_with_links(cls, rpc_node, expand=True): node = Node(**rpc_node.as_dict()) + assert_juno_provision_state_name(node) return cls._convert_with_links(node, pecan.request.host_url, expand) diff --git a/ironic/common/fsm.py b/ironic/common/fsm.py index ec45a2eaaf..f314054962 100644 --- a/ironic/common/fsm.py +++ b/ironic/common/fsm.py @@ -144,9 +144,8 @@ def process_event(self, event): if (self._target_state is not None and self._target_state == replacement.name): self._target_state = None - # set target if there is a new one - if (self._target_state is None and - self._states[replacement.name]['target'] is not None): + # if new state has a different target, update the target + if self._states[replacement.name]['target'] is not None: self._target_state = self._states[replacement.name]['target'] def is_valid_event(self, event): diff --git a/ironic/common/states.py b/ironic/common/states.py index 9bcdfb90ff..9ef7edc534 100644 --- a/ironic/common/states.py +++ b/ironic/common/states.py @@ -40,7 +40,14 @@ NOSTATE = None """ No state information. -Default for the power and provision state of newly created nodes. +This state is used with power_state to represent a lack of knowledge of +power state, and in target_*_state fields when there is no target. +""" + +AVAILABLE = 'available' +""" Node is available for use and scheduling. + +This state is replacing the NOSTATE state used prior to Kilo. """ ACTIVE = 'active' @@ -78,8 +85,10 @@ DELETED = 'deleted' """ Node tear down was successful. -This is mainly a target provision state used during node tear down. A -successful tear down leaves the node with a `provision_state` of NOSTATE. +In Juno, target_provision_state was set to this value during node tear down. + +In Kilo, this will be a transitory value of provision_state, and never +represented in target_provision_state. """ ERROR = 'error' @@ -131,7 +140,7 @@ def on_enter(new_state, event): machine = fsm.FSM() # Add stable states -machine.add_state(NOSTATE, **watchers) +machine.add_state(AVAILABLE, **watchers) machine.add_state(ACTIVE, **watchers) machine.add_state(ERROR, **watchers) @@ -145,11 +154,10 @@ def on_enter(new_state, event): # Add delete* states # NOTE(deva): Juno shows a target_provision_state of DELETED # this is changed in Kilo to AVAILABLE -# TODO(deva): change NOSTATE to AVAILABLE here -machine.add_state(DELETING, target=NOSTATE, **watchers) +machine.add_state(DELETING, target=AVAILABLE, **watchers) -# From NOSTATE, a deployment may be started -machine.add_transition(NOSTATE, DEPLOYING, 'deploy') +# From AVAILABLE, a deployment may be started +machine.add_transition(AVAILABLE, DEPLOYING, 'deploy') # A deployment may fail machine.add_transition(DEPLOYING, DEPLOYFAIL, 'fail') @@ -185,7 +193,7 @@ def on_enter(new_state, event): machine.add_transition(DEPLOYFAIL, DELETING, 'delete') # A delete may complete -machine.add_transition(DELETING, NOSTATE, 'done') +machine.add_transition(DELETING, AVAILABLE, 'done') # This state can also transition to error machine.add_transition(DELETING, ERROR, 'error') diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 67c3c0a98f..821e98b2ee 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -1480,12 +1480,6 @@ def do_node_tear_down(task): LOG.info(_LI('Successfully unprovisioned node %(node)s with ' 'instance %(instance)s.'), {'node': node.uuid, 'instance': node.instance_uuid}) - # NOTE(deva): Currently, NOSTATE is represented as None - # However, FSM class treats a target_state of None as - # the lack of a target state -- not a target of NOSTATE - # Thus, until we migrate to an explicit AVAILABLE state - # we need to clear the target_state here manually. - node.target_provision_state = None finally: # NOTE(deva): there is no need to unset conductor_affinity # because it is a reference to the most recent conductor which diff --git a/ironic/conductor/task_manager.py b/ironic/conductor/task_manager.py index 09a15063df..cd0ac4d14a 100644 --- a/ironic/conductor/task_manager.py +++ b/ironic/conductor/task_manager.py @@ -201,6 +201,13 @@ def reserve_node(): self.ports = objects.Port.list_by_node_id(context, self.node.id) self.driver = driver_factory.get_driver(driver_name or self.node.driver) + + # NOTE(deva): this handles the Juno-era NOSTATE state + # and should be deleted after Kilo is released + if self.node.provision_state is states.NOSTATE: + self.node.provision_state = states.AVAILABLE + self.node.save() + self.fsm.initialize(self.node.provision_state) except Exception: diff --git a/ironic/db/api.py b/ironic/db/api.py index 6bdcd89e47..e535b262f9 100644 --- a/ironic/db/api.py +++ b/ironic/db/api.py @@ -135,8 +135,8 @@ def create_node(self, values): { 'uuid': utils.generate_uuid(), 'instance_uuid': None, - 'power_state': states.NOSTATE, - 'provision_state': states.NOSTATE, + 'power_state': states.POWER_OFF, + 'provision_state': states.AVAILABLE, 'driver': 'pxe_ipmitool', 'driver_info': { ... }, 'properties': { ... }, diff --git a/ironic/db/sqlalchemy/alembic/versions/5674c57409b9_replace_nostate_with_available.py b/ironic/db/sqlalchemy/alembic/versions/5674c57409b9_replace_nostate_with_available.py new file mode 100644 index 0000000000..db311465c7 --- /dev/null +++ b/ironic/db/sqlalchemy/alembic/versions/5674c57409b9_replace_nostate_with_available.py @@ -0,0 +1,52 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""replace NOSTATE with AVAILABLE + +Revision ID: 5674c57409b9 +Revises: 242cc6a923b3 +Create Date: 2015-01-14 16:55:44.718196 + +""" + +# revision identifiers, used by Alembic. +revision = '5674c57409b9' +down_revision = '242cc6a923b3' + +from alembic import op +from sqlalchemy import String +from sqlalchemy.sql import table, column + +node = table('nodes', + column('uuid', String(36)), + column('provision_state', String(15))) + + +# NOTE(deva): We must represent the states as static strings in this migration +# file, rather than import ironic.common.states, because that file may change +# in the future. This migration script must still be able to be run with +# future versions of the code and still produce the same results. +AVAILABLE = 'available' + + +def upgrade(): + op.execute( + node.update().where( + node.c.provision_state == None).values( + {'provision_state': op.inline_literal(AVAILABLE)})) + + +def downgrade(): + op.execute( + node.update().where( + node.c.provision_state == op.inline_literal(AVAILABLE)).values( + {'provision_state': None})) diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index fb55eaf210..4b624f5273 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -253,12 +253,13 @@ def release_node(self, tag, node_id): def create_node(self, values): # ensure defaults are present for new nodes - if not values.get('uuid'): + if 'uuid' not in values: values['uuid'] = utils.generate_uuid() - if not values.get('power_state'): + if 'power_state' not in values: values['power_state'] = states.NOSTATE - if not values.get('provision_state'): - values['provision_state'] = states.NOSTATE + if 'provision_state' not in values: + # TODO(deva): change this to ENROLL + values['provision_state'] = states.AVAILABLE node = models.Node() node.update(values) diff --git a/ironic/tests/api/v1/test_nodes.py b/ironic/tests/api/v1/test_nodes.py index 2bcdd1e8f5..c8c62ed485 100644 --- a/ironic/tests/api/v1/test_nodes.py +++ b/ironic/tests/api/v1/test_nodes.py @@ -25,6 +25,7 @@ from testtools.matchers import HasLength from wsme import types as wtypes +from ironic.api.controllers import base as api_base from ironic.api.controllers.v1 import node as api_node from ironic.common import boot_devices from ironic.common import exception @@ -32,8 +33,8 @@ from ironic.common import utils from ironic.conductor import rpcapi from ironic import objects -from ironic.tests.api import base as api_base -from ironic.tests.api import utils as apiutils +from ironic.tests.api import base as test_api_base +from ironic.tests.api import utils as test_api_utils from ironic.tests import base from ironic.tests.db import utils as dbutils from ironic.tests.objects import utils as obj_utils @@ -42,7 +43,7 @@ # NOTE(lucasagomes): When creating a node via API (POST) # we have to use chassis_uuid def post_get_test_node(**kw): - node = apiutils.node_post_data(**kw) + node = test_api_utils.node_post_data(**kw) chassis = dbutils.get_test_chassis() node['chassis_id'] = None node['chassis_uuid'] = kw.get('chassis_uuid', chassis['uuid']) @@ -52,13 +53,13 @@ def post_get_test_node(**kw): class TestNodeObject(base.TestCase): def test_node_init(self): - node_dict = apiutils.node_post_data(chassis_id=None) + node_dict = test_api_utils.node_post_data(chassis_id=None) del node_dict['instance_uuid'] node = api_node.Node(**node_dict) self.assertEqual(wtypes.Unset, node.instance_uuid) -class TestListNodes(api_base.FunctionalTest): +class TestListNodes(test_api_base.FunctionalTest): def setUp(self): super(TestListNodes, self).setUp() @@ -151,6 +152,18 @@ def test_detail_against_single(self): expect_errors=True) self.assertEqual(404, response.status_int) + def test_mask_available_state(self): + node = obj_utils.create_test_node(self.context, + provision_state=states.AVAILABLE) + + data = self.get_json('/nodes/%s' % node['uuid'], + headers={api_base.Version.string: "1.0"}) + self.assertEqual(states.NOSTATE, data['provision_state']) + + data = self.get_json('/nodes/%s' % node['uuid'], + headers={api_base.Version.string: "1.1"}) + self.assertEqual(states.AVAILABLE, data['provision_state']) + def test_many(self): nodes = [] for id in range(5): @@ -486,7 +499,7 @@ def test_get_supported_boot_devices_iface_not_supported(self, mock_gsbd): mock_gsbd.assert_called_once_with(mock.ANY, node.uuid, 'test-topic') -class TestPatch(api_base.FunctionalTest): +class TestPatch(test_api_base.FunctionalTest): def setUp(self): super(TestPatch, self).setUp() @@ -779,7 +792,7 @@ def test_replace_provision_updated_at(self): self.assertTrue(response.json['error_message']) -class TestPost(api_base.FunctionalTest): +class TestPost(test_api_base.FunctionalTest): def setUp(self): super(TestPost, self).setUp() @@ -927,7 +940,7 @@ def test_vendor_passthru_without_method(self): def test_post_ports_subresource(self): node = obj_utils.create_test_node(self.context) - pdict = apiutils.port_post_data(node_id=None) + pdict = test_api_utils.port_post_data(node_id=None) pdict['node_uuid'] = node.uuid response = self.post_json('/nodes/ports', pdict, expect_errors=True) @@ -1012,7 +1025,7 @@ def test_vendor_passthru_methods(self, get_methods_mock): self.assertFalse(get_methods_mock.called) -class TestDelete(api_base.FunctionalTest): +class TestDelete(test_api_base.FunctionalTest): def setUp(self): super(TestDelete, self).setUp() @@ -1073,7 +1086,7 @@ def test_delete_node_maintenance_mode(self, mock_update, mock_get): topic='test-topic') -class TestPut(api_base.FunctionalTest): +class TestPut(test_api_base.FunctionalTest): def setUp(self): super(TestPut, self).setUp() diff --git a/ironic/tests/conductor/test_manager.py b/ironic/tests/conductor/test_manager.py index cc8ad0a545..cbf2208781 100644 --- a/ironic/tests/conductor/test_manager.py +++ b/ironic/tests/conductor/test_manager.py @@ -874,7 +874,7 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase): def test_do_node_deploy_invalid_state(self): self._start_service() - # test node['provision_state'] is not NOSTATE + # test that node deploy fails if the node is already provisioned node = obj_utils.create_test_node(self.context, driver='fake', provision_state=states.ACTIVE, target_provision_state=states.NOSTATE) @@ -1053,6 +1053,23 @@ def test__do_node_deploy_ok_2(self, mock_deploy): self.assertIsNone(node.last_error) mock_deploy.assert_called_once_with(mock.ANY) + @mock.patch('ironic.conductor.task_manager.TaskManager.process_event') + def test_deploy_with_nostate_converts_to_available(self, mock_pe): + # expressly create a node using the Juno-era NOSTATE state + # and assert that it does not result in an error, and that the state + # is converted to the new AVAILABLE state. + # Mock the process_event call, because the transitions from + # AVAILABLE are tested thoroughly elsewhere + # NOTE(deva): This test can be deleted after Kilo is released + self._start_service() + node = obj_utils.create_test_node(self.context, driver='fake', + provision_state=states.NOSTATE) + self.assertEqual(states.NOSTATE, node.provision_state) + self.service.do_node_deploy(self.context, node.uuid) + self.assertTrue(mock_pe.called) + node.refresh() + self.assertEqual(states.AVAILABLE, node.provision_state) + def test_do_node_deploy_partial_ok(self): self._start_service() thread = self.service._spawn_worker(lambda: None) @@ -1060,7 +1077,7 @@ def test_do_node_deploy_partial_ok(self): mock_spawn.return_value = thread node = obj_utils.create_test_node(self.context, driver='fake', - provision_state=states.NOSTATE) + provision_state=states.AVAILABLE) self.service.do_node_deploy(self.context, node.uuid) self.service._worker_pool.waitall() @@ -1177,11 +1194,11 @@ def test_do_node_deploy_rebuild_error_state(self, mock_deploy): self.assertIsNone(node.reservation) mock_deploy.assert_called_once_with(mock.ANY) - def test_do_node_deploy_rebuild_nostate_state(self): + def test_do_node_deploy_rebuild_from_available_state(self): self._start_service() - # test node will not rebuild if state is NOSTATE + # test node will not rebuild if state is AVAILABLE node = obj_utils.create_test_node(self.context, driver='fake', - provision_state=states.NOSTATE) + provision_state=states.AVAILABLE) exc = self.assertRaises(messaging.rpc.ExpectedException, self.service.do_node_deploy, self.context, node['uuid'], rebuild=True) @@ -1210,7 +1227,7 @@ def test__check_deploy_timeouts(self, mock_cleanup): mock_cleanup.assert_called_once_with(mock.ANY) def test_do_node_deploy_worker_pool_full(self): - prv_state = states.NOSTATE + prv_state = states.AVAILABLE tgt_prv_state = states.NOSTATE node = obj_utils.create_test_node(self.context, provision_state=prv_state, @@ -1239,7 +1256,7 @@ def test_do_node_tear_down_invalid_state(self): self._start_service() # test node.provision_state is incorrect for tear_down node = obj_utils.create_test_node(self.context, driver='fake', - provision_state=states.NOSTATE) + provision_state=states.AVAILABLE) exc = self.assertRaises(messaging.rpc.ExpectedException, self.service.do_node_tear_down, self.context, node['uuid']) @@ -1274,7 +1291,7 @@ def test_do_node_tear_down_driver_raises_error(self, mock_tear_down): manager.do_node_tear_down, task) node.refresh() self.assertEqual(states.ERROR, node.provision_state) - self.assertEqual(states.NOSTATE, node.target_provision_state) + self.assertEqual(states.AVAILABLE, node.target_provision_state) self.assertIsNotNone(node.last_error) # Assert instance_info was erased self.assertEqual({}, node.instance_info) @@ -1293,10 +1310,7 @@ def test_do_node_tear_down_ok(self, mock_tear_down): mock_tear_down.return_value = states.DELETED manager.do_node_tear_down(task) node.refresh() - self.assertEqual(states.NOSTATE, node.provision_state) - # NOTE(deva): this only works because manager.do_node_tear_down() - # explicitly sets target_provision_state to NOSTATE. Until we introduce - # the AVAILABLE state, this override should remain. + self.assertEqual(states.AVAILABLE, node.provision_state) self.assertEqual(states.NOSTATE, node.target_provision_state) self.assertIsNone(node.last_error) self.assertEqual({}, node.instance_info) @@ -1313,7 +1327,7 @@ def _test_do_node_tear_down_from_state(self, init_state, mock_tear_down): self.service.do_node_tear_down(self.context, node.uuid) self.service._worker_pool.waitall() node.refresh() - self.assertEqual(states.NOSTATE, node.provision_state) + self.assertEqual(states.AVAILABLE, node.provision_state) self.assertEqual(states.NOSTATE, node.target_provision_state) self.assertIsNone(node.last_error) self.assertEqual({}, node.instance_info) @@ -2451,7 +2465,7 @@ def test_acquire_node_locked(self, get_nodeinfo_mock, mapped_mock, def test_no_deploywait_after_lock(self, get_nodeinfo_mock, mapped_mock, acquire_mock): task = self._create_task( - node_attrs=dict(provision_state=states.NOSTATE, + node_attrs=dict(provision_state=states.AVAILABLE, uuid=self.node.uuid)) get_nodeinfo_mock.return_value = self._get_nodeinfo_list_response() mapped_mock.return_value = True diff --git a/ironic/tests/db/sqlalchemy/test_migrations.py b/ironic/tests/db/sqlalchemy/test_migrations.py index edb6cb8add..577ed47b15 100644 --- a/ironic/tests/db/sqlalchemy/test_migrations.py +++ b/ironic/tests/db/sqlalchemy/test_migrations.py @@ -328,6 +328,38 @@ def _check_242cc6a923b3(self, engine, data): self.assertIsInstance(nodes.c.maintenance_reason.type, sqlalchemy.types.String) + def _pre_upgrade_5674c57409b9(self, engine): + # add some nodes in various states so we can assert that "None" + # was replaced by "available", and nothing else changed. + nodes = db_utils.get_table(engine, 'nodes') + data = [{'uuid': utils.generate_uuid(), + 'provision_state': 'fake state'}, + {'uuid': utils.generate_uuid(), + 'provision_state': 'active'}, + {'uuid': utils.generate_uuid(), + 'provision_state': 'deleting'}, + {'uuid': utils.generate_uuid(), + 'provision_state': None}] + nodes.insert().values(data).execute() + return data + + def _check_5674c57409b9(self, engine, data): + nodes = db_utils.get_table(engine, 'nodes') + result = engine.execute(nodes.select()) + + def _get_state(uuid): + for row in data: + if row['uuid'] == uuid: + return row['provision_state'] + + for row in result: + old = _get_state(row['uuid']) + new = row['provision_state'] + if old is None: + self.assertEqual('available', new) + else: + self.assertEqual(old, new) + def test_upgrade_and_version(self): with patch_with_engine(self.engine): self.migration_api.upgrade('head') diff --git a/ironic/tests/drivers/ilo/test_deploy.py b/ironic/tests/drivers/ilo/test_deploy.py index a519a559e0..807adf5e12 100644 --- a/ironic/tests/drivers/ilo/test_deploy.py +++ b/ironic/tests/drivers/ilo/test_deploy.py @@ -402,7 +402,7 @@ def test__continue_deploy_good(self, cleanup_vmedia_boot_mock, def test__continue_deploy_bad(self, cleanup_vmedia_boot_mock): kwargs = {'method': 'pass_deploy_info', 'address': '123456'} - self.node.provision_state = states.NOSTATE + self.node.provision_state = states.AVAILABLE self.node.target_provision_state = states.NOSTATE self.node.save() with task_manager.acquire(self.context, self.node.uuid, @@ -411,7 +411,7 @@ def test__continue_deploy_bad(self, cleanup_vmedia_boot_mock): self.assertRaises(exception.InvalidState, vendor._continue_deploy, task, **kwargs) - self.assertEqual(states.NOSTATE, task.node.provision_state) + self.assertEqual(states.AVAILABLE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) self.assertFalse(cleanup_vmedia_boot_mock.called) diff --git a/ironic/tests/drivers/test_pxe.py b/ironic/tests/drivers/test_pxe.py index 5bde1f0cd7..4abcb6da7e 100644 --- a/ironic/tests/drivers/test_pxe.py +++ b/ironic/tests/drivers/test_pxe.py @@ -677,7 +677,7 @@ def fake_deploy(**kwargs): def test_continue_deploy_invalid(self): self.node.power_state = states.POWER_ON - self.node.provision_state = states.NOSTATE + self.node.provision_state = states.AVAILABLE self.node.target_provision_state = states.NOSTATE self.node.save() @@ -688,7 +688,7 @@ def test_continue_deploy_invalid(self): key='fake-56789', error='test ramdisk error') self.node.refresh() - self.assertEqual(states.NOSTATE, self.node.provision_state) + self.assertEqual(states.AVAILABLE, self.node.provision_state) self.assertEqual(states.NOSTATE, self.node.target_provision_state) self.assertEqual(states.POWER_ON, self.node.power_state)