diff --git a/baremetal/metal3data_manager.go b/baremetal/metal3data_manager.go index 0440538f07..3636a32080 100644 --- a/baremetal/metal3data_manager.go +++ b/baremetal/metal3data_manager.go @@ -1437,7 +1437,7 @@ func (m *DataManager) getM3Machine(ctx context.Context, m3dt *infrav1.Metal3Data } // not matching on UID since when pivoting it might change // Not matching on API version as this might change - if ownerRef.Kind == "Metal3Machine" && + if ownerRef.Kind == metal3MachineKind && oGV.Group == infrav1.GroupVersion.Group { metal3MachineName = ownerRef.Name break diff --git a/baremetal/metal3datatemplate_manager.go b/baremetal/metal3datatemplate_manager.go index 856670760e..eb865457e0 100644 --- a/baremetal/metal3datatemplate_manager.go +++ b/baremetal/metal3datatemplate_manager.go @@ -231,6 +231,27 @@ func (m *DataTemplateManager) updateData(ctx context.Context, return indexes, err } } else { + // Check the owner references for the Metal3Machine, if it is gone, delete the + // Metal3Data and Metal3DataClaim. + for _, ownerRef := range dataClaim.OwnerReferences { + if ownerRef.Kind == metal3MachineKind { + // Check if the Metal3Machine is gone + err = m.client.Get(ctx, client.ObjectKey{ + Name: ownerRef.Name, + Namespace: dataClaim.Namespace, + }, &infrav1.Metal3Machine{}) + if err != nil && apierrors.IsNotFound(err) { + // The Metal3Machine is gone. It is safe to delete the Metal3Data and Metal3DataClaim. + indexes, err = m.deleteData(ctx, dataClaim, indexes) + if err != nil { + return indexes, err + } + } + // The Metal3Machine is still there, do not delete the Metal3Data and Metal3DataClaim. + return indexes, nil + } + } + // There is no Metal3Machine owner reference, so the Metal3Data and Metal3DataClaim can be deleted. indexes, err = m.deleteData(ctx, dataClaim, indexes) if err != nil { return indexes, err @@ -271,7 +292,7 @@ func (m *DataTemplateManager) createData(ctx context.Context, if err != nil { return indexes, err } - if ownerRef.Kind == "Metal3Machine" && + if ownerRef.Kind == metal3MachineKind && aGV.Group == infrav1.GroupVersion.Group { m3mUID = ownerRef.UID m3mName = ownerRef.Name @@ -332,7 +353,7 @@ func (m *DataTemplateManager) createData(ctx context.Context, }, { APIVersion: dataClaim.APIVersion, - Kind: "Metal3Machine", + Kind: metal3MachineKind, Name: m3mName, UID: m3mUID, }, diff --git a/baremetal/metal3datatemplate_manager_test.go b/baremetal/metal3datatemplate_manager_test.go index 617107f2b9..38979c1e92 100644 --- a/baremetal/metal3datatemplate_manager_test.go +++ b/baremetal/metal3datatemplate_manager_test.go @@ -299,24 +299,24 @@ var _ = Describe("Metal3DataTemplate manager", func() { dataClaims: []*infrav1.Metal3DataClaim{ { ObjectMeta: metav1.ObjectMeta{ - Name: "abc", + Name: "claim-without-status", Namespace: namespaceName, }, Spec: infrav1.Metal3DataClaimSpec{ Template: corev1.ObjectReference{ - Name: "abc", + Name: templateMeta.Name, Namespace: namespaceName, }, }, }, { ObjectMeta: metav1.ObjectMeta{ - Name: "abcd", + Name: "orphaned-claim", Namespace: namespaceName, }, Spec: infrav1.Metal3DataClaimSpec{ Template: corev1.ObjectReference{ - Name: "abcd", + Name: "other-template", Namespace: namespaceName, }, }, @@ -329,12 +329,12 @@ var _ = Describe("Metal3DataTemplate manager", func() { }, { ObjectMeta: metav1.ObjectMeta{ - Name: "abce", + Name: "claim-with-status", Namespace: namespaceName, }, Spec: infrav1.Metal3DataClaimSpec{ Template: corev1.ObjectReference{ - Name: "abc", + Name: templateMeta.Name, Namespace: namespaceName, }, }, @@ -347,14 +347,14 @@ var _ = Describe("Metal3DataTemplate manager", func() { }, { ObjectMeta: metav1.ObjectMeta{ - Name: "abcf", + Name: "deleting-claim", Namespace: namespaceName, DeletionTimestamp: &timeNow, Finalizers: []string{"ipclaim.ipam.metal3.io"}, }, Spec: infrav1.Metal3DataClaimSpec{ Template: corev1.ObjectReference{ - Name: "abc", + Name: templateMeta.Name, Namespace: namespaceName, }, }, @@ -374,11 +374,11 @@ var _ = Describe("Metal3DataTemplate manager", func() { }, Spec: infrav1.Metal3DataSpec{ Template: corev1.ObjectReference{ - Name: "abc", + Name: templateMeta.Name, Namespace: namespaceName, }, Claim: corev1.ObjectReference{ - Name: "abc", + Name: "claim-without-status", Namespace: namespaceName, }, Index: 0, @@ -391,11 +391,11 @@ var _ = Describe("Metal3DataTemplate manager", func() { }, Spec: infrav1.Metal3DataSpec{ Template: corev1.ObjectReference{ - Name: "abc", + Name: templateMeta.Name, Namespace: namespaceName, }, Claim: corev1.ObjectReference{ - Name: "abce", + Name: "claim-with-status", Namespace: namespaceName, }, Index: 1, @@ -403,16 +403,16 @@ var _ = Describe("Metal3DataTemplate manager", func() { }, { ObjectMeta: metav1.ObjectMeta{ - Name: "abc-3", + Name: "data-to-delete", Namespace: namespaceName, }, Spec: infrav1.Metal3DataSpec{ Template: corev1.ObjectReference{ - Name: "abc", + Name: templateMeta.Name, Namespace: namespaceName, }, Claim: corev1.ObjectReference{ - Name: "abcf", + Name: "deleting-claim", Namespace: namespaceName, }, Index: 3, @@ -420,8 +420,8 @@ var _ = Describe("Metal3DataTemplate manager", func() { }, }, expectedIndexes: map[string]int{ - "abc": 0, - "abce": 1, + "claim-without-status": 0, + "claim-with-status": 1, }, expectedNbIndexes: 2, }), diff --git a/baremetal/utils.go b/baremetal/utils.go index 2db3d05114..ccd84100dd 100644 --- a/baremetal/utils.go +++ b/baremetal/utils.go @@ -38,6 +38,8 @@ import ( const ( // metal3SecretType defines the type of secret created by metal3. metal3SecretType corev1.SecretType = "infrastructure.cluster.x-k8s.io/secret" + // metal3MachineKind is the Kind of the Metal3Machine. + metal3MachineKind = "Metal3Machine" ) // Filter filters out occurrences of strToFilter from list and returns the new list. diff --git a/controllers/metal3data_controller.go b/controllers/metal3data_controller.go index e5e0d35fe0..626f821115 100644 --- a/controllers/metal3data_controller.go +++ b/controllers/metal3data_controller.go @@ -60,29 +60,29 @@ func (r *Metal3DataReconciler) Reconcile(ctx context.Context, req ctrl.Request) metadataLog := r.Log.WithName(dataControllerName).WithValues("metal3-data", req.NamespacedName) // Fetch the Metal3Data instance. - capm3Metadata := &infrav1.Metal3Data{} + metal3Data := &infrav1.Metal3Data{} - if err := r.Client.Get(ctx, req.NamespacedName, capm3Metadata); err != nil { + if err := r.Client.Get(ctx, req.NamespacedName, metal3Data); err != nil { if apierrors.IsNotFound(err) { return ctrl.Result{}, nil } return ctrl.Result{}, err } - helper, err := patch.NewHelper(capm3Metadata, r.Client) + helper, err := patch.NewHelper(metal3Data, r.Client) if err != nil { return ctrl.Result{}, errors.Wrap(err, "failed to init patch helper") } - // Always patch capm3Data exiting this function so we can persist any Metal3Data changes. + // Always patch Metal3Data exiting this function so we can persist any changes. defer func() { - err := helper.Patch(ctx, capm3Metadata) + err := helper.Patch(ctx, metal3Data) if err != nil { metadataLog.Info("failed to Patch Metal3Data") } }() // Fetch the Cluster. - cluster, err := util.GetClusterFromMetadata(ctx, r.Client, capm3Metadata.ObjectMeta) - if capm3Metadata.ObjectMeta.DeletionTimestamp.IsZero() { + cluster, err := util.GetClusterFromMetadata(ctx, r.Client, metal3Data.ObjectMeta) + if metal3Data.ObjectMeta.DeletionTimestamp.IsZero() { if err != nil { metadataLog.Info("Metal3Data is missing cluster label or cluster does not exist") return ctrl.Result{}, nil @@ -97,20 +97,32 @@ func (r *Metal3DataReconciler) Reconcile(ctx context.Context, req ctrl.Request) metadataLog = metadataLog.WithValues("cluster", cluster.Name) // Return early if the Metadata or Cluster is paused. - if annotations.IsPaused(cluster, capm3Metadata) { + if annotations.IsPaused(cluster, metal3Data) { metadataLog.Info("reconciliation is paused for this object") return ctrl.Result{Requeue: true, RequeueAfter: requeueAfter}, nil } } // Create a helper for managing the metadata object. - metadataMgr, err := r.ManagerFactory.NewDataManager(capm3Metadata, metadataLog) + metadataMgr, err := r.ManagerFactory.NewDataManager(metal3Data, metadataLog) if err != nil { return ctrl.Result{}, errors.Wrapf(err, "failed to create helper for managing the Metal3Data") } - // Handle deleted metadata - if !capm3Metadata.ObjectMeta.DeletionTimestamp.IsZero() { + // Handle deletion of Metal3Data + if !metal3Data.ObjectMeta.DeletionTimestamp.IsZero() { + // Check if the Metal3DataClaim is gone. We cannot clean up until it is. + for _, ownerRef := range metal3Data.OwnerReferences { + if ownerRef.Kind == "Metal3DataClaim" { + err := r.Client.Get(ctx, types.NamespacedName{Name: ownerRef.Name, Namespace: metal3Data.Namespace}, &infrav1.Metal3DataClaim{}) + if err != nil && apierrors.IsNotFound(err) { + return r.reconcileDelete(ctx, metadataMgr) + } + // We were unable to determine if the Metal3DataClaim is gone + return ctrl.Result{}, err + } + } + // There is no Metal3DataClaim in the ownerReferences, so we can delete the Metal3Data. return r.reconcileDelete(ctx, metadataMgr) }