Fix e2e test issues introduced by chained upgrades

Signed-off-by: Stefan Büringer [email protected]

Author: sbueringer

internal/webhooks/cluster.go

@@ -974,6 +974,10 @@ func (webhook *Cluster) pollClusterClassForCluster(ctx context.Context, cluster
 		return true, nil
 	})
 	if clusterClassPollErr != nil {
+		if errors.Is(clusterClassPollErr, errClusterClassNotReconciled) {
+			// Return ClusterClass if we were able to get it and it's just not reconciled.
+			return clusterClass, clusterClassPollErr
+		}
 		return nil, clusterClassPollErr
 	}
 	return clusterClass, nil

test/e2e/cluster_upgrade_runtimesdk.go

@@ -936,7 +936,8 @@ func runtimeHookTestHandler(ctx context.Context, c client.Client, cluster *clust
 }
 
 func computeHookName(hook string, attributes []string) string {
-	return strings.Join(append([]string{hook}, attributes...), "-")
+	// Note: + is not a valid character for ConfigMap keys (only alphanumeric characters, '-', '_' or '.')
+	return strings.ReplaceAll(strings.Join(append([]string{hook}, attributes...), "-"), "+", "_")
 }
 
 // clusterConditionShowsHookBlocking checks if the TopologyReconciled condition message contains both the hook name and hookFailedMessage.

test/e2e/cluster_upgrade_runtimesdk_test.go

@@ -100,8 +100,12 @@ var _ = Describe("When performing chained upgrades for workload cluster using Cl
 			DeployClusterClassInSeparateNamespace: true,
 			// Setting Kubernetes version from
 			KubernetesVersionFrom: e2eConfig.MustGetVariable(KubernetesVersionChainedUpgradeFrom),
-			// use Kubernetes versions from the kind mapper.
-			KubernetesVersions: kind.GetKubernetesVersions(),
+			// use Kubernetes versions from the kind mapper
+			// Note: Ensure KUBERNETES_VERSION_UPGRADE_TO is always part of the list, this is required for cases where
+			// KUBERNETES_VERSION_UPGRADE_TO is not in the kind mapper, e.g. in the `e2e-latestk8s` prowjob.
+			// Note: KUBERNETES_VERSION_UPGRADE_TO has to be set either to one version in kind.GetKubernetesVersions() or
+			// to a version greater than the last in the list by at most one minor version.
+			KubernetesVersions: appendIfNecessary(kind.GetKubernetesVersions(), e2eConfig.MustGetVariable(KubernetesVersionUpgradeTo)),
 			// The runtime extension gets deployed to the test-extension-system namespace and is exposed
 			// by the test-extension-webhook-service.
 			// The below values are used when creating the cluster-wide ExtensionConfig to refer
@@ -112,3 +116,12 @@ var _ = Describe("When performing chained upgrades for workload cluster using Cl
 		}
 	})
 })
+
+func appendIfNecessary(versions []string, v string) []string {
+	for _, version := range versions {
+		if version == v {
+			return versions
+		}
+	}
+	return append(versions, v)
+}

test/extension/handlers/lifecycle/handlers.go

@@ -293,5 +293,6 @@ func configMapName(clusterName, extensionConfigName string) string {
 }
 
 func computeHookName(hook runtimecatalog.Hook, attributes []string) string {
-	return strings.Join(append([]string{runtimecatalog.HookName(hook)}, attributes...), "-")
+	// Note: + is not a valid character for ConfigMap keys (only alphanumeric characters, '-', '_' or '.')
+	return strings.ReplaceAll(strings.Join(append([]string{runtimecatalog.HookName(hook)}, attributes...), "-"), "+", "_")
 }

test/framework/deployment_helpers.go

@@ -412,12 +412,12 @@ func dumpPodMetrics(ctx context.Context, client *kubernetes.Clientset, metricsPa
 		}
 
 		if !errorRetrievingMetrics {
-			Expect(verifyMetrics(data)).To(Succeed())
+			Expect(verifyMetrics(data, &pod)).To(Succeed())
 		}
 	}
 }
 
-func verifyMetrics(data []byte) error {
+func verifyMetrics(data []byte, pod *corev1.Pod) error {
 	var parser expfmt.TextParser
 	mf, err := parser.TextToMetricFamilies(bytes.NewReader(data))
 	if err != nil {
@@ -458,7 +458,7 @@ func verifyMetrics(data []byte) error {
 	}
 
 	if len(errs) > 0 {
-		return kerrors.NewAggregate(errs)
+		return errors.WithMessagef(kerrors.NewAggregate(errs), "panics occurred in Pod %s", klog.KObj(pod))
 	}
 
 	return nil

test/framework/deployment_helpers_test.go

@@ -20,6 +20,8 @@ import (
 	"testing"
 
 	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func Test_verifyMetrics(t *testing.T) {
@@ -68,7 +70,7 @@ controller_runtime_reconcile_panics_total{controller="clusterclass"} 0
 # TYPE controller_runtime_webhook_panics_total counter
 controller_runtime_webhook_panics_total 0
 `),
-			wantErr: "1 panics occurred in \"cluster\" controller (check logs for more details)",
+			wantErr: "panics occurred in Pod default/pod1: 1 panics occurred in \"cluster\" controller (check logs for more details)",
 		},
 		{
 			name: "panic occurred in webhooks",
@@ -88,7 +90,7 @@ controller_runtime_webhook_panics_total 1
 # TYPE controller_runtime_conversion_webhook_panics_total counter
 controller_runtime_conversion_webhook_panics_total 0
 `),
-			wantErr: "1 panics occurred in webhooks (check logs for more details)",
+			wantErr: "panics occurred in Pod default/pod1: 1 panics occurred in webhooks (check logs for more details)",
 		},
 		{
 			name: "panics occurred in conversion webhooks",
@@ -108,14 +110,14 @@ controller_runtime_webhook_panics_total 0
 # TYPE controller_runtime_conversion_webhook_panics_total counter
 controller_runtime_conversion_webhook_panics_total 2
 `),
-			wantErr: "2 panics occurred in conversion webhooks (check logs for more details)",
+			wantErr: "panics occurred in Pod default/pod1: 2 panics occurred in conversion webhooks (check logs for more details)",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			err := verifyMetrics(tt.data)
+			err := verifyMetrics(tt.data, &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "pod1"}})
 			if tt.wantErr == "" {
 				g.Expect(err).ToNot(HaveOccurred())
 			} else {