🌱 Fix flaky envTest and add Debug Output (#1551)

guettli · web-flow · commit dbae74fd6c8e · 2025-03-17T16:39:25.000+01:00
diff --git a/.github/actions/e2e/action.yaml b/.github/actions/e2e/action.yaml
@@ -80,7 +80,7 @@ runs:
       run: make ${{ inputs.e2e_make_target }}
     - name: Upload artifact
       uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4
-      if: ${{ always() }}
+      if: ${{ !cancelled() }}
       with:
         name: e2e-${{ inputs.e2e_name }}
         path: _artifacts
diff --git a/.github/actions/test-release/action.yaml b/.github/actions/test-release/action.yaml
@@ -26,6 +26,7 @@ runs:
       run: make test-release
     - name: Upload artifact
       uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4
+      if: ${{ !cancelled() }}
       with:
         name: test-release
         path: out
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -142,6 +142,7 @@ jobs:
       # Upload artifact digests
       - name: Upload artifact digests
         uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
+        if: ${{ !cancelled() }}
         with:
           name: image-digest caph
           path: image-digest
@@ -177,4 +178,4 @@ jobs:
         shell: bash
         run: |
           cd image-digest/
-          find -type f | sort | xargs -d '\n' cat
+          find -type f | sort | xargs -d '\n' cat
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -108,6 +108,7 @@ jobs:
       # Upload artifact digests
       - name: Upload artifact digests
         uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
+        if: ${{ !cancelled() }}
         with:
           name: image-digest caph
           path: image-digest
diff --git a/.github/workflows/report-bin-size.yml b/.github/workflows/report-bin-size.yml
@@ -27,7 +27,9 @@ jobs:
 
       - name: Upload Report
         uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
+        if: ${{ !cancelled() }}
         with:
           name: reports-${{ github.sha }}
           path: .reports
-          retention-days: 90
+          include-hidden-files: true
+          retention-days: 90
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -58,14 +58,20 @@ jobs:
       - name: Create Report
         run: make report-cover-html report-cover-treemap
 
+      - name: Report Slowest tests
+        run: hack/tools/bin/gotestsum tool slowest --num 10 --jsonfile .reports/go-test-output.json > .reports/slowest-tests.txt
+        if: ${{ !cancelled() }}
+
       - name: Test Summary
         uses: test-summary/action@31493c76ec9e7aa675f1585d3ed6f1da69269a86 # v2.4
         with:
           paths: ".coverage/junit.xml"
 
       - name: Upload Report
         uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
+        if: ${{ !cancelled() }}
         with:
           name: reports-${{ steps.name.outputs.name }}
           path: .reports
+          include-hidden-files: true
           retention-days: 30
diff --git a/controllers/controllers_suite_test.go b/controllers/controllers_suite_test.go
@@ -17,12 +17,15 @@ limitations under the License.
 package controllers
 
 import (
+	"fmt"
+	"os"
 	"sync"
 	"testing"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/prometheus/common/expfmt"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -32,6 +35,7 @@ import (
 	"sigs.k8s.io/cluster-api/util/conditions"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/metrics"
 
 	infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1"
 	hcloudclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/client"
@@ -132,7 +136,33 @@ var _ = BeforeSuite(func() {
 	Expect(testEnv.Create(ctx, ns)).To(Succeed())
 })
 
+func dumpMetrics() error {
+	metricFamilies, err := metrics.Registry.Gather()
+	if err != nil {
+		return fmt.Errorf("failed to gather metrics: %w", err)
+	}
+
+	if err := os.MkdirAll("../.reports", 0o750); err != nil {
+		return fmt.Errorf("Error creating directory: %w", err)
+	}
+	f, err := os.Create("../.reports/controller_suite_test-metrics.txt")
+	if err != nil {
+		return fmt.Errorf("Error creating file: %w", err)
+	}
+	defer f.Close()
+
+	// Encode the metrics into text format
+	encoder := expfmt.NewEncoder(f, expfmt.NewFormat(expfmt.TypeTextPlain))
+	for _, mf := range metricFamilies {
+		if err := encoder.Encode(mf); err != nil {
+			return fmt.Errorf("error encoding metric family: %w", err)
+		}
+	}
+	return nil
+}
+
 var _ = AfterSuite(func() {
+	Expect(dumpMetrics()).To(Succeed())
 	Expect(testEnv.Stop()).To(Succeed())
 	wg.Done() // Main manager has been stopped
 	wg.Wait() // Wait for target cluster manager
diff --git a/controllers/hcloudmachine_controller.go b/controllers/hcloudmachine_controller.go
@@ -74,18 +74,15 @@ func (r *HCloudMachineReconciler) Reconcile(ctx context.Context, req reconcile.R
 	hcloudMachine := &infrav1.HCloudMachine{}
 	err := r.Get(ctx, req.NamespacedName, hcloudMachine)
 	if err != nil {
-		if apierrors.IsNotFound(err) {
-			return reconcile.Result{}, nil
-		}
-		return reconcile.Result{}, err
+		return reconcile.Result{}, client.IgnoreNotFound(err)
 	}
 
 	log = log.WithValues("HCloudMachine", klog.KObj(hcloudMachine))
 
 	// Fetch the Machine.
 	machine, err := util.GetOwnerMachine(ctx, r.Client, hcloudMachine.ObjectMeta)
 	if err != nil {
-		return reconcile.Result{}, err
+		return reconcile.Result{}, fmt.Errorf("failed to get owner machine: %w", err)
 	}
 	if machine == nil {
 		log.Info("Machine Controller has not yet set OwnerRef")
diff --git a/controllers/hcloudmachine_controller_test.go b/controllers/hcloudmachine_controller_test.go
@@ -741,6 +741,7 @@ var _ = Describe("Hetzner secret", func() {
 			Eventually(func() bool {
 				return isPresentAndFalseWithReason(key, hcloudMachine, infrav1.HCloudTokenAvailableCondition, expectedReason)
 			}, timeout, interval).Should(BeTrue())
+			Expect(testEnv.Cleanup(ctx, hetznerSecret)).To(Succeed())
 		},
 		Entry("no Hetzner secret/wrong reference", func() *corev1.Secret {
 			return &corev1.Secret{
diff --git a/controllers/hcloudmachinetemplate_controller.go b/controllers/hcloudmachinetemplate_controller.go
@@ -60,7 +60,6 @@ func (r *HCloudMachineTemplateReconciler) Reconcile(ctx context.Context, req rec
 
 	machineTemplate := &infrav1.HCloudMachineTemplate{}
 	if err := r.Get(ctx, req.NamespacedName, machineTemplate); err != nil {
-		log.Error(err, "unable to fetch HCloudMachineTemplate")
 		return reconcile.Result{}, client.IgnoreNotFound(err)
 	}
 
@@ -110,7 +109,6 @@ func (r *HCloudMachineTemplateReconciler) Reconcile(ctx context.Context, req rec
 		Name:      cluster.Spec.InfrastructureRef.Name,
 	}
 	if err := r.Client.Get(ctx, hetznerClusterName, hetznerCluster); err != nil {
-		log.Info("HetznerCluster is not available yet")
 		return reconcile.Result{}, nil
 	}
 
diff --git a/controllers/hcloudremediation_controller.go b/controllers/hcloudremediation_controller.go
@@ -121,7 +121,6 @@ func (r *HCloudRemediationReconciler) Reconcile(ctx context.Context, req reconci
 		Name:      cluster.Spec.InfrastructureRef.Name,
 	}
 	if err := r.Client.Get(ctx, hetznerClusterName, hetznerCluster); err != nil {
-		log.Info("HetznerCluster is not available yet")
 		return reconcile.Result{}, nil
 	}
 
diff --git a/controllers/hetznerbaremetalmachine_controller.go b/controllers/hetznerbaremetalmachine_controller.go
@@ -106,7 +106,6 @@ func (r *HetznerBareMetalMachineReconciler) Reconcile(ctx context.Context, req r
 		Name:      cluster.Spec.InfrastructureRef.Name,
 	}
 	if err := r.Client.Get(ctx, hetznerClusterName, hetznerCluster); err != nil {
-		log.Info("HetznerCluster is not available yet")
 		return reconcile.Result{}, nil
 	}
 
diff --git a/controllers/hetznerbaremetalremediation_controller.go b/controllers/hetznerbaremetalremediation_controller.go
@@ -113,7 +113,6 @@ func (r *HetznerBareMetalRemediationReconciler) Reconcile(ctx context.Context, r
 		Name:      cluster.Spec.InfrastructureRef.Name,
 	}
 	if err := r.Client.Get(ctx, hetznerClusterName, hetznerCluster); err != nil {
-		log.Info("HetznerCluster is not available yet")
 		return reconcile.Result{}, nil
 	}
 
diff --git a/controllers/hetznercluster_controller_test.go b/controllers/hetznercluster_controller_test.go
@@ -559,7 +559,7 @@ var _ = Describe("Hetzner ClusterReconciler", func() {
 
 				Eventually(func() bool {
 					return isPresentAndTrue(key, instance, infrav1.LoadBalancerReadyCondition)
-				}, timeout, time.Second).Should(BeTrue())
+				}, 2*timeout, time.Second).Should(BeTrue()) // flaky ?
 
 				By("checking that load balancer has label set")
 
@@ -1001,6 +1001,7 @@ var _ = Describe("Hetzner secret", func() {
 			Eventually(func() bool {
 				return isPresentAndFalseWithReason(key, hetznerCluster, infrav1.HCloudTokenAvailableCondition, expectedReason)
 			}, timeout, time.Second).Should(BeTrue())
+			Expect(testEnv.Cleanup(ctx, hetznerSecret)).To(Succeed())
 		},
 		Entry("no Hetzner secret/wrong reference", func() *corev1.Secret {
 			return &corev1.Secret{
diff --git a/go.mod b/go.mod
@@ -10,6 +10,7 @@ require (
 	github.com/hetznercloud/hcloud-go/v2 v2.13.1
 	github.com/onsi/ginkgo/v2 v2.20.2
 	github.com/onsi/gomega v1.34.2
+	github.com/prometheus/common v0.48.0
 	github.com/spf13/pflag v1.0.6
 	github.com/stoewer/go-strcase v1.3.0
 	github.com/stretchr/testify v1.9.0
@@ -101,7 +102,6 @@ require (
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/prometheus/client_golang v1.19.1 // indirect
 	github.com/prometheus/client_model v0.6.0 // indirect
-	github.com/prometheus/common v0.48.0 // indirect
 	github.com/prometheus/procfs v0.12.0 // indirect
 	github.com/sagikazarmark/locafero v0.4.0 // indirect
 	github.com/sagikazarmark/slog-shim v0.1.0 // indirect
diff --git a/hack/run-github-ci-until-failure.sh b/hack/run-github-ci-until-failure.sh
@@ -0,0 +1,63 @@
+#!/usr/bin/env bash
+
+# Copyright 2025 The Kubernetes Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Run a Workflow in GitHub Actions until it fails.
+# Goal: Find flaky tests, which only fail in CI.
+
+# Bash Strict Mode: https://github.com/guettli/bash-strict-mode
+trap 'echo "Warning: A command has failed. Exiting the script. Line was ($0:$LINENO): $(sed -n "${LINENO}p" "$0")"; exit 3' ERR
+set -Eeuo pipefail
+
+# See `gh workflow list` for the workflow names.
+WORKFLOW="Test Code"
+
+while true; do
+    # Start the CI job for the current commit
+    echo "Triggering CI workflow..."
+    gh workflow run "$WORKFLOW"
+
+    # Get the most recent workflow run ID
+    echo "Waiting for the workflow to appear..."
+    sleep 5
+    RUN_ID=$(gh run list --limit 1 --json databaseId --jq '.[0].databaseId')
+
+    if [ -z "$RUN_ID" ]; then
+        echo "Failed to retrieve workflow run ID."
+        exit 1
+    fi
+
+    # Monitor the workflow until it finishes
+    while true; do
+        echo "Checking workflow status..."
+        STATUS=$(gh run view "$RUN_ID" --json status --jq '.status')
+        echo "... status: $STATUS"
+        if [ "$STATUS" == "in_progress" ] || [ "$STATUS" == "queued" ] || [ "$STATUS" == "pending" ]; then
+            echo "... waiting ..."
+            sleep 10
+            continue
+        fi
+        break
+    done
+
+    # Check the conclusion of the workflow
+    CONCLUSION=$(gh run view "$RUN_ID" --json conclusion --jq '.conclusion')
+    if [ "$CONCLUSION" != "success" ]; then
+        echo "Workflow failed"
+        gh run view "$RUN_ID"
+        exit 1
+    fi
+    echo "Workflow succeeded. Retrying..."
+done
diff --git a/hack/test-unit.sh b/hack/test-unit.sh
@@ -32,4 +32,9 @@ export KUBEBUILDER_ASSETS
 
 mkdir -p .coverage
 
-hack/tools/bin/gotestsum --junitfile=.coverage/junit.xml --format testname -- -covermode=atomic -coverprofile=.coverage/cover.out -p=4 -timeout 5m ./controllers/... ./pkg/... ./api/...
+hack/tools/bin/gotestsum \
+    --jsonfile=.reports/go-test-output.json \
+    --junitfile=.coverage/junit.xml \
+    --format testname -- \
+    -covermode=atomic -coverprofile=.coverage/cover.out -p=4 -timeout 5m \
+    ./controllers/... ./pkg/... ./api/...
diff --git a/test/e2e/basic_baremetal_test.go b/test/e2e/basic_baremetal_test.go
@@ -33,18 +33,6 @@ import (
 )
 
 func logBareMetalHostStatusContinously(ctx context.Context, c client.Client) {
-	caphDeployment := appsv1.Deployment{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "caph-controller-manager",
-			Namespace: "caph-system",
-		},
-	}
-	err := c.Get(ctx, client.ObjectKeyFromObject(&caphDeployment), &caphDeployment)
-	if err != nil {
-		By(fmt.Sprintf("Error getting caph-controller-manager deployment: %v", err))
-		return
-	}
-	By(fmt.Sprintf("caph-controller-manager image: %v", caphDeployment.Spec.Template.Spec.Containers[0].Image))
 	for {
 		t := time.After(30 * time.Second)
 		select {
@@ -65,6 +53,25 @@ func logBareMetalHostStatus(ctx context.Context, c client.Client) error {
 	if err != nil {
 		return err
 	}
+
+	if len(hbmhList.Items) == 0 {
+		return nil
+	}
+
+	caphDeployment := appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "caph-controller-manager",
+			Namespace: "caph-system",
+		},
+	}
+	err = c.Get(ctx, client.ObjectKeyFromObject(&caphDeployment), &caphDeployment)
+	if err != nil {
+		return fmt.Errorf("failed to get caph-controller-manager deployment: %w", err)
+	}
+
+	By(fmt.Sprintf("--------------------------------------------------- BareMetalHosts %s",
+		caphDeployment.Spec.Template.Spec.Containers[0].Image))
+
 	for i := range hbmhList.Items {
 		hbmh := &hbmhList.Items[i]
 		if hbmh.Spec.Status.ProvisioningState == "" {
@@ -88,7 +95,6 @@ func logBareMetalHostStatus(ctx context.Context, c client.Client) error {
 		}
 		By("  Ready Condition: " + state + " " + reason + " " + msg)
 	}
-	By("---------------------------------------------------")
 	return nil
 }
 
diff --git a/test/e2e/caph.go b/test/e2e/caph.go

Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,6 @@ func (r *HCloudMachineTemplateReconciler) Reconcile(ctx context.Context, req rec`
`60`	`60`
`61`	`61`	`machineTemplate := &infrav1.HCloudMachineTemplate{}`
`62`	`62`	`if err := r.Get(ctx, req.NamespacedName, machineTemplate); err != nil {`
`63`		`- log.Error(err, "unable to fetch HCloudMachineTemplate")`
`64`	`63`	`return reconcile.Result{}, client.IgnoreNotFound(err)`
`65`	`64`	`}`
`66`	`65`
`@@ -110,7 +109,6 @@ func (r *HCloudMachineTemplateReconciler) Reconcile(ctx context.Context, req rec`
`110`	`109`	`Name: cluster.Spec.InfrastructureRef.Name,`
`111`	`110`	`}`
`112`	`111`	`if err := r.Client.Get(ctx, hetznerClusterName, hetznerCluster); err != nil {`
`113`		`- log.Info("HetznerCluster is not available yet")`
`114`	`112`	`return reconcile.Result{}, nil`
`115`	`113`	`}`
`116`	`114`