Fix e2e test issues introduced by chained upgrades

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

Author: sbueringer

internal/webhooks/cluster.go

@@ -334,9 +334,8 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
 	allErrs = append(allErrs, validateAutoscalerAnnotationsForCluster(newCluster, clusterClass)...)
 
 	if oldCluster != nil { // On update
-		// The ClusterClass must exist to proceed with update validation. Return an error if the ClusterClass was
-		// not found.
-		if apierrors.IsNotFound(clusterClassPollErr) {
+		// Return an error if we couldn't get the ClusterClass (either because it was not found or not reconciled).
+		if clusterClassPollErr != nil {
 			allErrs = append(
 				allErrs, field.InternalError(
 					fldPath.Child("class"),
@@ -402,16 +401,6 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
 
 		// If the ClusterClass referenced in the Topology has changed compatibility checks are needed.
 		if oldCluster.GetClassKey() != newCluster.GetClassKey() {
-			if clusterClassPollErr != nil {
-				allErrs = append(
-					allErrs, field.Forbidden(
-						fldPath.Child("class"),
-						fmt.Sprintf("cannot rebase to ClusterClass %q: %s",
-							newCluster.GetClassKey(), clusterClassPollErr.Error())))
-				// Return early with errors if the new ClusterClass can't be retrieved.
-				return allWarnings, allErrs
-			}
-
 			// Check to see if the ClusterClass referenced in the old version of the Cluster exists.
 			oldClusterClass, err := webhook.pollClusterClassForCluster(ctx, oldCluster)
 			if err != nil {

test/e2e/cluster_upgrade_runtimesdk_test.go

@@ -100,8 +100,10 @@ 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.
+			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 +114,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/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.Wrapf(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 {