diff --git a/controllers/helmchartproxy/helmchartproxy_controller_phases.go b/controllers/helmchartproxy/helmchartproxy_controller_phases.go index 09f6ee97..dc820e2c 100644 --- 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 index 9c4516d7..370d36e5 100644 --- 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 index 67ae149f..698a4d23 100644 --- 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)