Labels override

The post for both clusters and nodegroups is adapted to wait for a
boolean flag called merge_labels. Based on this flag the API will
either merge the provided with the parent labels or just use the
provided labels.

At the same time, the get methods of both clusters and nodegroups
are adapted to include new fields in the response called
"labels_overridden", "labels_added", "labels_skipped". The fields
contain the differnces with the parent labels.

story: 2007515
task: 39691
Change-Id: I1054c54da96005a49e874de6f4cf60b5db57fc02

magnum/api/controllers/v1/cluster.py

@@ -182,6 +182,24 @@ class Cluster(base.APIBase):
     floating_ip_enabled = wsme.wsattr(types.boolean)
     """Indicates whether created clusters should have a floating ip or not."""
 
+    merge_labels = wsme.wsattr(types.boolean, default=False)
+    """Indicates whether the labels will be merged with the CT labels."""
+
+    labels_overridden = wtypes.DictType(
+            wtypes.text, types.MultiType(
+                wtypes.text, six.integer_types, bool, float))
+    """Contains labels that have a value different than the parent labels."""
+
+    labels_added = wtypes.DictType(
+            wtypes.text, types.MultiType(
+                wtypes.text, six.integer_types, bool, float))
+    """Contains labels that do not exist in the parent."""
+
+    labels_skipped = wtypes.DictType(
+            wtypes.text, types.MultiType(
+                wtypes.text, six.integer_types, bool, float))
+    """Contains labels that exist in the parent but were not inherited."""
+
     def __init__(self, **kwargs):
         super(Cluster, self).__init__()
         self.fields = []
@@ -198,7 +216,7 @@ class Cluster(base.APIBase):
             setattr(self, field, kwargs.get(field, wtypes.Unset))
 
     @staticmethod
-    def _convert_with_links(cluster, url, expand=True):
+    def _convert_with_links(cluster, url, expand=True, parent_labels=None):
         if not expand:
             cluster.unset_fields_except(['uuid', 'name', 'cluster_template_id',
                                          'keypair', 'docker_volume_size',
@@ -206,6 +224,12 @@ class Cluster(base.APIBase):
                                          'master_flavor_id', 'flavor_id',
                                          'create_timeout', 'master_count',
                                          'stack_id', 'health_status'])
+        else:
+            overridden, added, skipped = api_utils.get_labels_diff(
+                parent_labels, cluster.labels)
+            cluster.labels_overridden = overridden
+            cluster.labels_added = added
+            cluster.labels_skipped = skipped
 
         cluster.links = [link.Link.make_link('self', url,
                                              'clusters', cluster.uuid),
@@ -217,7 +241,9 @@ class Cluster(base.APIBase):
     @classmethod
     def convert_with_links(cls, rpc_cluster, expand=True):
         cluster = Cluster(**rpc_cluster.as_dict())
-        return cls._convert_with_links(cluster, pecan.request.host_url, expand)
+        parent_labels = rpc_cluster.cluster_template.labels
+        return cls._convert_with_links(cluster, pecan.request.host_url, expand,
+                                       parent_labels)
 
     @classmethod
     def sample(cls, expand=True):
@@ -474,6 +500,13 @@ class ClustersController(base.Controller):
         # If labels is not present, use cluster_template value
         if cluster.labels == wtypes.Unset:
             cluster.labels = cluster_template.labels
+        else:
+            # If labels are provided check if the user wishes to merge
+            # them with the values from the cluster template.
+            if cluster.merge_labels:
+                labels = cluster_template.labels
+                labels.update(cluster.labels)
+                cluster.labels = labels
 
         # If floating_ip_enabled is not present, use cluster_template value
         if cluster.floating_ip_enabled == wtypes.Unset:

magnum/api/controllers/v1/nodegroup.py

@@ -126,6 +126,24 @@ class NodeGroup(base.APIBase):
     version = wtypes.text
     """Version of the nodegroup"""
 
+    merge_labels = wsme.wsattr(types.boolean, default=False)
+    """Indicates whether the labels will be merged with the cluster labels."""
+
+    labels_overridden = wtypes.DictType(
+            wtypes.text, types.MultiType(
+                wtypes.text, six.integer_types, bool, float))
+    """Contains labels that have a value different than the parent labels."""
+
+    labels_added = wtypes.DictType(
+            wtypes.text, types.MultiType(
+                wtypes.text, six.integer_types, bool, float))
+    """Contains labels that do not exist in the parent."""
+
+    labels_skipped = wtypes.DictType(
+            wtypes.text, types.MultiType(
+                wtypes.text, six.integer_types, bool, float))
+    """Contains labels that exist in the parent but were not inherited."""
+
     def __init__(self, **kwargs):
         super(NodeGroup, self).__init__()
         self.fields = []
@@ -153,6 +171,14 @@ class NodeGroup(base.APIBase):
                         link.Link.make_link('bookmark', url,
                                             cluster_path, nodegroup_path,
                                             bookmark=True)]
