HPCC-32657 review changes 4

Signed-off-by: Jake Smith <jake.smith@lexisnexisrisk.com>
hpcc-systems · Sep 27, 2024 · 1c9223b · 1c9223b
1 parent 8ffeb78
commit 1c9223b
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 22 deletions.
diff --git a/helm/hpcc/templates/_helpers.tpl b/helm/hpcc/templates/_helpers.tpl
@@ -819,12 +819,9 @@ Generates image information into env. variables used at runtime for runtime plat
 {{- $baseImageVersion := "" -}}
 {{- if .me.image -}}
   {{- $baseImageRootName = printf "%s/%s" (.me.image.root | default .root.Values.global.image.root | default "hpccsystems") (.me.image.name | default .root.Values.global.image.name | default "platform-core") -}}
-{{- else -}}
-  {{- $baseImageRootName = printf "%s/%s" (.root.Values.global.image.root | default "hpccsystems") (.root.Values.global.image.name | default "platform-core") -}}
-{{- end -}}
-{{- if .me.image -}}
   {{- $baseImageVersion = .me.image.version | default .root.Values.global.image.version | default .root.Chart.Version -}}
 {{- else -}}
+  {{- $baseImageRootName = printf "%s/%s" (.root.Values.global.image.root | default "hpccsystems") (.root.Values.global.image.name | default "platform-core") -}}
   {{- $baseImageVersion = .root.Values.global.image.version | default .root.Chart.Version -}}
 {{- end }}
 - name: baseImageRootName

diff --git a/system/jlib/jcontainerized.cpp b/system/jlib/jcontainerized.cpp
@@ -222,36 +222,38 @@ bool applyYaml(const char *componentName, const char *wuid, const char *job, con
 
     VStringBuffer args("\"--workunit=%s\"", wuid);
     args.append(" \"--k8sJob=true\"");
-    const char *runtimeImageVersion = getenv("baseImageVersion"); // runtime image version will equal base version unless changed dynamically below
+    const char *baseImageVersion = getenv("baseImageVersion");
+    const char *runtimeImageVersion = baseImageVersion; // runtime image version will equal base version unless changed dynamically below
     for (const auto &p: extraParams)
     {
         // special handling _HPCC_JOB_VERSION_, not just a straight substitution
-        if (streq(p.first.c_str(), "_HPCC_JOB_VERSION_"))
+        if (streq(p.first.c_str(), "_HPCC_JOB_VERSION_") && !isEmptyString(baseImageVersion)) // NB: if baseImageVersion is empty implies incompatible helm chart/runtime image mismatch
         {
-            // locates "image: <baseImageRootName>:<baseImageVersion>" and replaces with "image: <baseImageRootName>:<p.second>"
-            const char *baseImageRootName = getenv("baseImageRootName");
-            if (!isEmptyString(baseImageRootName) && !isEmptyString(runtimeImageVersion))
+            const char *newVersion = p.second.c_str();
+            if (!isEmptyString(newVersion))
             {
-                VStringBuffer oriImagePatternSpec("image: %s:%s", baseImageRootName, runtimeImageVersion);
-                VStringBuffer newImagePatternSpec("image: %s:%s", baseImageRootName, p.second.c_str());
-                jobYaml.replaceString(oriImagePatternSpec, newImagePatternSpec);
-                DBGLOG("Job image version changed from '%s' to '%s'", runtimeImageVersion, p.second.c_str());
-                runtimeImageVersion = p.second.c_str(); // used to substitute _HPCC_JOB_VERSION_ in jobYaml (used in runtimeImageVersion env variable)
+                // locates "image: <baseImageRootName>:<baseImageVersion>" and replaces with "image: <baseImageRootName>:<p.second>"
+                const char *baseImageRootName = getenv("baseImageRootName");
+                if (!isEmptyString(baseImageRootName)) // NB: should never be empty (given baseImageVersion is not empty)
+                {
+                    VStringBuffer oriImagePatternSpec("image: %s:%s", baseImageRootName, baseImageVersion);
+                    VStringBuffer newImagePatternSpec("image: %s:%s", baseImageRootName, newVersion);
+                    jobYaml.replaceString(oriImagePatternSpec, newImagePatternSpec);
+                    DBGLOG("Job image version changed from '%s' to '%s'", baseImageVersion, newVersion);
+                    runtimeImageVersion = newVersion; // used to substitute _HPCC_JOB_VERSION_ in jobYaml (in runtimeImageVersion env variable)
+                }
             }
         }
         else if (hasPrefix(p.first.c_str(), "_HPCC_", false)) // job yaml substitution
             jobYaml.replaceString(p.first.c_str(), p.second.c_str());
         else
             args.append(" \"--").append(p.first.c_str()).append('=').append(p.second.c_str()).append("\"");
     }
-    // always substitute _HPCC_JOB_VERSION_ - runtimeImageVersion is either the original baseImageVersion or the one from _HPCC_JOB_VERSION_ from above)
-    if (isEmptyString(runtimeImageVersion)) // if it were empty, it implies there's an incompatible helm chart/runtime image mismatch
-    {
-        // we replace _HPCC_JOB_VERSION_ (helm env runtimeImageVersion) with an empty string
-        // so that the engine knows it can't use it.
-        runtimeImageVersion = "";
-    }
-    jobYaml.replaceString("_HPCC_JOB_VERSION_", runtimeImageVersion);
+    // always substitute _HPCC_JOB_VERSION_ - (as long as runtimeImageVersion is set)
+    // It is either the original baseImageVersion or the one from _HPCC_JOB_VERSION_ from above
+    // If it is empty, it implies a helm chart/runtime image mismatch
+    if (!isEmptyString(runtimeImageVersion))
+        jobYaml.replaceString("_HPCC_JOB_VERSION_", runtimeImageVersion);
     jobYaml.replaceString("_HPCC_ARGS_", args.str());
 
     // retrySecs=0 - I am not sure want to retry this command systematically..