test(agent): gate the simple and reap_old_logs operator-agent assertions

k8s-tests/operator-agent/check_node.sh

@@ -26,12 +26,19 @@ timeout=${4:-10}
 invert=${5:-false}
 
 
-# loop until the command returns a non-zero exit code or the timeout is reached
+# loop until the check passes or the retry budget is spent
 for i in $(seq 1 ${timeout}); do
     data=$(kubectl exec ${node}-debugger -- chroot /host bash -c "${cmd}")
+    status=$?
     check_result=$(echo "${data}" | grep -c "${check}")
     if [ "$invert" == "true" ]; then
         check_result=$((! check_result))
+    elif [ $status -ne 0 ]; then
+        ## A positive match only counts if the command itself succeeded: `cat a.log b.log`
+        ## prints a.log and exits non-zero when b.log is unreadable, and that output must
+        ## not pass a check. Inverted checks assert absence, where a command with nothing
+        ## to list exits non-zero by design, so they keep matching on output alone.
+        check_result=0
     fi
     if [ $check_result -gt 0 ]; then
         echo "Check passed"
@@ -42,6 +49,7 @@ for i in $(seq 1 ${timeout}); do
     sleep 1
 done
 echo "Data: ${data}"
+echo "Exit: ${status}"
 echo "Check: ${check}"
 
 exit 1

k8s-tests/operator-agent/reap_old_logs/chainsaw-test.yaml

@@ -28,7 +28,7 @@ spec:
     - script:
         content: |
           ## remove annotation from last run
-          ../../../operator/bin/nodewright reset reap-old-logs-agent-operator --confirm
+          ../../../operator/bin/nodewright reset reap-old-logs-agent-operator --confirm 2>/dev/null || true
     - script:
         content: |
           ## reinstall the debug pod in case it was deleted
@@ -40,6 +40,7 @@ spec:
           ../check_node.sh kind-worker "rm -rf /var/lib/skyhook/reap-old-logs-agent-operator || true" ".*" 2
     - script:
         content: |
+          set -e
           ## Seed more logs than the agent keeps, so the package has something to reap on its
           ## first attempt. Deliberately not driven by repeated failures: the operator gives a
           ## stage only JOB_BACKOFF_LIMIT+1 attempts, so a test that needs N failed attempts to
@@ -55,6 +56,7 @@ spec:
         file: assert.yaml
     - script:
         content: |
+          set -e
           ## 7 seeds plus this attempt's own logs come back down to 5. Counting survivors rather
           ## than naming them keeps this agnostic about which 5: the Python agent ranks by mtime,
           ## where all 7 seeds tie, and the Go agent ranks by the timestamp in the name. ls -rt

k8s-tests/operator-agent/simple/chainsaw-test.yaml

@@ -44,6 +44,9 @@ spec:
         file: assert.yaml
     - script:
         content: |
+          ## set -e: chainsaw runs script content as `sh -c`, so without it only the last
+          ## command decides the step and everything above it is decorative.
+          set -e
           ../check_node.sh kind-worker "cat /var/log/skyhook/simple-agent-operator/shellscript/1.1.1/*.log" ".*Hello, world!"
           ../check_node.sh kind-worker "cat /var/log/skyhook/simple-agent-operator/shellscript/1.1.1/*.log" ".*This is a test"
           ../check_node.sh kind-worker "ls /var/lib/skyhook/simple-agent-operator/flags" "START"