From e9d05ff9e82953f80e7304b0b23e5cffeadd956c Mon Sep 17 00:00:00 2001 From: Hans Arnholm Date: Sat, 16 May 2026 16:07:13 -0700 Subject: [PATCH 1/2] Conditionally set backendImages credentials For deployments that only use public images, and forego a key for NGC, it was difficuly to avoid image pull secrets. Even if a secret was not specified the config map would still contain ``` apiVersion: v1 data: config.yaml: | workflow: backend_images: credential: secretKey: .dockerconfigjson secretName: nvcr-secret ``` With the default values it was not easy to remove that configuration due to how Helm patches values. You would need to first set credential: null to delete the inherited map, then a second later file sets credential: {} Instead of changing the default values, which would be a breaking change, add a template that only adds the credential structure when needed. Signed-off-by: Hans Arnholm --- deployments/BUILD | 6 +++++ deployments/charts/BUILD | 9 ++++++++ .../charts/service/templates/_helpers.tpl | 23 ++++++++++++++++++- .../charts/service/templates/configs.yaml | 14 ++++++++++- 4 files changed, 50 insertions(+), 2 deletions(-) diff --git a/deployments/BUILD b/deployments/BUILD index fef4415f4..b4ff867d6 100644 --- a/deployments/BUILD +++ b/deployments/BUILD @@ -15,3 +15,9 @@ limitations under the License. SPDX-License-Identifier: Apache-2.0 """ + +filegroup( + name = "values", + srcs = glob(["values/**"]), + visibility = ["//visibility:public"], +) diff --git a/deployments/charts/BUILD b/deployments/charts/BUILD index ccb508b7a..09a4fd8f0 100644 --- a/deployments/charts/BUILD +++ b/deployments/charts/BUILD @@ -32,3 +32,12 @@ filegroup( srcs = glob(["quick-start/**"]), visibility = ["//visibility:public"], ) + +sh_test( + name = "service_public_registry_secret_render_test", + srcs = ["service/tests/public_registry_secret_render_test.sh"], + data = [ + ":service", + "//deployments:values", + ], +) diff --git a/deployments/charts/service/templates/_helpers.tpl b/deployments/charts/service/templates/_helpers.tpl index 0aab73267..193d353f0 100644 --- a/deployments/charts/service/templates/_helpers.tpl +++ b/deployments/charts/service/templates/_helpers.tpl @@ -178,17 +178,36 @@ OSMO_CONFIGMAP_NAME deliberately references services.service.serviceName {{- end }} {{- end -}} +{{/* +The minimal deploy values keep nvcr-secret as the private-registry default for +existing deployments. Public installs can omit that Secret; in that case, do +not render references that make pods wait on or configs load a missing Secret. +*/}} +{{- define "osmo.config-secret-ref-enabled" -}} +{{- $secretName := .secretName | default "" -}} +{{- $root := .root -}} +{{- $imagePullSecret := $root.Values.global.imagePullSecret | default "" -}} +{{- if and (eq $secretName "nvcr-secret") (ne $imagePullSecret $secretName) (not (lookup "v1" "Secret" $root.Release.Namespace $secretName)) -}} +false +{{- else -}} +true +{{- end -}} +{{- end -}} + {{- define "osmo.configmap-volume-mounts" -}} {{- if .Values.services.configs.enabled }} - name: configs mountPath: /etc/osmo/configs readOnly: true {{- range .Values.services.configs.secretRefs }} +{{- $secretName := .secretName | default "" }} +{{- if and $secretName (eq (include "osmo.config-secret-ref-enabled" (dict "root" $ "secretName" $secretName) | trim) "true") }} - name: secret-{{ .secretName }} mountPath: /etc/osmo/secrets/{{ .secretName }} readOnly: true {{- end }} {{- end }} +{{- end }} {{- end -}} {{- define "osmo.configmap-volumes" -}} @@ -197,10 +216,12 @@ OSMO_CONFIGMAP_NAME deliberately references services.service.serviceName configMap: name: {{ .Values.services.service.serviceName }}-configs {{- range .Values.services.configs.secretRefs }} +{{- $secretName := .secretName | default "" }} +{{- if and $secretName (eq (include "osmo.config-secret-ref-enabled" (dict "root" $ "secretName" $secretName) | trim) "true") }} - name: secret-{{ .secretName }} secret: secretName: {{ .secretName }} {{- end }} {{- end }} +{{- end }} {{- end -}} - diff --git a/deployments/charts/service/templates/configs.yaml b/deployments/charts/service/templates/configs.yaml index 1a1a0b98e..e1d2f928a 100644 --- a/deployments/charts/service/templates/configs.yaml +++ b/deployments/charts/service/templates/configs.yaml @@ -33,7 +33,19 @@ data: {{- end }} {{- if $cfg.workflow }} workflow: - {{- toYaml $cfg.workflow | nindent 6 }} + {{- $workflow := deepCopy $cfg.workflow }} + {{- $backendImages := get $workflow "backend_images" | default dict }} + {{- if kindIs "map" $backendImages }} + {{- $credential := get $backendImages "credential" | default dict }} + {{- if kindIs "map" $credential }} + {{- $secretName := get $credential "secretName" | default "" }} + {{- if and $secretName (eq (include "osmo.config-secret-ref-enabled" (dict "root" $ "secretName" $secretName) | trim) "false") }} + {{- $_ := set $backendImages "credential" dict }} + {{- $_ := set $workflow "backend_images" $backendImages }} + {{- end }} + {{- end }} + {{- end }} + {{- toYaml $workflow | nindent 6 }} {{- end }} {{- if $cfg.dataset }} dataset: From 4a05081e1957b6ed1eacd9b9d5721af052bcaa09 Mon Sep 17 00:00:00 2001 From: Vivian Pan Date: Mon, 18 May 2026 16:36:24 -0700 Subject: [PATCH 2/2] =?UTF-8?q?Revert=20configs.yaml=20strip=20=E2=80=94?= =?UTF-8?q?=20primary=20gate=20is=20=5Fhelpers.tpl?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The _helpers.tpl `osmo.config-secret-ref-enabled` gate is the load- bearing fix: it prevents `secret-nvcr-secret` volume + volumeMount from rendering when nvcr-secret isn't the imagePullSecret and the secret doesn't exist. That alone fixes the FailedMount + pod-start failure mode this PR targets. The configs.yaml `deepCopy`+`set` logic was a secondary measure stripping the rendered backend_images.credential block in the same case. Without it, the OSMO config loader inside the pod sees the credential reference, attempts to read /etc/osmo/secrets// .dockerconfigjson, gets an OSError because the volume was skipped, logs it, and proceeds with an empty RegistryCredential. Pod-spec generator then skips creating an OSMO-side docker auth (checks osmo_cred.registry / .username / .auth — all empty), and workflow pods pull anonymously. Functionally equivalent to the strip; the only delta is one log line per service. Tradeoffs in keeping the strip: - `lookup` only works at install time, not `helm template` / dry- run, so the rendered output differs by render mode. - The deepCopy + dict manipulation is verbose helm-template logic to maintain. - The skill is the canonical entry point for minimal deploys; it already gates `--set backend_images.credential.*` on the caller opting into NGC plumbing (vpan/microk8s-public-image-fixes), so the configs.yaml strip is unreachable from that path. Keep the _helpers.tpl gate, drop the configs.yaml strip. Co-Authored-By: Claude Opus 4.7 (1M context) --- deployments/charts/service/templates/configs.yaml | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/deployments/charts/service/templates/configs.yaml b/deployments/charts/service/templates/configs.yaml index e1d2f928a..1a1a0b98e 100644 --- a/deployments/charts/service/templates/configs.yaml +++ b/deployments/charts/service/templates/configs.yaml @@ -33,19 +33,7 @@ data: {{- end }} {{- if $cfg.workflow }} workflow: - {{- $workflow := deepCopy $cfg.workflow }} - {{- $backendImages := get $workflow "backend_images" | default dict }} - {{- if kindIs "map" $backendImages }} - {{- $credential := get $backendImages "credential" | default dict }} - {{- if kindIs "map" $credential }} - {{- $secretName := get $credential "secretName" | default "" }} - {{- if and $secretName (eq (include "osmo.config-secret-ref-enabled" (dict "root" $ "secretName" $secretName) | trim) "false") }} - {{- $_ := set $backendImages "credential" dict }} - {{- $_ := set $workflow "backend_images" $backendImages }} - {{- end }} - {{- end }} - {{- end }} - {{- toYaml $workflow | nindent 6 }} + {{- toYaml $cfg.workflow | nindent 6 }} {{- end }} {{- if $cfg.dataset }} dataset: