Clean up the edge cases around states
Currently show_instance returns an Instance even if the requested node is not actually an instance (e.g. just an available node). This change corrects it. Make list_instances consistent with it. Also make the states a proper enum to avoid consumers from using invalid values (I did it several times when working on this patch). Change-Id: If9aad0d7f4d10a7119d1f0bccc1cc32a918a72e3
This commit is contained in:
parent
a57d584935
commit
02932096df
|
@ -1,5 +1,6 @@
|
||||||
coverage==4.0
|
coverage==4.0
|
||||||
doc8==0.6.0
|
doc8==0.6.0
|
||||||
|
enum34==1.0.4
|
||||||
fixtures==3.0.0
|
fixtures==3.0.0
|
||||||
flake8-import-order==0.13
|
flake8-import-order==0.13
|
||||||
hacking==1.0.0
|
hacking==1.0.0
|
||||||
|
|
|
@ -15,6 +15,7 @@
|
||||||
|
|
||||||
from metalsmith._config import InstanceConfig
|
from metalsmith._config import InstanceConfig
|
||||||
from metalsmith._instance import Instance
|
from metalsmith._instance import Instance
|
||||||
|
from metalsmith._instance import InstanceState
|
||||||
from metalsmith._provisioner import Provisioner
|
from metalsmith._provisioner import Provisioner
|
||||||
|
|
||||||
__all__ = ['Instance', 'InstanceConfig', 'Provisioner']
|
__all__ = ['Instance', 'InstanceConfig', 'InstanceState', 'Provisioner']
|
||||||
|
|
|
@ -57,7 +57,8 @@ class DefaultFormat(object):
|
||||||
def show(self, instances):
|
def show(self, instances):
|
||||||
for instance in instances:
|
for instance in instances:
|
||||||
_print("Node %(node)s, current state is %(state)s",
|
_print("Node %(node)s, current state is %(state)s",
|
||||||
node=_utils.log_res(instance.node), state=instance.state)
|
node=_utils.log_res(instance.node),
|
||||||
|
state=instance.state.name)
|
||||||
|
|
||||||
if instance.is_deployed:
|
if instance.is_deployed:
|
||||||
ips = instance.ip_addresses()
|
ips = instance.ip_addresses()
|
||||||
|
|
|
@ -13,18 +13,74 @@
|
||||||
# See the License for the specific language governing permissions and
|
# See the License for the specific language governing permissions and
|
||||||
# limitations under the License.
|
# limitations under the License.
|
||||||
|
|
||||||
|
import enum
|
||||||
|
import warnings
|
||||||
|
|
||||||
from metalsmith import _utils
|
from metalsmith import _utils
|
||||||
|
|
||||||
|
|
||||||
_PROGRESS_STATES = frozenset(['deploying', 'wait call-back',
|
_PROGRESS_STATES = frozenset(['deploying', 'wait call-back',
|
||||||
'deploy complete'])
|
'deploy complete'])
|
||||||
# NOTE(dtantsur): include available since there is a period of time between
|
|
||||||
# claiming the instance and starting the actual provisioning via ironic.
|
|
||||||
_DEPLOYING_STATES = _PROGRESS_STATES | {'available'}
|
|
||||||
_ACTIVE_STATES = frozenset(['active'])
|
_ACTIVE_STATES = frozenset(['active'])
|
||||||
_ERROR_STATES = frozenset(['error', 'deploy failed'])
|
_ERROR_STATES = frozenset(['error', 'deploy failed'])
|
||||||
|
_RESERVED_STATES = frozenset(['available'])
|
||||||
|
|
||||||
_HEALTHY_STATES = _PROGRESS_STATES | _ACTIVE_STATES
|
|
||||||
|
class InstanceState(enum.Enum):
|
||||||
|
"""A state of an instance."""
|
||||||
|
|
||||||
|
DEPLOYING = 'deploying'
|
||||||
|
"""Provisioning is in progress.
|
||||||
|
|
||||||
|
This includes the case when a node is still in the ``available`` state, but
|
||||||
|
already has an instance associated with it.
|
||||||
|
"""
|
||||||
|
|
||||||
|
ACTIVE = 'active'
|
||||||
|
"""The instance is provisioned."""
|
||||||
|
|
||||||
|
MAINTENANCE = 'maintenance'
|
||||||
|
"""The instance is provisioned but is in the maintenance mode."""
|
||||||
|
|
||||||
|
ERROR = 'error'
|
||||||
|
"""The instance has a failure."""
|
||||||
|
|
||||||
|
UNKNOWN = 'unknown'
|
||||||
|
"""The node is in an unexpected state.
|
||||||
|
|
||||||
|
It can be unprovisioned or modified by a third party.
|
||||||
|
"""
|
||||||
|
|
||||||
|
@property
|
||||||
|
def is_deployed(self):
|
||||||
|
"""Whether the state designates a finished deployment."""
|
||||||
|
return self in _DEPLOYED_STATES
|
||||||
|
|
||||||
|
@property
|
||||||
|
def is_healthy(self):
|
||||||
|
"""Whether the state is considered healthy."""
|
||||||
|
return self in _HEALTHY_STATES
|
||||||
|
|
||||||
|
# TODO(dtantsur): remove before 1.0
|
||||||
|
|
||||||
|
def __eq__(self, other):
|
||||||
|
if isinstance(other, str):
|
||||||
|
warnings.warn("Comparing states with strings is deprecated, "
|
||||||
|
"use InstanceState instead", DeprecationWarning)
|
||||||
|
return self.value == other
|
||||||
|
else:
|
||||||
|
return super(InstanceState, self).__eq__(other)
|
||||||
|
|
||||||
|
def __ne__(self, other):
|
||||||
|
eq = self.__eq__(other)
|
||||||
|
return NotImplemented if eq is NotImplemented else not eq
|
||||||
|
|
||||||
|
def __hash__(self):
|
||||||
|
return hash(self.value)
|
||||||
|
|
||||||
|
|
||||||
|
_HEALTHY_STATES = frozenset([InstanceState.ACTIVE, InstanceState.DEPLOYING])
|
||||||
|
_DEPLOYED_STATES = frozenset([InstanceState.ACTIVE, InstanceState.MAINTENANCE])
|
||||||
|
|
||||||
|
|
||||||
class Instance(object):
|
class Instance(object):
|
||||||
|
@ -57,16 +113,12 @@ class Instance(object):
|
||||||
@property
|
@property
|
||||||
def is_deployed(self):
|
def is_deployed(self):
|
||||||
"""Whether the node is deployed."""
|
"""Whether the node is deployed."""
|
||||||
return self._node.provision_state in _ACTIVE_STATES
|
return self.state.is_deployed
|
||||||
|
|
||||||
@property
|
|
||||||
def _is_deployed_by_metalsmith(self):
|
|
||||||
return _utils.GetNodeMixin.HOSTNAME_FIELD in self._node.instance_info
|
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def is_healthy(self):
|
def is_healthy(self):
|
||||||
"""Whether the node is not at fault or maintenance."""
|
"""Whether the instance is not at fault or maintenance."""
|
||||||
return self.state in _HEALTHY_STATES and not self._node.is_maintenance
|
return self.state.is_healthy and not self._node.is_maintenance
|
||||||
|
|
||||||
def nics(self):
|
def nics(self):
|
||||||
"""List NICs for this instance.
|
"""List NICs for this instance.
|
||||||
|
@ -90,32 +142,23 @@ class Instance(object):
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def state(self):
|
def state(self):
|
||||||
"""Instance state.
|
"""Instance state, one of :py:class:`InstanceState`."""
|
||||||
|
|
||||||
``deploying``
|
|
||||||
deployment is in progress
|
|
||||||
``active``
|
|
||||||
node is provisioned
|
|
||||||
``maintenance``
|
|
||||||
node is provisioned but is in maintenance mode
|
|
||||||
``error``
|
|
||||||
node has a failure
|
|
||||||
``unknown``
|
|
||||||
node in unexpected state (maybe unprovisioned or modified by
|
|
||||||
a third party)
|
|
||||||
"""
|
|
||||||
prov_state = self._node.provision_state
|
prov_state = self._node.provision_state
|
||||||
if prov_state in _DEPLOYING_STATES:
|
if prov_state in _PROGRESS_STATES:
|
||||||
return 'deploying'
|
return InstanceState.DEPLOYING
|
||||||
|
# NOTE(dtantsur): include available since there is a period of time
|
||||||
|
# between claiming the instance and starting the actual provisioning.
|
||||||
|
elif prov_state in _RESERVED_STATES and self._node.instance_id:
|
||||||
|
return InstanceState.DEPLOYING
|
||||||
elif prov_state in _ERROR_STATES:
|
elif prov_state in _ERROR_STATES:
|
||||||
return 'error'
|
return InstanceState.ERROR
|
||||||
elif prov_state in _ACTIVE_STATES:
|
elif prov_state in _ACTIVE_STATES:
|
||||||
if self._node.is_maintenance:
|
if self._node.is_maintenance:
|
||||||
return 'maintenance'
|
return InstanceState.MAINTENANCE
|
||||||
else:
|
else:
|
||||||
return 'active'
|
return InstanceState.ACTIVE
|
||||||
else:
|
else:
|
||||||
return 'unknown'
|
return InstanceState.UNKNOWN
|
||||||
|
|
||||||
def to_dict(self):
|
def to_dict(self):
|
||||||
"""Convert instance to a dict."""
|
"""Convert instance to a dict."""
|
||||||
|
@ -123,7 +166,7 @@ class Instance(object):
|
||||||
'hostname': self.hostname,
|
'hostname': self.hostname,
|
||||||
'ip_addresses': self.ip_addresses(),
|
'ip_addresses': self.ip_addresses(),
|
||||||
'node': self._node.to_dict(),
|
'node': self._node.to_dict(),
|
||||||
'state': self.state,
|
'state': self.state.value,
|
||||||
'uuid': self._uuid,
|
'uuid': self._uuid,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -419,6 +419,8 @@ class Provisioner(_utils.GetNodeMixin):
|
||||||
|
|
||||||
:param instance_id: hostname, UUID or node name.
|
:param instance_id: hostname, UUID or node name.
|
||||||
:return: :py:class:`metalsmith.Instance` object.
|
:return: :py:class:`metalsmith.Instance` object.
|
||||||
|
:raises: :py:class:`metalsmith.exceptions.InvalidInstance`
|
||||||
|
if the instance is not a valid instance.
|
||||||
"""
|
"""
|
||||||
return self.show_instances([instance_id])[0]
|
return self.show_instances([instance_id])[0]
|
||||||
|
|
||||||
|
@ -431,14 +433,25 @@ class Provisioner(_utils.GetNodeMixin):
|
||||||
:param instances: list of hostnames, UUIDs or node names.
|
:param instances: list of hostnames, UUIDs or node names.
|
||||||
:return: list of :py:class:`metalsmith.Instance` objects in the same
|
:return: list of :py:class:`metalsmith.Instance` objects in the same
|
||||||
order as ``instances``.
|
order as ``instances``.
|
||||||
|
:raises: :py:class:`metalsmith.exceptions.InvalidInstance`
|
||||||
|
if one of the instances are not valid instances.
|
||||||
"""
|
"""
|
||||||
with self._cache_node_list_for_lookup():
|
with self._cache_node_list_for_lookup():
|
||||||
return [
|
result = [
|
||||||
_instance.Instance(
|
_instance.Instance(
|
||||||
self.connection,
|
self.connection,
|
||||||
self._get_node(inst, accept_hostname=True))
|
self._get_node(inst, accept_hostname=True))
|
||||||
for inst in instances
|
for inst in instances
|
||||||
]
|
]
|
||||||
|
# NOTE(dtantsur): do not accept node names as valid instances if they
|
||||||
|
# are not deployed or being deployed.
|
||||||
|
missing = [inst for (res, inst) in zip(result, instances)
|
||||||
|
if res.state == _instance.InstanceState.UNKNOWN]
|
||||||
|
if missing:
|
||||||
|
raise exceptions.InvalidInstance(
|
||||||
|
"Node(s)/instance(s) %s are not valid instances"
|
||||||
|
% ', '.join(map(str, missing)))
|
||||||
|
return result
|
||||||
|
|
||||||
def list_instances(self):
|
def list_instances(self):
|
||||||
"""List instances deployed by metalsmith.
|
"""List instances deployed by metalsmith.
|
||||||
|
@ -449,5 +462,5 @@ class Provisioner(_utils.GetNodeMixin):
|
||||||
instances = [i for i in
|
instances = [i for i in
|
||||||
(_instance.Instance(self.connection, node)
|
(_instance.Instance(self.connection, node)
|
||||||
for node in nodes)
|
for node in nodes)
|
||||||
if i._is_deployed_by_metalsmith]
|
if i.state != _instance.InstanceState.UNKNOWN]
|
||||||
return instances
|
return instances
|
||||||
|
|
|
@ -120,3 +120,7 @@ class DeploymentFailure(Error):
|
||||||
def __init__(self, message, nodes):
|
def __init__(self, message, nodes):
|
||||||
self.nodes = nodes
|
self.nodes = nodes
|
||||||
super(DeploymentFailure, self).__init__(message)
|
super(DeploymentFailure, self).__init__(message)
|
||||||
|
|
||||||
|
|
||||||
|
class InvalidInstance(Error):
|
||||||
|
"""The node(s) does not have a metalsmith instance associated."""
|
||||||
|
|
|
@ -76,7 +76,7 @@ class TestDeploy(testtools.TestCase):
|
||||||
instance.create_autospec(_instance.Instance)
|
instance.create_autospec(_instance.Instance)
|
||||||
instance.node.name = None
|
instance.node.name = None
|
||||||
instance.node.id = '123'
|
instance.node.id = '123'
|
||||||
instance.state = 'active'
|
instance.state = _instance.InstanceState.ACTIVE
|
||||||
instance.is_deployed = True
|
instance.is_deployed = True
|
||||||
instance.ip_addresses.return_value = {'private': ['1.2.3.4']}
|
instance.ip_addresses.return_value = {'private': ['1.2.3.4']}
|
||||||
|
|
||||||
|
@ -99,7 +99,7 @@ class TestDeploy(testtools.TestCase):
|
||||||
mock_log.getLogger.mock_calls)
|
mock_log.getLogger.mock_calls)
|
||||||
|
|
||||||
self.mock_print.assert_has_calls([
|
self.mock_print.assert_has_calls([
|
||||||
mock.call(mock.ANY, node='123', state='active'),
|
mock.call(mock.ANY, node='123', state='ACTIVE'),
|
||||||
mock.call(mock.ANY, ips='private=1.2.3.4')
|
mock.call(mock.ANY, ips='private=1.2.3.4')
|
||||||
])
|
])
|
||||||
|
|
||||||
|
@ -132,14 +132,14 @@ class TestDeploy(testtools.TestCase):
|
||||||
instance.ip_addresses.return_value = {}
|
instance.ip_addresses.return_value = {}
|
||||||
instance.node.name = None
|
instance.node.name = None
|
||||||
instance.node.id = '123'
|
instance.node.id = '123'
|
||||||
instance.state = 'active'
|
instance.state = _instance.InstanceState.ACTIVE
|
||||||
|
|
||||||
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
|
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
|
||||||
'--resource-class', 'compute']
|
'--resource-class', 'compute']
|
||||||
self._check(mock_pr, args, {}, {})
|
self._check(mock_pr, args, {}, {})
|
||||||
|
|
||||||
self.mock_print.assert_called_once_with(mock.ANY, node='123',
|
self.mock_print.assert_called_once_with(mock.ANY, node='123',
|
||||||
state='active'),
|
state='ACTIVE'),
|
||||||
|
|
||||||
def test_not_deployed_no_ips(self, mock_pr):
|
def test_not_deployed_no_ips(self, mock_pr):
|
||||||
instance = mock_pr.return_value.provision_node.return_value
|
instance = mock_pr.return_value.provision_node.return_value
|
||||||
|
@ -147,14 +147,14 @@ class TestDeploy(testtools.TestCase):
|
||||||
instance.is_deployed = False
|
instance.is_deployed = False
|
||||||
instance.node.name = None
|
instance.node.name = None
|
||||||
instance.node.id = '123'
|
instance.node.id = '123'
|
||||||
instance.state = 'deploying'
|
instance.state = _instance.InstanceState.DEPLOYING
|
||||||
|
|
||||||
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
|
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
|
||||||
'--resource-class', 'compute']
|
'--resource-class', 'compute']
|
||||||
self._check(mock_pr, args, {}, {})
|
self._check(mock_pr, args, {}, {})
|
||||||
|
|
||||||
self.mock_print.assert_called_once_with(mock.ANY, node='123',
|
self.mock_print.assert_called_once_with(mock.ANY, node='123',
|
||||||
state='deploying'),
|
state='DEPLOYING'),
|
||||||
|
|
||||||
@mock.patch.object(_cmd.LOG, 'info', autospec=True)
|
@mock.patch.object(_cmd.LOG, 'info', autospec=True)
|
||||||
def test_no_logs_not_deployed(self, mock_log, mock_pr):
|
def test_no_logs_not_deployed(self, mock_log, mock_pr):
|
||||||
|
@ -581,11 +581,12 @@ class TestShowWait(testtools.TestCase):
|
||||||
'metalsmith._format._print', autospec=True))
|
'metalsmith._format._print', autospec=True))
|
||||||
self.mock_print = self.print_fixture.mock
|
self.mock_print = self.print_fixture.mock
|
||||||
self.instances = [
|
self.instances = [
|
||||||
mock.Mock(spec=_instance.Instance, hostname=hostname,
|
mock.Mock(
|
||||||
uuid=hostname[-1], is_deployed=(hostname[-1] == '1'),
|
spec=_instance.Instance, hostname=hostname,
|
||||||
state=('active' if hostname[-1] == '1' else 'deploying'),
|
uuid=hostname[-1], is_deployed=(hostname[-1] == '1'),
|
||||||
**{'ip_addresses.return_value': {'private':
|
state=_instance.InstanceState.ACTIVE
|
||||||
['1.2.3.4']}})
|
if hostname[-1] == '1' else _instance.InstanceState.DEPLOYING,
|
||||||
|
**{'ip_addresses.return_value': {'private': ['1.2.3.4']}})
|
||||||
for hostname in ['hostname1', 'hostname2']
|
for hostname in ['hostname1', 'hostname2']
|
||||||
]
|
]
|
||||||
for inst in self.instances:
|
for inst in self.instances:
|
||||||
|
@ -599,9 +600,9 @@ class TestShowWait(testtools.TestCase):
|
||||||
_cmd.main(args)
|
_cmd.main(args)
|
||||||
|
|
||||||
self.mock_print.assert_has_calls([
|
self.mock_print.assert_has_calls([
|
||||||
mock.call(mock.ANY, node='name-1 (UUID 1)', state='active'),
|
mock.call(mock.ANY, node='name-1 (UUID 1)', state='ACTIVE'),
|
||||||
mock.call(mock.ANY, ips='private=1.2.3.4'),
|
mock.call(mock.ANY, ips='private=1.2.3.4'),
|
||||||
mock.call(mock.ANY, node='name-2 (UUID 2)', state='deploying'),
|
mock.call(mock.ANY, node='name-2 (UUID 2)', state='DEPLOYING'),
|
||||||
])
|
])
|
||||||
mock_pr.return_value.show_instances.assert_called_once_with(
|
mock_pr.return_value.show_instances.assert_called_once_with(
|
||||||
['uuid1', 'hostname2'])
|
['uuid1', 'hostname2'])
|
||||||
|
@ -612,9 +613,9 @@ class TestShowWait(testtools.TestCase):
|
||||||
_cmd.main(args)
|
_cmd.main(args)
|
||||||
|
|
||||||
self.mock_print.assert_has_calls([
|
self.mock_print.assert_has_calls([
|
||||||
mock.call(mock.ANY, node='name-1 (UUID 1)', state='active'),
|
mock.call(mock.ANY, node='name-1 (UUID 1)', state='ACTIVE'),
|
||||||
mock.call(mock.ANY, ips='private=1.2.3.4'),
|
mock.call(mock.ANY, ips='private=1.2.3.4'),
|
||||||
mock.call(mock.ANY, node='name-2 (UUID 2)', state='deploying'),
|
mock.call(mock.ANY, node='name-2 (UUID 2)', state='DEPLOYING'),
|
||||||
])
|
])
|
||||||
mock_pr.return_value.list_instances.assert_called_once_with()
|
mock_pr.return_value.list_instances.assert_called_once_with()
|
||||||
|
|
||||||
|
@ -625,9 +626,9 @@ class TestShowWait(testtools.TestCase):
|
||||||
_cmd.main(args)
|
_cmd.main(args)
|
||||||
|
|
||||||
self.mock_print.assert_has_calls([
|
self.mock_print.assert_has_calls([
|
||||||
mock.call(mock.ANY, node='name-1 (UUID 1)', state='active'),
|
mock.call(mock.ANY, node='name-1 (UUID 1)', state='ACTIVE'),
|
||||||
mock.call(mock.ANY, ips='private=1.2.3.4'),
|
mock.call(mock.ANY, ips='private=1.2.3.4'),
|
||||||
mock.call(mock.ANY, node='name-2 (UUID 2)', state='deploying'),
|
mock.call(mock.ANY, node='name-2 (UUID 2)', state='DEPLOYING'),
|
||||||
])
|
])
|
||||||
mock_pr.return_value.wait_for_provisioning.assert_called_once_with(
|
mock_pr.return_value.wait_for_provisioning.assert_called_once_with(
|
||||||
['uuid1', 'hostname2'], timeout=None)
|
['uuid1', 'hostname2'], timeout=None)
|
||||||
|
@ -639,9 +640,9 @@ class TestShowWait(testtools.TestCase):
|
||||||
_cmd.main(args)
|
_cmd.main(args)
|
||||||
|
|
||||||
self.mock_print.assert_has_calls([
|
self.mock_print.assert_has_calls([
|
||||||
mock.call(mock.ANY, node='name-1 (UUID 1)', state='active'),
|
mock.call(mock.ANY, node='name-1 (UUID 1)', state='ACTIVE'),
|
||||||
mock.call(mock.ANY, ips='private=1.2.3.4'),
|
mock.call(mock.ANY, ips='private=1.2.3.4'),
|
||||||
mock.call(mock.ANY, node='name-2 (UUID 2)', state='deploying'),
|
mock.call(mock.ANY, node='name-2 (UUID 2)', state='DEPLOYING'),
|
||||||
])
|
])
|
||||||
mock_pr.return_value.wait_for_provisioning.assert_called_once_with(
|
mock_pr.return_value.wait_for_provisioning.assert_called_once_with(
|
||||||
['uuid1', 'hostname2'], timeout=42)
|
['uuid1', 'hostname2'], timeout=42)
|
||||||
|
|
|
@ -14,6 +14,7 @@
|
||||||
# limitations under the License.
|
# limitations under the License.
|
||||||
|
|
||||||
import mock
|
import mock
|
||||||
|
import six
|
||||||
|
|
||||||
from metalsmith import _instance
|
from metalsmith import _instance
|
||||||
from metalsmith.test import test_provisioner
|
from metalsmith.test import test_provisioner
|
||||||
|
@ -57,47 +58,68 @@ class TestInstanceStates(test_provisioner.Base):
|
||||||
|
|
||||||
def test_state_deploying(self):
|
def test_state_deploying(self):
|
||||||
self.node.provision_state = 'wait call-back'
|
self.node.provision_state = 'wait call-back'
|
||||||
self.assertEqual('deploying', self.instance.state)
|
self.assertEqual(_instance.InstanceState.DEPLOYING,
|
||||||
|
self.instance.state)
|
||||||
self.assertFalse(self.instance.is_deployed)
|
self.assertFalse(self.instance.is_deployed)
|
||||||
self.assertTrue(self.instance.is_healthy)
|
self.assertTrue(self.instance.is_healthy)
|
||||||
|
self.assertTrue(self.instance.state.is_healthy)
|
||||||
|
self.assertFalse(self.instance.state.is_deployed)
|
||||||
|
|
||||||
def test_state_deploying_when_available(self):
|
def test_state_deploying_when_available(self):
|
||||||
self.node.provision_state = 'available'
|
self.node.provision_state = 'available'
|
||||||
self.assertEqual('deploying', self.instance.state)
|
self.node.instance_id = 'abcd'
|
||||||
|
self.assertEqual(_instance.InstanceState.DEPLOYING,
|
||||||
|
self.instance.state)
|
||||||
self.assertFalse(self.instance.is_deployed)
|
self.assertFalse(self.instance.is_deployed)
|
||||||
self.assertTrue(self.instance.is_healthy)
|
self.assertTrue(self.instance.is_healthy)
|
||||||
|
|
||||||
|
def test_state_unknown_when_available(self):
|
||||||
|
self.node.provision_state = 'available'
|
||||||
|
self.node.instance_id = None
|
||||||
|
self.assertEqual(_instance.InstanceState.UNKNOWN, self.instance.state)
|
||||||
|
self.assertFalse(self.instance.is_deployed)
|
||||||
|
self.assertFalse(self.instance.is_healthy)
|
||||||
|
self.assertFalse(self.instance.state.is_healthy)
|
||||||
|
|
||||||
def test_state_deploying_maintenance(self):
|
def test_state_deploying_maintenance(self):
|
||||||
self.node.is_maintenance = True
|
self.node.is_maintenance = True
|
||||||
self.node.provision_state = 'wait call-back'
|
self.node.provision_state = 'wait call-back'
|
||||||
self.assertEqual('deploying', self.instance.state)
|
self.assertEqual(_instance.InstanceState.DEPLOYING,
|
||||||
|
self.instance.state)
|
||||||
self.assertFalse(self.instance.is_deployed)
|
self.assertFalse(self.instance.is_deployed)
|
||||||
self.assertFalse(self.instance.is_healthy)
|
self.assertFalse(self.instance.is_healthy)
|
||||||
|
# The state itself is considered healthy
|
||||||
|
self.assertTrue(self.instance.state.is_healthy)
|
||||||
|
|
||||||
def test_state_active(self):
|
def test_state_active(self):
|
||||||
self.node.provision_state = 'active'
|
self.node.provision_state = 'active'
|
||||||
self.assertEqual('active', self.instance.state)
|
self.assertEqual(_instance.InstanceState.ACTIVE, self.instance.state)
|
||||||
self.assertTrue(self.instance.is_deployed)
|
self.assertTrue(self.instance.is_deployed)
|
||||||
self.assertTrue(self.instance.is_healthy)
|
self.assertTrue(self.instance.is_healthy)
|
||||||
|
self.assertTrue(self.instance.state.is_deployed)
|
||||||
|
|
||||||
def test_state_maintenance(self):
|
def test_state_maintenance(self):
|
||||||
self.node.is_maintenance = True
|
self.node.is_maintenance = True
|
||||||
self.node.provision_state = 'active'
|
self.node.provision_state = 'active'
|
||||||
self.assertEqual('maintenance', self.instance.state)
|
self.assertEqual(_instance.InstanceState.MAINTENANCE,
|
||||||
|
self.instance.state)
|
||||||
self.assertTrue(self.instance.is_deployed)
|
self.assertTrue(self.instance.is_deployed)
|
||||||
self.assertFalse(self.instance.is_healthy)
|
self.assertFalse(self.instance.is_healthy)
|
||||||
|
self.assertFalse(self.instance.state.is_healthy)
|
||||||
|
|
||||||
def test_state_error(self):
|
def test_state_error(self):
|
||||||
self.node.provision_state = 'deploy failed'
|
self.node.provision_state = 'deploy failed'
|
||||||
self.assertEqual('error', self.instance.state)
|
self.assertEqual(_instance.InstanceState.ERROR, self.instance.state)
|
||||||
self.assertFalse(self.instance.is_deployed)
|
self.assertFalse(self.instance.is_deployed)
|
||||||
self.assertFalse(self.instance.is_healthy)
|
self.assertFalse(self.instance.is_healthy)
|
||||||
|
self.assertFalse(self.instance.state.is_healthy)
|
||||||
|
|
||||||
def test_state_unknown(self):
|
def test_state_unknown(self):
|
||||||
self.node.provision_state = 'enroll'
|
self.node.provision_state = 'enroll'
|
||||||
self.assertEqual('unknown', self.instance.state)
|
self.assertEqual(_instance.InstanceState.UNKNOWN, self.instance.state)
|
||||||
self.assertFalse(self.instance.is_deployed)
|
self.assertFalse(self.instance.is_deployed)
|
||||||
self.assertFalse(self.instance.is_healthy)
|
self.assertFalse(self.instance.is_healthy)
|
||||||
|
self.assertFalse(self.instance.state.is_healthy)
|
||||||
|
|
||||||
@mock.patch.object(_instance.Instance, 'ip_addresses', autospec=True)
|
@mock.patch.object(_instance.Instance, 'ip_addresses', autospec=True)
|
||||||
def test_to_dict(self, mock_ips):
|
def test_to_dict(self, mock_ips):
|
||||||
|
@ -106,9 +128,17 @@ class TestInstanceStates(test_provisioner.Base):
|
||||||
self.node.instance_info = {'metalsmith_hostname': 'host'}
|
self.node.instance_info = {'metalsmith_hostname': 'host'}
|
||||||
mock_ips.return_value = {'private': ['1.2.3.4']}
|
mock_ips.return_value = {'private': ['1.2.3.4']}
|
||||||
|
|
||||||
|
to_dict = self.instance.to_dict()
|
||||||
self.assertEqual({'hostname': 'host',
|
self.assertEqual({'hostname': 'host',
|
||||||
'ip_addresses': {'private': ['1.2.3.4']},
|
'ip_addresses': {'private': ['1.2.3.4']},
|
||||||
'node': {'node': 'dict'},
|
'node': {'node': 'dict'},
|
||||||
'state': 'deploying',
|
'state': 'deploying',
|
||||||
'uuid': self.node.id},
|
'uuid': self.node.id},
|
||||||
self.instance.to_dict())
|
to_dict)
|
||||||
|
# States are converted to strings
|
||||||
|
self.assertIsInstance(to_dict['state'], six.string_types)
|
||||||
|
|
||||||
|
def test_deprecated_comparisons(self):
|
||||||
|
for item in _instance.InstanceState:
|
||||||
|
self.assertEqual(item, item.value)
|
||||||
|
self.assertFalse(item != item.value) # noqa
|
||||||
|
|
|
@ -1305,6 +1305,10 @@ class TestUnprovisionNode(Base):
|
||||||
|
|
||||||
|
|
||||||
class TestShowInstance(Base):
|
class TestShowInstance(Base):
|
||||||
|
def setUp(self):
|
||||||
|
super(TestShowInstance, self).setUp()
|
||||||
|
self.node.provision_state = 'active'
|
||||||
|
|
||||||
def test_show_instance(self):
|
def test_show_instance(self):
|
||||||
self.mock_get_node.side_effect = lambda n, *a, **kw: self.node
|
self.mock_get_node.side_effect = lambda n, *a, **kw: self.node
|
||||||
inst = self.pr.show_instance('id1')
|
inst = self.pr.show_instance('id1')
|
||||||
|
@ -1315,7 +1319,7 @@ class TestShowInstance(Base):
|
||||||
self.assertIs(inst.uuid, self.node.id)
|
self.assertIs(inst.uuid, self.node.id)
|
||||||
|
|
||||||
def test_show_instances(self):
|
def test_show_instances(self):
|
||||||
self.mock_get_node.side_effect = [self.node, mock.Mock()]
|
self.mock_get_node.side_effect = [self.node, self.node]
|
||||||
result = self.pr.show_instances(['1', '2'])
|
result = self.pr.show_instances(['1', '2'])
|
||||||
self.mock_get_node.assert_has_calls([
|
self.mock_get_node.assert_has_calls([
|
||||||
mock.call(self.pr, '1', accept_hostname=True),
|
mock.call(self.pr, '1', accept_hostname=True),
|
||||||
|
@ -1343,16 +1347,16 @@ class TestListInstances(Base):
|
||||||
super(TestListInstances, self).setUp()
|
super(TestListInstances, self).setUp()
|
||||||
self.nodes = [
|
self.nodes = [
|
||||||
mock.Mock(spec=NODE_FIELDS, provision_state=state,
|
mock.Mock(spec=NODE_FIELDS, provision_state=state,
|
||||||
instance_info={'metalsmith_hostname': '1234'})
|
instance_id='1234')
|
||||||
for state in ('active', 'active', 'deploying', 'wait call-back',
|
for state in ('active', 'active', 'deploying', 'wait call-back',
|
||||||
'deploy failed', 'available')
|
'deploy failed', 'available', 'available', 'enroll')
|
||||||
]
|
]
|
||||||
del self.nodes[-1].instance_info['metalsmith_hostname']
|
self.nodes[6].instance_id = None
|
||||||
self.api.baremetal.nodes.return_value = self.nodes
|
self.api.baremetal.nodes.return_value = self.nodes
|
||||||
|
|
||||||
def test_list(self):
|
def test_list(self):
|
||||||
instances = self.pr.list_instances()
|
instances = self.pr.list_instances()
|
||||||
self.assertTrue(isinstance(i, _instance.Instance) for i in instances)
|
self.assertTrue(isinstance(i, _instance.Instance) for i in instances)
|
||||||
self.assertEqual(self.nodes[:5], [i.node for i in instances])
|
self.assertEqual(self.nodes[:6], [i.node for i in instances])
|
||||||
self.api.baremetal.nodes.assert_called_once_with(associated=True,
|
self.api.baremetal.nodes.assert_called_once_with(associated=True,
|
||||||
details=True)
|
details=True)
|
||||||
|
|
|
@ -0,0 +1,17 @@
|
||||||
|
---
|
||||||
|
features:
|
||||||
|
- |
|
||||||
|
The ``Instance.state`` value is now a proper enumeration of type
|
||||||
|
``metalsmith.InstanceState``.
|
||||||
|
- |
|
||||||
|
The ``list_instances`` call now returns any valid instances, not only ones
|
||||||
|
created by metalsmith. This is consistent with the ``show_instance(s)``
|
||||||
|
behavior.
|
||||||
|
deprecations:
|
||||||
|
- |
|
||||||
|
Comparing an instance state with strings is deprecated, use enumeration
|
||||||
|
values instead.
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
Fixes the ``show_instance(s)`` calls to return an exception for nodes that
|
||||||
|
are not valid instances (state == ``UNKNOWN``).
|
|
@ -5,3 +5,4 @@ pbr!=2.1.0,>=2.0.0 # Apache-2.0
|
||||||
openstacksdk>=0.22.0 # Apache-2.0
|
openstacksdk>=0.22.0 # Apache-2.0
|
||||||
requests>=2.18.4 # Apache-2.0
|
requests>=2.18.4 # Apache-2.0
|
||||||
six>=1.10.0 # MIT
|
six>=1.10.0 # MIT
|
||||||
|
enum34>=1.0.4;python_version=='2.7' or python_version=='2.6' or python_version=='3.3' # BSD
|
||||||
|
|
Loading…
Reference in New Issue