Merge pull request #212 from Cray-HPE/CASMCMS-8799

CASMCMS-8799: Create v2-compatible session templates with v1 endpoint

CHANGELOG.md

@@ -9,9 +9,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 - Removed non-v2 fields from v1 session template template
 - Provide more useful example values in v1 and v2 session template templates
+- Modify v1 session template create function to strip away v1-specific fields and create a v2-compatible
+  session template.
 - Update API spec to reflect that no v1-format session template will exist inside BOS, because the
   v1 session template creation endpoint will strip v1-specific fields and create a v2-format session template,
   and even the v1 session template template endpoint will return a v2-compatible example template.
+- Update BOS migration code to properly convert v1 session templates to v2, both from old Etcd database and within
+  current redis DB.
 
 ## [2.6.3] - 08-22-2023
 ### Changed

api/openapi.yaml.in

@@ -1961,7 +1961,10 @@ paths:
       - $ref: '#/components/parameters/V1TenantHeaderParam'
     post:
       summary: Create Session Template
-      description: Create a new Session Template.
+      description: |
+        Create a new Session Template.
+
+        The created template will be modified if necessary to follow the BOS v2 session template format.
       tags:
         - sessiontemplate
       x-openapi-router-controller: bos.server.controllers.v1.sessiontemplate

src/bos/server/controllers/v1/sessiontemplate.py

@@ -30,7 +30,8 @@
 
 from bos.common.tenant_utils import no_v1_multi_tenancy_support
 from bos.server import redis_db_utils as dbutils
-from bos.server.models.v1_session_template import V1SessionTemplate as SessionTemplate  # noqa: E501
+from bos.server.models.v1_session_template import V1SessionTemplate  # noqa: E501
+from bos.server.models.v2_session_template import V2SessionTemplate  # noqa: E501
 from bos.server.utils import _canonize_xname
 from bos.common.tenant_utils import get_tenant_aware_key
 from ..v2.sessiontemplates import get_v2_sessiontemplate, get_v2_sessiontemplates, delete_v2_sessiontemplate
@@ -55,6 +56,8 @@
         "configuration": "desired-cfs-config"},
     "enable_cfs": True}
 
+V1_SPECIFIC_ST_FIELDS = [ "cfs_url", "cfs_branch", "partition" ]
+V1_SPECIFIC_BOOTSET_FIELDS = [ "network", "boot_ordinal", "shutdown_ordinal" ]
 
 def sanitize_xnames(st_json):
     """
@@ -75,6 +78,33 @@ def sanitize_xnames(st_json):
                 st_json['boot_sets'][boot_set]['node_list'] = clean_nl
     return st_json
 
+def strip_v1_only_fields(template_data):
+    """
+    Edits in-place the template data, removing any fields which are specific to BOS v1.
+    Returns True if any changes were made.
+    Returns False if nothing was removed.
+    """
+    changes_made=False
+    # Strip out the v1-specific fields from the dictionary
+    for v1_field_name in V1_SPECIFIC_ST_FIELDS:
+        try:
+            del template_data[v1_field_name]
+            changes_made=True
+        except KeyError:
+            pass
+
+    # Do the same for each boot set    
+    # Oddly, boot_sets is not a required field, so only do this if it is present
+    if "boot_sets" in template_data:
+        for bs in template_data["boot_sets"].values():
+            for v1_bs_field_name in V1_SPECIFIC_BOOTSET_FIELDS:
+                try:
+                    del bs[v1_bs_field_name]
+                    changes_made=True
+                except KeyError:
+                    pass
+
+    return changes_made
 
 @no_v1_multi_tenancy_support
 @dbutils.redis_error_handler
@@ -94,31 +124,52 @@ def create_v1_sessiontemplate():  # noqa: E501
     sessiontemplate = None
 
     try:
-        """Convert the JSON request data into a SessionTemplate object.
+        """Verify that we can convert the JSON request data into a
+           V1SessionTemplate object.
            Any exceptions caught here would be generated from the model
            (i.e. bos.server.models.session_template). Examples are
            an exception for a session template missing the required name
            field, or an exception for a session template name that does not
            confirm to Kubernetes naming convention.
            In this case return 400 with a description of the specific error.
         """
-        sessiontemplate = SessionTemplate.from_dict(connexion.request.get_json())
+        template_data = connexion.request.get_json()
+        V1SessionTemplate.from_dict(template_data)
     except Exception as err:
         return connexion.problem(
             status=400, title="The session template could not be created.",
             detail=str(err))
 
