Merge pull request #189 from jimmidyson/jimmi/skip-paused-clusters

🐛 Skip HelmChartProxy reconciliation for paused clusters
kubernetes-sigs · Mar 20, 2024 · fc3a5cd · fc3a5cd
2 parents e28f5bc + 369c00c
commit fc3a5cd
Show file tree

Hide file tree

Showing 3 changed files with 136 additions and 0 deletions.
diff --git a/controllers/helmchartproxy/helmchartproxy_controller_phases.go b/controllers/helmchartproxy/helmchartproxy_controller_phases.go
@@ -29,6 +29,7 @@ import (
 	"sigs.k8s.io/cluster-api-addon-provider-helm/internal"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util"
+	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/conditions"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -57,6 +58,12 @@ func (r *HelmChartProxyReconciler) deleteOrphanedHelmReleaseProxies(ctx context.
 func (r *HelmChartProxyReconciler) reconcileForCluster(ctx context.Context, helmChartProxy *addonsv1alpha1.HelmChartProxy, cluster clusterv1.Cluster) error {
 	log := ctrl.LoggerFrom(ctx)
 
+	// Don't reconcile if the Cluster or the helmChartProxy is paused.
+	if annotations.IsPaused(&cluster, helmChartProxy) {
+		log.V(2).Info("Reconciliation is paused for this Cluster", "cluster", cluster.Name)
+		return nil
+	}
+
 	existingHelmReleaseProxy, err := r.getExistingHelmReleaseProxy(ctx, helmChartProxy, &cluster)
 	if err != nil {
 		// TODO: Should we set a condition here?

diff --git a/controllers/helmchartproxy/helmchartproxy_controller_phases_test.go b/controllers/helmchartproxy/helmchartproxy_controller_phases_test.go
@@ -175,6 +175,26 @@ var (
 		},
 	}
 
+	fakeClusterPaused = &clusterv1.Cluster{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: clusterv1.GroupVersion.String(),
+			Kind:       "Cluster",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-cluster",
+			Namespace: "test-namespace",
+		},
+		Spec: clusterv1.ClusterSpec{
+			ClusterNetwork: &clusterv1.ClusterNetwork{
+				APIServerPort: ptr.To(int32(1234)),
+				Pods: &clusterv1.NetworkRanges{
+					CIDRBlocks: []string{"10.0.0.0/16", "20.0.0.0/16"},
+				},
+			},
+			Paused: true,
+		},
+	}
+
 	fakeHelmReleaseProxy = &addonsv1alpha1.HelmReleaseProxy{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "test-generated-name",
@@ -291,6 +311,17 @@ func TestReconcileForCluster(t *testing.T) {
 			},
 			expectedError: "",
 		},
+		{
+			name:                          "do not reconcile for a paused cluster",
+			helmChartProxy:                fakeReinstallHelmChartProxy,
+			existingHelmReleaseProxy:      fakeHelmReleaseProxy,
+			cluster:                       fakeClusterPaused,
+			expectHelmReleaseProxyToExist: false,
+			expect: func(g *WithT, hcp *addonsv1alpha1.HelmChartProxy, hrp *addonsv1alpha1.HelmReleaseProxy) {
+				g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeFalse())
+			},
+			expectedError: "",
+		},
 	}
 
 	for _, tc := range testcases {

diff --git a/controllers/helmchartproxy/helmchartproxy_controller_test.go b/controllers/helmchartproxy/helmchartproxy_controller_test.go
@@ -143,6 +143,23 @@ var (
 		},
 	}
 
+	clusterPaused = &clusterv1.Cluster{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: clusterv1.GroupVersion.String(),
+			Kind:       "Cluster",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-cluster-paused",
+			Namespace: "test-namespace",
+			Labels: map[string]string{
+				"test-label": "test-value",
+			},
+		},
+		Spec: clusterv1.ClusterSpec{
+			Paused: true,
+		},
+	}
+
 	hrpReady1 = &addonsv1alpha1.HelmReleaseProxy{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "test-hrp-1",
@@ -315,6 +332,46 @@ func TestReconcileNormal(t *testing.T) {
 			},
 			expectedError: "",
 		},
