Adapt fixtures and asserts in graceful-shutdown and product-config-compat tests by maltesander · Pull Request #765 · stackabletech/spark-k8s-operator

maltesander · 2026-09-03T12:19:37Z

Description

fix fixtures and asserts in graceful-shutdown and product-config-compat tests (failing in weekly)
improve assert loop to not spam output every second

    --- PASS: kuttl/harness (0.00s)
        --- PASS: kuttl/harness/product-config-compat_openshift-false_spark-regression-3.5.8 (11.01s)
        --- PASS: kuttl/harness/graceful-shutdown_openshift-false_spark-4.1.1 (43.11s)
        --- PASS: kuttl/harness/graceful-shutdown_openshift-false_spark-3.5.8 (37.93s)
        --- PASS: kuttl/harness/graceful-shutdown_openshift-false_spark-4.1.2 (43.77s)
PASS

Definition of Done Checklist

Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
Please make sure all these things are done and tick the boxes

Author

Changes are OpenShift compatible
CRD changes approved
CRD documentation for all fields, following the style guide.
Helm chart can be installed and deployed operator works
Integration tests passed (for non trivial changes)
Changes need to be "offline" compatible
Links to generated (nightly) docs added
Release note snippet added

Reviewer

Code contains useful comments
Code contains useful logging statements
(Integration-)Test cases added
Documentation added or updated. Follows the style guide.
Changelog updated
Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

Feature Tracker has been updated
Proper release label has been added
Links to generated (nightly) docs added
Release note snippet added
Add type/deprecation label & add to the deprecation schedule
Add type/experimental label & add to the experimental features tracker

…t-config-compat tests

Techassi

The scripts are... hard to look at. I think we can make it slightly easier to look at them.

Techassi · 2026-09-03T12:55:26Z

  "security.properties": "networkaddress.cache.negative.ttl=0\nnetworkaddress.cache.ttl=30\n",
  "spark-env.sh": "",
-  "template.yaml": "metadata:\n  labels:\n    app.kubernetes.io/component: spark\n    app.kubernetes.io/instance: pyspark-pi\n    app.kubernetes.io/managed-by: spark.stackable.tech_sparkapplication\n    app.kubernetes.io/name: spark-k8s\n    app.kubernetes.io/version: 3.5.8-stackable0.0.0-dev\n    prometheus.io/scrape: 'true'\n    stackable.tech/vendor: Stackable\n  name: spark\nspec:\n  affinity: {}\n  containers:\n  - env:\n    - name: CONTAINERDEBUG_LOG_DIRECTORY\n      value: /stackable/log/containerdebug\n    - name: _STACKABLE_PRE_HOOK\n      value: containerdebug --output=/stackable/log/containerdebug-state.json --loop &\n    image: oci.stackable.tech/sdp/spark-k8s:3.5.8-stackable0.0.0-dev\n    imagePullPolicy: IfNotPresent\n    name: spark\n    resources:\n      limits:\n        cpu: '2'\n        memory: 1Gi\n      requests:\n        cpu: '1'\n        memory: 1Gi\n    volumeMounts:\n    - mountPath: /stackable/log_config\n      name: log-config\n    - mountPath: /stackable/log\n      name: log\n  enableServiceLinks: false\n  securityContext:\n    fsGroup: 1000\n  serviceAccountName: pyspark-pi\n  volumes:\n  - emptyDir:\n      sizeLimit: 39Mi\n    name: log\n  - configMap:\n      name: pyspark-pi-driver-pod-template\n    name: log-config\n  - configMap:\n      name: pyspark-pi-driver-pod-template\n    name: config\n"
+  "template.yaml": "metadata:\n  labels:\n    app.kubernetes.io/component: spark\n    app.kubernetes.io/instance: pyspark-pi\n    app.kubernetes.io/managed-by: spark.stackable.tech_sparkapplication\n    app.kubernetes.io/name: spark-k8s\n    app.kubernetes.io/version: 3.5.8-stackable0.0.0-dev\n    prometheus.io/scrape: 'true'\n    stackable.tech/vendor: Stackable\n  name: spark\nspec:\n  affinity: {}\n  containers:\n  - env:\n    - name: CONTAINERDEBUG_LOG_DIRECTORY\n      value: /stackable/log/containerdebug\n    - name: _STACKABLE_PRE_HOOK\n      value: containerdebug --output=/stackable/log/containerdebug-state.json --loop &\n    image: oci.stackable.tech/sdp/spark-k8s:3.5.8-stackable0.0.0-dev\n    imagePullPolicy: IfNotPresent\n    name: spark\n    resources:\n      limits:\n        cpu: '2'\n        memory: 1Gi\n      requests:\n        cpu: '1'\n        memory: 1Gi\n    volumeMounts:\n    - mountPath: /stackable/log_config\n      name: log-config\n    - mountPath: /stackable/log\n      name: log\n  enableServiceLinks: false\n  securityContext:\n    fsGroup: 1000\n    runAsNonRoot: true\n  serviceAccountName: pyspark-pi\n  volumes:\n  - emptyDir:\n      sizeLimit: 39Mi\n    name: log\n  - configMap:\n      name: pyspark-pi-driver-pod-template\n    name: log-config\n  - configMap:\n      name: pyspark-pi-driver-pod-template\n    name: config\n"


