Merge pull request SatelliteQE#383 from svtkachenko/63_image

Update Image and Host entities

nailgun/entities.py

@@ -2858,18 +2858,37 @@ def read(self, entity=None, attrs=None, ignore=None):
 
         `content_facet_attributes` are returned only in case any of facet
         attributes were actually set.
+
+        Also add image to the response if needed, as
+        :meth:`nailgun.entity_mixins.EntityReadMixin.read` can't initialize
+        image.
         """
         if attrs is None:
             attrs = self.read_json()
         if ignore is None:
             ignore = set()
+        attrs['host_parameters_attributes'] = attrs.pop('parameters')
         if 'content_facet_attributes' not in attrs:
             ignore.add('content_facet_attributes')
         ignore.add('root_pass')
-        attrs['host_parameters_attributes'] = attrs.pop('parameters')
+        # Image entity requires compute_resource_id to initialize as it is
+        # part of its path. The thing is that entity_mixins.read() initializes
+        # entities by id only.
+        # Workaround is to add image to ignore, call entity_mixins.read()
+        # and then add 'manually' initialized image to the result.
+        # If image_id is None set image to None as it is done by default.
+        ignore.add('image')
         # host id is required for interface initialization
         ignore.add('interface')
         result = super(Host, self).read(entity, attrs, ignore)
+        if attrs.get('image_id'):
+            result.image = Image(
+                server_config=self._server_config,
+                id=attrs.get('image_id'),
+                compute_resource=attrs.get('compute_resource_id'),
+            )
+        else:
+            result.image = None
         if 'interfaces' in attrs and attrs['interfaces']:
             result.interface = [
                 Interface(
@@ -3028,6 +3047,7 @@ class Image(
     """A representation of a Image entity."""
 
     def __init__(self, server_config=None, **kwargs):
+        _check_for_value('compute_resource', kwargs)
         self._fields = {
             'architecture': entity_fields.OneToOneField(
                 Architecture,
@@ -3049,11 +3069,49 @@ def __init__(self, server_config=None, **kwargs):
             'username': entity_fields.StringField(required=True),
             'uuid': entity_fields.StringField(required=True),
         }
+        super(Image, self).__init__(server_config, **kwargs)
         self._meta = {
-            'api_path': 'api/v2/compute_resources/:compute_resource_id/images',
+            'api_path': '{0}/images'.format(
+                # pylint:disable=no-member
+                self.compute_resource.path('self')),
             'server_modes': ('sat'),
         }
-        super(Image, self).__init__(server_config, **kwargs)
+
+    def create_payload(self):
+        """Wrap submitted data within an extra dict."""
+        return {u'image': super(Image, self).create_payload()}
+
+    def update_payload(self, fields=None):
+        """Wrap submitted data within an extra dict."""
+        return {u'image': super(Image, self).update_payload(fields)}
+
+    def read(self, entity=None, attrs=None, ignore=None):
+        """Provide a default value for ``entity``.
+
+        By default, ``nailgun.entity_mixins.EntityReadMixin.read`` provides a
+        default value for ``entity`` like so::
+
+            entity = type(self)()
+
+        However, :class:`Image` requires that an
+        ``compute_resource`` be provided, so this technique will not work. Do
+        this instead::
+
+            entity = type(self)(compute_resource=self.compute_resource.id)
+
+        """
+        # read() should not change the state of the object it's called on, but
+        # super() alters the attributes of any entity passed in. Creating a new
+        # object and passing it to super() lets this one avoid changing state.
+        if entity is None:
+            entity = type(self)(
+                self._server_config,
+                compute_resource=self.compute_resource,  # pylint:disable=E1101
+            )
+        if ignore is None:
+            ignore = set()
+        ignore.add('compute_resource')
+        return super(Image, self).read(entity, attrs, ignore)
 
 
 class Interface(

tests/test_entities.py

@@ -124,7 +124,6 @@ def test_init_succeeds(self):
                 entities.HostCollectionErrata,
                 entities.HostCollectionPackage,
                 entities.HostGroup,
-                entities.Image,
                 entities.LibvirtComputeResource,
                 entities.LifecycleEnvironment,
                 entities.Location,
@@ -177,6 +176,7 @@ def test_init_succeeds(self):
             (entities.HostPackage, {'host': 1}),
             (entities.HostSubscription, {'host': 1}),
             (entities.Interface, {'host': 1}),
+            (entities.Image, {'compute_resource': 1}),
             (entities.OperatingSystemParameter, {'operatingsystem': 1}),
             (entities.OverrideValue, {'smart_class_parameter': 1}),
             (entities.OverrideValue, {'smart_variable': 1}),
@@ -210,6 +210,7 @@ def test_required_params(self):
                 entities.ContentViewPuppetModule,
                 entities.HostPackage,
                 entities.HostSubscription,
+                entities.Image,
                 entities.OverrideValue,
                 entities.OperatingSystemParameter,
                 entities.RepositorySet,
@@ -570,6 +571,7 @@ def test_no_attributes(self):
             )
         ]
         entities_.extend([
+            (entities.Image, {'compute_resource': 1}),
             (entities.SyncPlan, {'organization': 1})
         ])
         for entity, params in entities_:
@@ -599,6 +601,14 @@ def test_host_collection(self):
         self.assertNotIn('system_ids', payload)
         self.assertIn('system_uuids', payload)
 
+    def test_image(self):
+        """Create a :class:`nailgun.entities.Image`."""
+        payload = entities.Image(
+            self.cfg,
+            compute_resource=1,
+        ).create_payload()
+        self.assertEqual({'image': {'compute_resource_id': 1}}, payload)
+
     def test_media(self):
         """Create a :class:`nailgun.entities.Media`."""
         payload = entities.Media(self.cfg, path_='foo').create_payload()
@@ -917,6 +927,7 @@ def test_entity_arg(self):
                 ),
                 entities.ContentViewPuppetModule(self.cfg, content_view=2),
                 entities.Interface(self.cfg, host=2),
+                entities.Image(self.cfg, compute_resource=1),
                 entities.OperatingSystemParameter(self.cfg, operatingsystem=2),
                 entities.OverrideValue(self.cfg, smart_class_parameter=2),
                 entities.OverrideValue(self.cfg, smart_variable=2),
@@ -1232,6 +1243,37 @@ def test_interface(self):
                 self.assertIn('host_id', args)
                 self.assertEqual(args['host_id'], 3)
 
+    def test_host_with_image(self):
+        """Call :meth:`nailgun.entities.Host.read` for a host with image
+        assigned.
+
+        Ensure that the image entity was correctly fetched.
+        """
+        image = entities.Image(self.cfg, id=1, compute_resource=1)
+        host = entities.Host(self.cfg, id=1)
+        with mock.patch.object(EntityReadMixin, 'read_json') as read_json:
+            with mock.patch.object(EntityReadMixin, 'read') as read:
+                # Image was set
+                read_json.return_value = {
+                    'image_id': 1,
+                    'compute_resource_id': 1,
+                    'parameters': {},
+                }
+                read.return_value = host
+                host = host.read()
+                self.assertTrue(hasattr(host, 'image'))
+                # pylint:disable=no-member
+                self.assertEqual(host.image.id, image.id)
+                # Image wasn't set
+                read_json.return_value = {
+                    'parameters': {},
+                }
+                read.return_value = host
+                host = host.read()
+                self.assertTrue(hasattr(host, 'image'))
+                # pylint:disable=no-member
+                self.assertIsNone(host.image)
+
 
 class SearchRawTestCase(TestCase):
     """Tests for :meth:`nailgun.entity_mixins.EntitySearchMixin.search_raw`."""
@@ -1406,6 +1448,19 @@ def test_discovery_rule_search(self):
         self.assertNotIn('search_', payload['discovery_rule'])
         self.assertIn('search', payload['discovery_rule'])
 
+    def test_image(self):
+        """Check whether ``Image`` updates its ``path_`` field.
+
+        The field should be renamed from ``path_`` to ``path`` when
+        ``update_payload`` is called.
+
+        """
+        payload = entities.Image(
+            self.cfg,
+            compute_resource=1,
+        ).update_payload()
+        self.assertEqual({'image': {'compute_resource_id': 1}}, payload)
+
     def test_media_path(self):
         """Check whether ``Media`` updates its ``path_`` field.