+		{
+			name:           "mark HelmChartProxy as ready once HelmReleaseProxies ready conditions are true ignoring paused clusters",
+			helmChartProxy: defaultProxy,
+			objects:        []client.Object{cluster1, cluster2, clusterPaused, hrpReady1, hrpReady2},
+			expect: func(g *WithT, c client.Client, hcp *addonsv1alpha1.HelmChartProxy) {
+				g.Expect(hcp.Status.MatchingClusters).To(BeEquivalentTo([]corev1.ObjectReference{
+					{
+						APIVersion: clusterv1.GroupVersion.String(),
+						Kind:       "Cluster",
+						Name:       "test-cluster-1",
+						Namespace:  "test-namespace",
+					},
+					{
+						APIVersion: clusterv1.GroupVersion.String(),
+						Kind:       "Cluster",
+						Name:       "test-cluster-2",
+						Namespace:  "test-namespace",
+					},
+					{
+						APIVersion: clusterv1.GroupVersion.String(),
+						Kind:       "Cluster",
+						Name:       "test-cluster-paused",
+						Namespace:  "test-namespace",
+					},
+				}))
+				g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeTrue())
+				g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeTrue())
+				g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue())
+				g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue())
+				g.Expect(conditions.Has(hcp, clusterv1.ReadyCondition)).To(BeTrue())
+				g.Expect(conditions.IsTrue(hcp, clusterv1.ReadyCondition)).To(BeTrue())
+				g.Expect(hcp.Status.ObservedGeneration).To(Equal(hcp.Generation))
+				hrpList := &addonsv1alpha1.HelmReleaseProxyList{}
+				g.Expect(c.List(ctx, hrpList, &client.ListOptions{Namespace: hcp.Namespace})).To(Succeed())
+
+				// There should be 2 HelmReleaseProxies as the paused cluster should not have a HelmReleaseProxy.
+				g.Expect(hrpList.Items).To(HaveLen(2))
+			},
+			expectedError: "",
+		},
 	}
 
 	for _, tc := range testcases {
@@ -353,6 +410,47 @@ func TestReconcileNormal(t *testing.T) {
 	}
 }
 
+func TestReconcileAfterMatchingClusterUnpaused(t *testing.T) {
+	g := NewWithT(t)
+
+	request := reconcile.Request{
+		NamespacedName: util.ObjectKey(defaultProxy),
+	}
+
+	c := fake.NewClientBuilder().
+		WithScheme(fakeScheme).
+		WithObjects(cluster1, cluster2, clusterPaused, defaultProxy).
+		WithStatusSubresource(&addonsv1alpha1.HelmChartProxy{}).
+		WithStatusSubresource(&addonsv1alpha1.HelmReleaseProxy{}).
+		Build()
+
+	r := &HelmChartProxyReconciler{
+		Client: c,
+	}
+	result, err := r.Reconcile(ctx, request)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(result).To(Equal(reconcile.Result{}))
+
+	// There should be 2 HelmReleaseProxies as the paused cluster should not have a HelmReleaseProxy.
+	hrpList := &addonsv1alpha1.HelmReleaseProxyList{}
+	g.Expect(c.List(ctx, hrpList, &client.ListOptions{Namespace: request.Namespace})).To(Succeed())
+	g.Expect(hrpList.Items).To(HaveLen(2))
+
+	// Unpause the cluster and reconcile again.
+	unpausedCluster := clusterPaused.DeepCopy()
+	unpausedCluster.Spec.Paused = false
+	g.Expect(c.Update(ctx, unpausedCluster)).To(Succeed())
+
+	result, err = r.Reconcile(ctx, request)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(result).To(Equal(reconcile.Result{}))
+
+	// Now there should be 3 HelmReleaseProxies.
+	hrpList = &addonsv1alpha1.HelmReleaseProxyList{}
+	g.Expect(c.List(ctx, hrpList, &client.ListOptions{Namespace: request.Namespace})).To(Succeed())
+	g.Expect(hrpList.Items).To(HaveLen(3))
+}
+
 func init() {
 	_ = scheme.AddToScheme(fakeScheme)
 	_ = clusterv1.AddToScheme(fakeScheme)