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

@@ -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,

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):

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
 
-
+from magnum.objects import fields as m_fields
-# possible status
-ERROR = 'Error'
-RUNNING = 'Running'
-STOPPED = 'Stopped'
-PAUSED = 'Paused'
-UNKNOWN = 'Unknown'
 
 
 @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

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()

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')

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')

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')