Merge pull request #3220 from CecileRobertMichon/cherry-pick-3198

[release-1.7] Fix e2e Helm chart retry when install fails

Author: k8s-ci-robot

test/e2e/azure_privatecluster.go

@@ -103,8 +103,8 @@ func AzurePrivateClusterSpec(ctx context.Context, inputGetter func() AzurePrivat
 
 	By("Ensure public API server is stable before creating private cluster")
 	Consistently(func() error {
-		kubeSystem := &corev1.Namespace{}
-		return publicClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: "kube-system"}, kubeSystem)
+		ns := &corev1.Namespace{}
+		return publicClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: kubesystem}, ns)
 	}, "5s", "100ms").Should(BeNil(), "Failed to assert public API server stability")
 
 	// **************

test/e2e/azure_selfhosted.go

@@ -153,12 +153,12 @@ func SelfHostedSpec(ctx context.Context, inputGetter func() SelfHostedSpecInput)
 		// More specifically, we were observing the test failing to get objects from the API server during move, so we
 		// are now testing the API servers are stable before starting move.
 		Consistently(func() error {
-			kubeSystem := &corev1.Namespace{}
-			return input.BootstrapClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: "kube-system"}, kubeSystem)
+			ns := &corev1.Namespace{}
+			return input.BootstrapClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: kubesystem}, ns)
 		}, "5s", "100ms").Should(BeNil(), "Failed to assert bootstrap API server stability")
 		Consistently(func() error {
-			kubeSystem := &corev1.Namespace{}
-			return selfHostedClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: "kube-system"}, kubeSystem)
+			ns := &corev1.Namespace{}
+			return selfHostedClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: kubesystem}, ns)
 		}, "5s", "100ms").Should(BeNil(), "Failed to assert self-hosted API server stability")
 
 		By("Moving the cluster to self hosted")
@@ -205,12 +205,12 @@ func SelfHostedSpec(ctx context.Context, inputGetter func() SelfHostedSpecInput)
 			// More specifically, we were observing the test failing to get objects from the API server during move, so we
 			// are now testing the API servers are stable before starting move.
 			Consistently(func() error {
-				kubeSystem := &corev1.Namespace{}
-				return input.BootstrapClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: "kube-system"}, kubeSystem)
+				ns := &corev1.Namespace{}
+				return input.BootstrapClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: kubesystem}, ns)
 			}, "5s", "100ms").Should(BeNil(), "Failed to assert bootstrap API server stability")
 			Consistently(func() error {
-				kubeSystem := &corev1.Namespace{}
-				return selfHostedClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: "kube-system"}, kubeSystem)
+				ns := &corev1.Namespace{}
+				return selfHostedClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: kubesystem}, ns)
 			}, "5s", "100ms").Should(BeNil(), "Failed to assert self-hosted API server stability")
 
 			By("Moving the cluster back to bootstrap")

test/e2e/common.go

@@ -45,7 +45,7 @@ import (
 	"sigs.k8s.io/cluster-api/test/framework"
 	"sigs.k8s.io/cluster-api/test/framework/clusterctl"
 	"sigs.k8s.io/cluster-api/util/kubeconfig"
-	crclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // Test suite constants for e2e config variables
@@ -219,7 +219,7 @@ func redactLogs() {
 }
 
 func createRestConfig(ctx context.Context, tmpdir, namespace, clusterName string) *rest.Config {
-	cluster := crclient.ObjectKey{
+	cluster := client.ObjectKey{
 		Namespace: namespace,
 		Name:      clusterName,
 	}
@@ -247,13 +247,24 @@ func EnsureControlPlaneInitialized(ctx context.Context, input clusterctl.ApplyCl
 		Namespace: input.ConfigCluster.Namespace,
 	})
 	kubeadmControlPlane := &kubeadmv1.KubeadmControlPlane{}
-	key := crclient.ObjectKey{
+	key := client.ObjectKey{
 		Namespace: cluster.Spec.ControlPlaneRef.Namespace,
 		Name:      cluster.Spec.ControlPlaneRef.Name,
 	}
-	Eventually(func() error {
-		return getter.Get(ctx, key, kubeadmControlPlane)
-	}, input.WaitForControlPlaneIntervals...).Should(Succeed(), "Failed to get KubeadmControlPlane object %s/%s", cluster.Spec.ControlPlaneRef.Namespace, cluster.Spec.ControlPlaneRef.Name)
+
+	By("Ensuring KubeadmControlPlane is initialized")
+	Eventually(func(g Gomega) {
+		g.Expect(getter.Get(ctx, key, kubeadmControlPlane)).To(Succeed(), "Failed to get KubeadmControlPlane object %s/%s", cluster.Spec.ControlPlaneRef.Namespace, cluster.Spec.ControlPlaneRef.Name)
+		g.Expect(kubeadmControlPlane.Status.Initialized).To(BeTrue(), "KubeadmControlPlane is not yet initialized")
+	}, input.WaitForControlPlaneIntervals...).Should(Succeed(), "KubeadmControlPlane object %s/%s was not initialized in time", cluster.Spec.ControlPlaneRef.Namespace, cluster.Spec.ControlPlaneRef.Name)
+
+	By("Ensuring API Server is reachable before applying Helm charts")
+	Eventually(func(g Gomega) {
+		ns := &corev1.Namespace{}
+		clusterProxy := input.ClusterProxy.GetWorkloadCluster(ctx, input.ConfigCluster.Namespace, input.ConfigCluster.ClusterName)
+		g.Expect(clusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: kubesystem}, ns)).To(Succeed(), "Failed to get kube-system namespace")
+	}, input.WaitForControlPlaneIntervals...).Should(Succeed(), "API Server was not reachable in time")
+
 	_, hasWindows := cluster.Labels["cni-windows"]
 	if kubeadmControlPlane.Spec.KubeadmConfigSpec.ClusterConfiguration.ControllerManager.ExtraArgs["cloud-provider"] == "external" {
 		// There is a co-dependency between cloud-provider and CNI so we install both together if cloud-provider is external.

test/e2e/helpers.go

@@ -874,6 +874,14 @@ func InstallHelmChart(ctx context.Context, input clusterctl.ApplyClusterTemplate
 			}
 			release, err := i.RunWithContext(ctx, chartRequested, vals)
 			if err != nil {
+				Logf("Failed to install release %s, attempting to cleanup so we can retry", releaseName)
+				// Best effort attempt to delete the failed release so we can retry.
+				_, delErr := helmAction.NewUninstall(actionConfig).Run(releaseName)
+				if delErr != nil {
+					Logf("Failed to delete release %s", releaseName)
+				} else {
+					Logf("Deleted failed release %s", releaseName)
+				}
 				return err
 			}
 			Logf(release.Info.Description)