job: Change setPXEFilename -> getPXEFilename

This way its easier to test since its side effect free. I was getting a panic
when trying to rework the not-our-ipxe case for an unknown machine because
it would try to call j.Error(). This is just better code.

Signed-off-by: Manuel Mendez <mmendez@equinix.com>

job/dhcp.go

@@ -74,7 +74,9 @@ func (j Job) configureDHCP(rep, req *dhcp4.Packet) bool {
 			ipxe.Setup(rep)
 		}
 
-		j.setPXEFilename(rep, isOuriPXE, isARM, isUEFI)
+		if filename := j.getPXEFilename(isOuriPXE, isARM, isUEFI); filename != "" {
+			dhcp.SetFilename(rep, filename, conf.PublicIPv4)
+		}
 	}
 	return true
 }
@@ -97,15 +99,15 @@ func (j Job) areWeProvisioner() bool {
 	return j.hardware.HardwareProvisioner() == j.ProvisionerEngineName()
 }
 
-func (j Job) setPXEFilename(rep *dhcp4.Packet, isOuriPXE, isARM, isUEFI bool) {
+func (j Job) getPXEFilename(isOuriPXE, isARM, isUEFI bool) string {
 	if !j.isPXEAllowed() {
 		if j.instance != nil && j.instance.State == "active" {
 			// We set a filename because if a machine is actually trying to PXE and nothing is sent it may hang for
 			// a while waiting for any possible ProxyDHCP packets and it would delay booting from disks.
 			// This short cuts all that when we know we want to be booting from disk.
-			dhcp.SetFilename(rep, "/pxe-is-not-allowed", conf.PublicIPv4)
+			return "/pxe-is-not-allowed"
 		}
-		return
+		return ""
 	}
 
 	var filename string
@@ -119,14 +121,8 @@ func (j Job) setPXEFilename(rep *dhcp4.Packet, isOuriPXE, isARM, isUEFI bool) {
 		} else {
 			filename = "undionly.kpxe"
 		}
-
-		if filename == "" {
-			j.Error(errors.New("unable to figure out iPXE to serve"))
-			return
-		}
 	} else {
 		filename = "http://" + conf.PublicFQDN + "/auto.ipxe"
 	}
-
-	dhcp.SetFilename(rep, filename, conf.PublicIPv4)
+	return filename
 }

job/dhcp_test.go

@@ -1,21 +1,19 @@
 package job
 
 import (
-	"bytes"
 	"fmt"
 	"testing"
 
 	"github.com/google/uuid"
-	dhcp4 "github.com/packethost/dhcp4-go"
 	assert "github.com/stretchr/testify/require"
 	"github.com/tinkerbell/boots/conf"
 	"github.com/tinkerbell/boots/packet"
 )
 
-func TestSetPXEFilename(t *testing.T) {
+func TestGetPXEFilename(t *testing.T) {
 	conf.PublicFQDN = "boots-testing.packet.net"
 
-	var setPXEFilenameTests = []struct {
+	var getPXEFilenameTests = []struct {
 		name     string
 		iState   string
 		plan     string
@@ -53,7 +51,7 @@ func TestSetPXEFilename(t *testing.T) {
 			filename: "ipxe.efi"},
 	}
 
-	for i, tt := range setPXEFilenameTests {
+	for i, tt := range getPXEFilenameTests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Logf("index=%d iState=%q plan=%q allowPXE=%v ouriPXE=%v arm=%v uefi=%v filename=%q",
 				i, tt.iState, tt.plan, tt.allowPXE, tt.ouriPXE, tt.arm, tt.uefi, tt.filename)
@@ -76,10 +74,7 @@ func TestSetPXEFilename(t *testing.T) {
 				},
 				instance: instance,
 			}
-			rep := dhcp4.NewPacket(dhcp4.BootReply)
-			j.setPXEFilename(&rep, tt.ouriPXE, tt.arm, tt.uefi)
-			filename := string(bytes.TrimRight(rep.File(), "\x00"))
-
+			filename := j.getPXEFilename(tt.ouriPXE, tt.arm, tt.uefi)
 			if tt.filename != filename {
 				t.Fatalf("unexpected filename want:%q, got:%q", tt.filename, filename)
 			}