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.

⚠️ Now, BMH object/CRD needs to have The object has the clusterctl.cluster.x-k8s.io/move label or the clusterctl.cluster.x-k8s.io/move-hierarchy  label to make sure BMH is pivoted to target cluster and removed from the source. clusterctl.cluster.x-k8s.io/move and clusterctl.cluster.x-k8s.io/move-hierarchy labels could be applied to single objects or at the CRD level (the label applies to all the objects).

We still keep the removal of owner reference code for one minor release cycle to facilitate upgrade scenario where a BMH could still have an owner reference set from previous version. Starting from v1.11.x minor cycle we will remove this code also.
Signed-off-by: Kashif Khan <[email protected]>
  • Loading branch information
kashifest committed Jan 13, 2025
1 parent 86e7a50 commit 715a0d0
Showing 3 changed files with 10 additions and 27 deletions.
9 changes: 2 additions & 7 deletions baremetal/metal3machine_manager.go
Original file line number Diff line number Diff line change
@@ -458,6 +458,7 @@ 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)
// The following removal of ownerreference code will be removed in v1.11
// Remove the ownerreference to this machine, even if the consumer ref
// references another machine.
host.OwnerReferences, err = m.DeleteOwnerRef(host.OwnerReferences)
@@ -644,6 +645,7 @@ func (m *MachineManager) Delete(ctx context.Context) error {

host.Spec.ConsumerRef = nil

// The following removal of ownerreference code will be removed in v1.11
// Remove the ownerreference to this machine.
host.OwnerReferences, err = m.DeleteOwnerRef(host.OwnerReferences)
if err != nil {
@@ -1099,13 +1101,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")

26 changes: 6 additions & 20 deletions baremetal/metal3machine_manager_test.go
Original file line number Diff line number Diff line change
@@ -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(""))
@@ -3024,7 +3022,6 @@ var _ = Describe("Metal3Machine manager", func() {
Data *infrav1.Metal3Data
ExpectRequeue bool
ExpectClusterLabel bool
ExpectOwnerRef bool
}

DescribeTable("Test Associate function",
@@ -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{}
@@ -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",
@@ -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",
@@ -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",
@@ -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",
@@ -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",
@@ -3225,7 +3212,6 @@ var _ = Describe("Metal3Machine manager", func() {
},
ExpectClusterLabel: true,
ExpectRequeue: false,
ExpectOwnerRef: true,
},
),
)
2 changes: 2 additions & 0 deletions test/e2e/pivoting.go
Original file line number Diff line number Diff line change
@@ -396,6 +396,8 @@ func RemoveDeployment(ctx context.Context, inputGetter func() RemoveDeploymentIn
func labelBMOCRDs(ctx context.Context, targetCluster framework.ClusterProxy) {
labels := map[string]string{}
labels[clusterctlv1.ClusterctlLabel] = ""
labels[clusterctlv1.ClusterctlMoveLabel] = ""
labels[clusterctlv1.ClusterctlMoveHierarchyLabel] = ""
labels[clusterv1.ProviderNameLabel] = "metal3"
crdName := "baremetalhosts.metal3.io"
err := LabelCRD(ctx, targetCluster.GetClient(), crdName, labels)

0 comments on commit 715a0d0

Please sign in to comment.