improve tests

.github/workflows/on-pr.yaml

@@ -88,6 +88,7 @@ jobs:
           - "TestE2EWithCommand"
           - "TestE2EWithSignal"
           - "TestE2EConcurrentWithCommand"
+          - "TestE2EConcurrentWithSignal"
         kubernetes_version:
           - "previous"
           - "current"
@@ -126,6 +127,7 @@ jobs:
         testname:
           - "TestCordonningIsKept/concurrency1"
           - "TestCordonningIsKept/concurrency2"
+          - "TestE2EBlocker/podblocker"
     steps:
       - uses: actions/checkout@v4
       - name: Ensure go version
@@ -147,5 +149,9 @@ jobs:
         with:
           install_only: true
           version: v0.22.0
+          # Keep this until v1.31 (or superior) becomes the default kubectl version for the kind-action.
+          # It is used in podblocker shell script test to use --all-pods.
+          # If the podblocker e2e test relies on another way, this can also be removed.
+          kubectl_version: v1.31.0
       - name: Run specific e2e tests
-        run: make e2e-test ARGS="-run ^${{ matrix.testname }}"
+        run: make e2e-test ARGS="-run ^${{ matrix.testname }}"

Makefile

@@ -19,7 +19,7 @@ bootstrap-tools: $(HACKDIR)
 	command -v  $(HACKDIR)/cosign || curl -sSfL https://github.com/sigstore/cosign/releases/download/v2.2.3/cosign-linux-amd64 -o $(HACKDIR)/cosign
 	command -v  $(HACKDIR)/shellcheck || (curl -sSfL https://github.com/koalaman/shellcheck/releases/download/stable/shellcheck-stable.linux.x86_64.tar.xz | tar -J -v -x shellcheck-stable/shellcheck && mv shellcheck-stable/shellcheck $(HACKDIR)/shellcheck && rmdir shellcheck-stable)
 	chmod +x $(HACKDIR)/goreleaser $(HACKDIR)/cosign $(HACKDIR)/syft $(HACKDIR)/shellcheck
-	# go install honnef.co/go/tools/cmd/staticcheck@latest
+	command -v staticcheck || go install honnef.co/go/tools/cmd/staticcheck@latest
 
 clean:
 	rm -rf ./dist
@@ -47,9 +47,12 @@ dev-manifest:
 	sed -e "s#image: ghcr.io/.*kured.*#image: kured:dev#g" -e 's/#\(.*\)--period=1h/\1--period=20s/g' kured-ds.yaml > tests/kind/testfiles/kured-ds.yaml
 	# signal e2e scenario
 	sed -e "s#image: ghcr.io/.*kured.*#image: kured:dev#g" -e 's/#\(.*\)--period=1h/\1--period=20s/g' kured-ds-signal.yaml > tests/kind/testfiles/kured-ds-signal.yaml
-	# concurrency e2e scenario
-	sed -e "s#image: ghcr.io/.*kured.*#image: kured:dev#g" -e 's/#\(.*\)--period=1h/\1--period=20s/g' -e 's/#\(.*\)--concurrency=1/\1--concurrency=2/g' kured-ds.yaml > tests/kind/testfiles/kured-ds-concurrent.yaml
-
+	# concurrency e2e command scenario
+	sed -e "s#image: ghcr.io/.*kured.*#image: kured:dev#g" -e 's/#\(.*\)--period=1h/\1--period=20s/g' -e 's/#\(.*\)--concurrency=1/\1--concurrency=2/g' kured-ds.yaml > tests/kind/testfiles/kured-ds-concurrent-command.yaml
+	# concurrency e2e signal scenario
+	sed -e "s#image: ghcr.io/.*kured.*#image: kured:dev#g" -e 's/#\(.*\)--period=1h/\1--period=20s/g' -e 's/#\(.*\)--concurrency=1/\1--concurrency=2/g' kured-ds-signal.yaml > tests/kind/testfiles/kured-ds-concurrent-signal.yaml
+	# pod blocker e2e signal scenario
+	sed -e "s#image: ghcr.io/.*kured.*#image: kured:dev#g" -e 's/#\(.*\)--period=1h/\1--period=20s/g' -e 's/#\(.*\)--blocking-pod-selector=name=temperamental/\1--blocking-pod-selector=app=blocker/g' kured-ds-signal.yaml > tests/kind/testfiles/kured-ds-podblocker.yaml
 
 e2e-test: dev-manifest dev-image
 	echo "Running ALL go tests"
@@ -68,4 +71,4 @@ test: bootstrap-tools
 	go test -test.short -json ./... > test.json
 	echo "Running shellcheck"
 	find . -name '*.sh' | xargs -n1 $(HACKDIR)/shellcheck
-	# Need to add staticcheck to replace golint as golint is deprecated, and staticcheck is the recommendation
+	staticcheck ./...

cmd/kured/main.go

@@ -320,11 +320,11 @@ func validateNotificationURL(notifyURL string, slackHookURL string) string {
 			log.Errorf("slack-hook-url is not properly formatted... no notification will be sent: %v\n", err)
 			return ""
 		}
-		if len(strings.Split(strings.Trim(parsedURL.Path, "/services/"), "/")) != 3 {
+		if len(strings.Split(strings.Replace(parsedURL.Path, "/services/", "", -1), "/")) != 3 {
 			log.Errorf("slack-hook-url is not properly formatted... no notification will be sent: unexpected number of / in URL\n")
 			return ""
 		}
-		return fmt.Sprintf("slack://%s", strings.Trim(parsedURL.Path, "/services/"))
+		return fmt.Sprintf("slack://%s", strings.Replace(parsedURL.Path, "/services/", "", -1))
 	}
 	return ""
 }

