From 5ac2dce563d4e5ebacb65acfebee4f3fb9cfeaf1 Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Wed, 26 Aug 2015 21:44:39 +0000 Subject: [PATCH] Add field for container status This adds an enum for the different statuses and changes over to use those constants in docker conducter. An UNKNOWN value was also added because the API tests expected 'Unknown' so this value needed to be added to the field in order to be valid. Change-Id: Ic79317f0fbdaf99c1979d3023f04c4eba44b412a Closes-Bug: #1489136 --- magnum/api/controllers/v1/container.py | 3 +- magnum/conductor/handlers/docker_conductor.py | 30 +++++++++-------- magnum/objects/container.py | 10 ++---- magnum/objects/fields.py | 16 ++++++++++ .../unit/api/controllers/v1/test_container.py | 3 +- .../handlers/test_docker_conductor.py | 32 ++++++++++--------- magnum/tests/unit/objects/test_fields.py | 21 ++++++++++++ 7 files changed, 76 insertions(+), 39 deletions(-) diff --git a/magnum/api/controllers/v1/container.py b/magnum/api/controllers/v1/container.py index 7b52ce68cc..8262cebed5 100644 --- a/magnum/api/controllers/v1/container.py +++ b/magnum/api/controllers/v1/container.py @@ -32,6 +32,7 @@ from magnum.common import exception from magnum.common import policy from magnum.i18n import _LE from magnum import objects +from magnum.objects import fields LOG = logging.getLogger(__name__) @@ -291,7 +292,7 @@ class ContainersController(rest.RestController): LOG.exception(_LE("Error while list container %(uuid)s: " "%(e)s."), {'uuid': c.uuid, 'e': e}) - containers[i].status = objects.container.UNKNOWN + containers[i].status = fields.ContainerStatus.UNKNOWN return ContainerCollection.convert_with_links(containers, limit, url=resource_url, diff --git a/magnum/conductor/handlers/docker_conductor.py b/magnum/conductor/handlers/docker_conductor.py index 8f1f6c7138..de47767a08 100644 --- a/magnum/conductor/handlers/docker_conductor.py +++ b/magnum/conductor/handlers/docker_conductor.py @@ -25,7 +25,7 @@ from magnum.conductor.handlers.common import docker_client from magnum.conductor import utils as conductor_utils from magnum.i18n import _LE from magnum import objects -from magnum.objects import container as obj_container +from magnum.objects import fields LOG = logging.getLogger(__name__) CONF = cfg.CONF @@ -130,10 +130,10 @@ class Handler(object): docker.create_container(image, name=name, hostname=container_uuid, command=container.command) - container.status = obj_container.STOPPED + container.status = fields.ContainerStatus.STOPPED return container except errors.APIError as api_error: - container.status = obj_container.ERROR + container.status = fields.ContainerStatus.ERROR raise exception.ContainerException( "Docker API Error : %s" % str(api_error)) finally: @@ -162,26 +162,26 @@ class Handler(object): if not docker_id: LOG.exception(_LE("Can not find docker instance with %s," "set it to Error status"), container_uuid) - container.status = obj_container.ERROR + container.status = fields.ContainerStatus.ERROR container.save() return container result = docker.inspect_container(docker_id) status = result.get('State') if status: if status.get('Error') is True: - container.status = obj_container.ERROR + container.status = fields.ContainerStatus.ERROR elif status.get('Running'): - container.status = obj_container.RUNNING + container.status = fields.ContainerStatus.RUNNING elif status.get('Paused'): - container.status = obj_container.PAUSED + container.status = fields.ContainerStatus.PAUSED else: - container.status = obj_container.STOPPED + container.status = fields.ContainerStatus.STOPPED container.save() return container except errors.APIError as api_error: error_message = str(api_error) if '404' in error_message: - container.status = obj_container.ERROR + container.status = fields.ContainerStatus.ERROR container.save() return container raise exception.ContainerException( @@ -204,23 +204,25 @@ class Handler(object): def container_reboot(self, context, container_uuid): return self._container_action(context, container_uuid, - obj_container.RUNNING, 'restart') + fields.ContainerStatus.RUNNING, + 'restart') def container_stop(self, context, container_uuid): return self._container_action(context, container_uuid, - obj_container.STOPPED, 'stop') + fields.ContainerStatus.STOPPED, 'stop') def container_start(self, context, container_uuid): return self._container_action(context, container_uuid, - obj_container.RUNNING, 'start') + fields.ContainerStatus.RUNNING, 'start') def container_pause(self, context, container_uuid): return self._container_action(context, container_uuid, - obj_container.PAUSED, 'pause') + fields.ContainerStatus.PAUSED, 'pause') def container_unpause(self, context, container_uuid): return self._container_action(context, container_uuid, - obj_container.RUNNING, 'unpause') + fields.ContainerStatus.RUNNING, + 'unpause') @wrap_container_exception def container_logs(self, context, container_uuid): diff --git a/magnum/objects/container.py b/magnum/objects/container.py index 84716892db..7605e0ebd9 100644 --- a/magnum/objects/container.py +++ b/magnum/objects/container.py @@ -18,13 +18,7 @@ from oslo_versionedobjects import fields from magnum.db import api as dbapi from magnum.objects import base - -# possible status -ERROR = 'Error' -RUNNING = 'Running' -STOPPED = 'Stopped' -PAUSED = 'Paused' -UNKNOWN = 'Unknown' +from magnum.objects import fields as m_fields @base.MagnumObjectRegistry.register @@ -44,7 +38,7 @@ class Container(base.MagnumPersistentObject, base.MagnumObject, 'image': fields.StringField(nullable=True), 'command': fields.StringField(nullable=True), 'bay_uuid': fields.StringField(nullable=True), - 'status': fields.StringField(nullable=True), + 'status': m_fields.ContainerStatusField(nullable=True), } @staticmethod diff --git a/magnum/objects/fields.py b/magnum/objects/fields.py index 447e37d64a..59e783d5ce 100644 --- a/magnum/objects/fields.py +++ b/magnum/objects/fields.py @@ -34,9 +34,25 @@ class BayStatus(fields.Enum): super(BayStatus, self).__init__(valid_values=BayStatus.ALL) +class ContainerStatus(fields.Enum): + ALL = ( + ERROR, RUNNING, STOPPED, PAUSED, UNKNOWN, + ) = ( + 'Error', 'Running', 'Stopped', 'Paused', 'Unknown', + ) + + def __init__(self): + super(ContainerStatus, self).__init__( + valid_values=ContainerStatus.ALL) + + class ListOfDictsField(fields.AutoTypedField): AUTO_TYPE = fields.List(fields.Dict(fields.FieldType())) class BayStatusField(fields.BaseEnumField): AUTO_TYPE = BayStatus() + + +class ContainerStatusField(fields.BaseEnumField): + AUTO_TYPE = ContainerStatus() diff --git a/magnum/tests/unit/api/controllers/v1/test_container.py b/magnum/tests/unit/api/controllers/v1/test_container.py index a1a49b0270..c0460a6773 100644 --- a/magnum/tests/unit/api/controllers/v1/test_container.py +++ b/magnum/tests/unit/api/controllers/v1/test_container.py @@ -12,6 +12,7 @@ from magnum.common import utils as comm_utils from magnum import objects +from magnum.objects import fields from magnum.tests.unit.api import base as api_base from magnum.tests.unit.db import utils @@ -253,7 +254,7 @@ class TestContainerController(api_base.FunctionalTest): test_container['uuid']) self.assertEqual(actual_containers[0].get('status'), - objects.container.UNKNOWN) + fields.ContainerStatus.UNKNOWN) @patch('magnum.conductor.api.API.container_show') @patch('magnum.objects.Container.get_by_uuid') diff --git a/magnum/tests/unit/conductor/handlers/test_docker_conductor.py b/magnum/tests/unit/conductor/handlers/test_docker_conductor.py index 360b7e544f..704eb51a4e 100644 --- a/magnum/tests/unit/conductor/handlers/test_docker_conductor.py +++ b/magnum/tests/unit/conductor/handlers/test_docker_conductor.py @@ -20,7 +20,7 @@ import six from magnum.common import exception from magnum.conductor.handlers import docker_conductor from magnum import objects -from magnum.objects import container as obj_container +from magnum.objects import fields from magnum.tests import base from mock import patch @@ -129,7 +129,7 @@ class TestDockerConductor(base.BaseTestCase): name='some-name', hostname='some-uuid', command=None) - self.assertEqual(obj_container.STOPPED, container.status) + self.assertEqual(fields.ContainerStatus.STOPPED, container.status) @mock.patch.object(docker_conductor.Handler, 'get_docker_client') def test_container_create_with_command(self, mock_get_docker_client): @@ -154,7 +154,7 @@ class TestDockerConductor(base.BaseTestCase): name='some-name', hostname='some-uuid', command='env') - self.assertEqual(obj_container.STOPPED, container.status) + self.assertEqual(fields.ContainerStatus.STOPPED, container.status) def test_encode_utf8_unicode(self): image = 'some_image:some_tag' @@ -181,7 +181,8 @@ class TestDockerConductor(base.BaseTestCase): tag='some_tag') self.assertFalse(mock_docker.create_container.called) mock_init.assert_called_once_with() - self.assertEqual(obj_container.ERROR, mock_container.status) + self.assertEqual(fields.ContainerStatus.ERROR, + mock_container.status) def test_find_container_by_name_not_found(self): mock_docker = mock.MagicMock() @@ -282,7 +283,7 @@ class TestDockerConductor(base.BaseTestCase): mock_docker.restart.assert_called_once_with(mock_docker_id) mock_find_container.assert_called_once_with(mock_docker, mock_container_uuid) - self.assertEqual(obj_container.RUNNING, mock_container.status) + self.assertEqual(fields.ContainerStatus.RUNNING, mock_container.status) @patch.object(docker_conductor.Handler, '_find_container_by_name') @mock.patch.object(docker_conductor.Handler, 'get_docker_client') @@ -323,7 +324,7 @@ class TestDockerConductor(base.BaseTestCase): mock_docker.start.assert_called_once_with(mock_docker_id) mock_find_container.assert_called_once_with(mock_docker, mock_container_uuid) - self.assertEqual(obj_container.RUNNING, mock_container.status) + self.assertEqual(fields.ContainerStatus.RUNNING, mock_container.status) @patch.object(docker_conductor.Handler, '_find_container_by_name') @mock.patch.object(docker_conductor.Handler, 'get_docker_client') @@ -364,7 +365,7 @@ class TestDockerConductor(base.BaseTestCase): mock_docker.stop.assert_called_once_with(mock_docker_id) mock_find_container.assert_called_once_with(mock_docker, mock_container_uuid) - self.assertEqual(obj_container.STOPPED, mock_container.status) + self.assertEqual(fields.ContainerStatus.STOPPED, mock_container.status) @patch.object(docker_conductor.Handler, '_find_container_by_name') @mock.patch.object(docker_conductor.Handler, 'get_docker_client') @@ -404,7 +405,7 @@ class TestDockerConductor(base.BaseTestCase): mock_docker.pause.assert_called_once_with(mock_docker_id) mock_find_container.assert_called_once_with(mock_docker, mock_container_uuid) - self.assertEqual(obj_container.PAUSED, mock_container.status) + self.assertEqual(fields.ContainerStatus.PAUSED, mock_container.status) @patch.object(docker_conductor.Handler, '_find_container_by_name') @mock.patch.object(docker_conductor.Handler, 'get_docker_client') @@ -444,7 +445,7 @@ class TestDockerConductor(base.BaseTestCase): mock_docker.unpause.assert_called_once_with(mock_docker_id) mock_find_container.assert_called_once_with(mock_docker, mock_container_uuid) - self.assertEqual(obj_container.RUNNING, mock_container.status) + self.assertEqual(fields.ContainerStatus.RUNNING, mock_container.status) @patch.object(docker_conductor.Handler, '_find_container_by_name') @mock.patch.object(docker_conductor.Handler, 'get_docker_client') @@ -505,7 +506,7 @@ class TestDockerConductor(base.BaseTestCase): 'Paused': False}} mock_docker.inspect_container.return_value = mock_container_detail self.conductor.container_show(None, mock_container_uuid) - self.assertEqual(obj_container.RUNNING, mock_container.status) + self.assertEqual(fields.ContainerStatus.RUNNING, mock_container.status) @mock.patch.object(objects.Container, 'get_by_uuid') @mock.patch.object(docker_conductor.Handler, '_find_container_by_name') @@ -525,7 +526,7 @@ class TestDockerConductor(base.BaseTestCase): 'Paused': False}} mock_docker.inspect_container.return_value = mock_container_detail self.conductor.container_show(None, mock_container_uuid) - self.assertEqual(obj_container.STOPPED, mock_container.status) + self.assertEqual(fields.ContainerStatus.STOPPED, mock_container.status) @mock.patch.object(objects.Container, 'get_by_uuid') @mock.patch.object(docker_conductor.Handler, '_find_container_by_name') @@ -545,7 +546,7 @@ class TestDockerConductor(base.BaseTestCase): 'Paused': True}} mock_docker.inspect_container.return_value = mock_container_detail self.conductor.container_show(None, mock_container_uuid) - self.assertEqual(obj_container.PAUSED, mock_container.status) + self.assertEqual(fields.ContainerStatus.PAUSED, mock_container.status) @mock.patch.object(objects.Container, 'get_by_uuid') @mock.patch.object(docker_conductor.Handler, '_find_container_by_name') @@ -565,7 +566,7 @@ class TestDockerConductor(base.BaseTestCase): 'Paused': False}} mock_docker.inspect_container.return_value = mock_container_detail self.conductor.container_show(None, mock_container_uuid) - self.assertEqual(obj_container.ERROR, mock_container.status) + self.assertEqual(fields.ContainerStatus.ERROR, mock_container.status) @mock.patch.object(objects.Container, 'get_by_uuid') @patch.object(docker_conductor.Handler, '_find_container_by_name') @@ -615,7 +616,8 @@ class TestDockerConductor(base.BaseTestCase): mock_find_container.assert_called_once_with(mock_docker, mock_container_uuid) mock_init.assert_called_once_with() - self.assertEqual(obj_container.ERROR, mock_container.status) + self.assertEqual(fields.ContainerStatus.ERROR, + mock_container.status) @mock.patch.object(objects.Container, 'get_by_uuid') @patch.object(docker_conductor.Handler, '_find_container_by_name') @@ -634,7 +636,7 @@ class TestDockerConductor(base.BaseTestCase): self.conductor.container_show(None, mock_container_uuid) mock_find_container.assert_called_once_with(mock_docker, mock_container_uuid) - self.assertEqual(obj_container.ERROR, mock_container.status) + self.assertEqual(fields.ContainerStatus.ERROR, mock_container.status) @patch.object(docker_conductor.Handler, '_find_container_by_name') @mock.patch.object(docker_conductor.Handler, 'get_docker_client') diff --git a/magnum/tests/unit/objects/test_fields.py b/magnum/tests/unit/objects/test_fields.py index 8b11aee772..22f542c2f3 100644 --- a/magnum/tests/unit/objects/test_fields.py +++ b/magnum/tests/unit/objects/test_fields.py @@ -43,3 +43,24 @@ class TestBayStatus(test_fields.TestField): def test_stringify_invalid(self): self.assertRaises(ValueError, self.field.stringify, 'DELETE_STOPPED') + + +class TestContainerStatus(test_fields.TestField): + def setUp(self): + super(TestContainerStatus, self).setUp() + self.field = fields.ContainerStatusField() + self.coerce_good_values = [('Error', 'Error'), ('Running', 'Running'), + ('Stopped', 'Stopped'), + ('Paused', 'Paused'), + ('Unknown', 'Unknown'), ] + self.coerce_bad_values = ['DELETED'] + + self.to_primitive_values = self.coerce_good_values[0:1] + self.from_primitive_values = self.coerce_good_values[0:1] + + def test_stringify(self): + self.assertEqual("'Stopped'", + self.field.stringify('Stopped')) + + def test_stringify_invalid(self): + self.assertRaises(ValueError, self.field.stringify, 'DELETED')