fix(chart): RBAC settings for job patch finalizers (#2239)

SeleniumHQ · May 1, 2024 · 07e13f6 · 07e13f6
1 parent 0762e87
commit 07e13f6
Show file tree

Hide file tree

Showing 9 changed files with 110 additions and 81 deletions.
diff --git a/charts/selenium-grid/README.md b/charts/selenium-grid/README.md
@@ -704,6 +704,7 @@ This table contains the configuration parameters of the chart and their default
 | `ingress.tls`                                 | `[]`                                        | TLS backend configuration for ingress resource                                                                             |
 | `autoscaling.enableWithExistingKEDA`          | `false`                                     | Enable autoscaling of browser nodes.                                                                                       |
 | `autoscaling.enabled`                         | `false`                                     | Same as above plus installation of KEDA                                                                                    |
+| `autoscaling.patchObjectFinalizers.enabled`   | `true`                                      | Enabled job to execute `kubectl` to patch scaled object finalizers when chart hooks failed with object existed             |
 | `autoscaling.scalingType`                     | `job`                                       | Which typ of KEDA scaling to use: `job` or `deployment`                                                                    |
 | `autoscaling.scaledOptions`                   | See `values.yaml`                           | Common options for KEDA scaled resources (both ScaledJobs and ScaledObjects)                                               |
 | `autoscaling.scaledOptions.minReplicaCount`   | `0`                                         | Min number of replicas that each browser nodes has when autoscaling                                                        |

diff --git a/charts/selenium-grid/templates/_nameHelpers.tpl b/charts/selenium-grid/templates/_nameHelpers.tpl
@@ -43,7 +43,7 @@ helm.sh/chart: {{ include "seleniumGrid.chart" . }}
 Autoscaling labels
 */}}
 {{- define "seleniumGrid.autoscalingLabels" -}}
-component.autoscaling: "true"
+component.autoscaling: "{{ .Release.Name }}"
 {{- end -}}
 
 {{- define "seleniumGrid.component.name" -}}
@@ -193,19 +193,19 @@ Server ConfigMap fullname
 Patch scaledObjects finalizers job fullname
 */}}
 {{- define "seleniumGrid.keda.patchObjectsJob.fullname" -}}
-{{- printf "%s-%s" .Release.Name "patch-scaledobjects-finalizers" | trunc 63 | trimSuffix "-" -}}
+{{- tpl (include "seleniumGrid.component.name" (list "selenium-patch-scaledobjects-finalizers" $)) $ | trunc 63 | trimSuffix "-" -}}
 {{- end -}}
 
 {{/*
-Patch scaled objects RoleBinding fullname
+RBAC RoleBinding fullname
 */}}
-{{- define "seleniumGrid.keda.roleBinding.fullname" -}}
-{{- printf "%s-%s" .Release.Name "patch-keda-rb" | trunc 63 | trimSuffix "-" -}}
+{{- define "seleniumGrid.rbac.roleBinding.fullname" -}}
+{{- tpl (default (include "seleniumGrid.component.name" (list "selenium-rolebinding" $)) .Values.rbacRoleBinding.nameOverride) $ | trunc 63 | trimSuffix "-" -}}
 {{- end -}}
 
 {{/*
-Patch scaled objects Role fullname
+RBAC Role fullname
 */}}
-{{- define "seleniumGrid.keda.role.fullname" -}}
-{{- printf "%s-%s" .Release.Name "patch-keda-role" | trunc 63 | trimSuffix "-" -}}
+{{- define "seleniumGrid.rbac.role.fullname" -}}
+{{- tpl (default (include "seleniumGrid.component.name" (list "selenium-role" $)) .Values.rbacRole.nameOverride) $ | trunc 63 | trimSuffix "-" -}}
 {{- end -}}
diff --git a/charts/selenium-grid/templates/patch-keda/patch-keda-objects-job.yaml b/charts/selenium-grid/templates/patch-keda/patch-keda-objects-job.yaml
@@ -1,13 +1,11 @@
-{{- if eq (include "seleniumGrid.useKEDA" $) "true" }}
+{{- if and (eq (include "seleniumGrid.useKEDA" $) "true") $.Values.autoscaling.patchObjectFinalizers.enabled }}
 apiVersion: batch/v1
 kind: Job
 metadata:
   name: {{ template "seleniumGrid.keda.patchObjectsJob.fullname" $ }}
   namespace: {{ .Release.Namespace }}
   annotations:
