Skip to content

Commit

Permalink
Remove Metal3Machine owner reference from BMH
Browse files Browse the repository at this point in the history
Metal3Machine should not own BMH. It should consume it and release it when not needed. we already put a M3M consumer reference on BMH. This also would mean that for pivoting use cases we must add the clusterctl move labels in the CRDs. Otherwise BMH wont be pivoted to target clusters.

Signed-off-by: Kashif Khan <[email protected]>
  • Loading branch information
kashifest committed Nov 25, 2024
1 parent 476b7e1 commit cfbdb57
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 39 deletions.
19 changes: 0 additions & 19 deletions baremetal/metal3machine_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -458,12 +458,6 @@ func (m *MachineManager) Delete(ctx context.Context) error {
if !consumerRefMatches(host.Spec.ConsumerRef, m.Metal3Machine) {
m.Log.Info("host already associated with another metal3 machine",
"host", host.Name)
// Remove the ownerreference to this machine, even if the consumer ref
// references another machine.
host.OwnerReferences, err = m.DeleteOwnerRef(host.OwnerReferences)
if err != nil {
return err
}
return nil
}

Expand Down Expand Up @@ -644,12 +638,6 @@ func (m *MachineManager) Delete(ctx context.Context) error {

host.Spec.ConsumerRef = nil

// Remove the ownerreference to this machine.
host.OwnerReferences, err = m.DeleteOwnerRef(host.OwnerReferences)
if err != nil {
return err
}

if host.Labels != nil && host.Labels[clusterv1.ClusterNameLabel] == m.Machine.Spec.ClusterName {
delete(host.Labels, clusterv1.ClusterNameLabel)
}
Expand Down Expand Up @@ -1099,13 +1087,6 @@ func (m *MachineManager) setHostConsumerRef(_ context.Context, host *bmov1alpha1
APIVersion: m.Metal3Machine.APIVersion,
}

// Set OwnerReferences.
hostOwnerReferences, err := m.SetOwnerRef(host.OwnerReferences, true)
if err != nil {
return err
}
host.OwnerReferences = hostOwnerReferences

// Delete nodeReuseLabelName from host.
m.Log.Info("Deleting nodeReuseLabelName from host, if any")

Expand Down
26 changes: 6 additions & 20 deletions baremetal/metal3machine_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1213,8 +1213,6 @@ var _ = Describe("Metal3Machine manager", func() {
Expect(tc.Host.Spec.ConsumerRef.Namespace).
To(Equal(m3mconfig.Namespace))
Expect(tc.Host.Spec.ConsumerRef.Kind).To(Equal("Metal3Machine"))
_, err = machineMgr.FindOwnerRef(tc.Host.OwnerReferences)
Expect(err).NotTo(HaveOccurred())

if tc.expectNodeReuseLabelDeleted {
Expect(tc.Host.Labels[nodeReuseLabelName]).To(Equal(""))
Expand Down Expand Up @@ -3024,7 +3022,6 @@ var _ = Describe("Metal3Machine manager", func() {
Data *infrav1.Metal3Data
ExpectRequeue bool
ExpectClusterLabel bool
ExpectOwnerRef bool
}

DescribeTable("Test Associate function",
Expand Down Expand Up @@ -3075,12 +3072,7 @@ var _ = Describe("Metal3Machine manager", func() {
&savedHost,
)
Expect(err).NotTo(HaveOccurred())
_, err = machineMgr.FindOwnerRef(savedHost.OwnerReferences)
if tc.ExpectOwnerRef {
Expect(err).NotTo(HaveOccurred())
} else {
Expect(err).To(HaveOccurred())
}

if tc.ExpectClusterLabel {
// get the BMC credential
savedCred := corev1.Secret{}
Expand All @@ -3105,8 +3097,7 @@ var _ = Describe("Metal3Machine manager", func() {
Host: newBareMetalHost(baremetalhostName, nil, bmov1alpha1.StateNone, nil,
false, "metadata", false, "",
),
ExpectRequeue: false,
ExpectOwnerRef: true,
ExpectRequeue: false,
},
),
Entry("Associate empty machine, Metal3 machine spec set",
Expand All @@ -3118,9 +3109,8 @@ var _ = Describe("Metal3Machine manager", func() {
Host: newBareMetalHost(baremetalhostName, bmhSpecBMC(), bmov1alpha1.StateNone, nil,
false, "metadata", false, "",
),
BMCSecret: newBMCSecret("mycredentials", false),
ExpectRequeue: false,
ExpectOwnerRef: true,
BMCSecret: newBMCSecret("mycredentials", false),
ExpectRequeue: false,
},
),
Entry("Associate empty machine, host empty, Metal3 machine spec set",
Expand All @@ -3129,9 +3119,8 @@ var _ = Describe("Metal3Machine manager", func() {
M3Machine: newMetal3Machine(metal3machineName, m3mSpecAll(), nil,
m3mObjectMetaWithValidAnnotations(),
),
Host: newBareMetalHost("", nil, bmov1alpha1.StateNone, nil, false, "metadata", false, ""),
ExpectRequeue: true,
ExpectOwnerRef: false,
Host: newBareMetalHost("", nil, bmov1alpha1.StateNone, nil, false, "metadata", false, ""),
ExpectRequeue: true,
},
),
Entry("Associate machine, host nil, Metal3 machine spec set, requeue",
Expand All @@ -3152,7 +3141,6 @@ var _ = Describe("Metal3Machine manager", func() {
BMCSecret: newBMCSecret("mycredentials", false),
ExpectClusterLabel: true,
ExpectRequeue: false,
ExpectOwnerRef: true,
},
),
Entry("Associate machine with DataTemplate missing",
Expand All @@ -3179,7 +3167,6 @@ var _ = Describe("Metal3Machine manager", func() {
BMCSecret: newBMCSecret("mycredentials", false),
ExpectClusterLabel: true,
ExpectRequeue: true,
ExpectOwnerRef: true,
},
),
Entry("Associate machine with DataTemplate and Data ready",
Expand Down Expand Up @@ -3225,7 +3212,6 @@ var _ = Describe("Metal3Machine manager", func() {
},
ExpectClusterLabel: true,
ExpectRequeue: false,
ExpectOwnerRef: true,
},
),
)
Expand Down

0 comments on commit cfbdb57

Please sign in to comment.