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

chore: error technical debt #239

Merged
merged 1 commit into from
Aug 21, 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
2 changes: 1 addition & 1 deletion .web-docs/components/builder/vmx/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ JSON Example:

- `linked` (bool) - By default Packer creates a 'full' clone of the virtual machine
specified in source_path. The resultant virtual machine is fully
independant from the parent it was cloned from.
independent from the parent it was cloned from.

Setting linked to true instead causes Packer to create the virtual
machine as a 'linked' clone. Linked clones use and require ongoing
Expand Down
26 changes: 14 additions & 12 deletions builder/vmware/common/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,15 @@ func NewDriver(dconfig *DriverConfig, config *SSHConfig, vmName string) (Driver,
NewPlayer5Driver(config),
}
default:
return nil, fmt.Errorf("can't find driver for OS: %s", runtime.GOOS)
return nil, fmt.Errorf("error finding a driver for %s", runtime.GOOS)
}
}

errs := ""
for _, driver := range drivers {
err := driver.Verify()

log.Printf("Testing against vmware driver %T, Success: %t", driver, err == nil)
log.Printf("Testing against driver %T, Success: %t", driver, err == nil)
if err == nil {
return driver, nil
}
Expand Down Expand Up @@ -170,20 +170,22 @@ func runAndLog(cmd *exec.Cmd) (string, string, error) {
message = stdoutString
}

err = fmt.Errorf("VMware error: %s", message)
err = fmt.Errorf("error: %s", message)

// If "unknown error" is in there, add some additional notes
re := regexp.MustCompile(`(?i)unknown error`)
if re.MatchString(message) {
err = fmt.Errorf(
"%s\n\n%s", err,
"Packer detected a VMware 'Unknown Error'. Unfortunately VMware\n"+
"often has extremely vague error messages such as this and Packer\n"+
"itself can't do much about that. Please check the vmware.log files\n"+
"created by VMware when a VM is started (in the directory of the\n"+
"vmx file), which often contains more detailed error information.\n\n"+
"You may need to set the command line flag --on-error=abort to\n\n"+
"prevent Packer from cleaning up the vmx file directory.")
"Packer detected an error from the VMware hypervisor "+
"platform. Unfortunately, the error message provided is "+
"not very specific. Please check the `vmware.log` files "+
"created by the hypervisor platform when a virtual "+
"machine is started. The logs are located in the "+
"directory of the .vmx file and often contain more "+
"detailed error information.\n\nYou may need to set the "+
"command line flag --on-error=abort to prevent the plugin "+
"from cleaning up the file directory.")
}
}