+            cluster = api_utils.get_resource('Cluster', ng.cluster_id)
+
+            overridden, added, skipped = api_utils.get_labels_diff(
+                    cluster.labels, ng.labels)
+            ng.labels_overridden = overridden
+            ng.labels_added = added
+            ng.labels_skipped = skipped
+
         return ng
 
 
@@ -314,6 +340,13 @@ class NodeGroupController(base.Controller):
             nodegroup.flavor_id = cluster.flavor_id
         if nodegroup.labels is None or nodegroup.labels == wtypes.Unset:
             nodegroup.labels = cluster.labels
+        else:
+            # If labels are provided check if the user wishes to merge
+            # them with the values from the cluster.
+            if nodegroup.merge_labels:
+                labels = cluster.labels
+                labels.update(nodegroup.labels)
+                nodegroup.labels = labels
 
         nodegroup_dict = nodegroup.as_dict()
         nodegroup_dict['cluster_id'] = cluster.uuid

magnum/api/utils.py

@@ -136,3 +136,24 @@ def get_openstack_resource(manager, resource_ident, resource_type):
             raise exception.Conflict(msg)
         resource_data = matches[0]
     return resource_data
+
+
+def get_labels_diff(parent_labels, labels):
+    # Overriddent are the labels that exist in both the parent and the object
+    # but have a different value.
+    labels_overridden = {}
+    # Added are the labels that exist in the object and not in the parent.
+    labels_added = {}
+    # We consider as skipped, the labels that exist in the parent but not in
+    # the object's labels.
+    labels_skipped = {
+        k: v for k, v in parent_labels.items() if k not in labels
+    }
+    for key, value in labels.items():
+        try:
+            parent_value = parent_labels[key]
+            if parent_value != value:
+                labels_overridden[key] = parent_value
+        except KeyError:
+            labels_added[key] = value
+    return labels_overridden, labels_added, labels_skipped

magnum/tests/unit/api/controllers/v1/test_cluster.py

@@ -116,9 +116,27 @@ class TestListCluster(api_base.FunctionalTest):
     def test_get_one_by_uuid(self):
         temp_uuid = uuidutils.generate_uuid()
         obj_utils.create_test_cluster(self.context, uuid=temp_uuid)
-        response = self.get_json(
-            '/clusters/%s' % temp_uuid)
+        response = self.get_json('/clusters/%s' % temp_uuid)
         self.assertEqual(temp_uuid, response['uuid'])
+        self.assertIn('labels_overridden', response)
+        self.assertIn('labels_added', response)
+        self.assertIn('labels_skipped', response)
+
+    def test_get_one_merged_labels(self):
+        ct_uuid = uuidutils.generate_uuid()
+        ct_labels = {'label1': 'value1', 'label2': 'value2'}
+        obj_utils.create_test_cluster_template(self.context, uuid=ct_uuid,
+                                               labels=ct_labels)
+        c_uuid = uuidutils.generate_uuid()
+        c_labels = {'label1': 'value3', 'label4': 'value4'}
+        obj_utils.create_test_cluster(self.context, uuid=c_uuid,
+                                      labels=c_labels,
+                                      cluster_template_id=ct_uuid)
+        response = self.get_json('/clusters/%s' % c_uuid)
+        self.assertEqual(c_labels, response['labels'])
+        self.assertEqual({'label1': 'value1'}, response['labels_overridden'])
+        self.assertEqual({'label2': 'value2'}, response['labels_skipped'])
+        self.assertEqual({'label4': 'value4'}, response['labels_added'])
 
     def test_get_one_by_uuid_not_found(self):
         temp_uuid = uuidutils.generate_uuid()
