Skip to content

Conversation

@ddelnano
Copy link
Member

@ddelnano ddelnano commented Dec 19, 2024

Summary: Ensure helm release build consistently handles deployOLM toggle

Splitting the csv-deleter Job into its own file in #2063 introduced a bug where the px-operator namespace becomes stuck on helm uninstall again. This is because helm's deployOLM configuration key is patched for the 00_olm.yaml file during the release build. When I initially tested in #2059, the csv-deleter Job existed in the file and received this special treatment, but after #2063 it no longer did.

In order to address this, this change introduces a Go template comment placeholder that sed will look to make it clear that helm operates with a different conditional check. It also runs this sed command across all template files and not just the 00_olm.yaml file. This comment will be stripped out during the px cli's yaml templating and will leave it's deployOLM conditional check intact.

Relevant Issues: #1917

Type of change: /kind bug

Test Plan: Ran the sed command and verified that it works

# Verify bash variables are set so sed command can be copy and pasted from this PR
$ echo $repo_path
./
$ echo $helm_tmpl_checks
{{- $olmCRDFound := false }} \ {{- $nsLookup := len (lookup "v1" "Namespace" "" "") }} \ {{- range $index, $crdLookup := (lookup "apiextensions.k8s.io/v1" "CustomResourceDefinition" "" "").items -}}{{ if eq $crdLookup.metadata.name "operators.operators.coreos.com"}}{{ $olmCRDFound = true }}{{ end }}{{end}} \ {{ if and (not $olmCRDFound) (not (eq $nsLookup 0))}}{{ fail "CRDs missing! Please deploy CRDs from https://github.com/pixie-io/pixie/tree/main/k8s/operator/helm/crds to continue with deploy." }}{{end}} \ {{- $lookupLen := 0 -}}{{- $opLookup := (lookup "operators.coreos.com/v1" "OperatorGroup" "" "").items -}}{{if $opLookup }}{{ $lookupLen = len $opLookup }}{{ end }}\n{{ if (or (eq (.Values.deployOLM | toString) "true") (and (not (eq (.Values.deployOLM | toString) "false")) (eq $lookupLen 0))) }}

# Run copy and pasted sed command
$ find "${repo_path}/k8s/operator/helm/templates" -type f -exec sed -i "/HELM_DEPLOY_OLM_PLACEHOLDER/c\\${helm_tmpl_checks}" {} \;

$ git diff
diff --git a/k8s/operator/helm/templates/00_olm.yaml b/k8s/operator/helm/templates/00_olm.yaml
index 381c8569d..cd4ccafb2 100644
--- a/k8s/operator/helm/templates/00_olm.yaml
+++ b/k8s/operator/helm/templates/00_olm.yaml
@@ -1,4 +1,9 @@
-{{if .Values.deployOLM}}{{- /* HELM_DEPLOY_OLM_PLACEHOLDER */ -}}
+{{- $olmCRDFound := false }}
+{{- $nsLookup := len (lookup "v1" "Namespace" "" "") }}
+{{- range $index, $crdLookup := (lookup "apiextensions.k8s.io/v1" "CustomResourceDefinition" "" "").items -}}{{ if eq $crdLookup.metadata.name "operators.operators.coreos.com"}}{{ $olmCRDFound = true }}{{ end }}{{end}}
+{{ if and (not $olmCRDFound) (not (eq $nsLookup 0))}}{{ fail "CRDs missing! Please deploy CRDs from https://github.com/pixie-io/pixie/tree/main/k8s/operator/helm/crds to continue with deploy." }}{{end}}
+{{- $lookupLen := 0 -}}{{- $opLookup := (lookup "operators.coreos.com/v1" "OperatorGroup" "" "").items -}}{{if $opLookup }}{{ $lookupLen = len $opLookup }}{{ end }}
+{{ if (or (eq (.Values.deployOLM | toString) "true") (and (not (eq (.Values.deployOLM | toString) "false")) (eq $lookupLen 0))) }}
 {{ if not (eq .Values.olmNamespace .Release.Namespace) }}
 ---
 apiVersion: v1
