Fix unterminating DataTemplate deletion

DataTemplate deletion has a watch over DataClaims but counts Data. We ensure that only Data for which a live claim exist are counted to avoid races during multiple DataClaim deletion that result with a positive count of data when all claims have been deleted. Fixes: #1994 Co-authored-by: Pierre Crégut <[email protected]> Co-authored-by: Thomas Morin <[email protected]> Signed-off-by: Pierre Crégut <[email protected]>
metal3-io · Sep 26, 2024 · 515943d · 515943d
1 parent 348375c
commit 515943d
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 17 deletions.
diff --git a/baremetal/metal3datatemplate_manager.go b/baremetal/metal3datatemplate_manager.go
@@ -104,11 +104,12 @@ func (m *DataTemplateManager) SetClusterOwnerRef(cluster *clusterv1.Cluster) err
 }
 
 // RecreateStatus recreates the status if empty.
-func (m *DataTemplateManager) getIndexes(ctx context.Context) (map[int]string, error) {
+func (m *DataTemplateManager) getIndexes(ctx context.Context) (map[int]string, map[string]int, error) {
 	m.Log.Info("Fetching Metal3Data objects")
 
 	// start from empty maps
 	m.DataTemplate.Status.Indexes = make(map[string]int)
+	statusIndexes := make(map[string]int)
 
 	indexes := make(map[int]string)
 
@@ -121,7 +122,7 @@ func (m *DataTemplateManager) getIndexes(ctx context.Context) (map[int]string, e
 
 	err := m.client.List(ctx, &dataObjects, opts)
 	if err != nil {
-		return indexes, err
+		return indexes, statusIndexes, err
 	}
 
 	// Iterate over the Metal3Data objects to find all indexes and objects
@@ -135,17 +136,12 @@ func (m *DataTemplateManager) getIndexes(ctx context.Context) (map[int]string, e
 			continue
 		}
 
-		// Get the claim Name, if unset use empty string, to still record the
-		// index being used, to avoid conflicts
-		claimName := ""
-		if dataObject.Spec.Claim.Name != "" {
-			claimName = dataObject.Spec.Claim.Name
-		}
-		m.DataTemplate.Status.Indexes[claimName] = dataObject.Spec.Index
+		claimName := dataObject.Spec.Claim.Name
+		statusIndexes[claimName] = dataObject.Spec.Index
 		indexes[dataObject.Spec.Index] = claimName
 	}
 	m.updateStatusTimestamp()
-	return indexes, nil
+	return indexes, statusIndexes, nil
 }
 
 func (m *DataTemplateManager) dataObjectBelongsToTemplate(dataObject infrav1.Metal3Data) bool {
@@ -172,7 +168,7 @@ func (m *DataTemplateManager) updateStatusTimestamp() {
 // UpdateDatas handles the Metal3DataClaims and creates or deletes Metal3Data accordingly.
 // It returns the number of current allocations.
 func (m *DataTemplateManager) UpdateDatas(ctx context.Context) (int, error) {
-	indexes, err := m.getIndexes(ctx)
+	indexes, statusIndexes, err := m.getIndexes(ctx)
 	if err != nil {
 		return 0, err
 	}
@@ -196,9 +192,13 @@ func (m *DataTemplateManager) UpdateDatas(ctx context.Context) (int, error) {
 		if dataClaim.Spec.Template.Name != m.DataTemplate.Name {
 			continue
 		}
-
-		if dataClaim.Status.RenderedData != nil && dataClaim.DeletionTimestamp.IsZero() {
-			continue
+		if dataClaim.DeletionTimestamp.IsZero() {
+			if index, ok := statusIndexes[dataClaim.Name]; ok {
+				m.DataTemplate.Status.Indexes[dataClaim.Name] = index
+			}
+			if dataClaim.Status.RenderedData != nil {
+				continue
+			}
 		}
 
 		indexes, err = m.updateData(ctx, &dataClaim, indexes)
@@ -207,7 +207,7 @@ func (m *DataTemplateManager) UpdateDatas(ctx context.Context) (int, error) {
 		}
 	}
 	m.updateStatusTimestamp()
-	return len(indexes), nil
+	return len(m.DataTemplate.Status.Indexes), nil
 }
 
 func (m *DataTemplateManager) updateData(ctx context.Context,

diff --git a/baremetal/metal3datatemplate_manager_test.go b/baremetal/metal3datatemplate_manager_test.go
@@ -149,14 +149,14 @@ var _ = Describe("Metal3DataTemplate manager", func() {
 			)
 			Expect(err).NotTo(HaveOccurred())
 
-			addressMap, err := templateMgr.getIndexes(context.TODO())
+			addressMap, coMap, err := templateMgr.getIndexes(context.TODO())
 			if tc.expectError {
 				Expect(err).To(HaveOccurred())
 			} else {
 				Expect(err).NotTo(HaveOccurred())
 			}
 			Expect(addressMap).To(Equal(tc.expectedMap))
-			Expect(tc.template.Status.Indexes).To(Equal(tc.expectedIndexes))
+			Expect(coMap).To(Equal(tc.expectedIndexes))
 			Expect(tc.template.Status.LastUpdated.IsZero()).To(BeFalse())
 		},
 		Entry("No indexes", testGetIndexes{