I assume this change was triggered by the addition of "runAsNonRoot": true in the ...-spec.json file?

Not really, the json wasnt refreshed properly after #744.

Techassi · 2026-09-03T13:04:28Z

+      executors=$(kubectl -n "$NAMESPACE" get pods -l "spark-role=executor,app.kubernetes.io/instance=$app" -o json \
+        | jq -r '.items[]
+                 | select(.metadata.deletionTimestamp == null and .status.phase == "Running")
+                 | .metadata.name')


Better formatting would make this slightly less cursed.

Suggested change

executors=$(kubectl -n "$NAMESPACE" get pods -l "spark-role=executor,app.kubernetes.io/instance=$app" -o json \

| jq -r '.items[]

| select(.metadata.deletionTimestamp == null and .status.phase == "Running")

| .metadata.name')

executors=$(

kubectl -n "$NAMESPACE" get pods \

-l "spark-role=executor,app.kubernetes.io/instance=$app" \

-o json | jq -r \

'

.items[]

| select(.metadata.deletionTimestamp == null and .status.phase == "Running")

| .metadata.name'

)

cleaned up 6a8d8ba

Techassi · 2026-09-03T13:08:45Z

      fi
-      if [ -z "$executors" ]; then
-        echo "FAIL: no executor Pods found for $app, so no shutdown can be propagated to any"
+      running=$(printf '%s\n' "$executors" | grep -c . || true)


Is this for counting the number of running executor? If so, this can be done better by using jq's length function.

done 6a8d8ba

Techassi · 2026-09-03T13:10:25Z

-      if [ -z "$executors" ]; then
-        echo "FAIL: no executor Pods found for $app, so no shutdown can be propagated to any"
+      running=$(printf '%s\n' "$executors" | grep -c . || true)
+      if [ "$running" -ne 2 ]; then


Also, numerical equality checks are tricky. We have to be really certain bash thinks $running is a number and not a string containing a number.

The jq length fixed this automatically (and it is not bash: POSIX test is an arithmetic comparison).

Techassi · 2026-09-03T13:12:31Z

+        for pod in $(kubectl -n "$NAMESPACE" get pods -l "$selector" -o json \
+          | jq -r '.items[]
+                   | select(.metadata.deletionTimestamp == null and .status.phase == "Running")
+                   | .metadata.name'); do


I feel like we should pull this out of the loop header. It is currently cumbersome to read.

fixed 678d450

Techassi · 2026-09-03T13:15:01Z

+          echo "FAIL: ${budget_seconds}s after the executor was stopped, $registered of 2"
+          echo "executors are registered and running a task, so no replacement came back"


Why not use a single echo -e here for newlines?

The echo -e does not work for kuttls sh -c. Could do printf?

Co-authored-by: Techassi <git@techassi.dev>

fix(test): adapt fixtures and asserts in graceful-shutdown and produc…

fdedafc

…t-config-compat tests

maltesander self-assigned this Sep 3, 2026

fix: improve test comment

58568ea

Techassi requested changes Sep 3, 2026

View reviewed changes

maltesander and others added 3 commits September 4, 2026 07:36

Update tests/templates/kuttl/graceful-shutdown/12-assert.yaml

acee5ec

Co-authored-by: Techassi <git@techassi.dev>

fix(test): count running executors with jq length

6a8d8ba

fix(test): pull the executor listing out of the for loop header

678d450

maltesander requested a review from Techassi September 4, 2026 05:51

		echo "FAIL: ${budget_seconds}s after the executor was stopped, $registered of 2"
		echo "executors are registered and running a task, so no replacement came back"

Uh oh!

Uh oh!

Conversation

maltesander commented Sep 3, 2026

Description

Definition of Done Checklist

Author

Reviewer

Acceptance

Uh oh!

Techassi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maltesander Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

maltesander Sep 4, 2026 •

edited

Loading