Expand Down Expand Up @@ -688,7 +690,7 @@ func CheckOvfToolVersion(ovftoolPath string) error {
func (d *VmwareDriver) Export(args []string) error {
ovftool := GetOvfTool()
if ovftool == "" {
return fmt.Errorf("error finding ovftool in path")
return errors.New("error finding ovftool in path")
}
cmd := exec.Command(ovftool, args...)
if _, _, err := runAndLog(cmd); err != nil {
Expand All @@ -706,7 +708,7 @@ func (d *VmwareDriver) VerifyOvfTool(SkipExport, _ bool) error {
log.Printf("Verifying that ovftool exists...")
ovftoolPath := GetOvfTool()
if ovftoolPath == "" {
return fmt.Errorf("ovftool not found; install and include it in your PATH")
return errors.New("ovftool not found; install and include it in your PATH")
}

log.Printf("Checking ovftool version...")
Expand Down
14 changes: 7 additions & 7 deletions builder/vmware/common/driver_esx5.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,13 +436,13 @@ func (d *ESX5Driver) HostAddress(multistep.StateBag) (string, error) {
// get the local address (the host)
host, _, err := net.SplitHostPort(conn.LocalAddr().String())
if err != nil {
return "", fmt.Errorf("unable to determine host address : %v", err)
return "", fmt.Errorf("unable to determine host address : %s", err)
}

// iterate through all the interfaces..
interfaces, err := net.Interfaces()
if err != nil {
return "", fmt.Errorf("unable to enumerate host interfaces : %v", err)
return "", fmt.Errorf("unable to enumerate host interfaces : %s", err)
}

for _, intf := range interfaces {
Expand All @@ -467,7 +467,7 @@ func (d *ESX5Driver) GuestAddress(multistep.StateBag) (string, error) {
// list all the interfaces on the ESXi host
r, err := d.esxcli("network", "ip", "interface", "list")
if err != nil {
return "", fmt.Errorf("unable to retrieve host interfaces : %v", err)
return "", fmt.Errorf("unable to retrieve host interfaces : %s", err)
}

// rip out the interface name and the MAC address from the csv output
Expand All @@ -482,7 +482,7 @@ func (d *ESX5Driver) GuestAddress(multistep.StateBag) (string, error) {
// list all the addresses on the ESXi host
r, err = d.esxcli("network", "ip", "interface", "ipv4", "get")
if err != nil {
return "", fmt.Errorf("unable to retrieve host addresses : %v", err)
return "", fmt.Errorf("unable to retrieve host addresses : %s", err)
}

// figure out the interface name that matches the specified d.Host address
Expand All @@ -501,7 +501,7 @@ func (d *ESX5Driver) GuestAddress(multistep.StateBag) (string, error) {
// find the MAC address according to the interface name
result, ok := addrs[intf]
if !ok {
return "", fmt.Errorf("unable to find address for guest interface")
return "", errors.New("unable to find address for guest interface")
}

// ..and we're good
Expand All @@ -516,7 +516,7 @@ func (d *ESX5Driver) VNCAddress(ctx context.Context, _ string, portMin, portMax
//it will ignore any ports listened to by only localhost
r, err := d.esxcli("network", "ip", "connection", "list")
if err != nil {
err = fmt.Errorf("unable to retrieve network information for host : %v", err)
err = fmt.Errorf("unable to retrieve network information for host : %s", err)
return "", 0, err
}

Expand Down Expand Up @@ -856,7 +856,7 @@ func (d *ESX5Driver) VerifyChecksum(hash string, file string) bool {
Src: file + "?checksum=" + hash,
})
if err != nil {
log.Printf("[ERROR] Error parsing the checksum: %v", err)
log.Printf("[ERROR] Error parsing the checksum: %s", err)
return false
}

Expand Down
2 changes: 1 addition & 1 deletion builder/vmware/common/driver_esx5_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestESX5Driver_CommHost(t *testing.T) {
var commConfig communicator.Config
err := config.Decode(&commConfig, nil, conf)
if err != nil {
t.Fatalf("error decoding config: %v", err)
t.Fatalf("error decoding config: %s", err)
}
state := new(multistep.BasicStateBag)
sshConfig := SSHConfig{Comm: commConfig}
Expand Down
2 changes: 1 addition & 1 deletion builder/vmware/common/driver_fusion6.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (d *Fusion6Driver) ToolsIsoPath(k string) string {
cmd := exec.Command(vmxpath, "-v")
cmd.Stderr = &stderr
if err := cmd.Run(); err != nil {
log.Printf("[DEBUG] failed to execute vmware-vmx command to get version %v", err)
log.Printf("[DEBUG] failed to execute vmware-vmx command to get version %s", err)
log.Printf("[DEBUG] continuing with default iso path for fusion6+.")
return filepath.Join(d.AppPath, "Contents", "Library", "isoimages", "x86_x64", k+".iso")
}
Expand Down
17 changes: 9 additions & 8 deletions builder/vmware/common/driver_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package common
import (
"bytes"
"encoding/hex"
"errors"
"fmt"
"log"
"math"
Expand Down Expand Up @@ -68,7 +69,7 @@ func tokenizeDhcpConfig(in chan byte) chan string {
}

// If we're in a quote, then we continue until we're not in a quote
// before we start looing for tokens
// before we start looking for tokens
if quote {
if by == '"' {
out <- state + string(by)
Expand All @@ -81,7 +82,7 @@ func tokenizeDhcpConfig(in chan byte) chan string {

switch by {
case '"':
// Otherwise we're outside any quotes and can process bytes normaly
// Otherwise we're outside any quotes and can process bytes normally
quote = true
state += string(by)
continue
Expand Down Expand Up @@ -240,10 +241,10 @@ func parseDhcpConfig(in chan string) (tkGroup, error) {
// that was because they were unterminated. Raise an error in that case.

if node.parent == nil {
return tkGroup{}, fmt.Errorf("refused to close the global declaration")
return tkGroup{}, errors.New("refused to close the global declaration")
}
if len(tokens) > 0 {
return tkGroup{}, fmt.Errorf("list of tokens was left unterminated : %v", tokens)
return tkGroup{}, fmt.Errorf("list of tokens was left unterminated: %v", tokens)
}
node = node.parent

Expand Down Expand Up @@ -415,12 +416,12 @@ func parseNetworkMapConfig(in chan string) (NetworkMap, error) {
switch tk {
case ".":
if len(state) != 1 {
return nil, fmt.Errorf("network index missing")
return nil, errors.New("network index missing")
}

case "=":
if len(state) != 2 {
return nil, fmt.Errorf("assigned to empty attribute")
return nil, errors.New("assigned to empty attribute")
}

case "\n":
Expand Down Expand Up @@ -1031,7 +1032,7 @@ func (e *configDeclaration) IP4() (net.IP, error) {
return nil, fmt.Errorf("more than one ipv4 address returned : %v", result)

} else if len(result) == 0 {
return nil, fmt.Errorf("no ipv4 address found")
return nil, errors.New("no IPv4 address found")
}

// Try and parse it as an IP4. If so, then it's good to return it as-is.
Expand Down Expand Up @@ -1064,7 +1065,7 @@ func (e *configDeclaration) IP6() (net.IP, error) {
return nil, fmt.Errorf("more than one ipv6 address returned : %v", result)

} else if len(result) == 0 {
return nil, fmt.Errorf("no ipv6 address found")
return nil, errors.New("no IPv6 address found")
}

// If we were able to parse it into an IP, then we can just return it.
Expand Down
16 changes: 8 additions & 8 deletions builder/vmware/common/driver_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ parameters : map[default-lease-time:1800 max-lease-time:7200]

config, err := ReadDhcpConfiguration(f)
if err != nil {
t.Fatalf("Unable to read dhcpd.conf samplpe: %s", err)
t.Fatalf("Unable to read dhcpd.conf sample: %s", err)
}

if len(config) != 3 {
Expand Down Expand Up @@ -429,7 +429,7 @@ func TestParserReadNetworkMap(t *testing.T) {

netmap, err := ReadNetworkMap(f)
if err != nil {
t.Fatalf("Unable to read netmap.conf samplpe: %s", err)
t.Fatalf("Unable to read netmap.conf sample: %s", err)
}

expected_keys := []string{"device", "name"}
Expand Down Expand Up @@ -711,7 +711,7 @@ func TestParserReadDhcpdLeaseEntry(t *testing.T) {

result, err := readDhcpdLeaseEntry(consumeLeaseString(test_1))
if err != nil {
t.Errorf("error parsing entry: %v", err)
t.Errorf("error parsing entry: %s", err)
}
if result.address != expected_1["address"] {
t.Errorf("expected address %v, got %v", expected_1["address"], result.address)
Expand All @@ -733,7 +733,7 @@ func TestParserReadDhcpdLeaseEntry(t *testing.T) {
}
result, err = readDhcpdLeaseEntry(consumeLeaseString(test_2))
if err != nil {
t.Errorf("error parsing entry: %v", err)
t.Errorf("error parsing entry: %s", err)
}
if result.address != expected_2["address"] {
t.Errorf("expected address %v, got %v", expected_2["address"], result.address)
Expand Down Expand Up @@ -896,7 +896,7 @@ func TestParserReadAppleDhcpdLeaseEntry(t *testing.T) {

result, err := readAppleDhcpdLeaseEntry(consumeAppleLeaseString(test_1))
if err != nil {
t.Errorf("error parsing entry: %v", err)
t.Errorf("error parsing entry: %s", err)
}
if result.ipAddress != expected_1["ipAddress"] {
t.Errorf("expected ipAddress %v, got %v", expected_1["ipAddress"], result.ipAddress)
Expand Down Expand Up @@ -930,7 +930,7 @@ func TestParserReadAppleDhcpdLeaseEntry(t *testing.T) {

result, err = readAppleDhcpdLeaseEntry(consumeAppleLeaseString(test_2))
if err != nil {
t.Errorf("error parsing entry: %v", err)
t.Errorf("error parsing entry: %s", err)
}
if result.ipAddress != expected_2["ipAddress"] {
t.Errorf("expected ipAddress %v, got %v", expected_2["ipAddress"], result.ipAddress)
Expand Down Expand Up @@ -1212,13 +1212,13 @@ func TestParserReadNetworingConfig(t *testing.T) {

f, err := os.Open(filepath.Join("testdata", "networking-example"))
if err != nil {
t.Fatalf("Unable to open networking-example sample: %v", err)
t.Fatalf("Unable to open networking-example sample: %s", err)
}
defer f.Close()

config, err := ReadNetworkingConfig(f)
if err != nil {
t.Fatalf("error parsing networking-example: %v", err)
t.Fatalf("error parsing networking-example: %s", err)
}

if vnet, ok := config.answer[1]; ok {
Expand Down
5 changes: 3 additions & 2 deletions builder/vmware/common/run_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package common

import (
"errors"
"fmt"

"github.com/hashicorp/packer-plugin-sdk/template/interpolate"
Expand Down Expand Up @@ -68,11 +69,11 @@ type RunConfig struct {
func (c *RunConfig) Prepare(_ *interpolate.Context, driverConfig *DriverConfig) (warnings []string, errs []error) {
if c.VNCOverWebsocket {
if driverConfig.RemoteType == "" {
errs = append(errs, fmt.Errorf("'vnc_over_websocket' can only be used with remote hypervisor builds"))
errs = append(errs, errors.New("'vnc_over_websocket' can only be used with remote hypervisor builds"))
return
}
if c.VNCPortMin != 0 || c.VNCPortMax != 0 || c.VNCBindAddress != "" || c.VNCDisablePassword {
warnings = append(warnings, "[WARN] 'vnc_over_websocket' enabled, any other VNC configuration will be ignored.")
warnings = append(warnings, "[WARN] 'vnc_over_websocket' enabled; other VNC configurations will be ignored.")
}
return
}
Expand Down
2 changes: 1 addition & 1 deletion builder/vmware/common/run_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func TestRunConfig_Prepare(t *testing.T) {
},
driver: &DriverConfig{RemoteType: "esx5"},
errs: nil,
warnings: []string{"[WARN] 'vnc_over_websocket' enabled, any other VNC configuration will be ignored."},
warnings: []string{"[WARN] 'vnc_over_websocket' enabled; other VNC configurations will be ignored."},
},
}

Expand Down
2 changes: 1 addition & 1 deletion builder/vmware/common/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func CommHost(config *SSHConfig) func(multistep.StateBag) (string, error) {
hosts, err := driver.PotentialGuestIP(state)
if err != nil {
log.Printf("IP lookup failed: %s", err)
return "", fmt.Errorf("IP lookup failed: %s", err)
return "", fmt.Errorf("failed to lookup IP: %s", err)
}

if len(hosts) == 0 {
Expand Down
2 changes: 1 addition & 1 deletion builder/vmware/common/step_clean_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (StepCleanFiles) Run(ctx context.Context, state multistep.StateBag) multist
dir := state.Get("dir").(OutputDir)
ui := state.Get("ui").(packersdk.Ui)

ui.Say("Deleting unnecessary VMware files...")
ui.Say("Deleting unnecessary files...")
files, err := dir.ListFiles()
if err != nil {
state.Put("error", err)
Expand Down
9 changes: 5 additions & 4 deletions builder/vmware/common/step_configure_vmx.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package common

import (
"context"
"errors"
"fmt"
"log"
"regexp"
Expand Down Expand Up @@ -37,7 +38,7 @@ func (s *StepConfigureVMX) Run(ctx context.Context, state multistep.StateBag) mu
vmxPath := state.Get("vmx_path").(string)
vmxData, err := ReadVMX(vmxPath)
if err != nil {
err := fmt.Errorf("error reading VMX file: %s", err)
err = fmt.Errorf("error reading VMX file: %s", err)
state.Put("error", err)
ui.Error(err.Error())
return multistep.ActionHalt
Expand Down Expand Up @@ -110,7 +111,7 @@ func (s *StepConfigureVMX) Run(ctx context.Context, state multistep.StateBag) mu
} else {
displayName, ok := vmxData["displayname"]
if !ok { // Packer converts key names to lowercase!
err := fmt.Errorf("error returning value of displayName from VMX data")
err := errors.New("error returning value of displayName from VMX data")
state.Put("error", err)
ui.Error(err.Error())
return multistep.ActionHalt
Expand All @@ -122,7 +123,7 @@ func (s *StepConfigureVMX) Run(ctx context.Context, state multistep.StateBag) mu
// Set the extendedConfigFile setting for the .vmxf filename to the VMName
// if displayName is not set. This is needed so that when VMware creates
// the .vmxf file it matches the displayName if it is set. When just using
// the sisplayName if it was empty VMware would make a file named ".vmxf".
// the displayName if it was empty VMware would make a file named ".vmxf".
// The ".vmxf" file would not get deleted when the VM got deleted.
if s.DisplayName != "" {
vmxData["extendedconfigfile"] = fmt.Sprintf("%s.vmxf", s.DisplayName)
Expand All @@ -133,7 +134,7 @@ func (s *StepConfigureVMX) Run(ctx context.Context, state multistep.StateBag) mu
err = WriteVMX(vmxPath, vmxData)

if err != nil {
err := fmt.Errorf("error writing VMX file: %s", err)
err = fmt.Errorf("error writing VMX file: %s", err)
state.Put("error", err)
ui.Error(err.Error())
return multistep.ActionHalt
Expand Down
2 changes: 1 addition & 1 deletion builder/vmware/common/step_configure_vmx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func testVMXFile(t *testing.T) string {
// displayName must always be set
err = WriteVMX(tf.Name(), map[string]string{"displayName": "PackerBuild"})
if err != nil {
t.Fatalf("error writing .vmx file: %v", err)
t.Fatalf("error writing .vmx file: %s", err)
}
tf.Close()

Expand Down
Loading