Skip to content

Commit

Permalink
Fix tests, make less hardware lookups:
Browse files Browse the repository at this point in the history 
This makes it so that we don't make get calls
as much to the kube api server.

Signed-off-by: Jacob Weinstock <[email protected]>
  • Loading branch information
@jacobweinstock
jacobweinstock committed Oct 11, 2024
1 parent 2ab8867 commit c3c2163
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 120 deletions.
17 changes: 15 additions & 2 deletions internal/deprecated/workflow/hardware.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,24 @@ func (s *state) toggleHardware(ctx context.Context, allowPXE bool) error {
// 2. if not, set the allowPXE field to the desired value
// 3. return a WorkflowCondition with the result of the operation

hw, err := hardwareFrom(ctx, s.client, s.workflow)
if err != nil {
s.workflow.Status.SetCondition(v1alpha1.WorkflowCondition{
Type: v1alpha1.ToggleAllowNetbootTrue,
Status: metav1.ConditionFalse,
Reason: "Error",
Message: fmt.Sprintf("error getting hardware: %v", err),
Time: &metav1.Time{Time: metav1.Now().UTC()},
})

return err
}

if allowPXE {
if s.workflow.Status.BootOptions.AllowNetboot.ToggledTrue {
return nil
}
if err := setAllowPXE(ctx, s.client, s.workflow, s.hardware, allowPXE); err != nil {
if err := setAllowPXE(ctx, s.client, s.workflow, hw, allowPXE); err != nil {
s.workflow.Status.SetCondition(v1alpha1.WorkflowCondition{
Type: v1alpha1.ToggleAllowNetbootTrue,
Status: metav1.ConditionFalse,
Expand All @@ -87,7 +100,7 @@ func (s *state) toggleHardware(ctx context.Context, allowPXE bool) error {
if s.workflow.Status.BootOptions.AllowNetboot.ToggledFalse {
return nil
}
if err := setAllowPXE(ctx, s.client, s.workflow, s.hardware, allowPXE); err != nil {
if err := setAllowPXE(ctx, s.client, s.workflow, hw, allowPXE); err != nil {
s.workflow.Status.SetCondition(v1alpha1.WorkflowCondition{
Type: v1alpha1.ToggleAllowNetbootFalse,
Status: metav1.ConditionFalse,
Expand Down
11 changes: 6 additions & 5 deletions internal/deprecated/workflow/job.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,15 @@ func (s *state) createJob(ctx context.Context, actions []rufio.Action, name jobN
}

// create a new job
if s.hardware == nil {
return reconcile.Result{}, fmt.Errorf("hardware is nil")
hw , err := hardwareFrom(ctx, s.client, s.workflow)
if err != nil {
return reconcile.Result{}, fmt.Errorf("error getting hardware: %w", err)
}
if s.hardware.Spec.BMCRef == nil {
return reconcile.Result{}, fmt.Errorf("hardware %q does not have a BMC", s.hardware.Name)
if hw.Spec.BMCRef == nil {
return reconcile.Result{}, fmt.Errorf("hardware %q does not have a BMC", hw.Name)
}

if err := create(ctx, s.client, name.String(), s.hardware, s.workflow.Namespace, actions); err != nil {
if err := create(ctx, s.client, name.String(), hw, s.workflow.Namespace, actions); err != nil {
return reconcile.Result{}, fmt.Errorf("error creating job: %w", err)
}
journal.Log(ctx, "job created", "name", name)
Expand Down
23 changes: 15 additions & 8 deletions internal/deprecated/workflow/job_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

Expand Down Expand Up @@ -90,10 +91,17 @@ func TestHandleJob(t *testing.T) {
},
},
name: jobNameNetboot,
hardware: new(v1alpha1.Hardware),
wantResult: reconcile.Result{Requeue: true},
},
"existing job deleted, create new job": {
workflow: &v1alpha1.Workflow{
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
},
Spec: v1alpha1.WorkflowSpec{
HardwareRef: "test-hardware",
},
Status: v1alpha1.WorkflowStatus{
BootOptions: v1alpha1.BootOptionsStatus{
Jobs: map[string]v1alpha1.JobStatus{
Expand Down Expand Up @@ -176,7 +184,7 @@ func TestHandleJob(t *testing.T) {
AllowNetboot: v1alpha1.AllowNetbootStatus{},
},
},
hardware: &v1alpha1.Hardware{},
hardware: new(v1alpha1.Hardware),
actions: []rufio.Action{},
name: jobNameNetboot,
wantResult: reconcile.Result{},
Expand All @@ -200,17 +208,16 @@ func TestHandleJob(t *testing.T) {

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
runtimescheme := runtime.NewScheme()
rufio.AddToScheme(runtimescheme)
v1alpha1.AddToScheme(runtimescheme)
clientBulider := GetFakeClientBuilder().WithScheme(runtimescheme)
scheme := runtime.NewScheme()
rufio.AddToScheme(scheme)
v1alpha1.AddToScheme(scheme)
clientBuilder := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(tc.hardware, tc.workflow)
if tc.job != nil {
clientBulider.WithRuntimeObjects(tc.job)
clientBuilder.WithRuntimeObjects(tc.job)
}
s := &state{
workflow: tc.workflow,
hardware: tc.hardware,
client: clientBulider.Build(),
client: clientBuilder.Build(),
}
ctx := context.Background()
r, err := s.handleJob(ctx, tc.actions, tc.name)
Expand Down
40 changes: 21 additions & 19 deletions internal/deprecated/workflow/post.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,40 +7,42 @@ import (
"github.com/pkg/errors"
rufio "github.com/tinkerbell/rufio/api/v1alpha1"
"github.com/tinkerbell/tink/api/v1alpha1"
"github.com/tinkerbell/tink/internal/deprecated/workflow/journal"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

func (s *state) postActions(ctx context.Context) (reconcile.Result, error) {
// 1. Handle toggling allowPXE in a hardware object if toggleAllowNetboot is true.
if s.workflow.Spec.BootOptions.ToggleAllowNetboot {
if s.workflow.Spec.BootOptions.ToggleAllowNetboot && !s.workflow.Status.BootOptions.AllowNetboot.ToggledFalse {
journal.Log(ctx, "toggling allowPXE false")
if err := s.toggleHardware(ctx, false); err != nil {
return reconcile.Result{}, err
}
}

// 2. Handle ISO eject scenario.
if s.workflow.Spec.BootOptions.BootMode == v1alpha1.BootModeISO {
if s.hardware == nil {
return reconcile.Result{}, errors.New("hardware is nil")
}
if s.workflow.Spec.BootOptions.ISOURL == "" {
return reconcile.Result{}, errors.New("iso url must be a valid url")
}
name := jobName(fmt.Sprintf("%s-%s", jobNameISOEject, s.hardware.Name))
actions := []rufio.Action{
{
VirtualMediaAction: &rufio.VirtualMediaAction{
MediaURL: "", // empty to unmount/eject the media
Kind: rufio.VirtualMediaCD,
name := jobName(fmt.Sprintf("%s-%s", jobNameISOEject, s.workflow.GetName()))
if j := s.workflow.Status.BootOptions.Jobs[name.String()]; !j.ExistingJobDeleted || j.UID == "" || !j.Complete {
journal.Log(ctx, "boot mode iso")
if s.workflow.Spec.BootOptions.ISOURL == "" {
return reconcile.Result{}, errors.New("iso url must be a valid url")
}
actions := []rufio.Action{
{
VirtualMediaAction: &rufio.VirtualMediaAction{
MediaURL: "", // empty to unmount/eject the media
Kind: rufio.VirtualMediaCD,
},
},
},
}
}

r, err := s.handleJob(ctx, actions, name)
if s.workflow.Status.BootOptions.Jobs[name.String()].Complete {
s.workflow.Status.State = v1alpha1.WorkflowStateSuccess
r, err := s.handleJob(ctx, actions, name)
if s.workflow.Status.BootOptions.Jobs[name.String()].Complete {
s.workflow.Status.State = v1alpha1.WorkflowStateSuccess
}
return r, err
}
return r, err
}

s.workflow.Status.State = v1alpha1.WorkflowStateSuccess
Expand Down
155 changes: 81 additions & 74 deletions internal/deprecated/workflow/pre.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ import (
// prepareWorkflow prepares the workflow for execution.
// The workflow (s.workflow) can be updated even if an error occurs.
// Any patching of the workflow object in a cluster is left up to the caller.
// At the moment prepareWorkflow requires the workflow have a hardwareRef and the object exists.
func (s *state) prepareWorkflow(ctx context.Context) (reconcile.Result, error) {
// handle bootoptions
// 1. Handle toggling allowPXE in a hardware object if toggleAllowNetboot is true.
if s.workflow.Spec.BootOptions.ToggleAllowNetboot {
if s.workflow.Spec.BootOptions.ToggleAllowNetboot && !s.workflow.Status.BootOptions.AllowNetboot.ToggledTrue {
journal.Log(ctx, "toggling allowPXE true")
if err := s.toggleHardware(ctx, true); err != nil {
return reconcile.Result{}, err
Expand All @@ -27,92 +28,98 @@ func (s *state) prepareWorkflow(ctx context.Context) (reconcile.Result, error) {
// 2. Handle booting scenarios.
switch s.workflow.Spec.BootOptions.BootMode {
case v1alpha1.BootModeNetboot:
journal.Log(ctx, "boot mode netboot")
if s.hardware == nil {
return reconcile.Result{}, errors.New("hardware is nil")
}
name := jobName(fmt.Sprintf("%s-%s", jobNameNetboot, s.hardware.Name))
efiBoot := func() bool {
for _, iface := range s.hardware.Spec.Interfaces {
if iface.DHCP != nil && iface.DHCP.UEFI {
return true
}
name := jobName(fmt.Sprintf("%s-%s", jobNameNetboot, s.workflow.GetName()))
if j := s.workflow.Status.BootOptions.Jobs[name.String()]; !j.ExistingJobDeleted || j.UID == "" || !j.Complete {
journal.Log(ctx, "boot mode netboot")
hw, err := hardwareFrom(ctx, s.client, s.workflow)
if err != nil {
return reconcile.Result{}, errors.Wrap(err, "failed to get hardware")
}
return false
}()
actions := []rufio.Action{
{
PowerAction: rufio.PowerHardOff.Ptr(),
},
{
OneTimeBootDeviceAction: &rufio.OneTimeBootDeviceAction{
Devices: []rufio.BootDevice{
rufio.PXE,
efiBoot := func() bool {
for _, iface := range hw.Spec.Interfaces {
if iface.DHCP != nil && iface.DHCP.UEFI {
return true
}
}
return false
}()
actions := []rufio.Action{
{
PowerAction: rufio.PowerHardOff.Ptr(),
},
{
OneTimeBootDeviceAction: &rufio.OneTimeBootDeviceAction{
Devices: []rufio.BootDevice{
rufio.PXE,
},
EFIBoot: efiBoot,
},
EFIBoot: efiBoot,
},
},
{
PowerAction: rufio.PowerOn.Ptr(),
},
}
{
PowerAction: rufio.PowerOn.Ptr(),
},
}

r, err := s.handleJob(ctx, actions, name)
if s.workflow.Status.BootOptions.Jobs[name.String()].Complete && s.workflow.Status.State == v1alpha1.WorkflowStatePreparing {
s.workflow.Status.State = v1alpha1.WorkflowStatePending
r, err := s.handleJob(ctx, actions, name)
if s.workflow.Status.BootOptions.Jobs[name.String()].Complete && s.workflow.Status.State == v1alpha1.WorkflowStatePreparing {
s.workflow.Status.State = v1alpha1.WorkflowStatePending
}
return r, err
}
return r, err
case v1alpha1.BootModeISO:
journal.Log(ctx, "boot mode iso")
if s.hardware == nil {
return reconcile.Result{}, errors.New("hardware is nil")
}
if s.workflow.Spec.BootOptions.ISOURL == "" {
return reconcile.Result{}, errors.New("iso url must be a valid url")
}
name := jobName(fmt.Sprintf("%s-%s", jobNameISOMount, s.hardware.Name))
efiBoot := func() bool {
for _, iface := range s.hardware.Spec.Interfaces {
if iface.DHCP != nil && iface.DHCP.UEFI {
return true
}
name := jobName(fmt.Sprintf("%s-%s", jobNameISOMount, s.workflow.GetName()))
if j := s.workflow.Status.BootOptions.Jobs[name.String()]; !j.ExistingJobDeleted || j.UID == "" || !j.Complete {
journal.Log(ctx, "boot mode iso")
if s.workflow.Spec.BootOptions.ISOURL == "" {
return reconcile.Result{}, errors.New("iso url must be a valid url")
}
hw, err := hardwareFrom(ctx, s.client, s.workflow)
if err != nil {
return reconcile.Result{}, errors.Wrap(err, "failed to get hardware")
}
return false
}()
actions := []rufio.Action{
{
PowerAction: rufio.PowerHardOff.Ptr(),
},
{
VirtualMediaAction: &rufio.VirtualMediaAction{
MediaURL: "", // empty to unmount/eject the media
Kind: rufio.VirtualMediaCD,
efiBoot := func() bool {
for _, iface := range hw.Spec.Interfaces {
if iface.DHCP != nil && iface.DHCP.UEFI {
return true
}
}
return false
}()
actions := []rufio.Action{
{
PowerAction: rufio.PowerHardOff.Ptr(),
},
},
{
VirtualMediaAction: &rufio.VirtualMediaAction{
MediaURL: s.workflow.Spec.BootOptions.ISOURL,
Kind: rufio.VirtualMediaCD,
{
VirtualMediaAction: &rufio.VirtualMediaAction{
MediaURL: "", // empty to unmount/eject the media
Kind: rufio.VirtualMediaCD,
},
},
},
{
OneTimeBootDeviceAction: &rufio.OneTimeBootDeviceAction{
Devices: []rufio.BootDevice{
rufio.CDROM,
{
VirtualMediaAction: &rufio.VirtualMediaAction{
MediaURL: s.workflow.Spec.BootOptions.ISOURL,
Kind: rufio.VirtualMediaCD,
},
EFIBoot: efiBoot,
},
},
{
PowerAction: rufio.PowerOn.Ptr(),
},
}
{
OneTimeBootDeviceAction: &rufio.OneTimeBootDeviceAction{
Devices: []rufio.BootDevice{
rufio.CDROM,
},
EFIBoot: efiBoot,
},
},
{
PowerAction: rufio.PowerOn.Ptr(),
},
}

r, err := s.handleJob(ctx, actions, name)
if s.workflow.Status.BootOptions.Jobs[name.String()].Complete && s.workflow.Status.State == v1alpha1.WorkflowStatePreparing {
s.workflow.Status.State = v1alpha1.WorkflowStatePending
r, err := s.handleJob(ctx, actions, name)
if s.workflow.Status.BootOptions.Jobs[name.String()].Complete && s.workflow.Status.State == v1alpha1.WorkflowStatePreparing {
s.workflow.Status.State = v1alpha1.WorkflowStatePending
}
return r, err
}
return r, err
}

return reconcile.Result{}, nil
Expand Down
Loading

0 comments on commit c3c2163

Please sign in to comment.