Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use netboot.AllowPXE to determine hardware ready status #367

Merged
merged 1 commit into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 30 additions & 12 deletions controllers/machine_reconcile_scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"text/template"

"k8s.io/apimachinery/pkg/labels"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"

corev1 "k8s.io/api/core/v1"
Expand All @@ -49,8 +50,6 @@ import (

const (
providerIDPlaceholder = "PROVIDER_ID"
inUse = "in_use"
provisioned = "provisioned"
)

var (
Expand Down Expand Up @@ -107,7 +106,20 @@ func (scope *machineReconcileScope) addFinalizer() error {
}

func isHardwareReady(hw *tinkv1.Hardware) bool {
return hw.Spec.Metadata.State == inUse && hw.Spec.Metadata.Instance.State == provisioned
// if allowpxe false for all interface, hardware ready
if len(hw.Spec.Interfaces) == 0 {
return false
}

for _, ifc := range hw.Spec.Interfaces {
if ifc.Netboot != nil {
if *ifc.Netboot.AllowPXE {
return false
}
}
}

return true
}

type errRequeueRequested struct{}
Expand Down Expand Up @@ -184,7 +196,7 @@ func (scope *machineReconcileScope) reconcile(hw *tinkv1.Hardware) error {
return nil
}

if err := scope.patchHardwareStates(hw, inUse, provisioned); err != nil {
if err := scope.patchHardwareStates(hw, false); err != nil {
return fmt.Errorf("failed to patch hardware: %w", err)
}

Expand All @@ -195,14 +207,17 @@ func (scope *machineReconcileScope) reconcile(hw *tinkv1.Hardware) error {
}

// patchHardwareStates patches a hardware's metadata and instance states.
func (scope *machineReconcileScope) patchHardwareStates(hw *tinkv1.Hardware, mdState, iState string) error {
func (scope *machineReconcileScope) patchHardwareStates(hw *tinkv1.Hardware, allowpxe bool) error {
patchHelper, err := patch.NewHelper(hw, scope.client)
if err != nil {
return fmt.Errorf("initializing patch helper for selected hardware: %w", err)
}

hw.Spec.Metadata.State = mdState
hw.Spec.Metadata.Instance.State = iState
for _, ifc := range hw.Spec.Interfaces {
if ifc.Netboot != nil {
ifc.Netboot.AllowPXE = ptr.To(allowpxe)
}
}

if err := patchHelper.Patch(scope.ctx, hw); err != nil {
return fmt.Errorf("patching Hardware object: %w", err)
Expand Down Expand Up @@ -716,12 +731,15 @@ func (scope *machineReconcileScope) releaseHardware(hardware *tinkv1.Hardware) e

delete(hardware.ObjectMeta.Labels, HardwareOwnerNameLabel)
delete(hardware.ObjectMeta.Labels, HardwareOwnerNamespaceLabel)
// setting these Metadata.State and Metadata.Instance.State = "" indicates to Boots
// that this hardware should be allowed to netboot. FYI, this is not authoritative.
// setting the AllowPXE=true indicates to Smee that this hardware should be allowed
// to netboot. FYI, this is not authoritative.
// Other hardware values can be set to prohibit netbooting of a machine.
// See this Boots function for the logic around this: https://github.com/tinkerbell/boots/blob/main/job/dhcp.go#L115
hardware.Spec.Metadata.State = ""
hardware.Spec.Metadata.Instance.State = ""
// See this Boots function for the logic around this: https://github.com/tinkerbell/smee/blob/main/internal/ipxe/script/ipxe.go#L112
for _, ifc := range hardware.Spec.Interfaces {
if ifc.Netboot != nil {
ifc.Netboot.AllowPXE = ptr.To(true)
}
}

controllerutil.RemoveFinalizer(hardware, infrastructurev1.MachineFinalizer)

Expand Down
17 changes: 7 additions & 10 deletions controllers/tinkerbellmachine_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,9 @@ func validHardware(name, uuid, ip string, options ...testOptions) *tinkv1.Hardwa
Address: ip,
},
},
Netboot: &tinkv1.Netboot{
AllowPXE: ptr.To(true),
},
},
},
Metadata: &tinkv1.HardwareMetadata{
Expand Down Expand Up @@ -417,7 +420,7 @@ func Test_Machine_reconciliation_with_available_hardware(t *testing.T) {
g.Expect(updatedMachine.Status.Addresses).NotTo(BeEmpty(), "Machine status should be updated on every reconciliation")
})

t.Run("in_use_states_are_not_set", func(t *testing.T) {
t.Run("allowPXE_is_not_updated", func(t *testing.T) {
t.Parallel()
g := NewWithT(t)

Expand All @@ -428,10 +431,7 @@ func Test_Machine_reconciliation_with_available_hardware(t *testing.T) {

updatedHardware := &tinkv1.Hardware{}
g.Expect(client.Get(ctx, hardwareNamespacedName, updatedHardware)).To(Succeed())
if diff := cmp.Diff(updatedHardware.Spec.Metadata.State, ""); diff != "" {
t.Errorf(diff)
}
if diff := cmp.Diff(updatedHardware.Spec.Metadata.Instance.State, ""); diff != "" {
if diff := cmp.Diff(updatedHardware.Spec.Interfaces[0].Netboot.AllowPXE, ptr.To(true)); diff != "" {
t.Errorf(diff)
}
})
Expand Down Expand Up @@ -554,7 +554,7 @@ func Test_Machine_reconciliation_workflow_complete(t *testing.T) {
g.Expect(updatedMachine.Status.Addresses).NotTo(BeEmpty(), "Machine status should be updated on every reconciliation")
})

t.Run("in_use_states_are_set", func(t *testing.T) {
t.Run("allowPXE_is_updated", func(t *testing.T) {
t.Parallel()
g := NewWithT(t)

Expand All @@ -565,10 +565,7 @@ func Test_Machine_reconciliation_workflow_complete(t *testing.T) {

updatedHardware := &tinkv1.Hardware{}
g.Expect(client.Get(ctx, hardwareNamespacedName, updatedHardware)).To(Succeed())
if diff := cmp.Diff(updatedHardware.Spec.Metadata.State, "in_use"); diff != "" {
t.Errorf(diff)
}
if diff := cmp.Diff(updatedHardware.Spec.Metadata.Instance.State, "provisioned"); diff != "" {
if diff := cmp.Diff(updatedHardware.Spec.Interfaces[0].Netboot.AllowPXE, ptr.To(false)); diff != "" {
t.Errorf(diff)
}
})
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/tinkerbell/cluster-api-provider-tinkerbell

go 1.21
go 1.21.0

toolchain go1.21.2

Expand Down
Loading