@@ -911,6 +929,42 @@ class TestPost(api_base.FunctionalTest):
         # Verify flavor_id from ClusterTemplate is used
         self.assertEqual('m1.small', cluster[0].flavor_id)
 
+    def test_create_cluster_without_merge_labels(self):
+        self.cluster_template.labels = {'label1': 'value1', 'label2': 'value2'}
+        self.cluster_template.save()
+        cluster_labels = {'label2': 'value3', 'label4': 'value4'}
+        bdict = apiutils.cluster_post_data(labels=cluster_labels)
+        response = self.post_json('/clusters', bdict)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(202, response.status_int)
+        cluster, timeout = self.mock_cluster_create.call_args
+        self.assertEqual(cluster_labels, cluster[0].labels)
+
+    def test_create_cluster_with_merge_labels(self):
+        self.cluster_template.labels = {'label1': 'value1', 'label2': 'value2'}
+        self.cluster_template.save()
+        cluster_labels = {'label2': 'value3', 'label4': 'value4'}
+        bdict = apiutils.cluster_post_data(labels=cluster_labels,
+                                           merge_labels=True)
+        response = self.post_json('/clusters', bdict)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(202, response.status_int)
+        cluster, timeout = self.mock_cluster_create.call_args
+        expected = self.cluster_template.labels
+        expected.update(cluster_labels)
+        self.assertEqual(expected, cluster[0].labels)
+
+    def test_create_cluster_with_merge_labels_no_labels(self):
+        self.cluster_template.labels = {'label1': 'value1', 'label2': 'value2'}
+        self.cluster_template.save()
+        bdict = apiutils.cluster_post_data(merge_labels=True)
+        del bdict['labels']
+        response = self.post_json('/clusters', bdict)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(202, response.status_int)
+        cluster, timeout = self.mock_cluster_create.call_args
+        self.assertEqual(self.cluster_template.labels, cluster[0].labels)
+
 
 class TestDelete(api_base.FunctionalTest):
     def setUp(self):

magnum/tests/unit/api/controllers/v1/test_nodegroup.py

@@ -176,6 +176,45 @@ class TestListNodegroups(NodeGroupControllerTest):
         self.assertEqual(worker.name, response['name'])
         self._verify_attrs(self._nodegroup_attrs, response)
         self._verify_attrs(self._expanded_attrs, response)
+        self.assertEqual({}, response['labels_overridden'])
+        self.assertEqual({}, response['labels_skipped'])
+        self.assertEqual({}, response['labels_added'])
+
+    def test_get_one_non_default(self):
+        self.cluster.labels = {'label1': 'value1', 'label2': 'value2'}
+        self.cluster.save()
+        ng_name = 'non_default_ng'
+        ng_labels = {
+            'label1': 'value3', 'label2': 'value2', 'label4': 'value4'
+        }
+        db_utils.create_test_nodegroup(cluster_id=self.cluster.uuid,
+                                       name=ng_name, labels=ng_labels)
+        url = '/clusters/%s/nodegroups/%s' % (self.cluster.uuid, ng_name)
+        response = self.get_json(url)
+        self._verify_attrs(self._nodegroup_attrs, response)
+        self._verify_attrs(self._expanded_attrs, response)
+        self.assertEqual(ng_labels, response['labels'])
+        overridden_labels = {'label1': 'value1'}
+        self.assertEqual(overridden_labels, response['labels_overridden'])
+        self.assertEqual({'label4': 'value4'}, response['labels_added'])
+        self.assertEqual({}, response['labels_skipped'])
+
+    def test_get_one_non_default_skipped_labels(self):
+        self.cluster.labels = {'label1': 'value1', 'label2': 'value2'}
+        self.cluster.save()
+        ng_name = 'non_default_ng'
+        ng_labels = {'label1': 'value3', 'label4': 'value4'}
+        db_utils.create_test_nodegroup(cluster_id=self.cluster.uuid,
+                                       name=ng_name, labels=ng_labels)
+        url = '/clusters/%s/nodegroups/%s' % (self.cluster.uuid, ng_name)
+        response = self.get_json(url)
+        self._verify_attrs(self._nodegroup_attrs, response)
+        self._verify_attrs(self._expanded_attrs, response)
+        self.assertEqual(ng_labels, response['labels'])
+        overridden_labels = {'label1': 'value1'}
+        self.assertEqual(overridden_labels, response['labels_overridden'])
+        self.assertEqual({'label4': 'value4'}, response['labels_added'])
+        self.assertEqual({'label2': 'value2'}, response['labels_skipped'])
 
     def test_get_one_non_existent_ng(self):
         url = '/clusters/%s/nodegroups/not-here' % self.cluster.uuid
