Skip to content

Commit

Permalink
Use netboot.AllowPXE to determine hardware ready status
Browse files Browse the repository at this point in the history
Signed-off-by: Ahree Hong <[email protected]>
  • Loading branch information
ahreehong committed May 8, 2024
1 parent e27df5d commit d49f40b
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 20 deletions.
34 changes: 25 additions & 9 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/pointer"

Check failure on line 31 in controllers/machine_reconcile_scope.go

View workflow job for this annotation

GitHub Actions / Validate lint

SA1019: "k8s.io/utils/pointer" is deprecated: Use functions in k8s.io/utils/ptr instead: ptr.To to obtain a pointer, ptr.Deref to dereference a pointer, ptr.Equal to compare dereferenced pointers. (staticcheck)
"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,18 @@ 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 {

Check failure on line 113 in controllers/machine_reconcile_scope.go

View workflow job for this annotation

GitHub Actions / Validate lint

ranges should only be cuddled with assignments used in the iteration (wsl)
if ifc.Netboot != nil {
if *ifc.Netboot.AllowPXE {
return false
}
}
}
return true

Check failure on line 120 in controllers/machine_reconcile_scope.go

View workflow job for this annotation

GitHub Actions / Validate lint

return statements should not be cuddled if block has more than two lines (wsl)
}

type errRequeueRequested struct{}
Expand Down Expand Up @@ -184,7 +194,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); err != nil {
return fmt.Errorf("failed to patch hardware: %w", err)
}

Expand All @@ -195,14 +205,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) 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 = pointer.Bool(false)
}
}

if err := patchHelper.Patch(scope.ctx, hw); err != nil {
return fmt.Errorf("patching Hardware object: %w", err)
Expand Down Expand Up @@ -720,8 +733,11 @@ func (scope *machineReconcileScope) releaseHardware(hardware *tinkv1.Hardware) e
// 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 = ""
for _, ifc := range hardware.Spec.Interfaces {
if ifc.Netboot != nil {
ifc.Netboot.AllowPXE = pointer.Bool(true)
}
}

controllerutil.RemoveFinalizer(hardware, infrastructurev1.MachineFinalizer)

Expand Down
18 changes: 8 additions & 10 deletions controllers/tinkerbellmachine_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/pointer"

Check failure on line 31 in controllers/tinkerbellmachine_controller_test.go

View workflow job for this annotation

GitHub Actions / Validate lint

SA1019: "k8s.io/utils/pointer" is deprecated: Use functions in k8s.io/utils/ptr instead: ptr.To to obtain a pointer, ptr.Deref to dereference a pointer, ptr.Equal to compare dereferenced pointers. (staticcheck)
"k8s.io/utils/ptr"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -190,6 +191,9 @@ func validHardware(name, uuid, ip string, options ...testOptions) *tinkv1.Hardwa
Address: ip,
},
},
Netboot: &tinkv1.Netboot{
AllowPXE: pointer.Bool(true),
},
},
},
Metadata: &tinkv1.HardwareMetadata{
Expand Down Expand Up @@ -417,7 +421,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 +432,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, pointer.Bool(true)); diff != "" {
t.Errorf(diff)
}
})
Expand Down Expand Up @@ -554,7 +555,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 +566,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, pointer.Bool(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

0 comments on commit d49f40b

Please sign in to comment.