-    """For now overwrite any existing template by name w/o warning.
-       Later this can be changed when we support patching operations.
-       This could also be changed to result in an HTTP 409 Conflict. TBD.
-    """
-    LOGGER.debug("create_v1_sessiontemplate name: %s", sessiontemplate.name)
-    st_json = connexion.request.get_json()
+    strip_v1_only_fields(template_data)
+
+    # BOS v2 doesn't want the session template name inside the dictionary itself
+    # name is a required v1 field, though, so we can safely pop it here
+    session_template_id = template_data.pop("name")
+
+    # Now basically follow the same process as when creating a V2 session template (except in the end,
+    # if successful, we will return 201 status and the name of the template, to match the v1 API spec)
+    try:
+        """Verify that we can convert the JSON request data into a
+           V2SessionTemplate object.
+           Any exceptions caught here would be generated from the model
+           (i.e. bos.server.models.session_template).
+           An example is an exception for a session template name that
+           does not conform to Kubernetes naming convention.
+           In this case return 400 with a description of the specific error.
+        """
+        V2SessionTemplate.from_dict(template_data)
+    except Exception as err:
+        return connexion.problem(
+            status=400, title="The session template could not be created as a v2 template.",
+            detail=str(err))
+
+    template_data = sanitize_xnames(template_data)
+    template_data['name'] = session_template_id
     # Tenants are not used in v1, but v1 and v2 share template storage
-    st_json["tenant"] = ""
-    template_key = get_tenant_aware_key(sessiontemplate.name, "")
-    DB.put(template_key, st_json)
-    return sessiontemplate.name, 201
+    template_data['tenant'] = ""
+    template_key = get_tenant_aware_key(session_template_id, "")
+    DB.put(template_key, template_data)
+    return session_template_id, 201
 
 
 @no_v1_multi_tenancy_support

src/bos/server/migrations.py

@@ -26,6 +26,7 @@
 import logging
 import os
 
+from bos.server.controllers.v1.sessiontemplate import strip_v1_only_fields
 from bos.server.dbclient import BosEtcdClient
 from bos.common.utils import requests_retry_session
 from bos.common.tenant_utils import get_tenant_aware_key
@@ -62,9 +63,11 @@ def pod_ip():
     pods = v1.list_namespaced_pod("services",
                                   label_selector=f"app.kubernetes.io/name=cray-bos,app.kubernetes.io/version={version}")
     # Get the pod's IP address
-    if pods:
-        pod_ip = pods.items[0].status.pod_ip
-    return pod_ip
+    if pods and pods.items:
+        return pods.items[0].status.pod_ip
+    msg = "Could not determine BOS pod IP address. Aborting."
+    LOGGER.error(msg)
+    raise ValueError(msg)
 
 
 def convert_v1_to_v2(v1_st):
@@ -84,8 +87,7 @@ def convert_v1_to_v2(v1_st):
                    exception.
     """
     session_template_keys = ['name', 'description',
-                             'enable_cfs', 'cfs', 'partition',
-                             'boot_sets', 'links']
+                             'enable_cfs', 'cfs', 'boot_sets', 'links']
     boot_set_keys = ['name', 'path', 'type', 'etag', 'kernel_parameters',
                      'node_list', 'node_roles_groups', 'node_groups',
                      'rootfs_provider', 'rootfs_provider_passthrough']
@@ -97,7 +99,7 @@ def convert_v1_to_v2(v1_st):
         raise MissingName()
     for k, v in v1_st.items():
         if k in session_template_keys:
-            if k != "boot_sets" and k != "name":
+            if k != "boot_sets" and k != "name" and k!= "links":
                 v2_st[k] = v
         else:
             LOGGER.warning("Discarding attribute: '{}' from session template: '{}'".format(k, v1_st['name']))
@@ -114,7 +116,7 @@ def convert_v1_to_v2(v1_st):
     return v2_st, name
 
 
-def migrate_v1_to_v2_session_templates():
+def migrate_v1_etcd_to_v2_redis_session_templates():
     """
     Read the session templates out of the V1 etcd key/value store and
     write them into the v2 Redis database.
@@ -153,20 +155,23 @@ def migrate_v1_to_v2_session_templates():
                                  "to error: {}".format(v1_st['name'],
                                                        response.reason))
                     LOGGER.error("Error specifics: {}".format(response.text))
-
-                v1_st["name"] = v1_st["name"] + "_v1_deprecated"
-                response = session.post("{}".format(st_v1_endpoint), json=v1_st)
-                if not response.ok:
-                    LOGGER.error("Session template: '{}' was not migrated for v1 due "
-                                 "to error: {}".format(v1_st['name'],
-                                                       response.reason))
-                    LOGGER.error("Error specifics: {}".format(response.text))
             else:
                 LOGGER.error("Session template: '{}' was not migrated due "
                                  "to error: {}".format(v1_st['name'],
                                                        response.reason))
                 LOGGER.error("Error specifics: {}".format(response.text))
 
+# Convert existing v1 session templates to v2 format
+def convert_v1_to_v2_session_templates():
+    db=dbutils.get_wrapper(db='session_templates')
+    response = db.get_keys()
+    for st_key in response:
+        data = db.get(st_key)
+        if strip_v1_only_fields(data):
+            name = data.get("name")
+            LOGGER.info(f"Converting {name} to BOS v2")
+            db.put(st_key, data)
+
 # Multi-tenancy key migrations
 
 def migrate_database(db):
@@ -188,8 +193,9 @@ def migrate_to_tenant_aware_keys():
 
 
 def perform_migrations():
-    migrate_v1_to_v2_session_templates()
+    migrate_v1_etcd_to_v2_redis_session_templates()
     migrate_to_tenant_aware_keys()
+    convert_v1_to_v2_session_templates()
 
 
 if __name__ == "__main__":