Skip to content

Commit

Permalink
Enable all errorlint checks
Browse files Browse the repository at this point in the history
Use %w instead of %v to wrap errors

Use errors Is() and As() and the govmomi fault equivalents
instead of type casting/checking
  • Loading branch information
Bryan Venteicher committed Dec 23, 2024
1 parent d55c5f2 commit 74c81d7
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 65 deletions.
7 changes: 1 addition & 6 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,21 +143,16 @@ linters-settings:
desc: "replaced by internal packages like pkg/util/ptr"
- pkg: github.com/onsi/ginkgo$
desc: "replaced by github.com/onsi/ginkgo/v2"
errorlint:
# Check whether fmt.Errorf uses the %w verb for formatting errors.
errorf: false
# Check for plain type assertions and type switches.
asserts: false

linters:
disable-all: true
enable:
- errorlint
- asciicheck
- bodyclose
- depguard
- dogsled
- errcheck
- errorlint
- exportloopref
- goconst
- gocritic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ func (r *Reconciler) ReconcileNormal(ctx *pkgctx.VirtualMachineReplicaSetContext
client.MatchingLabels(selectorMap),
); err != nil {
return ctrl.Result{},
fmt.Errorf("failed to list virtual machines matched by the VirtualMachineReplicaSet. selector: %q, err: %v",
fmt.Errorf("failed to list virtual machines matched by the VirtualMachineReplicaSet. selector: %q, err: %w",
selectorMap, err)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ func (cs *provider) generateDownloadURLForLibraryItem(

if info.Status == "ERROR" {
// Log message used by VMC LINT. Refer to before making changes
return false, fmt.Errorf("error occurred preparing file for download %v", info.ErrorMessage)
return false, fmt.Errorf("error occurred preparing file for download %w", info.ErrorMessage)
}

if info.Status != "PREPARED" {
Expand Down
4 changes: 2 additions & 2 deletions pkg/providers/vsphere/virtualmachine/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ func trimBackupFields(obj client.Object) {
func getEncodedVMYaml(vm *vmopv1.VirtualMachine) (string, error) {
vmYAML, err := k8syaml.Marshal(vm)
if err != nil {
return "", fmt.Errorf("failed to marshal VM into YAML %+v: %v", vm, err)
return "", fmt.Errorf("failed to marshal VM into YAML %+v: %w", vm, err)
}

return pkgutil.EncodeGzipBase64(string(vmYAML))
Expand Down Expand Up @@ -363,7 +363,7 @@ func getDesiredAdditionalResourceYAMLForBackup(
trimBackupFields(resource)
marshaledYaml, err := k8syaml.Marshal(resource)
if err != nil {
return "", fmt.Errorf("failed to marshal object %q: %v", resource.GetName(), err)
return "", fmt.Errorf("failed to marshal object %q: %w", resource.GetName(), err)
}
sb.Write(marshaledYaml)
if i != len(resources)-1 {
Expand Down
13 changes: 2 additions & 11 deletions pkg/providers/vsphere/vmlifecycle/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"slices"
"strings"

"github.com/vmware/govmomi/fault"
"github.com/vmware/govmomi/object"
"github.com/vmware/govmomi/task"
vimtypes "github.com/vmware/govmomi/vim25/types"
apiEquality "k8s.io/apimachinery/pkg/api/equality"
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -272,7 +272,7 @@ func doCustomize(
if err := resources.NewVMFromObject(vcVM).Customize(vmCtx, *customSpec); err != nil {
// isCustomizationPendingExtraConfig() above is supposed to prevent this error, but
// handle it explicitly here just in case so VM reconciliation can proceed.
if !isCustomizationPendingError(err) {
if !fault.Is(err, &vimtypes.CustomizationPending{}) {
return err
}
}
Expand All @@ -291,15 +291,6 @@ func IsCustomizationPendingExtraConfig(extraConfig []vimtypes.BaseOptionValue) b
return false
}

func isCustomizationPendingError(err error) bool {
if te, ok := err.(task.Error); ok {
if _, ok := te.Fault().(*vimtypes.CustomizationPending); ok {
return true
}
}
return false
}

// linuxGuestIDPrefixes is derived from the vimtypes.VirtualMachineGuestOsIdentifier values.
var linuxGuestIDPrefixes = [...]string{
"redhat",
Expand Down
4 changes: 3 additions & 1 deletion pkg/util/kube/spq/spq.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package spq

import (
"context"
"errors"
"fmt"
"strings"

Expand Down Expand Up @@ -66,7 +67,8 @@ func GetStorageClassesForPolicy(
namespace,
o.Name)
if err != nil {
if _, ok := err.(NotFoundInNamespace); !ok {
var notFoundInNamespace NotFoundInNamespace
if !errors.As(err, &notFoundInNamespace) {
return nil, err
}
}
Expand Down
38 changes: 12 additions & 26 deletions pkg/util/vsphere/vm/hardware_version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,15 @@
package vm_test

import (
"errors"
"fmt"

"github.com/go-logr/logr"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"github.com/vmware/govmomi/fault"
"github.com/vmware/govmomi/object"
"github.com/vmware/govmomi/simulator"
"github.com/vmware/govmomi/task"
"github.com/vmware/govmomi/vim25/mo"
vimtypes "github.com/vmware/govmomi/vim25/types"

Expand Down Expand Up @@ -73,36 +72,23 @@ func hardwareVersionTests() {
}

assertInvalidPowerStateFaultPoweredOn := func(err error) {
ExpectWithOffset(1, err).To(HaveOccurred())
err = errors.Unwrap(err)
ExpectWithOffset(1, err).To(HaveOccurred())
ExpectWithOffset(1, err).To(BeAssignableToTypeOf(task.Error{}))
ExpectWithOffset(1, err.(task.Error).Fault()).ToNot(BeNil())
ExpectWithOffset(1, err.(task.Error).Fault()).To(BeAssignableToTypeOf(&vimtypes.InvalidPowerStateFault{}))
fault := err.(task.Error).Fault().(*vimtypes.InvalidPowerStateFault)
ExpectWithOffset(1, fault.ExistingState).To(Equal(vimtypes.VirtualMachinePowerStatePoweredOn))
ExpectWithOffset(1, fault.RequestedState).To(Equal(vimtypes.VirtualMachinePowerStatePoweredOff))
var psFault *vimtypes.InvalidPowerStateFault
_, ok := fault.As(err, &psFault)
ExpectWithOffset(1, ok).To(BeTrue())
ExpectWithOffset(1, psFault.ExistingState).To(Equal(vimtypes.VirtualMachinePowerStatePoweredOn))
ExpectWithOffset(1, psFault.RequestedState).To(Equal(vimtypes.VirtualMachinePowerStatePoweredOff))
}

assertInvalidPowerStateFaultSuspended := func(err error) {
ExpectWithOffset(1, err).To(HaveOccurred())
err = errors.Unwrap(err)
ExpectWithOffset(1, err).To(HaveOccurred())
ExpectWithOffset(1, err).To(BeAssignableToTypeOf(task.Error{}))
ExpectWithOffset(1, err.(task.Error).Fault()).ToNot(BeNil())
ExpectWithOffset(1, err.(task.Error).Fault()).To(BeAssignableToTypeOf(&vimtypes.InvalidPowerStateFault{}))
fault := err.(task.Error).Fault().(*vimtypes.InvalidPowerStateFault)
ExpectWithOffset(1, fault.ExistingState).To(Equal(vimtypes.VirtualMachinePowerStateSuspended))
ExpectWithOffset(1, fault.RequestedState).To(Equal(vimtypes.VirtualMachinePowerStatePoweredOff))
var psFault *vimtypes.InvalidPowerStateFault
_, ok := fault.As(err, &psFault)
ExpectWithOffset(1, ok).To(BeTrue())
ExpectWithOffset(1, psFault.ExistingState).To(Equal(vimtypes.VirtualMachinePowerStateSuspended))
ExpectWithOffset(1, psFault.RequestedState).To(Equal(vimtypes.VirtualMachinePowerStatePoweredOff))
}

assertAlreadyUpgradedFault := func(err error) {
ExpectWithOffset(1, err).To(HaveOccurred())
err = errors.Unwrap(err)
ExpectWithOffset(1, err).To(HaveOccurred())
ExpectWithOffset(1, err).To(BeAssignableToTypeOf(task.Error{}))
ExpectWithOffset(1, err.(task.Error).Fault()).ToNot(BeNil())
ExpectWithOffset(1, err.(task.Error).Fault()).To(BeAssignableToTypeOf(&vimtypes.AlreadyUpgradedFault{}))
Expect(fault.Is(err, &vimtypes.AlreadyUpgradedFault{})).To(BeTrue())
}

assertFailedToRetrievePropsNotFound := func(err error) {
Expand Down
18 changes: 8 additions & 10 deletions pkg/util/vsphere/vm/power_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import (

"github.com/go-logr/logr"

"github.com/vmware/govmomi/fault"
"github.com/vmware/govmomi/object"
"github.com/vmware/govmomi/task"
"github.com/vmware/govmomi/vim25"
"github.com/vmware/govmomi/vim25/mo"
vimtypes "github.com/vmware/govmomi/vim25/types"
Expand Down Expand Up @@ -330,15 +330,13 @@ func doAndWaitOnHardPowerOp(
"failed to invoke hard power op for %s %w", desiredPowerState, err)
}
if ti, err := t.WaitForResult(ctx); err != nil {
if err, ok := err.(task.Error); ok {
// Ignore error if desired power state already set.
if ips, ok := err.Fault().(*vimtypes.InvalidPowerState); ok && ips.ExistingState == ips.RequestedState {
log.Info(
"Power state already set",
"desiredPowerState",
desiredPowerState)
return PowerOpResultNone, nil
}
var ips *vimtypes.InvalidPowerState
if _, ok := fault.As(err, &ips); ok && ips.ExistingState == ips.RequestedState {
log.Info(
"Power state already set",
"desiredPowerState",
desiredPowerState)
return PowerOpResultNone, nil
}
if ti != nil {
log.Error(err, "Change power state task failed", "taskInfo", ti)
Expand Down
11 changes: 6 additions & 5 deletions pkg/util/vsphere/watcher/watcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,20 +213,21 @@ var _ = Describe("Start", func() {
ExpectWithOffset(1, errors.As(w.Err(), &urlErr)).To(BeTrue())
ExpectWithOffset(1, urlErr.Op).To(Equal("Post"))

switch tErr := urlErr.Err.(type) {
case *net.OpError:
var tErr *net.OpError
switch {
case errors.As(urlErr.Err, &tErr):
ExpectWithOffset(1, tErr.Op).To(Equal("dial"))
ExpectWithOffset(1, tErr.Net).To(Equal("tcp"))
ExpectWithOffset(1, tErr.Err).ToNot(BeNil())
var sysErr *os.SyscallError
ExpectWithOffset(1, errors.As(tErr.Err, &sysErr)).To(BeTrue())
ExpectWithOffset(1, sysErr.Syscall).To(Equal("connect"))
default:
ExpectWithOffset(1, tErr).To(HaveOccurred())
ExpectWithOffset(1, urlErr.Err).To(HaveOccurred())
}
}

When("the context is cancelled", FlakeAttempts(5), func() {
When("the context is cancelled", FlakeAttempts(1), func() {
JustBeforeEach(func() {
cancel()
})
Expand All @@ -247,7 +248,7 @@ var _ = Describe("Start", func() {
})
})

When("the connection to vSphere is interrupted", FlakeAttempts(5), func() {
When("the connection to vSphere is interrupted", FlakeAttempts(1), func() {
JustBeforeEach(func() {
closeServerOnce.Do(server.Close)
})
Expand Down
5 changes: 4 additions & 1 deletion test/testutil/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ func ContainsError(err error, message string) bool {
if err.Error() == message {
return true
}
switch tErr := err.(type) {

switch tErr := err.(type) { //nolint:errorlint
case unwrappableError:
err = tErr.Unwrap()
case unwrappableErrorSlice:
Expand All @@ -102,6 +103,8 @@ func ContainsError(err error, message string) bool {
return true
}
}
default:
return false
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1332,7 +1332,7 @@ func (v validator) isNetworkRestrictedForReadinessProbe(ctx *pkgctx.WebhookReque
configMap := &corev1.ConfigMap{}
configMapKey := ctrlclient.ObjectKey{Name: config.ProviderConfigMapName, Namespace: ctx.Namespace}
if err := v.client.Get(ctx, configMapKey, configMap); err != nil {
return false, fmt.Errorf("error get ConfigMap: %s while validating TCP readiness probe port: %v", configMapKey, err)
return false, fmt.Errorf("error get ConfigMap: %s while validating TCP readiness probe port: %w", configMapKey, err)
}

return configMap.Data[isRestrictedNetworkKey] == "true", nil
Expand Down

0 comments on commit 74c81d7

Please sign in to comment.