Skip to content

Commit

Permalink
Ensure M3M is gone before cleaning up M3DC and M3D
Browse files Browse the repository at this point in the history 
Before cleaning up Metal3DataClaims, check if the Metal3Machine
(consumer) is gone.
This commit also renames some of the test objects and variables to make them easier to
understand and adds a constant for "Metal3Machine".

Signed-off-by: Lennart Jern <[email protected]>
  • Loading branch information
@lentzi90
lentzi90 committed May 21, 2024
1 parent fe486bb commit 35181ed
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 31 deletions.
2 changes: 1 addition & 1 deletion baremetal/metal3data_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 23 additions & 2 deletions baremetal/metal3datatemplate_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -332,7 +353,7 @@ func (m *DataTemplateManager) createData(ctx context.Context,
},
{
APIVersion: dataClaim.APIVersion,
Kind: "Metal3Machine",
Kind: metal3MachineKind,
Name: m3mName,
UID: m3mUID,
},
Expand Down
34 changes: 17 additions & 17 deletions baremetal/metal3datatemplate_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
Expand All @@ -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,
},
},
Expand All @@ -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,
},
},
Expand All @@ -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,
Expand All @@ -391,37 +391,37 @@ 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,
},
},
{
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,
},
},
},
expectedIndexes: map[string]int{
"abc": 0,
"abce": 1,
"claim-without-status": 0,
"claim-with-status": 1,
},
expectedNbIndexes: 2,
}),
Expand Down
2 changes: 2 additions & 0 deletions baremetal/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
34 changes: 23 additions & 11 deletions controllers/metal3data_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}

Expand Down

0 comments on commit 35181ed

Please sign in to comment.