pkg/daemonsetlock/daemonsetlock.go

@@ -214,10 +214,10 @@ func (dsl *DaemonSetSingleLock) Release() error {
 			}
 
 			if value.NodeID != dsl.nodeID {
-				return fmt.Errorf("Not lock holder: %v", value.NodeID)
+				return fmt.Errorf("not lock holder: %v", value.NodeID)
 			}
 		} else {
-			return fmt.Errorf("Lock not held")
+			return fmt.Errorf("lock not held")
 		}
 
 		delete(ds.ObjectMeta.Annotations, dsl.annotation)

pkg/timewindow/days.go

@@ -81,11 +81,11 @@ func parseWeekday(day string) (time.Weekday, error) {
 		if n >= 0 && n < 7 {
 			return time.Weekday(n), nil
 		}
-		return time.Sunday, fmt.Errorf("Invalid weekday, number out of range: %s", day)
+		return time.Sunday, fmt.Errorf("invalid weekday, number out of range: %s", day)
 	}
 
 	if weekday, ok := dayStrings[strings.ToLower(day)]; ok {
 		return weekday, nil
 	}
-	return time.Sunday, fmt.Errorf("Invalid weekday: %s", day)
+	return time.Sunday, fmt.Errorf("invalid weekday: %s", day)
 }

pkg/timewindow/timewindow.go

@@ -77,5 +77,5 @@ func parseTime(s string, loc *time.Location) (time.Time, error) {
 		}
 	}
 
-	return time.Now(), fmt.Errorf("Invalid time format: %s", s)
+	return time.Now(), fmt.Errorf("invalid time format: %s", s)
 }

tests/kind/main_test.go

