diff --git a/internal/controllers/helpers.go b/internal/controllers/helpers.go index e81ad35e..ba81fbff 100644 --- a/internal/controllers/helpers.go +++ b/internal/controllers/helpers.go @@ -26,6 +26,7 @@ import ( "net/http" "time" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -179,6 +180,41 @@ func createImportManifest(ctx context.Context, remoteClient client.Client, in io return nil } +func validateImportReadiness(ctx context.Context, remoteClient client.Client, in io.Reader) (bool, error) { + log := log.FromContext(ctx) + + jobs := &batchv1.JobList{} + if err := remoteClient.List(ctx, jobs, client.MatchingLabels(map[string]string{"cattle.io/creator": "norman"})); err != nil { + return false, fmt.Errorf("error looking for cleanup job: %w", err) + } + + for _, job := range jobs.Items { + if job.GenerateName == "cattle-cleanup-" { + log.Info("cleanup job is being performed, waiting...", "gvk", job.GroupVersionKind(), "name", job.GetName(), "namespace", job.GetNamespace()) + return true, nil + } + } + + reader := yamlDecoder.NewYAMLReader(bufio.NewReaderSize(in, 4096)) + + for { + raw, err := reader.Read() + if errors.Is(err, io.EOF) { + break + } + + if err != nil { + return false, err + } + + if requeue, err := verifyRawManifest(ctx, remoteClient, raw); err != nil || requeue { + return requeue, err + } + } + + return false, nil +} + func createRawManifest(ctx context.Context, remoteClient client.Client, bytes []byte) error { items, err := utilyaml.ToUnstructured(bytes) if err != nil { @@ -194,6 +230,41 @@ func createRawManifest(ctx context.Context, remoteClient client.Client, bytes [] return nil } +func verifyRawManifest(ctx context.Context, remoteClient client.Client, bytes []byte) (bool, error) { + items, err := utilyaml.ToUnstructured(bytes) + if err != nil { + return false, fmt.Errorf("error unmarshalling bytes or empty object passed: %w", err) + } + + for _, obj := range items { + if requeue, err := checkDeletion(ctx, remoteClient, obj.DeepCopy()); err != nil || requeue { + return requeue, err + } + } + + return false, nil +} + +func checkDeletion(ctx context.Context, c client.Client, obj client.Object) (bool, error) { + log := log.FromContext(ctx) + gvk := obj.GetObjectKind().GroupVersionKind() + + err := c.Get(ctx, client.ObjectKeyFromObject(obj), obj) + if apierrors.IsNotFound(err) { + log.V(4).Info("object is missing, ready to be created", "gvk", gvk, "name", obj.GetName(), "namespace", obj.GetNamespace()) + return false, nil + } else if err != nil { + return false, fmt.Errorf("checking object in remote cluster: %w", err) + } + + if obj.GetDeletionTimestamp() != nil { + log.Info("object is being deleted, waiting", "gvk", gvk, "name", obj.GetName(), "namespace", obj.GetNamespace()) + return true, nil + } + + return false, nil +} + func createObject(ctx context.Context, c client.Client, obj client.Object) error { log := log.FromContext(ctx) gvk := obj.GetObjectKind().GroupVersionKind() diff --git a/internal/controllers/import_controller.go b/internal/controllers/import_controller.go index 5d4280af..ee909f7b 100644 --- a/internal/controllers/import_controller.go +++ b/internal/controllers/import_controller.go @@ -251,7 +251,7 @@ func (r *CAPIImportReconciler) reconcileNormal(ctx context.Context, capiCluster log.Info("found cluster name", "name", rancherCluster.Status.ClusterName) if rancherCluster.Status.AgentDeployed { - log.Info("agent already deployed, no action needed") + log.Info("agent is deployed, no action needed") return ctrl.Result{}, nil } @@ -273,6 +273,13 @@ func (r *CAPIImportReconciler) reconcileNormal(ctx context.Context, capiCluster return ctrl.Result{}, fmt.Errorf("getting remote cluster client: %w", err) } + if requeue, err := validateImportReadiness(ctx, remoteClient, strings.NewReader(manifest)); err != nil { + return ctrl.Result{}, fmt.Errorf("verifying import manifest: %w", err) + } else if requeue { + log.Info("Import manifests are being deleted, not ready to be applied yet, requeue") + return ctrl.Result{RequeueAfter: defaultRequeueDuration}, nil + } + if err := createImportManifest(ctx, remoteClient, strings.NewReader(manifest)); err != nil { return ctrl.Result{}, fmt.Errorf("creating import manifest: %w", err) } diff --git a/internal/controllers/import_controller_v3.go b/internal/controllers/import_controller_v3.go index 3fa92000..38936675 100644 --- a/internal/controllers/import_controller_v3.go +++ b/internal/controllers/import_controller_v3.go @@ -150,18 +150,17 @@ func (r *CAPIImportManagementV3Reconciler) Reconcile(ctx context.Context, req ct return ctrl.Result{Requeue: true}, err } + log = log.WithValues("cluster", capiCluster.Name) + if capiCluster.ObjectMeta.DeletionTimestamp.IsZero() && !turtlesannotations.HasClusterImportAnnotation(capiCluster) && - !controllerutil.ContainsFinalizer(capiCluster, managementv3.CapiClusterFinalizer) { - log.Info("CAPI cluster is imported, adding finalizer") - controllerutil.AddFinalizer(capiCluster, managementv3.CapiClusterFinalizer) + controllerutil.AddFinalizer(capiCluster, managementv3.CapiClusterFinalizer) { + log.Info("CAPI cluster is marked for import, adding finalizer") if err := r.Client.Update(ctx, capiCluster); err != nil { return ctrl.Result{}, fmt.Errorf("error adding finalizer: %w", err) } } - log = log.WithValues("cluster", capiCluster.Name) - // Wait for controlplane to be ready. This should never be false as the predicates // do the filtering. if !capiCluster.Status.ControlPlaneReady && !conditions.IsTrue(capiCluster, clusterv1.ControlPlaneReadyCondition) { @@ -231,16 +230,25 @@ func (r *CAPIImportManagementV3Reconciler) reconcile(ctx context.Context, capiCl rancherCluster = &rancherClusterList.Items[0] } + if !rancherCluster.ObjectMeta.DeletionTimestamp.IsZero() { + if err := r.reconcileDelete(ctx, capiCluster); err != nil { + log.Error(err, "Removing CAPI Cluster failed, retrying") + return ctrl.Result{}, err + } + + if controllerutil.RemoveFinalizer(rancherCluster, managementv3.CapiClusterFinalizer) { + if err := r.Client.Update(ctx, rancherCluster); err != nil { + return ctrl.Result{}, fmt.Errorf("error removing rancher cluster finalizer: %w", err) + } + } + } + if !capiCluster.ObjectMeta.DeletionTimestamp.IsZero() { if err := r.deleteDependentRancherCluster(ctx, capiCluster); err != nil { return ctrl.Result{}, fmt.Errorf("error deleting associated managementv3.Cluster resources: %w", err) } } - if !rancherCluster.ObjectMeta.DeletionTimestamp.IsZero() { - return r.reconcileDelete(ctx, capiCluster) - } - return r.reconcileNormal(ctx, capiCluster, rancherCluster) } @@ -270,6 +278,9 @@ func (r *CAPIImportManagementV3Reconciler) reconcileNormal(ctx context.Context, capiClusterOwnerNamespace: capiCluster.Namespace, ownedLabelName: "", }, + Finalizers: []string{ + managementv3.CapiClusterFinalizer, + }, }, Spec: managementv3.ClusterSpec{ DisplayName: capiCluster.Name, @@ -296,9 +307,10 @@ func (r *CAPIImportManagementV3Reconciler) reconcileNormal(ctx context.Context, return ctrl.Result{}, err } - if feature.Gates.Enabled(feature.PropagateLabels) { - patchBase := client.MergeFromWithOptions(rancherCluster.DeepCopy(), client.MergeFromWithOptimisticLock{}) + 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{} } @@ -312,10 +324,14 @@ func (r *CAPIImportManagementV3Reconciler) reconcileNormal(ctx context.Context, } 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) + } } - if conditions.IsTrue(rancherCluster, managementv3.ClusterConditionAgentDeployed) { - log.Info("agent already deployed, no action needed") + if conditions.IsTrue(rancherCluster, managementv3.ClusterConditionReady) { + log.Info("agent is ready, no action needed") return ctrl.Result{}, nil } @@ -337,6 +353,13 @@ func (r *CAPIImportManagementV3Reconciler) reconcileNormal(ctx context.Context, return ctrl.Result{}, fmt.Errorf("getting remote cluster client: %w", err) } + if requeue, err := validateImportReadiness(ctx, remoteClient, strings.NewReader(manifest)); err != nil { + return ctrl.Result{}, fmt.Errorf("verifying import manifest: %w", err) + } else if requeue { + log.Info("Import manifests are being deleted, not ready to be applied yet, requeue") + return ctrl.Result{RequeueAfter: defaultRequeueDuration}, nil + } + if err := createImportManifest(ctx, remoteClient, strings.NewReader(manifest)); err != nil { return ctrl.Result{}, fmt.Errorf("creating import manifest: %w", err) } @@ -424,7 +447,7 @@ func (r *CAPIImportManagementV3Reconciler) rancherV1ClusterToCapiCluster(ctx con } } -func (r *CAPIImportManagementV3Reconciler) reconcileDelete(ctx context.Context, capiCluster *clusterv1.Cluster) (ctrl.Result, error) { +func (r *CAPIImportManagementV3Reconciler) reconcileDelete(ctx context.Context, capiCluster *clusterv1.Cluster) error { log := log.FromContext(ctx) log.Info("Reconciling rancher cluster deletion") @@ -440,16 +463,13 @@ func (r *CAPIImportManagementV3Reconciler) reconcileDelete(ctx context.Context, annotations[turtlesannotations.ClusterImportedAnnotation] = "true" capiCluster.SetAnnotations(annotations) + controllerutil.RemoveFinalizer(capiCluster, managementv3.CapiClusterFinalizer) - if controllerutil.ContainsFinalizer(capiCluster, managementv3.CapiClusterFinalizer) { - controllerutil.RemoveFinalizer(capiCluster, managementv3.CapiClusterFinalizer) - - if err := r.Client.Update(ctx, capiCluster); err != nil { - return ctrl.Result{}, fmt.Errorf("error removing finalizer: %w", err) - } + if err := r.Client.Update(ctx, capiCluster); err != nil { + return fmt.Errorf("error removing finalizer: %w", err) } - return ctrl.Result{}, nil + return nil } func (r *CAPIImportManagementV3Reconciler) deleteDependentRancherCluster(ctx context.Context, capiCluster *clusterv1.Cluster) error { @@ -464,7 +484,7 @@ func (r *CAPIImportManagementV3Reconciler) deleteDependentRancherCluster(ctx con }, } - return r.RancherClient.DeleteAllOf(ctx, &managementv3.Cluster{}, selectors...) + return client.IgnoreNotFound(r.RancherClient.DeleteAllOf(ctx, &managementv3.Cluster{}, selectors...)) } // verifyV1ClusterMigration verifies if a v1 cluster has been successfully migrated. diff --git a/internal/controllers/import_controller_v3_test.go b/internal/controllers/import_controller_v3_test.go index 53010ac0..7cfb0764 100644 --- a/internal/controllers/import_controller_v3_test.go +++ b/internal/controllers/import_controller_v3_test.go @@ -170,7 +170,7 @@ var _ = Describe("reconcile CAPI Cluster", func() { }).Should(Succeed()) }) - It("should reconcile a CAPI cluster when rancher cluster doesn't exist", func() { + It("should reconcile a CAPI cluster when rancher cluster doesn't exist, and set finalizers", func() { ns.Labels = map[string]string{} Expect(cl.Update(ctx, ns)).To(Succeed()) capiCluster.Labels = map[string]string{ @@ -195,9 +195,15 @@ var _ = Describe("reconcile CAPI Cluster", func() { Eventually(ctx, func(g Gomega) { g.Expect(cl.List(ctx, rancherClusters, selectors...)).ToNot(HaveOccurred()) g.Expect(rancherClusters.Items).To(HaveLen(1)) + g.Expect(rancherClusters.Items[0].Name).To(ContainSubstring("c-")) + g.Expect(rancherClusters.Items[0].Labels).To(HaveKeyWithValue(testLabelName, testLabelVal)) + g.Expect(rancherClusters.Items[0].Finalizers).To(ContainElement(managementv3.CapiClusterFinalizer)) + }).Should(Succeed()) + + Eventually(ctx, func(g Gomega) { + g.Expect(cl.Get(ctx, client.ObjectKeyFromObject(capiCluster), capiCluster)).ToNot(HaveOccurred()) + g.Expect(capiCluster.Finalizers).To(ContainElement(managementv3.CapiClusterFinalizer)) }).Should(Succeed()) - Expect(rancherClusters.Items[0].Name).To(ContainSubstring("c-")) - Expect(rancherClusters.Items[0].Labels).To(HaveKeyWithValue(testLabelName, testLabelVal)) }) It("should reconcile a CAPI cluster when rancher cluster doesn't exist and annotation is set on the namespace", func() { @@ -223,7 +229,7 @@ var _ = Describe("reconcile CAPI Cluster", func() { Expect(rancherClusters.Items[0].Name).To(ContainSubstring("c-")) }) - It("should reconcile a CAPI cluster when rancher cluster exists", func() { + 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) w.Write([]byte(sampleTemplate)) @@ -244,6 +250,7 @@ var _ = Describe("reconcile CAPI Cluster", func() { Eventually(ctx, func(g Gomega) { g.Expect(cl.List(ctx, rancherClusters, selectors...)).ToNot(HaveOccurred()) g.Expect(rancherClusters.Items).To(HaveLen(1)) + g.Expect(rancherClusters.Items[0].Finalizers).ToNot(ContainElement(managementv3.CapiClusterFinalizer)) }).Should(Succeed()) cluster := rancherClusters.Items[0] Expect(cluster.Name).To(ContainSubstring("c-")) @@ -282,11 +289,16 @@ var _ = Describe("reconcile CAPI Cluster", func() { Name: unstructuredObj.GetName(), }, unstructuredObj)).To(Succeed()) - g.Expect(cl.List(ctx, rancherClusters, selectors...)).ToNot(HaveOccurred()) - g.Expect(rancherClusters.Items).To(HaveLen(1)) - g.Expect(rancherClusters.Items[0].Name).To(ContainSubstring("c-")) - g.Expect(rancherClusters.Items[0].Labels).To(HaveKeyWithValue(testLabelName, testLabelVal)) } + + g.Expect(cl.Get(ctx, client.ObjectKeyFromObject(capiCluster), capiCluster)).ToNot(HaveOccurred()) + g.Expect(capiCluster.Finalizers).To(ContainElement(managementv3.CapiClusterFinalizer)) + + g.Expect(cl.List(ctx, rancherClusters, selectors...)).ToNot(HaveOccurred()) + g.Expect(rancherClusters.Items).To(HaveLen(1)) + g.Expect(rancherClusters.Items[0].Name).To(ContainSubstring("c-")) + g.Expect(rancherClusters.Items[0].Labels).To(HaveKeyWithValue(testLabelName, testLabelVal)) + g.Expect(rancherClusters.Items[0].Finalizers).To(ContainElement(managementv3.CapiClusterFinalizer)) }, 10*time.Second).Should(Succeed()) }) @@ -332,8 +344,8 @@ var _ = Describe("reconcile CAPI Cluster", func() { cluster := rancherClusters.Items[0] Expect(cluster.Name).To(ContainSubstring("c-")) - conditions.Set(&cluster, conditions.TrueCondition(managementv3.ClusterConditionAgentDeployed)) - Expect(conditions.IsTrue(&cluster, managementv3.ClusterConditionAgentDeployed)).To(BeTrue()) + conditions.Set(&cluster, conditions.TrueCondition(managementv3.ClusterConditionReady)) + Expect(conditions.IsTrue(&cluster, managementv3.ClusterConditionReady)).To(BeTrue()) Expect(cl.Status().Update(ctx, &cluster)).To(Succeed()) _, err := r.Reconcile(ctx, reconcile.Request{ @@ -477,8 +489,8 @@ var _ = Describe("reconcile CAPI Cluster", func() { Eventually(ctx, func(g Gomega) { g.Expect(cl.Get(ctx, client.ObjectKeyFromObject(rancherCluster), rancherCluster)).To(Succeed()) - conditions.Set(rancherCluster, conditions.TrueCondition(managementv3.ClusterConditionAgentDeployed)) - g.Expect(conditions.IsTrue(rancherCluster, managementv3.ClusterConditionAgentDeployed)).To(BeTrue()) + conditions.Set(rancherCluster, conditions.TrueCondition(managementv3.ClusterConditionReady)) + g.Expect(conditions.IsTrue(rancherCluster, managementv3.ClusterConditionReady)).To(BeTrue()) g.Expect(cl.Status().Update(ctx, rancherCluster)).To(Succeed()) }).Should(Succeed()) @@ -534,4 +546,5 @@ var _ = Describe("reconcile CAPI Cluster", func() { Expect(rancherClusters.Items).To(HaveLen(0)) }).Should(Succeed()) }) + }) diff --git a/internal/rancher/provisioning/v1/cluster.go b/internal/rancher/provisioning/v1/cluster.go index 5453b002..f229c08e 100644 --- a/internal/rancher/provisioning/v1/cluster.go +++ b/internal/rancher/provisioning/v1/cluster.go @@ -18,6 +18,8 @@ package v1 import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ) // Cluster is the struct representing a Rancher Cluster. @@ -41,6 +43,8 @@ type ClusterStatus struct { ClusterName string `json:"clusterName,omitempty"` AgentDeployed bool `json:"agentDeployed,omitempty"` Ready bool `json:"ready,omitempty"` + + Conditions clusterv1.Conditions `json:"conditions,omitempty"` } // ClusterList contains a list of ClusterList. @@ -51,6 +55,16 @@ type ClusterList struct { Items []Cluster `json:"items"` } +// GetConditions method to implement capi conditions getter interface. +func (c *Cluster) GetConditions() clusterv1.Conditions { + return c.Status.Conditions +} + +// SetConditions method to implement capi conditions setter interface. +func (c *Cluster) SetConditions(conditions clusterv1.Conditions) { + c.Status.Conditions = conditions +} + func init() { SchemeBuilder.Register(&Cluster{}, &ClusterList{}) } diff --git a/internal/rancher/provisioning/v1/zz_generated.deepcopy.go b/internal/rancher/provisioning/v1/zz_generated.deepcopy.go index f70e6fb9..971c78fb 100644 --- a/internal/rancher/provisioning/v1/zz_generated.deepcopy.go +++ b/internal/rancher/provisioning/v1/zz_generated.deepcopy.go @@ -23,6 +23,7 @@ package v1 import ( corev1 "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/cluster-api/api/v1beta1" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -31,7 +32,7 @@ func (in *Cluster) DeepCopyInto(out *Cluster) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Cluster. @@ -107,6 +108,13 @@ func (in *ClusterSpec) DeepCopy() *ClusterSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterStatus) DeepCopyInto(out *ClusterStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make(v1beta1.Conditions, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterStatus. diff --git a/test/e2e/helpers.go b/test/e2e/helpers.go index 21cb1eee..49c2208f 100644 --- a/test/e2e/helpers.go +++ b/test/e2e/helpers.go @@ -60,20 +60,6 @@ func CreateRepoName(specName string) string { } func DumpSpecResourcesAndCleanup(ctx context.Context, specName string, clusterProxy framework.ClusterProxy, artifactFolder string, namespace *corev1.Namespace, cancelWatches context.CancelFunc, capiCluster *types.NamespacedName, intervalsGetter func(spec, key string) []interface{}, skipCleanup bool) { - turtlesframework.Byf("Dumping logs from the %q workload cluster", capiCluster.Name) - - // Dump all the logs from the workload cluster before deleting them. - clusterProxy.CollectWorkloadClusterLogs(ctx, capiCluster.Namespace, capiCluster.Name, filepath.Join(artifactFolder, "clusters", capiCluster.Name)) - - turtlesframework.Byf("Dumping all the Cluster API resources in the %q namespace", namespace.Name) - - // Dump all Cluster API related resources to artifacts before deleting them. - framework.DumpAllResources(ctx, framework.DumpAllResourcesInput{ - Lister: clusterProxy.GetClient(), - Namespace: namespace.Name, - LogPath: filepath.Join(artifactFolder, "clusters", clusterProxy.GetName(), "resources"), - }) - if !skipCleanup { turtlesframework.Byf("Deleting cluster %s", capiCluster) // While https://github.com/kubernetes-sigs/cluster-api/issues/2955 is addressed in future iterations, there is a chance diff --git a/test/e2e/suites/embedded-capi-disabled/embedded_capi_disabled_test.go b/test/e2e/suites/embedded-capi-disabled/embedded_capi_disabled_test.go index d4f240c3..50ebace9 100644 --- a/test/e2e/suites/embedded-capi-disabled/embedded_capi_disabled_test.go +++ b/test/e2e/suites/embedded-capi-disabled/embedded_capi_disabled_test.go @@ -30,7 +30,6 @@ import ( ) var _ = Describe("[AWS] [EKS] Create and delete CAPI cluster functionality should work with namespace auto-import (embedded capi disable from start)", Label(e2e.FullTestLabel), func() { - BeforeEach(func() { SetClient(setupClusterResult.BootstrapClusterProxy.GetClient()) SetContext(ctx) diff --git a/test/e2e/suites/embedded-capi-disabled/suite_test.go b/test/e2e/suites/embedded-capi-disabled/suite_test.go index 24600492..5a2c749a 100644 --- a/test/e2e/suites/embedded-capi-disabled/suite_test.go +++ b/test/e2e/suites/embedded-capi-disabled/suite_test.go @@ -197,8 +197,9 @@ var _ = BeforeSuite(func() { Tag: "v0.0.1", WaitDeploymentsReadyInterval: e2eConfig.GetIntervals(setupClusterResult.BootstrapClusterProxy.GetName(), "wait-controllers"), AdditionalValues: map[string]string{ - "cluster-api-operator.cert-manager.enabled": "false", - "rancherTurtles.features.embedded-capi.disabled": "false", + "cluster-api-operator.cert-manager.enabled": "false", + "rancherTurtles.features.embedded-capi.disabled": "false", + "rancherTurtles.features.managementv3-cluster.enabled": "false", }, } if flagVals.UseEKS {