@@ -381,6 +420,45 @@ class TestPost(NodeGroupControllerTest):
         self.assertEqual('application/json', response.content_type)
         self.assertEqual(409, response.status_int)
 
+    def test_create_ng_with_labels(self):
+        cluster_labels = {'label1': 'value1', 'label2': 'value2'}
+        self.cluster.labels = cluster_labels
+        self.cluster.save()
+        ng_labels = {'label3': 'value3'}
+        ng_dict = apiutils.nodegroup_post_data(labels=ng_labels)
+        response = self.post_json(self.url, ng_dict)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(202, response.status_int)
+        (cluster, ng), _ = self.mock_ng_create.call_args
+        self.assertEqual(ng_labels, ng.labels)
+
+    def test_create_ng_with_merge_labels(self):
+        cluster_labels = {'label1': 'value1', 'label2': 'value2'}
+        self.cluster.labels = cluster_labels
+        self.cluster.save()
+        ng_labels = {'label1': 'value3', 'label4': 'value4'}
+        ng_dict = apiutils.nodegroup_post_data(labels=ng_labels,
+                                               merge_labels=True)
+        response = self.post_json(self.url, ng_dict)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(202, response.status_int)
+        (cluster, ng), _ = self.mock_ng_create.call_args
+        expected_labels = cluster.labels
+        expected_labels.update(ng_labels)
+        self.assertEqual(expected_labels, ng.labels)
+
+    def test_create_ng_with_merge_labels_no_labels(self):
+        cluster_labels = {'label1': 'value1', 'label2': 'value2'}
+        self.cluster.labels = cluster_labels
+        self.cluster.save()
+        ng_dict = apiutils.nodegroup_post_data(merge_labels=True)
+        ng_dict.pop('labels')
+        response = self.post_json(self.url, ng_dict)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(202, response.status_int)
+        (cluster, ng), _ = self.mock_ng_create.call_args
+        self.assertEqual(cluster.labels, ng.labels)
+
 
 class TestDelete(NodeGroupControllerTest):
 

magnum/tests/unit/api/utils.py

@@ -57,6 +57,7 @@ def cluster_post_data(**kw):
     kw.update({'for_api_use': True})
     cluster = utils.get_test_cluster(**kw)
     cluster['create_timeout'] = kw.get('create_timeout', 15)
+    cluster['merge_labels'] = kw.get('merge_labels', False)
     internal = cluster_controller.ClusterPatchType.internal_attrs()
     return remove_internal(cluster, internal)
 
@@ -102,4 +103,5 @@ def nodegroup_post_data(**kw):
                 '/created_at', '/updated_at', '/status', '/status_reason',
                 '/version', '/stack_id']
     nodegroup = utils.get_test_nodegroup(**kw)
+    nodegroup['merge_labels'] = kw.get('merge_labels', False)
     return remove_internal(nodegroup, internal)

releasenotes/notes/merge-labels-9ba7deffc5bb3c7f.yaml

@@ -0,0 +1,10 @@
+---
+features:
+  - |
+    A new boolean flag is introduced in the CLuster and Nodegroup create API
+    calls. Using this flag, users can override label values when clusters or
+    nodegroups are created without having to specify all the inherited values.
+    To do that, users have to specify the labels with their new values and use
+    the flag --merge-labels. At the same time, three new fields are added in
+    the cluster and nodegroup show outputs, showing the differences between the
+    actual and the iherited labels.