diff --git a/k8s/operator/helm/templates/csv-deleter.yaml b/k8s/operator/helm/templates/csv-deleter.yaml
index f785a63e7..5c0efb20e 100644
--- a/k8s/operator/helm/templates/csv-deleter.yaml
+++ b/k8s/operator/helm/templates/csv-deleter.yaml
@@ -1,4 +1,9 @@
-{{if .Values.deployOLM}}{{- /* HELM_DEPLOY_OLM_PLACEHOLDER */ -}}
+{{- $olmCRDFound := false }}
+{{- $nsLookup := len (lookup "v1" "Namespace" "" "") }}
+{{- range $index, $crdLookup := (lookup "apiextensions.k8s.io/v1" "CustomResourceDefinition" "" "").items -}}{{ if eq $crdLookup.metadata.name "operators.operators.coreos.com"}}{{ $olmCRDFound = true }}{{ end }}{{end}}
+{{ if and (not $olmCRDFound) (not (eq $nsLookup 0))}}{{ fail "CRDs missing! Please deploy CRDs from https://github.com/pixie-io/pixie/tree/main/k8s/operator/helm/crds to continue with deploy." }}{{end}}
+{{- $lookupLen := 0 -}}{{- $opLookup := (lookup "operators.coreos.com/v1" "OperatorGroup" "" "").items -}}{{if $opLookup }}{{ $lookupLen = len $opLookup }}{{ end }}
+{{ if (or (eq (.Values.deployOLM | toString) "true") (and (not (eq (.Values.deployOLM | toString) "false")) (eq $lookupLen 0))) }}
 ---
 apiVersion: batch/v1
 kind: Job

…bug caused by separating out csv-deleter Job

Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
@ddelnano ddelnano requested review from a team as code owners December 19, 2024 03:47
@ddelnano ddelnano merged commit ae80b43 into pixie-io:main Dec 19, 2024
33 checks passed
@ddelnano ddelnano deleted the ddelnano/fix-csv-deleter-bug-and-clean-up-helm-template branch December 19, 2024 04:15
ddelnano added a commit to ddelnano/pixie that referenced this pull request Aug 6, 2025
…e-io#2066)

Summary: Ensure helm release build consistently handles deployOLM toggle

