host-label validation via kubernetes-client

The host label key and value are changed to be stored in
separate fields in the database. A unique constraint
between the host and the label key is added.
The validation of the label key and value is performed in the
kubernetes client api instead of replicating the checking
in the host-label api. The error message from the kubernetes
client api is passed to the host-label api.

Story: 2002845
Task: 24936

Change-Id: I7e318d5e58602d33652d49452e35d99322565148

sysinv/cgts-client/cgts-client/cgtsclient/v1/label_shell.py

@@ -16,7 +16,7 @@ from cgtsclient.v1 import ihost as ihost_utils
 
 
 def _print_label_show(obj):
-    fields = ['uuid', 'host_uuid', 'label']
+    fields = ['uuid', 'host_uuid', 'label_key', 'label_value']
     data = [(f, getattr(obj, f, '')) for f in fields]
     utils.print_tuple_list(data)
 
@@ -30,8 +30,8 @@ def do_host_label_list(cc, args):
     host_label = cc.label.list(ihost.uuid)
     for i in host_label[:]:
         setattr(i, 'hostname', ihost.hostname)
-    field_labels = ['hostname', 'label', ]
-    fields = ['hostname', 'label', ]
+    field_labels = ['hostname', 'label key', 'label value']
+    fields = ['hostname', 'label_key', 'label_value']
     utils.print_list(host_label, fields, field_labels, sortby=1)
 
 
@@ -81,7 +81,7 @@ def do_host_label_remove(cc, args):
 def _find_host_label(cc, host, label):
     host_labels = cc.label.list(host.uuid)
     for lbl in host_labels:
-        if lbl.host_uuid == host.uuid and lbl.label.split('=')[0] == label:
+        if lbl.host_uuid == host.uuid and lbl.label_key == label:
             break
     else:
         lbl = None

sysinv/sysinv/sysinv/sysinv/api/controllers/v1/label.py

@@ -4,7 +4,6 @@
 #
 
 import pecan
-import re
 import wsme
 import wsmeext.pecan as wsme_pecan
 from pecan import rest
@@ -43,8 +42,11 @@ class Label(base.APIBase):
     uuid = types.uuid
     "Unique UUID for this label"
 
-    label = wtypes.text
-    "Represents a label assigned to the host"
+    label_key = wtypes.text
+    "Represents a label key assigned to the host"
+
+    label_value = wtypes.text
+    "Represents a label value assigned to the host"
 
     host_id = int
     "Represent the host_id the label belongs to"
@@ -65,7 +67,8 @@ class Label(base.APIBase):
         if not expand:
             label.unset_fields_except(['uuid',
                                        'host_uuid',
-                                       'label'])
+                                       'label_key',
+                                       'label_value'])
 
         # do not expose the id attribute
         label.host_id = wtypes.Unset
@@ -181,26 +184,6 @@ class LabelController(rest.RestController):
 
         return Label.convert_with_links(sp_label)
 
-    @staticmethod
-    def _check_label_validity(label):
-        """Perform checks on validity of label
-        """
-        expr = re.compile("([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]")
-        if not expr.match(label):
-            return False
-        return True
-
-    @staticmethod
-    def _check_duplicate_label(host, label_key):
-        """Perform checks whether label already exists
-        """
-        try:
-            pecan.request.dbapi.label_query(host.id, label_key)
-        except exception.HostLabelNotFoundByKey:
-            return None
-        raise exception.HostLabelAlreadyExists(host=host.hostname,
-                                               label=label_key)
-
     @cutils.synchronized(LOCK_NAME)
     @wsme_pecan.wsexpose(LabelCollection, types.uuid,
                          body=types.apidict)
@@ -213,32 +196,6 @@ class LabelController(rest.RestController):
         LOG.info("patch_data: %s" % body)
         host = objects.host.get_by_uuid(pecan.request.context, uuid)
 
-        new_records = []
-        for key, value in body.iteritems():
-            values = {
-                'host_id': host.id,
-                'label': "=".join([key, str(value)])
-            }
-            # syntax check
-            if not self._check_label_validity(values['label']):
-                msg = _("Label must consist of alphanumeric characters, "
-                        "'-', '_' or '.', and must start and end with an "
-                        "alphanumeric character with an optional DNS "
-                        "subdomain prefix and '/'")
-                raise wsme.exc.ClientSideError(msg)
-
-            # check for duplicate
-            self._check_duplicate_label(host, key)
-
-            try:
-                new_label = pecan.request.dbapi.label_create(uuid, values)
-            except exception.HostLabelAlreadyExists:
-                msg = _("Host label add failed: "
-                        "host %s label %s "
-                        % (host.hostname, values['label']))
-                raise wsme.exc.ClientSideError(msg)
-            new_records.append(new_label)
-
         try:
             pecan.request.rpcapi.update_kubernetes_label(
                 pecan.request.context,
@@ -246,19 +203,24 @@ class LabelController(rest.RestController):
                 body
             )
         except rpc_common.RemoteError as e:
-            # rollback
-            for p in new_records:
-                try:
-                    pecan.request.dbapi.label_destroy(p.uuid)
-                    LOG.warn(_("Rollback host label create: "
-                               "destroy uuid {}".format(p.uuid)))
-                except exception.SysinvException:
-                    pass
             raise wsme.exc.ClientSideError(str(e.value))
         except Exception as e:
             with excutils.save_and_reraise_exception():
                 LOG.exception(e)
 
+        new_records = []
+        for key, value in body.iteritems():
+            values = {
+                'host_id': host.id,
+                'label_key': key,
+                'label_value': value
+            }
+            try:
+                new_label = pecan.request.dbapi.label_create(uuid, values)
+                new_records.append(new_label)
+            except exception.HostLabelAlreadyExists:
+                pass
+
         return LabelCollection.convert_with_links(
             new_records, limit=None, url=None, expand=False,
             sort_key='id', sort_dir='asc')
@@ -272,7 +234,7 @@ class LabelController(rest.RestController):
 
         lbl_obj = objects.label.get_by_uuid(pecan.request.context, uuid)
         host = objects.host.get_by_uuid(pecan.request.context, lbl_obj.host_id)
-        label_dict = {lbl_obj.label.split('=')[0]: None}
+        label_dict = {lbl_obj.label_key: None}
 
         try:
             pecan.request.rpcapi.update_kubernetes_label(
@@ -288,6 +250,6 @@ class LabelController(rest.RestController):
         try:
             pecan.request.dbapi.label_destroy(lbl_obj.uuid)
         except exception.HostLabelNotFound:
-            msg = _("Delete host label failed: host %s label %s"
-                    % (host.hostname, lbl_obj.label.split('=')[0]))
+            msg = _("Delete host label failed: host %s label %s=%s"
+                    % (host.hostname, lbl_obj.label_key, lbl_obj.label_value))
             raise wsme.exc.ClientSideError(msg)

sysinv/sysinv/sysinv/sysinv/common/exception.py

@@ -1184,6 +1184,10 @@ class HostLabelNotFoundByKey(NotFound):
     message = _("Host label %(label)s could not be found.")
 
 
+class HostLabelInvalid(Invalid):
+    message = _("Host label is invalid. Reason: %(reason)s")
+
+
 class PickleableException(Exception):
     """
     Pickleable Exception

sysinv/sysinv/sysinv/sysinv/common/kubernetes.py

@@ -12,10 +12,14 @@
 """ System Inventory Kubernetes Utilities and helper functions."""
 
 from __future__ import absolute_import
+import httplib
+import json
 
 from kubernetes import config
 from kubernetes import client
 from kubernetes.client import Configuration
+from kubernetes.client.rest import ApiException
+from sysinv.common import exception
 from sysinv.openstack.common import log as logging
 
 LOG = logging.getLogger(__name__)
@@ -43,6 +47,10 @@ class KubeOperator(object):
         try:
             api_response = self._get_kubernetesclient().patch_node(name, body)
             LOG.debug("Response: %s" % api_response)
+        except ApiException as e:
+            if e.status == httplib.UNPROCESSABLE_ENTITY:
+                reason = json.loads(e.body).get('message', "")
+                raise exception.HostLabelInvalid(reason=reason)
         except Exception as e:
             LOG.error("Kubernetes exception: %s" % e)
             raise

sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/api.py

@@ -7446,9 +7446,9 @@ class Connection(api.Connection):
             except db_exc.DBDuplicateEntry:
                 LOG.error("Failed to add host label %s. "
                           "Already exists with this uuid" %
-                          (values['label']))
+                          (values['label_key']))
                 raise exception.HostLabelAlreadyExists(
-                    label=values['label'], host=values['host_uuid'])
+                    label=values['label_key'], host=values['host_uuid'])
             return self._label_get(values['uuid'])
 
     @objects.objectify(objects.label)
@@ -7499,16 +7499,16 @@ class Connection(api.Connection):
         return _paginate_query(models.Label, limit, marker,
                                sort_key, sort_dir, query)
 
-    def _label_query(self, host_id, values, session=None):
+    def _label_query(self, host_id, label_key, session=None):
         query = model_query(models.Label, session=session)
         query = query.filter(models.Label.host_id == host_id)
-        query = query.filter(models.Label.label.startswith(values))
+        query = query.filter(models.Label.label_key == label_key)
         try:
             result = query.one()
         except NoResultFound:
-            raise exception.HostLabelNotFoundByKey(label=values)
+            raise exception.HostLabelNotFoundByKey(label=label_key)
         return result
 
     @objects.objectify(objects.label)
-    def label_query(self, host_id, values):
-        return self._label_query(host_id, values)
+    def label_query(self, host_id, label_key):
+        return self._label_query(host_id, label_key)

sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/migrate_repo/versions/076_host_label.py

@@ -7,7 +7,7 @@
 
 from sqlalchemy import Column, MetaData, Table
 from sqlalchemy import DateTime, Integer, String
-from sqlalchemy import ForeignKey
+from sqlalchemy import ForeignKey, UniqueConstraint
 from sysinv.openstack.common import log
 
 ENGINE = 'InnoDB'
@@ -37,7 +37,9 @@ def upgrade(migrate_engine):
         Column('host_id', Integer, ForeignKey('i_host.id',
                                               ondelete='CASCADE')),
 
-        Column('label', String(255)),
+        Column('label_key', String(384)),
+        Column('label_value', String(128)),
+        UniqueConstraint('host_id', 'label_key', name='u_host_id@label_key'),
 
         mysql_engine=ENGINE,
         mysql_charset=CHARSET,

sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/models.py

@@ -1629,5 +1629,6 @@ class Label(Base):
     host_id = Column(Integer, ForeignKey('i_host.id',
                                          ondelete='CASCADE'))
     host = relationship("ihost", lazy="joined", join_depth=1)
-    label = Column(String(255))
-    UniqueConstraint('host_id', 'label', name='u_host_label')
+    label_key = Column(String(384))
+    label_value = Column(String(128))
+    UniqueConstraint('host_id', 'label_key', name='u_host_id@label_key')

sysinv/sysinv/sysinv/sysinv/objects/label.py

@@ -19,7 +19,8 @@ class Label(base.SysinvObject):
 
     fields = {
         'uuid': utils.str_or_none,
-        'label': utils.str_or_none,
+        'label_key': utils.str_or_none,
+        'label_value': utils.str_or_none,
         'host_id': utils.int_or_none,
         'host_uuid': utils.str_or_none,
     }