Add extra finalizers to stop metal3data/dataClaim from being deletabl…

…e while in use. Signed-off-by: Max Rantil <[email protected]>
metal3-io · Apr 2, 2024 · dffc5b7 · dffc5b7
1 parent c9a0a3b
commit dffc5b7
Show file tree

Hide file tree

Showing 2 changed files with 108 additions and 44 deletions.
diff --git a/baremetal/metal3datatemplate_manager.go b/baremetal/metal3datatemplate_manager.go
@@ -18,7 +18,6 @@ package baremetal
 
 import (
 	"context"
-	"fmt"
 	"strconv"
 
 	"github.com/go-logr/logr"
@@ -232,6 +231,14 @@ func (m *DataTemplateManager) updateData(ctx context.Context,
 			return indexes, err
 		}
 	} else {
+		// Check if this claim is in use. Does it have any owners?
+		// If it is no longer in use, proceed to delete the associated data
+		if len(dataClaim.OwnerReferences) > 0 {
+			m.Log.Info("Metal3DataClaim is still in use (has owners). Cannot delete Metal3Data.",
+				"Metal3DataClaim", dataClaim.Name, "Owners", dataClaim.OwnerReferences)
+			return indexes, nil
+		}
+
 		indexes, err = m.deleteData(ctx, dataClaim, indexes)
 		if err != nil {
 			return indexes, err
@@ -306,16 +313,17 @@ func (m *DataTemplateManager) createData(ctx context.Context,
 	m.Log.Info("Index", "Claim", dataClaim.Name, "index", claimIndex)
 
 	// Create the Metal3Data object, with an Owner ref to the Metal3Machine
-	// (curOwnerRef) and to the Metal3DataTemplate
+	// (curOwnerRef) and to the Metal3DataTemplate. Also add a finalizer.
 	dataObject := &infrav1.Metal3Data{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Metal3Data",
 			APIVersion: infrav1.GroupVersion.String(),
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      dataName,
-			Namespace: m.DataTemplate.Namespace,
-			Labels:    dataClaim.Labels,
+			Name:       dataName,
+			Namespace:  m.DataTemplate.Namespace,
+			Finalizers: []string{infrav1.MachineFinalizer},
+			Labels:     dataClaim.Labels,
 			OwnerReferences: []metav1.OwnerReference{
 				{
 					Controller: ptr.To(true),
@@ -374,18 +382,18 @@ func (m *DataTemplateManager) createData(ctx context.Context,
 	return indexes, nil
 }
 
-// DeleteDatas deletes old secrets.
+// deleteData removes the finalizers from the Data and deletes the associated Metal3Data.
 func (m *DataTemplateManager) deleteData(ctx context.Context,
 	dataClaim *infrav1.Metal3DataClaim, indexes map[int]string,
 ) (map[int]string, error) {
-	var dataName string
-	m.Log.Info("Deleting Claim", "Metal3DataClaim", dataClaim.Name)
+	m.Log.Info("Deleting Metal3Data associated with Metal3DataClaim", "Metal3DataClaim", dataClaim.Name)
 
 	dataClaimIndex, ok := m.DataTemplate.Status.Indexes[dataClaim.Name]
 	if ok {
 		// Try to get the Metal3Data. if it succeeds, delete it
 		tmpM3Data := &infrav1.Metal3Data{}
 
+		var dataName string
 		if m.DataTemplate.Spec.TemplateReference != "" {
 			dataName = m.DataTemplate.Spec.TemplateReference + "-" + strconv.Itoa(dataClaimIndex)
 		} else {
@@ -401,26 +409,42 @@ func (m *DataTemplateManager) deleteData(ctx context.Context,
 			dataClaim.Status.ErrorMessage = ptr.To("Failed to get associated Metal3Data object")
 			return indexes, err
 		} else if err == nil {
-			// Delete the secret with metadata
-			fmt.Println(tmpM3Data.Name)
-			err = m.client.Delete(ctx, tmpM3Data)
+			// Remove the finalizer
+			tmpM3Data.Finalizers = Filter(tmpM3Data.Finalizers,
+				infrav1.MachineFinalizer,
+			)
+			err = updateObject(ctx, m.client, tmpM3Data)
+			if err != nil && !apierrors.IsNotFound(err) {
+				m.Log.Info("Unable to remove finalizer from Metal3Data", "Metal3Data", tmpM3Data.Name)
+				return indexes, err
+			}
+			// Delete the Metal3Data
+			err = deleteObject(ctx, m.client, tmpM3Data)
 			if err != nil && !apierrors.IsNotFound(err) {
 				dataClaim.Status.ErrorMessage = ptr.To("Failed to delete associated Metal3Data object")
 				return indexes, err
 			}
+			m.Log.Info("Deleted Metal3Data", "Metal3Data", tmpM3Data.Name)
 		}
 	}
+
 	dataClaim.Status.RenderedData = nil
 	dataClaim.Finalizers = Filter(dataClaim.Finalizers,
 		infrav1.DataClaimFinalizer,
 	)
 
-	m.Log.Info("Deleted Claim", "Metal3DataClaim", dataClaim.Name)
+	err := updateObject(ctx, m.client, dataClaim)
+	if err != nil && !apierrors.IsNotFound(err) {
+		m.Log.Info("Unable to remove finalizers from Metal3DataClaim", "Metal3DataClaim", dataClaim.Name)
+		return indexes, err
+	}
 
 	if ok {
 		delete(m.DataTemplate.Status.Indexes, dataClaim.Name)
 		delete(indexes, dataClaimIndex)
 	}
+
+	m.Log.Info("Deleted Metal3DataClaim", "Metal3DataClaim", dataClaim.Name)
 	m.updateStatusTimestamp()
 	return indexes, nil
 }
diff --git a/baremetal/metal3datatemplate_manager_test.go b/baremetal/metal3datatemplate_manager_test.go
@@ -425,6 +425,78 @@ var _ = Describe("Metal3DataTemplate manager", func() {
 			},
 			expectedNbIndexes: 2,
 		}),
+		// Entry("Metal3DataClaim with deletion timestamp without owner nor finalizer", testCaseUpdateDatas{
+		// 	template: &infrav1.Metal3DataTemplate{
+		// 		ObjectMeta: templateMeta,
+		// 		Spec:       infrav1.Metal3DataTemplateSpec{},
+		// 	},
+		// 	dataClaims: []*infrav1.Metal3DataClaim{
+		// 		{
+		// 			ObjectMeta: metav1.ObjectMeta{
+		// 				Name:              "dataClaimNoOwner",
+		// 				Namespace:         namespaceName,
+		// 				DeletionTimestamp: &timeNow,
+		// 			},
+		// 			Spec: infrav1.Metal3DataClaimSpec{
+		// 				Template: corev1.ObjectReference{
+		// 					Name:      "dataClaimNoOwnerTemplate",
+		// 					Namespace: namespaceName,
+		// 				},
+		// 			},
+		// 		},
+		// 	},
+		// 	expectedNbIndexes: 0,
+		// }),
+		Entry("Metal3DataClaim with deletion timestamp, owner and finalizer", testCaseUpdateDatas{
+			template: &infrav1.Metal3DataTemplate{
+				ObjectMeta: templateMeta,
+				Spec:       infrav1.Metal3DataTemplateSpec{},
+			},
+			dataClaims: []*infrav1.Metal3DataClaim{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:              "dataClaimWithOwner",
+						Namespace:         namespaceName,
+						DeletionTimestamp: &timeNow,
+						Finalizers:        []string{infrav1.DataClaimFinalizer, infrav1.MachineFinalizer},
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								APIVersion: "v1",
+								Kind:       "SomeKind",
+								Name:       "SomeName",
+								UID:        "SomeUID",
+							},
+						},
+					},
+					Spec: infrav1.Metal3DataClaimSpec{
+						Template: corev1.ObjectReference{
+							Name:      "dataClaimWithOwnerTemplate",
+							Namespace: namespaceName,
+						},
+					},
+				},
+			},
+			datas: []*infrav1.Metal3Data{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "dataWithOwner",
+						Namespace: namespaceName,
+					},
+					Spec: infrav1.Metal3DataSpec{
+						Template: corev1.ObjectReference{
+							Name:      "dataClaimWithOwnerTemplate",
+							Namespace: namespaceName,
+						},
+						Claim: corev1.ObjectReference{
+							Name:      "dataClaimWithOwner",
+							Namespace: namespaceName,
+						},
+						Index: 0,
+					},
+				},
+			},
+			expectedNbIndexes: 1,
+		}),
 	)
 
 	type testCaseTemplateReference struct {
@@ -818,37 +890,5 @@ var _ = Describe("Metal3DataTemplate manager", func() {
 			expectedIndexes: map[string]int{},
 			expectedMap:     map[int]string{},
 		}),
-		Entry("Deletion needed", testCaseDeleteDatas{
-			template: &infrav1.Metal3DataTemplate{
-				ObjectMeta: testObjectMeta("abc", "", ""),
-				Spec:       infrav1.Metal3DataTemplateSpec{},
-				Status: infrav1.Metal3DataTemplateStatus{
-					Indexes: map[string]int{
-						"TestRef": 0,
-					},
-				},
-			},
-			dataClaim: &infrav1.Metal3DataClaim{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "TestRef",
-					Finalizers: []string{
-						infrav1.DataClaimFinalizer,
-					},
-				},
-			},
-			indexes: map[int]string{
-				0: "TestRef",
-			},
-			expectedMap:     map[int]string{},
-			expectedIndexes: map[string]int{},
-			datas: []*infrav1.Metal3Data{
-				{
-					ObjectMeta: metav1.ObjectMeta{
-						Name: "abc-0",
-					},
-				},
-			},
-		}),
 	)
-
 })