Splitting the csv-deleter Job into its own file in pixie-io#2063 introduced a
bug where the `px-operator` namespace becomes stuck on `helm uninstall`
again. This is because helm's `deployOLM` configuration key is patched
for the [00_olm.yaml
file](https://github.com/pixie-io/pixie/blob/5c5e9dcb259c6548c9ab2c4b31c80d31f5f90124/ci/operator_helm_build_release.sh#L60)
during the release build. When I initially tested in
pixie-io#2059, the csv-deleter Job existed
in the file and received this special treatment, but after pixie-io#2063 it no
longer did.

In order to address this, this change introduces a Go template comment
placeholder that `sed` will look to make it clear that helm operates
with a different conditional check. It also runs this sed command across
all template files and not just the `00_olm.yaml` file. This comment
will be stripped out during the `px` cli's yaml templating and will
leave it's `deployOLM` conditional check intact.

Relevant Issues: pixie-io#1917

Type of change: /kind bug

Test Plan: Ran the sed command and verified that it works
```
# Verify bash variables are set so sed command can be copy and pasted from this PR
$ echo $repo_path
./
$ echo $helm_tmpl_checks
{{- $olmCRDFound := false }} \ {{- $nsLookup := len (lookup "v1" "Namespace" "" "") }} \ {{- range $index, $crdLookup := (lookup "apiextensions.k8s.io/v1" "CustomResourceDefinition" "" "").items -}}{{ if eq $crdLookup.metadata.name "operators.operators.coreos.com"}}{{ $olmCRDFound = true }}{{ end }}{{end}} \ {{ if and (not $olmCRDFound) (not (eq $nsLookup 0))}}{{ fail "CRDs missing! Please deploy CRDs from https://github.com/pixie-io/pixie/tree/main/k8s/operator/helm/crds to continue with deploy." }}{{end}} \ {{- $lookupLen := 0 -}}{{- $opLookup := (lookup "operators.coreos.com/v1" "OperatorGroup" "" "").items -}}{{if $opLookup }}{{ $lookupLen = len $opLookup }}{{ end }}\n{{ if (or (eq (.Values.deployOLM | toString) "true") (and (not (eq (.Values.deployOLM | toString) "false")) (eq $lookupLen 0))) }}

# Run copy and pasted sed command
$ find "${repo_path}/k8s/operator/helm/templates" -type f -exec sed -i "/HELM_DEPLOY_OLM_PLACEHOLDER/c\\${helm_tmpl_checks}" {} \;

$ git diff
diff --git a/k8s/operator/helm/templates/00_olm.yaml b/k8s/operator/helm/templates/00_olm.yaml
index 381c856..cd4ccafb2 100644
--- a/k8s/operator/helm/templates/00_olm.yaml
+++ b/k8s/operator/helm/templates/00_olm.yaml
@@ -1,4 +1,9 @@
-{{if .Values.deployOLM}}{{- /* HELM_DEPLOY_OLM_PLACEHOLDER */ -}}
+{{- $olmCRDFound := false }}
+{{- $nsLookup := len (lookup "v1" "Namespace" "" "") }}
+{{- range $index, $crdLookup := (lookup "apiextensions.k8s.io/v1" "CustomResourceDefinition" "" "").items -}}{{ if eq $crdLookup.metadata.name "operators.operators.coreos.com"}}{{ $olmCRDFound = true }}{{ end }}{{end}}
+{{ if and (not $olmCRDFound) (not (eq $nsLookup 0))}}{{ fail "CRDs missing! Please deploy CRDs from https://github.com/pixie-io/pixie/tree/main/k8s/operator/helm/crds to continue with deploy." }}{{end}}
+{{- $lookupLen := 0 -}}{{- $opLookup := (lookup "operators.coreos.com/v1" "OperatorGroup" "" "").items -}}{{if $opLookup }}{{ $lookupLen = len $opLookup }}{{ end }}
+{{ if (or (eq (.Values.deployOLM | toString) "true") (and (not (eq (.Values.deployOLM | toString) "false")) (eq $lookupLen 0))) }}
 {{ if not (eq .Values.olmNamespace .Release.Namespace) }}
 ---
 apiVersion: v1
diff --git a/k8s/operator/helm/templates/csv-deleter.yaml b/k8s/operator/helm/templates/csv-deleter.yaml
index f785a63..5c0efb20e 100644
--- a/k8s/operator/helm/templates/csv-deleter.yaml
+++ b/k8s/operator/helm/templates/csv-deleter.yaml
@@ -1,4 +1,9 @@
-{{if .Values.deployOLM}}{{- /* HELM_DEPLOY_OLM_PLACEHOLDER */ -}}
+{{- $olmCRDFound := false }}
+{{- $nsLookup := len (lookup "v1" "Namespace" "" "") }}
+{{- range $index, $crdLookup := (lookup "apiextensions.k8s.io/v1" "CustomResourceDefinition" "" "").items -}}{{ if eq $crdLookup.metadata.name "operators.operators.coreos.com"}}{{ $olmCRDFound = true }}{{ end }}{{end}}
+{{ if and (not $olmCRDFound) (not (eq $nsLookup 0))}}{{ fail "CRDs missing! Please deploy CRDs from https://github.com/pixie-io/pixie/tree/main/k8s/operator/helm/crds to continue with deploy." }}{{end}}
+{{- $lookupLen := 0 -}}{{- $opLookup := (lookup "operators.coreos.com/v1" "OperatorGroup" "" "").items -}}{{if $opLookup }}{{ $lookupLen = len $opLookup }}{{ end }}
+{{ if (or (eq (.Values.deployOLM | toString) "true") (and (not (eq (.Values.deployOLM | toString) "false")) (eq $lookupLen 0))) }}
 ---
 apiVersion: batch/v1
 kind: Job

```

Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
GitOrigin-RevId: ae80b43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants