openstack
/
ironic
Code Issues Proposed changes

Merge "Rename NOSTATE to AVAILABLE"

This commit is contained in:
Jenkins 2015-01-30 21:06:10 +00:00 committed by Gerrit Code Review
parent f47c7e1156 e7958dee65
commit 84de1bbf21
14 changed files with 182 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
ironic/api/controllers/v1/__init__.py
View File

@ -38,7 +38,7 @@ from ironic.common.i18n import _
MIN_VER = 0
MAX_VER = 0
MAX_VER = 1
class MediaType(base.APIBase):

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

@ -53,6 +53,13 @@ LOG = log.getLogger(__name__)
_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 @@ class NodeStates(base.APIBase):
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 @@ class Node(base.APIBase):
@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)

5
ironic/common/fsm.py
View File

@ -144,9 +144,8 @@ class FSM(object):
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):

26
ironic/common/states.py
View File

@ -40,7 +40,14 @@ LOG = logging.getLogger(__name__)
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 @@ DELETING = 'deleting'
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 @@ watchers['on_enter'] = on_enter
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 @@ machine.add_state(DEPLOYFAIL, target=ACTIVE, **watchers)
# 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 @@ machine.add_transition(DEPLOYWAIT, DELETING, 'delete')
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')

6
ironic/conductor/manager.py
View File

@ -1481,12 +1481,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

7
ironic/conductor/task_manager.py
View File

@ -201,6 +201,13 @@ class TaskManager(object):
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:

4
ironic/db/api.py
View File

@ -135,8 +135,8 @@ class Connection(object):
{
'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': { ... },

52
ironic/db/sqlalchemy/alembic/versions/5674c57409b9_replace_nostate_with_available.py Normal file
View File

@ -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}))

9
ironic/db/sqlalchemy/api.py
View File

@ -253,12 +253,13 @@ class Connection(api.Connection):
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)

33
ironic/tests/api/v1/test_nodes.py
View File

@ -25,6 +25,7 @@ from six.moves.urllib import parse as urlparse
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 states
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 @@ from ironic.tests.objects import utils as obj_utils
# 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 @@ class TestListNodes(api_base.FunctionalTest):
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 @@ class TestListNodes(api_base.FunctionalTest):
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 @@ class TestPatch(api_base.FunctionalTest):
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 @@ class TestPost(api_base.FunctionalTest):
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 @@ class TestPost(api_base.FunctionalTest):
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 @@ class TestDelete(api_base.FunctionalTest):
topic='test-topic')
class TestPut(api_base.FunctionalTest):
class TestPut(test_api_base.FunctionalTest):
def setUp(self):
super(TestPut, self).setUp()

42
ironic/tests/conductor/test_manager.py
View File

@ -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 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
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 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
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 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
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 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
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 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
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 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
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 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
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 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
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 @@ class ManagerCheckDeployTimeoutsTestCase(_CommonMixIn,
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

32
ironic/tests/db/sqlalchemy/test_migrations.py
View File

@ -328,6 +328,38 @@ class MigrationCheckersMixin(object):
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')

4
ironic/tests/drivers/ilo/test_deploy.py
View File

@ -402,7 +402,7 @@ class VendorPassthruTestCase(db_base.DbTestCase):
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 @@ class VendorPassthruTestCase(db_base.DbTestCase):
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)

4
ironic/tests/drivers/test_pxe.py
View File

@ -677,7 +677,7 @@ class PXEDriverTestCase(db_base.DbTestCase):
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 @@ class PXEDriverTestCase(db_base.DbTestCase):
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)