From 74c7290cd44f4ae7df59fbfb7f318c4906da2845 Mon Sep 17 00:00:00 2001 From: Danil-Grigorev Date: Tue, 10 Dec 2024 13:09:46 +0100 Subject: [PATCH] Populate fleet external annotation on management clusters Signed-off-by: Danil-Grigorev --- .../rancher-turtles/templates/deployment.yaml | 2 +- feature/feature.go | 6 + internal/controllers/helpers.go | 1 + internal/controllers/import_controller_v3.go | 170 ++++++++++-------- .../controllers/import_controller_v3_test.go | 68 +++++++ internal/controllers/suite_test.go | 1 + 6 files changed, 169 insertions(+), 79 deletions(-) diff --git a/charts/rancher-turtles/templates/deployment.yaml b/charts/rancher-turtles/templates/deployment.yaml index 8219fed8..6041d6cf 100644 --- a/charts/rancher-turtles/templates/deployment.yaml +++ b/charts/rancher-turtles/templates/deployment.yaml @@ -26,7 +26,7 @@ spec: containers: - args: - --leader-elect - - --feature-gates=propagate-labels={{ index .Values "rancherTurtles" "features" "propagate-labels" "enabled"}},managementv3-cluster={{ index .Values "rancherTurtles" "features" "managementv3-cluster" "enabled"}},rancher-kube-secret-patch={{ index .Values "rancherTurtles" "features" "rancher-kubeconfigs" "label"}} + - --feature-gates=propagate-labels={{ index .Values "rancherTurtles" "features" "propagate-labels" "enabled"}},managementv3-cluster={{ index .Values "rancherTurtles" "features" "managementv3-cluster" "enabled"}},rancher-kube-secret-patch={{ index .Values "rancherTurtles" "features" "rancher-kubeconfigs" "label"}},addon-provider-fleet={{ index .Values "rancherTurtles" "features" "addon-provider-fleet" "enabled"}} {{- range .Values.rancherTurtles.managerArguments }} - {{ . }} {{- end }} diff --git a/feature/feature.go b/feature/feature.go index dac6b6a3..da4aec92 100644 --- a/feature/feature.go +++ b/feature/feature.go @@ -32,6 +32,11 @@ const ( // PropagateLabels is used to enable copying the labels from the CAPI cluster // to the Rancher cluster. PropagateLabels featuregate.Feature = "propagate-labels" + + // ExternalFleet allows to disable in-tree management of the Fleet clusters + // in the imported rancher clusters, by setting "provisioning.cattle.io/externally-managed" + // annotation. + ExternalFleet featuregate.Feature = "addon-provider-fleet" ) func init() { @@ -42,4 +47,5 @@ var defaultGates = map[featuregate.Feature]featuregate.FeatureSpec{ RancherKubeSecretPatch: {Default: false, PreRelease: featuregate.Beta}, ManagementV3Cluster: {Default: false, PreRelease: featuregate.Beta}, PropagateLabels: {Default: false, PreRelease: featuregate.Beta}, + ExternalFleet: {Default: false, PreRelease: featuregate.Beta}, } diff --git a/internal/controllers/helpers.go b/internal/controllers/helpers.go index 46cee3fa..9cd7bbfd 100644 --- a/internal/controllers/helpers.go +++ b/internal/controllers/helpers.go @@ -51,6 +51,7 @@ const ( capiClusterOwner = "cluster-api.cattle.io/capi-cluster-owner" capiClusterOwnerNamespace = "cluster-api.cattle.io/capi-cluster-owner-ns" v1ClusterMigrated = "cluster-api.cattle.io/migrated" + externalFleetAnnotation = "provisioning.cattle.io/externally-managed" defaultRequeueDuration = 1 * time.Minute trueAnnotationValue = "true" diff --git a/internal/controllers/import_controller_v3.go b/internal/controllers/import_controller_v3.go index 984dd2f1..237e7311 100644 --- a/internal/controllers/import_controller_v3.go +++ b/internal/controllers/import_controller_v3.go @@ -17,6 +17,7 @@ limitations under the License. package controllers import ( + "cmp" "context" "fmt" "strings" @@ -27,7 +28,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" errorutils "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/tools/record" - "k8s.io/client-go/util/retry" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" @@ -184,13 +184,8 @@ func (r *CAPIImportManagementV3Reconciler) Reconcile(ctx context.Context, req ct errs = append(errs, fmt.Errorf("error reconciling cluster: %w", err)) } - if err := retry.RetryOnConflict(retry.DefaultBackoff, func() error { - if err := r.Client.Patch(ctx, capiCluster, patchBase); err != nil { - errs = append(errs, fmt.Errorf("failed to patch cluster: %w", err)) - } - return nil - }); err != nil { - return ctrl.Result{}, err + if err := r.Client.Patch(ctx, capiCluster, patchBase); err != nil { + errs = append(errs, fmt.Errorf("failed to patch cluster: %w", err)) } if len(errs) > 0 { @@ -200,7 +195,7 @@ func (r *CAPIImportManagementV3Reconciler) Reconcile(ctx context.Context, req ct return result, nil } -func (r *CAPIImportManagementV3Reconciler) reconcile(ctx context.Context, capiCluster *clusterv1.Cluster) (ctrl.Result, error) { +func (r *CAPIImportManagementV3Reconciler) reconcile(ctx context.Context, capiCluster *clusterv1.Cluster) (res ctrl.Result, reterr error) { log := log.FromContext(ctx) migrated, err := r.verifyV1ClusterMigration(ctx, capiCluster) @@ -214,7 +209,7 @@ func (r *CAPIImportManagementV3Reconciler) reconcile(ctx context.Context, capiCl ownedLabelName: "", } - rancherCluster := &managementv3.Cluster{} + var rancherCluster *managementv3.Cluster rancherClusterList := &managementv3.ClusterList{} selectors := []client.ListOption{ @@ -234,7 +229,7 @@ func (r *CAPIImportManagementV3Reconciler) reconcile(ctx context.Context, capiCl rancherCluster = &rancherClusterList.Items[0] } - if !rancherCluster.ObjectMeta.DeletionTimestamp.IsZero() { + if rancherCluster != nil && !rancherCluster.ObjectMeta.DeletionTimestamp.IsZero() { if err := r.reconcileDelete(ctx, capiCluster); err != nil { log.Error(err, "Removing CAPI Cluster failed, retrying") return ctrl.Result{}, err @@ -253,7 +248,23 @@ func (r *CAPIImportManagementV3Reconciler) reconcile(ctx context.Context, capiCl } } - return r.reconcileNormal(ctx, capiCluster, rancherCluster) + patchBase := client.MergeFromWithOptions(rancherCluster.DeepCopy(), client.MergeFromWithOptimisticLock{}) + + defer func() { + // As the rancherCluster is created inside reconcileNormal, we can only client-side patch existing object + // Skipping all requeues or existing errors, overlapping with scenario of missing cluster + if reterr != nil || rancherCluster == nil { + return + } + + if err := r.Client.Patch(ctx, rancherCluster, patchBase); err != nil { + reterr = fmt.Errorf("failed to patch Rancher cluster: %w", err) + } + }() + + res, reterr = r.reconcileNormal(ctx, capiCluster, rancherCluster) + + return res, reterr } func (r *CAPIImportManagementV3Reconciler) reconcileNormal(ctx context.Context, capiCluster *clusterv1.Cluster, @@ -261,78 +272,48 @@ func (r *CAPIImportManagementV3Reconciler) reconcileNormal(ctx context.Context, ) (ctrl.Result, error) { log := log.FromContext(ctx) - err := r.RancherClient.Get(ctx, client.ObjectKeyFromObject(rancherCluster), rancherCluster) - if apierrors.IsNotFound(err) { - if autoImport, err := r.shouldAutoImportUncached(ctx, capiCluster); err != nil || !autoImport { - return ctrl.Result{}, err - } + clusterMissing := rancherCluster == nil - newCluster := &managementv3.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: capiCluster.Namespace, - GenerateName: "c-", - Labels: map[string]string{ - capiClusterOwner: capiCluster.Name, - capiClusterOwnerNamespace: capiCluster.Namespace, - ownedLabelName: "", - }, - Annotations: map[string]string{ - turtlesannotations.NoCreatorRBACAnnotation: trueAnnotationValue, - }, - Finalizers: []string{ - managementv3.CapiClusterFinalizer, - }, + updatedCluster := &managementv3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: capiCluster.Namespace, + GenerateName: "c-", + Labels: map[string]string{ + capiClusterOwner: capiCluster.Name, + capiClusterOwnerNamespace: capiCluster.Namespace, + ownedLabelName: "", }, - Spec: managementv3.ClusterSpec{ - DisplayName: capiCluster.Name, - Description: "CAPI cluster imported to Rancher", + Finalizers: []string{ + managementv3.CapiClusterFinalizer, }, - } - - if feature.Gates.Enabled(feature.PropagateLabels) { - for labelKey, labelVal := range capiCluster.Labels { - newCluster.Labels[labelKey] = labelVal - } - } - - if err := r.RancherClient.Create(ctx, newCluster); err != nil { - return ctrl.Result{}, fmt.Errorf("error creating rancher cluster: %w", err) - } - - return ctrl.Result{Requeue: true}, nil + }, + Spec: managementv3.ClusterSpec{ + DisplayName: capiCluster.Name, + Description: "CAPI cluster imported to Rancher", + }, } - if err != nil { - log.Error(err, fmt.Sprintf("Unable to fetch rancher cluster %s", client.ObjectKeyFromObject(rancherCluster))) + rancherCluster = cmp.Or(rancherCluster, updatedCluster) - return ctrl.Result{}, err - } + r.optOutOfClusterOwner(ctx, rancherCluster) + r.optOutOfFleetManagement(ctx, rancherCluster) + r.propagateLabels(ctx, capiCluster, rancherCluster) - if err := r.optOutOfClusterOwner(ctx, rancherCluster); err != nil { - return ctrl.Result{}, fmt.Errorf("error annotating rancher cluster %s to opt out of cluster owner: %w", rancherCluster.Name, err) + addedFinalizer := controllerutil.AddFinalizer(rancherCluster, managementv3.CapiClusterFinalizer) + if addedFinalizer { + log.Info("Successfully added capicluster.turtles.cattle.io finalizer to Rancher cluster") } - patchBase := client.MergeFromWithOptions(rancherCluster.DeepCopy(), client.MergeFromWithOptimisticLock{}) - needsFinalizer := controllerutil.AddFinalizer(rancherCluster, managementv3.CapiClusterFinalizer) - - if feature.Gates.Enabled(feature.PropagateLabels) { - if rancherCluster.Labels == nil { - rancherCluster.Labels = map[string]string{} - } - - for labelKey, labelVal := range capiCluster.Labels { - rancherCluster.Labels[labelKey] = labelVal + if clusterMissing { + if autoImport, err := r.shouldAutoImportUncached(ctx, capiCluster); err != nil || !autoImport { + return ctrl.Result{}, err } - if err := r.Client.Patch(ctx, rancherCluster, patchBase); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch Rancher cluster: %w", err) + if err := r.RancherClient.Create(ctx, rancherCluster); err != nil { + return ctrl.Result{}, fmt.Errorf("error creating rancher cluster: %w", err) } - log.Info("Successfully propagated labels to Rancher cluster") - } else if needsFinalizer { - if err := r.Client.Patch(ctx, rancherCluster, patchBase); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch Rancher cluster: %w", err) - } + return ctrl.Result{Requeue: true}, nil } if conditions.IsTrue(rancherCluster, managementv3.ClusterConditionReady) { @@ -548,7 +529,7 @@ func (r *CAPIImportManagementV3Reconciler) verifyV1ClusterMigration(ctx context. // optOutOfClusterOwner annotates the cluster with the opt-out annotation. // Rancher will detect this annotation and it won't create ProjectOwner or ClusterOwner roles. -func (r *CAPIImportManagementV3Reconciler) optOutOfClusterOwner(ctx context.Context, rancherCluster *managementv3.Cluster) error { +func (r *CAPIImportManagementV3Reconciler) optOutOfClusterOwner(ctx context.Context, rancherCluster *managementv3.Cluster) { log := log.FromContext(ctx) annotations := rancherCluster.GetAnnotations() @@ -561,15 +542,48 @@ func (r *CAPIImportManagementV3Reconciler) optOutOfClusterOwner(ctx context.Cont rancherCluster.Name, turtlesannotations.ClusterImportedAnnotation)) - patchBase := client.MergeFromWithOptions(rancherCluster.DeepCopy(), client.MergeFromWithOptimisticLock{}) - annotations[turtlesannotations.NoCreatorRBACAnnotation] = trueAnnotationValue rancherCluster.SetAnnotations(annotations) + } +} - if err := r.Client.Patch(ctx, rancherCluster, patchBase); err != nil { - return fmt.Errorf("error patching rancher cluster: %w", err) - } +// optOutOfFleetManagement annotates the cluster with the fleet provisioning opt-out annotation, +// allowing external fleet cluster management. +func (r *CAPIImportManagementV3Reconciler) optOutOfFleetManagement(ctx context.Context, rancherCluster *managementv3.Cluster) { + log := log.FromContext(ctx) + + annotations := rancherCluster.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} } - return nil + if _, found := annotations[externalFleetAnnotation]; !found && feature.Gates.Enabled(feature.ExternalFleet) { + annotations[externalFleetAnnotation] = "true" + rancherCluster.SetAnnotations(annotations) + + log.Info("Added fleet annotation to Rancher cluster") + } +} + +func (r *CAPIImportManagementV3Reconciler) propagateLabels( + ctx context.Context, + capiCluster *clusterv1.Cluster, + rancherCluster *managementv3.Cluster, +) { + log := log.FromContext(ctx) + + labels := rancherCluster.GetLabels() + if rancherCluster.Labels == nil { + labels = map[string]string{} + } + + if feature.Gates.Enabled(feature.PropagateLabels) { + for labelKey, labelVal := range capiCluster.Labels { + labels[labelKey] = labelVal + } + + rancherCluster.SetLabels(labels) + + log.V(5).Info("Propagated labels to Rancher cluster") + } } diff --git a/internal/controllers/import_controller_v3_test.go b/internal/controllers/import_controller_v3_test.go index 7f6cdbc0..c9616ad0 100644 --- a/internal/controllers/import_controller_v3_test.go +++ b/internal/controllers/import_controller_v3_test.go @@ -233,6 +233,29 @@ var _ = Describe("reconcile CAPI Cluster", func() { Expect(rancherClusters.Items[0].Name).To(ContainSubstring("c-")) }) + It("should set fleet annotation on a freshly imported rancher cluster", func() { + Expect(cl.Create(ctx, capiCluster)).To(Succeed()) + capiCluster.Status.ControlPlaneReady = true + Expect(cl.Status().Update(ctx, capiCluster)).To(Succeed()) + + Eventually(ctx, func(g Gomega) { + res, err := r.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: capiCluster.Namespace, + Name: capiCluster.Name, + }, + }) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(res.Requeue).To(BeTrue()) + }).Should(Succeed()) + + Eventually(ctx, func(g Gomega) { + g.Expect(cl.List(ctx, rancherClusters, selectors...)).ToNot(HaveOccurred()) + g.Expect(rancherClusters.Items).To(HaveLen(1)) + }).Should(Succeed()) + Expect(rancherClusters.Items[0].Annotations).To(HaveKeyWithValue(externalFleetAnnotation, testLabelVal)) + }) + It("should reconcile a CAPI cluster when rancher cluster exists, and have finalizers set", func() { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) @@ -404,6 +427,51 @@ var _ = Describe("reconcile CAPI Cluster", func() { }).Should(Succeed()) }) + It("should set the fleet annotation on an already imported cluster", func() { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(sampleTemplate)) + })) + defer server.Close() + + Expect(cl.Create(ctx, capiCluster)).To(Succeed()) + capiCluster.Status.ControlPlaneReady = true + Expect(cl.Status().Update(ctx, capiCluster)).To(Succeed()) + + Expect(cl.Create(ctx, capiKubeconfigSecret)).To(Succeed()) + + Expect(cl.Create(ctx, rancherCluster)).To(Succeed()) + Eventually(ctx, func(g Gomega) { + g.Expect(cl.List(ctx, rancherClusters, selectors...)).ToNot(HaveOccurred()) + g.Expect(rancherClusters.Items).To(HaveLen(1)) + }).Should(Succeed()) + cluster := rancherClusters.Items[0] + Expect(cluster.Name).To(ContainSubstring("c-")) + + clusterRegistrationToken.Name = cluster.Name + clusterRegistrationToken.Namespace = cluster.Name + _, err := testEnv.CreateNamespaceWithName(ctx, cluster.Name) + Expect(err).ToNot(HaveOccurred()) + Expect(cl.Create(ctx, clusterRegistrationToken)).To(Succeed()) + token := clusterRegistrationToken.DeepCopy() + token.Status.ManifestURL = server.URL + Expect(cl.Status().Update(ctx, token)).To(Succeed()) + + Eventually(ctx, func(g Gomega) { + _, err := r.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: capiCluster.Namespace, + Name: capiCluster.Name, + }, + }) + g.Expect(err).ToNot(HaveOccurred()) + + rancherCluster := cluster.DeepCopy() + g.Expect(cl.Get(ctx, client.ObjectKeyFromObject(&cluster), rancherCluster)).To(Succeed()) + g.Expect(rancherCluster.Annotations).To(HaveKeyWithValue(externalFleetAnnotation, testLabelVal)) + }, 5*time.Second).Should(Succeed()) + }) + It("should reconcile a CAPI cluster when rancher cluster exists and a cluster registration token does not exist", func() { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index 9c3b6ad4..936ff6c7 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -54,6 +54,7 @@ var ( func init() { utilruntime.Must(feature.MutableGates.SetFromMap(map[string]bool{ string(feature.PropagateLabels): true, + string(feature.ExternalFleet): true, })) }