@@ -152,7 +152,7 @@ func TestE2EWithCommand(t *testing.T) {
 		t.Run(version, func(t *testing.T) {
 			t.Parallel() // Allow tests to run in parallel
 
-			randomInt := fmt.Sprintf(strconv.Itoa(rand.Intn(100)))
+			randomInt := strconv.Itoa(rand.Intn(100))
 			kindClusterName := fmt.Sprintf("kured-e2e-command-%v-%v", version, randomInt)
 			kindClusterConfigFile := fmt.Sprintf("../../.github/kind-cluster-%v.yaml", version)
 			kindContext := fmt.Sprintf("kind-%v", kindClusterName)
@@ -202,7 +202,7 @@ func TestE2EWithSignal(t *testing.T) {
 		t.Run(version, func(t *testing.T) {
 			t.Parallel() // Allow tests to run in parallel
 
-			randomInt := fmt.Sprintf(strconv.Itoa(rand.Intn(100)))
+			randomInt := strconv.Itoa(rand.Intn(100))
 			kindClusterName := fmt.Sprintf("kured-e2e-signal-%v-%v", version, randomInt)
 			kindClusterConfigFile := fmt.Sprintf("../../.github/kind-cluster-%v.yaml", version)
 			kindContext := fmt.Sprintf("kind-%v", kindClusterName)
@@ -252,12 +252,12 @@ func TestE2EConcurrentWithCommand(t *testing.T) {
 		t.Run(version, func(t *testing.T) {
 			t.Parallel() // Allow tests to run in parallel
 
-			randomInt := fmt.Sprintf(strconv.Itoa(rand.Intn(100)))
+			randomInt := strconv.Itoa(rand.Intn(100))
 			kindClusterName := fmt.Sprintf("kured-e2e-concurrentcommand-%v-%v", version, randomInt)
 			kindClusterConfigFile := fmt.Sprintf("../../.github/kind-cluster-%v.yaml", version)
 			kindContext := fmt.Sprintf("kind-%v", kindClusterName)
 
-			k := NewKindTester(kindClusterName, kindClusterConfigFile, t, LocalImage(kuredDevImage), Deploy("../../kured-rbac.yaml"), Deploy("testfiles/kured-ds-concurrent.yaml"))
+			k := NewKindTester(kindClusterName, kindClusterConfigFile, t, LocalImage(kuredDevImage), Deploy("../../kured-rbac.yaml"), Deploy("testfiles/kured-ds-concurrent-command.yaml"))
 			defer k.FlushLog()
 
 			err := k.Create()
@@ -284,15 +284,16 @@ func TestE2EConcurrentWithCommand(t *testing.T) {
 	}
 }
 
-func TestCordonningIsKept(t *testing.T) {
+func TestE2EConcurrentWithSignal(t *testing.T) {
 	t.Parallel()
 	if testing.Short() {
 		t.Skip("skipping test in short mode.")
 	}
 
 	var kindClusterConfigs = []string{
-		"concurrency1",
-		"concurrency2",
+		"previous",
+		"current",
+		"next",
 	}
 	// Iterate over each Kubernetes version
 	for _, version := range kindClusterConfigs {
@@ -301,16 +302,65 @@ func TestCordonningIsKept(t *testing.T) {
 		t.Run(version, func(t *testing.T) {
 			t.Parallel() // Allow tests to run in parallel
 
-			randomInt := fmt.Sprintf(strconv.Itoa(rand.Intn(100)))
-			kindClusterName := fmt.Sprintf("kured-e2e-cordon-%v-%v", version, randomInt)
-			kindClusterConfigFile := fmt.Sprintf("../../.github/kind-cluster-next.yaml")
+			randomInt := strconv.Itoa(rand.Intn(100))
+			kindClusterName := fmt.Sprintf("kured-e2e-concurrentsignal-%v-%v", version, randomInt)
+			kindClusterConfigFile := fmt.Sprintf("../../.github/kind-cluster-%v.yaml", version)
+			kindContext := fmt.Sprintf("kind-%v", kindClusterName)
+
+			k := NewKindTester(kindClusterName, kindClusterConfigFile, t, LocalImage(kuredDevImage), Deploy("../../kured-rbac.yaml"), Deploy("testfiles/kured-ds-concurrent-signal.yaml"))
+			defer k.FlushLog()
+
+			err := k.Create()
+			if err != nil {
+				t.Fatalf("Error creating cluster %v", err)
+			}
+			defer func(k *KindTest) {
+				err := k.Destroy()
+				if err != nil {
+					t.Fatalf("Error destroying cluster %v", err)
+				}
+			}(k)
+
+			k.Write([]byte("Now running e2e tests"))
+
+			if err := k.RunCmd("bash", "testfiles/create-reboot-sentinels.sh", kindContext); err != nil {
+				t.Fatalf("failed to create sentinels: %v", err)
+			}
+
+			if err := k.RunCmd("bash", "testfiles/follow-coordinated-reboot.sh", kindContext); err != nil {
+				t.Fatalf("failed to follow reboot: %v", err)
+			}
+		})
+	}
+}
+
+func TestCordonningIsKept(t *testing.T) {
+	t.Parallel()
+	if testing.Short() {
+		t.Skip("skipping test in short mode.")
+	}
+
+	var kindClusterConfigs = []string{
+		"concurrency1",
+		"concurrency2",
+	}
+	// Iterate over each test variant
+	for _, variant := range kindClusterConfigs {
+		variant := variant
+		// Define a subtest for each combination
+		t.Run(variant, func(t *testing.T) {
+			t.Parallel() // Allow tests to run in parallel
+
+			randomInt := strconv.Itoa(rand.Intn(100))
+			kindClusterName := fmt.Sprintf("kured-e2e-cordon-%v-%v", variant, randomInt)
+			kindClusterConfigFile := "../../.github/kind-cluster-next.yaml"
 			kindContext := fmt.Sprintf("kind-%v", kindClusterName)
 
 			var manifest string
-			if version == "concurrency1" {
-				manifest = fmt.Sprintf("testfiles/kured-ds.yaml")
+			if variant == "concurrency1" {
+				manifest = "testfiles/kured-ds-signal.yaml"
 			} else {
-				manifest = fmt.Sprintf("testfiles/kured-ds-concurrent.yaml")
+				manifest = "testfiles/kured-ds-concurrent-signal.yaml"
 			}
 			k := NewKindTester(kindClusterName, kindClusterConfigFile, t, LocalImage(kuredDevImage), Deploy("../../kured-rbac.yaml"), Deploy(manifest))
 			defer k.FlushLog()
@@ -334,3 +384,46 @@ func TestCordonningIsKept(t *testing.T) {
 		})
 	}
 }
+func TestE2EBlocker(t *testing.T) {
+	t.Parallel()
+	if testing.Short() {
+		t.Skip("skipping test in short mode.")
+	}
+
+	var kindClusterConfigs = []string{
+		"podblocker",
+	}
+	// Iterate over each variant of the test
+	for _, variant := range kindClusterConfigs {
+		variant := variant
+		// Define a subtest for each combination
+		t.Run(variant, func(t *testing.T) {
+			t.Parallel() // Allow tests to run in parallel
+
+			randomInt := strconv.Itoa(rand.Intn(100))
+			kindClusterName := fmt.Sprintf("kured-e2e-cordon-%v-%v", variant, randomInt)
+			kindClusterConfigFile := "../../.github/kind-cluster-next.yaml"
+			kindContext := fmt.Sprintf("kind-%v", kindClusterName)
+
+			k := NewKindTester(kindClusterName, kindClusterConfigFile, t, LocalImage(kuredDevImage), Deploy("../../kured-rbac.yaml"), Deploy(fmt.Sprintf("testfiles/kured-ds-%v.yaml", variant)))
+			defer k.FlushLog()
+
+			err := k.Create()
+			if err != nil {
+				t.Fatalf("Error creating cluster %v", err)
+			}
+			defer func(k *KindTest) {
+				err := k.Destroy()
+				if err != nil {
+					t.Fatalf("Error destroying cluster %v", err)
+				}
+			}(k)
+
+			k.Write([]byte("Now running e2e tests"))
+
+			if err := k.RunCmd("bash", fmt.Sprintf("testfiles/%v.sh", variant), kindContext); err != nil {
+				t.Fatalf("node blocker test did not succeed: %v", err)
+			}
+		})
+	}
+}

tests/kind/testfiles/podblocker.sh

@@ -0,0 +1,54 @@
+#!/usr/bin/env bash
+
+kubectl_flags=( )
+[[ "$1" != "" ]] && kubectl_flags=("${kubectl_flags[@]}" --context "$1")
+
+function gather_logs_and_cleanup {
+      for id in $(docker ps -q); do
+          echo "############################################################"
+          echo "docker logs for container $id:"
+          docker logs "$id"
+      done
+      ${KUBECTL_CMD:-kubectl} "${kubectl_flags[@]}" logs ds/kured --all-pods -n kube-system
+}
+trap gather_logs_and_cleanup EXIT
+
+set +o errexit
+worker=$(${KUBECTL_CMD:-kubectl} "${kubectl_flags[@]}" get nodes -o custom-columns=name:metadata.name --no-headers | grep worker | head -n 1)
+
+${KUBECTL_CMD:-kubectl} "${kubectl_flags[@]}" label nodes "$worker" blocked-host=yes
+
+${KUBECTL_CMD:-kubectl} "${kubectl_flags[@]}" apply -f - << EOF
+apiVersion: v1
+kind: Pod
+metadata:
+  name: nginx
+  labels:
+    app: blocker
+spec:
+  containers:
+    - name: nginx
+      image: nginx
+      imagePullPolicy: IfNotPresent
+  nodeSelector:
+    blocked-host: "yes"
+EOF
+
+docker exec "$worker" touch "${SENTINEL_FILE:-/var/run/reboot-required}"
+
+set -o errexit
+max_attempts="100"
+attempt_num=1
+sleep_time=5
+
+until ${KUBECTL_CMD:-kubectl} "${kubectl_flags[@]}" logs ds/kured --all-pods -n kube-system  | grep -i -e "Reboot.*blocked"
+do
+  if (( attempt_num == max_attempts )); then
+      echo "Attempt $attempt_num failed and there are no more attempts left!"
+      exit 1
+  else
+      echo "Did not find 'reboot blocked' in the log, retrying in $sleep_time seconds (Attempt #$attempt_num)"
+      sleep "$sleep_time"
+  fi
+  (( attempt_num++ ))
+done