-    "helm.sh/hook-delete-policy": hook-succeeded
-    "helm.sh/resource-policy": delete
-    {{- with $.Values.autoscaling.annotations }}
+    {{- with $.Values.autoscaling.patchObjectFinalizers.annotations }}
       {{- toYaml . | nindent 4 }}
     {{- end }}
   labels:
@@ -21,15 +19,17 @@ spec:
     metadata:
       name: {{ template "seleniumGrid.keda.patchObjectsJob.fullname" $ }}
     spec:
-      activeDeadlineSeconds: 120
+      activeDeadlineSeconds: {{ $.Values.autoscaling.patchObjectFinalizers.activeDeadlineSeconds }}
       serviceAccountName: {{ template "seleniumGrid.serviceAccount.fullname" $ }}
       serviceAccount: {{ template "seleniumGrid.serviceAccount.fullname" $ }}
       containers:
         - name: {{ template "seleniumGrid.keda.patchObjectsJob.fullname" $ }}
           image: {{ $.Values.global.seleniumGrid.kubectlImage }}
-          command:
-            - "bin/bash"
-            - "-c"
-            - "kubectl get ScaledObjects,ScaledJobs -n {{ .Release.Namespace }} -l component.autoscaling=true -o=json | jq '.metadata.finalizers = null' | kubectl apply -f -"
+          command: ["/bin/bash", "-c"]
+          args:
+            - |
+              kubectl get ScaledObjects,ScaledJobs -n {{ .Release.Namespace }} -l component.autoscaling={{ .Release.Name }} -o=json | jq '.metadata.finalizers = null' | kubectl apply -f - || true ;
+              kubectl delete ScaledObjects,ScaledJobs -n {{ .Release.Namespace }} -l component.autoscaling={{ .Release.Name }} --wait || true ;
+              kubectl delete hpa -n {{ .Release.Namespace }} -l component.autoscaling={{ .Release.Name }} --wait || true ;
       restartPolicy: Never
 {{- end }}
diff --git a/charts/selenium-grid/templates/patch-keda/patch-keda-rb.yaml b/charts/selenium-grid/templates/patch-keda/patch-keda-rb.yaml
diff --git a/charts/selenium-grid/templates/patch-keda/patch-keda-role.yaml b/charts/selenium-grid/templates/patch-keda/patch-keda-role.yaml
diff --git a/charts/selenium-grid/templates/rbac-role.yaml b/charts/selenium-grid/templates/rbac-role.yaml
@@ -0,0 +1,21 @@
+{{- if $.Values.rbacRole.create }}
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: {{ template "seleniumGrid.rbac.role.fullname" $ }}
+  namespace: {{ .Release.Namespace }}
+  annotations:
+    {{- with $.Values.rbacRole.annotations }}
+      {{- toYaml . | nindent 4 }}
+    {{- end }}
+  labels:
+    deploymentName: {{ template "seleniumGrid.rbac.role.fullname" $ }}
+    {{- include "seleniumGrid.commonLabels" $ | nindent 4 }}
+    {{- with $.Values.customLabels }}
+      {{- toYaml . | nindent 4 }}
+    {{- end }}
+{{- with $.Values.rbacRole.rules }}
+rules:
+  {{- toYaml . | nindent 2 }}
+{{- end }}
+{{- end }}
diff --git a/charts/selenium-grid/templates/rbac-rolebinding.yaml b/charts/selenium-grid/templates/rbac-rolebinding.yaml
@@ -0,0 +1,25 @@
+{{- if $.Values.rbacRoleBinding.create }}
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: {{ template "seleniumGrid.rbac.roleBinding.fullname" $ }}
+  namespace: {{ .Release.Namespace }}
+  annotations:
+    {{- with $.Values.rbacRoleBinding.annotations }}
+      {{- toYaml . | nindent 4 }}
+    {{- end }}
+  labels:
+    deploymentName: {{ template "seleniumGrid.rbac.roleBinding.fullname" $ }}
+    {{- include "seleniumGrid.commonLabels" $ | nindent 4 }}
+    {{- with $.Values.customLabels }}
+      {{- toYaml . | nindent 4 }}
+    {{- end }}
+subjects:
+  - kind: ServiceAccount
+    name: {{ template "seleniumGrid.serviceAccount.fullname" $ }}
+{{ with $.Values.rbacRoleBinding.roleRef }}
+{{- $roleRef := merge (dict "name" (include "seleniumGrid.rbac.role.fullname" $)) . -}}
+roleRef:
+  {{ $roleRef | toYaml | nindent 2 }}
+{{- end }}
+{{- end }}
diff --git a/charts/selenium-grid/values.yaml b/charts/selenium-grid/values.yaml
@@ -70,6 +70,36 @@ serviceAccount:
   annotations: {}
   #  eks.amazonaws.com/role-arn: "arn:aws:iam::12345678:role/video-bucket-permissions"
 
+# RBAC settings
+rbacRole:
+  create: true
+  # nameOverride:
+  annotations: {}
+  rules:
+    - apiGroups:
+        - keda.sh
+      resources:
+        - scaledjobs
+      verbs: [get, list, patch, update, delete]
+    - apiGroups:
+        - keda.sh
+      resources:
+        - scaledobjects
+      verbs: [get, list, patch, update, delete]
+    - apiGroups:
+        - autoscaling
+      resources:
+        - horizontalpodautoscalers
+      verbs: [get, list, patch, update, delete]
+
+rbacRoleBinding:
+  create: true
+  # nameOverride:
+  annotations: {}
+  roleRef:
+    kind: Role
+    apiGroup: rbac.authorization.k8s.io
+
 # Configure the ingress resource to access the Grid installation.
 ingress:
   # Enable or disable ingress resource
@@ -548,7 +578,16 @@ autoscaling:
   scalingType: job
   # Annotations for KEDA resources: ScaledObject and ScaledJob
   annotations:
-    "helm.sh/hook": post-install,post-upgrade,post-rollback,pre-delete
+    "helm.sh/hook": post-install,post-upgrade,post-rollback
+    "helm.sh/hook-weight": "1"
+  # Workaround for Hook post-upgrade selenium-grid/templates/x-node-hpa.yaml failed: object is being deleted: scaledobjects.keda.sh "x" already exists
+  patchObjectFinalizers:
+    enabled: true
+    activeDeadlineSeconds: 120
+    annotations:
+      "helm.sh/hook": post-install,post-upgrade,post-rollback,pre-delete
+      "helm.sh/hook-delete-policy": hook-succeeded,before-hook-creation
+      "helm.sh/hook-weight": "-1"
   # Options for KEDA scaled resources (keep only common options used for both ScaledJob and ScaledObject)
   scaledOptions:
     minReplicaCount: 0
@@ -1187,7 +1226,7 @@ customLabels: {}
 # Configuration for dependency chart keda
 keda:
   additionalAnnotations:
-    "helm.sh/hook": pre-install,pre-upgrade,pre-rollback,post-delete
+    "helm.sh/hook": pre-install,pre-upgrade,pre-rollback
   http:
     timeout: 60000
   webhooks:
@@ -1223,19 +1262,19 @@ prometheus-stack:
   defaultRules:
     create: true
     annotations:
-      "helm.sh/hook": post-install,post-upgrade,post-rollback,pre-delete
+      "helm.sh/hook": post-install,post-upgrade,post-rollback
   alertmanager:
     enabled: true
     annotations:
-      "helm.sh/hook": post-install,post-upgrade,post-rollback,pre-delete
+      "helm.sh/hook": post-install,post-upgrade,post-rollback
   grafana:
     enabled: true
     adminPassword: admin
     forceDeployDatasources: true
     forceDeployDashboards: true
     annotations:
-      "helm.sh/hook": post-install,post-upgrade,post-rollback,pre-delete
+      "helm.sh/hook": post-install,post-upgrade,post-rollback
   prometheus:
     enabled: true
     annotations:
-      "helm.sh/hook": post-install,post-upgrade,post-rollback,pre-delete
+      "helm.sh/hook": post-install,post-upgrade,post-rollback
diff --git a/tests/charts/make/chart_setup_env.sh b/tests/charts/make/chart_setup_env.sh
@@ -110,7 +110,7 @@ if [ "$(uname -m)" = "x86_64" ]; then
     echo "==============================="
 
     echo "Installing Helm for AMD64 / x86_64"
-    HELM_VERSION=${HELM_VERSION:-"v3.14.3"}
+    HELM_VERSION=${HELM_VERSION:-"v3.14.4"}
     curl -fsSL -o helm.tar.gz https://get.helm.sh/helm-${HELM_VERSION}-linux-amd64.tar.gz
     mkdir -p helm
     tar -xf helm.tar.gz --strip-components 1 -C helm