fix(chart): autoscaling.terminationGracePeriodSeconds is not set in…

… Node spec Fixed #2153 node.terminationGracePeriodSeconds should be `3600` by default instead of `30` Signed-off-by: Viet Nguyen Duc <[email protected]>
SeleniumHQ · Mar 2, 2024 · 37cefa4 · 37cefa4
1 parent 57a8b08
commit 37cefa4
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 2 deletions.
diff --git a/charts/selenium-grid/templates/_helpers.tpl b/charts/selenium-grid/templates/_helpers.tpl
@@ -424,7 +424,7 @@ template:
   {{- with .node.priorityClassName }}
     priorityClassName: {{ . }}
   {{- end }}
-    terminationGracePeriodSeconds: {{ .node.terminationGracePeriodSeconds }}
+    terminationGracePeriodSeconds: {{ template "seleniumGrid.node.terminationGracePeriodSeconds" $ }}
     volumes:
       - name: {{ tpl (default (include "seleniumGrid.node.configmap.fullname" $) $.Values.nodeConfigMap.scriptVolumeMountName) $ }}
         configMap:
@@ -582,7 +582,7 @@ Define terminationGracePeriodSeconds of the node pod.
 {{- $autoscalingPeriod := default 0 .Values.autoscaling.terminationGracePeriodSeconds -}}
 {{- $nodePeriod := default 0 .node.terminationGracePeriodSeconds -}}
 {{- $period := $nodePeriod -}}
-{{- if and (eq .Values.autoscaling.scalingType "deployment") (eq (include "seleniumGrid.useKEDA" .) "true") -}}
+{{- if and (eq .Values.autoscaling.scalingType "deployment") (eq (include "seleniumGrid.useKEDA" $) "true") -}}
   {{- $period = ternary $nodePeriod $autoscalingPeriod (gt $nodePeriod $autoscalingPeriod) -}}
 {{- end -}}
 {{- $period -}}

diff --git a/tests/charts/templates/render/dummy.yaml b/tests/charts/templates/render/dummy.yaml
@@ -76,9 +76,11 @@ components:
 
 chromeNode:
   affinity: *affinity
+  terminationGracePeriodSeconds: 7200
 
 firefoxNode:
     affinity: *affinity
+    terminationGracePeriodSeconds: 720
 
 edgeNode:
     affinity: *affinity

diff --git a/tests/charts/templates/render/dummy_solution.yaml b/tests/charts/templates/render/dummy_solution.yaml
@@ -77,9 +77,11 @@ selenium-grid:
 
   chromeNode:
     affinity: *affinity
+    terminationGracePeriodSeconds: 7200
 
   firefoxNode:
       affinity: *affinity
+      terminationGracePeriodSeconds: 720
 
   edgeNode:
       affinity: *affinity

diff --git a/tests/charts/templates/test.py b/tests/charts/templates/test.py
@@ -171,6 +171,26 @@ def test_extra_script_import_to_recorder_configmap(self):
                 count += 1
         self.assertEqual(count, len(resources_name), "No recorder config resources found")
 
+    def test_terminationGracePeriodSeconds_in_deployment_autoscaling(self):
+        resources_name = ['{0}selenium-chrome-node'.format(RELEASE_NAME),]
+        count = 0
+        for doc in LIST_OF_DOCUMENTS:
+            if doc['metadata']['name'] in resources_name and doc['kind'] == 'Deployment':
+                logger.info(f"Assert terminationGracePeriodSeconds is set in resource {doc['metadata']['name']}")
+                self.assertTrue(doc['spec']['template']['spec']['terminationGracePeriodSeconds'] == 7200)
+                count += 1
+        self.assertEqual(count, len(resources_name), "node.terminationGracePeriodSeconds doesn't override a higher value than autoscaling.terminationGracePeriodSeconds")
+
+        resources_name = ['{0}selenium-edge-node'.format(RELEASE_NAME),
+                          '{0}selenium-firefox-node'.format(RELEASE_NAME),]
+        count = 0
+        for doc in LIST_OF_DOCUMENTS:
+            if doc['metadata']['name'] in resources_name and doc['kind'] == 'Deployment':
+                logger.info(f"Assert terminationGracePeriodSeconds is set in resource {doc['metadata']['name']}")
+                self.assertTrue(doc['spec']['template']['spec']['terminationGracePeriodSeconds'] == 3600)
+                count += 1
+        self.assertEqual(count, len(resources_name), "node.terminationGracePeriodSeconds doesn't inherit the global value autoscaling.terminationGracePeriodSeconds")
+
 if __name__ == '__main__':
     failed = False
     try: