Merge pull request #4269 from twz123/wrap-errors

Wrap errors correctly, part 3

Author: twz123

.golangci.yml

@@ -10,10 +10,11 @@ run:
 
 linters:
   enable:
-    - depguard # Checks if package imports are in a list of acceptable packages
-    - gofmt    # Checks whether code was gofmt-ed
-    - goheader # Checks is file headers matche a given pattern
-    - revive   # Stricter drop-in replacement for golint
+    - depguard  # Checks if package imports are in a list of acceptable packages
+    - errorlint # Find code that will cause problems with Go's error wrapping scheme
+    - gofmt     # Checks whether code was gofmt-ed
+    - goheader  # Checks is file headers matche a given pattern
+    - revive    # Stricter drop-in replacement for golint
 
 linters-settings:
   depguard:

cmd/root.go

@@ -17,6 +17,7 @@ limitations under the License.
 package cmd
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"os"
@@ -70,7 +71,7 @@ func NewRootCmd() *cobra.Command {
 				go func() {
 					log := logrus.WithField("debug_server", config.DebugListenOn)
 					log.Debug("Starting debug server")
-					if err := http.ListenAndServe(config.DebugListenOn, nil); err != http.ErrServerClosed {
+					if err := http.ListenAndServe(config.DebugListenOn, nil); !errors.Is(err, http.ErrServerClosed) {
 						log.WithError(err).Debug("Failed to start debug server")
 					} else {
 						log.Debug("Debug server closed")

internal/pkg/archive/archive.go

@@ -19,6 +19,7 @@ package archive
 import (
 	"archive/tar"
 	"compress/gzip"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -50,7 +51,7 @@ func Extract(input io.Reader, dst string) error {
 
 	for {
 		header, err := tarReader.Next()
-		if err == io.EOF {
+		if errors.Is(err, io.EOF) {
 			break
 		}
 

internal/pkg/sysinfo/probes/linux/cgroups.go

@@ -19,6 +19,7 @@ limitations under the License.
 package linux
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"sync"
@@ -80,8 +81,9 @@ func (c *CgroupsProbes) probe(reporter probes.Reporter) error {
 }
 
 func reportCgroupSystemErr(reporter probes.Reporter, d probes.ProbeDesc, err error) error {
-	if detectionFailed, ok := err.(cgroupFsDetectionFailed); ok {
-		return reporter.Reject(d, detectionFailed, "")
+	var detectionFailedErr cgroupFsDetectionFailed
+	if errors.As(err, &detectionFailedErr) {
+		return reporter.Reject(d, detectionFailedErr, "")
 	}
 
 	return reporter.Error(d, err)

internal/pkg/sysinfo/probes/linux/kernel.go

@@ -206,8 +206,9 @@ func (k *kConfigProbe) DisplayName() string {
 func (k *kConfigProbe) Probe(reporter probes.Reporter) error {
 	option, err := k.probeConfig(k.kConfig)
 	if err != nil {
-		if err, notFound := err.(*noKConfigsFound); notFound {
-			return reporter.Warn(k, err, "")
+		var notFoundErr *noKConfigsFound
+		if errors.As(err, &notFoundErr) {
+			return reporter.Warn(k, notFoundErr, "")
 		}
 		return reporter.Error(k, err)
 	}

internal/pkg/sysinfo/probes/linux/kernel_test.go

@@ -117,8 +117,9 @@ func TestKConfigProber(t *testing.T) {
 
 		option, err := probeKConfig(ensureKConfig("I_CERTAINLY_DONT_EXIST"))
 		assert.Equal(t, option, kConfigUnknown)
-		if _, ok := err.(*noKConfigsFound); ok {
-			t.Log("system doesn't expose its kernel config")
+		var notFoundErr *noKConfigsFound
+		if errors.As(err, &notFoundErr) {
+			t.Logf("System doesn't seem to expose its kernel config: %v", err)
 		} else {
 			assert.NoError(t, err)
 		}

internal/pkg/sysinfo/probes/memory.go

@@ -16,7 +16,10 @@ limitations under the License.
 
 package probes
 
-import "fmt"
+import (
+	"errors"
+	"fmt"
+)
 
 // AssertTotalMemory asserts a minimum amount of system RAM.
 func AssertTotalMemory(parent ParentProbe, min uint64) {
@@ -39,7 +42,8 @@ type assertTotalMem struct {
 func (a *assertTotalMem) Probe(reporter Reporter) error {
 	desc := NewProbeDesc("Total memory", a.path)
 	if totalMemory, err := a.probeTotalMemory(); err != nil {
-		if unsupportedErr, unsupported := err.(probeUnsupported); unsupported {
+		var unsupportedErr probeUnsupported
+		if errors.As(err, &unsupportedErr) {
 			return reporter.Warn(desc, unsupportedErr, "")
 		}
 		return reporter.Error(desc, err)

pkg/apis/k0s/v1beta1/cplb.go

@@ -86,7 +86,7 @@ func (c *ControlPlaneLoadBalancingSpec) ValidateVRRPInstances(getDefaultNICFn fu
 		if vi.Interface == "" {
 			nic, err := getDefaultNICFn()
 			if err != nil {
-				return fmt.Errorf("failed to get default NIC: %v", err)
+				return fmt.Errorf("failed to get default NIC: %w", err)
 			}
 			c.VRRPInstances[i].Interface = nic
 		}

pkg/component/controller/cplb_unix.go

@@ -65,11 +65,11 @@ func (k *Keepalived) Init(_ context.Context) error {
 
 	basepath := filepath.Dir(k.K0sVars.KeepalivedConfigFile)
 	if err = dir.Init(basepath, constant.KeepalivedDirMode); err != nil {
-		return fmt.Errorf("failed to create keepalived data dir: %v", err)
+		return fmt.Errorf("failed to create keepalived data dir: %w", err)
 	}
 
 	if err = os.Chown(basepath, k.uid, -1); err != nil {
-		return fmt.Errorf("failed to chown keepalived data dir: %v", err)
+		return fmt.Errorf("failed to chown keepalived data dir: %w", err)
 	}
 
 	return assets.Stage(k.K0sVars.BinDir, "keepalived", constant.BinDirMode)
@@ -82,14 +82,14 @@ func (k *Keepalived) Start(_ context.Context) error {
 	}
 
 	if err := k.configureDummy(); err != nil {
-		return fmt.Errorf("failed to configure dummy interface: %v", err)
+		return fmt.Errorf("failed to configure dummy interface: %w", err)
 	}
 
 	if err := k.Config.ValidateVRRPInstances(nil); err != nil {
-		return fmt.Errorf("failed to validate VRRP instances: %v", err)
+		return fmt.Errorf("failed to validate VRRP instances: %w", err)
 	}
 	if err := k.generateKeepalivedTemplate(); err != nil {
-		return fmt.Errorf("failed to generate keepalived template: %v", err)
+		return fmt.Errorf("failed to generate keepalived template: %w", err)
 
 	}
 
@@ -123,7 +123,7 @@ func (k *Keepalived) Stop() error {
 	k.log.Infof("Stopping keepalived")
 	if err := k.supervisor.Stop(); err != nil {
 		// Failed to stop keepalived. Don't delete the VIP, just in case.
-		return fmt.Errorf("failed to stop keepalived: %v", err)
+		return fmt.Errorf("failed to stop keepalived: %w", err)
 	}
 
 	k.log.Infof("Deleting dummy interface")
@@ -159,7 +159,7 @@ func (k *Keepalived) configureDummy() error {
 		// If the dummy interface fails, attempt to define the addresses just
 		// in case.
 		if err := k.ensureLinkAddresses(dummyLinkName, vips); err != nil {
-			return fmt.Errorf("failed to ensure link addresses: %v", err)
+			return fmt.Errorf("failed to ensure link addresses: %w", err)
 		}
 	}
 	return nil
@@ -183,7 +183,7 @@ func (k *Keepalived) ensureDummyInterface(linkName string) error {
 
 	// This happens if the interface exists but it's not a dummy interface
 	if err = netlink.LinkDel(link); err != nil {
-		return fmt.Errorf("failed to delete %s: %v", linkName, err)
+		return fmt.Errorf("failed to delete %s: %w", linkName, err)
 	}
 
 	return k.createDummyInterface(linkName)
@@ -201,12 +201,12 @@ func (k *Keepalived) createDummyInterface(linkName string) error {
 func (k *Keepalived) ensureLinkAddresses(linkName string, expectedAddresses []string) error {
 	link, err := netlink.LinkByName(linkName)
 	if err != nil {
-		return fmt.Errorf("failed to get link by name %s: %v", linkName, err)
+		return fmt.Errorf("failed to get link by name %s: %w", linkName, err)
 	}
 
 	linkAddrs, strAddrs, err := k.getLinkAddresses(link)
 	if err != nil {
-		return fmt.Errorf("failed to get addresses for link %s: %v", linkName, err)
+		return fmt.Errorf("failed to get addresses for link %s: %w", linkName, err)
 	}
 
 	// Remove unexpected addresses
@@ -216,7 +216,7 @@ func (k *Keepalived) ensureLinkAddresses(linkName string, expectedAddresses []st
 		if !slices.Contains(expectedAddresses, strAddrs[i]) {
 			k.log.Infof("Deleting address %s from link %s", strAddr, linkName)
 			if err = netlink.AddrDel(link, &linkAddr); err != nil {
-				return fmt.Errorf("failed to delete address %s from link %s: %v", linkAddr.IPNet.String(), linkName, err)
+				return fmt.Errorf("failed to delete address %s from link %s: %w", linkAddr.IPNet.String(), linkName, err)
 			}
 		}
 	}
@@ -225,7 +225,7 @@ func (k *Keepalived) ensureLinkAddresses(linkName string, expectedAddresses []st
 	for _, addr := range expectedAddresses {
 		if !slices.Contains(strAddrs, addr) {
 			if err = k.setLinkIP(addr, linkName, link); err != nil {
-				return fmt.Errorf("failed to add address %s to link %s: %v", addr, linkName, err)
+				return fmt.Errorf("failed to add address %s to link %s: %w", addr, linkName, err)
 			}
 		}
 	}
@@ -236,7 +236,7 @@ func (k *Keepalived) ensureLinkAddresses(linkName string, expectedAddresses []st
 func (k *Keepalived) setLinkIP(addr string, linkName string, link netlink.Link) error {
 	ipAddr, _, err := net.ParseCIDR(addr)
 	if err != nil {
-		return fmt.Errorf("failed to parse CIDR %s: %v", addr, err)
+		return fmt.Errorf("failed to parse CIDR %s: %w", addr, err)
 	}
 
 	var mask net.IPMask
@@ -255,15 +255,15 @@ func (k *Keepalived) setLinkIP(addr string, linkName string, link netlink.Link)
 
 	k.log.Infof("Adding address %s to link %s", addr, linkName)
 	if err := netlink.AddrAdd(link, linkAddr); err != nil {
-		return fmt.Errorf("failed to add address %s to link %s: %v", addr, linkName, err)
+		return fmt.Errorf("failed to add address %s to link %s: %w", addr, linkName, err)
 	}
 	return nil
 }
 
 func (*Keepalived) getLinkAddresses(link netlink.Link) ([]netlink.Addr, []string, error) {
 	linkAddrs, err := netlink.AddrList(link, netlink.FAMILY_ALL)
 	if err != nil {
-		return nil, nil, fmt.Errorf("failed to list addresses for link %s: %v", link.Attrs().Name, err)
+		return nil, nil, fmt.Errorf("failed to list addresses for link %s: %w", link.Attrs().Name, err)
 	}
 
 	strAddrs := make([]string, len(linkAddrs))
@@ -276,28 +276,28 @@ func (*Keepalived) getLinkAddresses(link netlink.Link) ([]netlink.Addr, []string
 func (k *Keepalived) generateKeepalivedTemplate() error {
 	f, err := os.OpenFile(k.K0sVars.KeepalivedConfigFile, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, fs.FileMode(0500))
 	if err != nil {
-		return fmt.Errorf("failed to open keepalived config file: %v", err)
+		return fmt.Errorf("failed to open keepalived config file: %w", err)
 	}
 	defer f.Close()
 
 	template, err := template.New("keepalived").Parse(keepalivedConfigTemplate)
 	if err != nil {
-		return fmt.Errorf("failed to parse keepalived template: %v", err)
+		return fmt.Errorf("failed to parse keepalived template: %w", err)
 	}
 
 	kc := keepalivedConfig{
 		VRRPInstances: k.Config.VRRPInstances,
 	}
 	if err = template.Execute(f, kc); err != nil {
-		return fmt.Errorf("failed to execute keepalived template: %v", err)
+		return fmt.Errorf("failed to execute keepalived template: %w", err)
 	}
 
 	// TODO: Do we really need to this every single time?
 	if err = os.Chown(k.K0sVars.KeepalivedConfigFile, k.uid, -1); err != nil {
-		return fmt.Errorf("failed to chown keepalived config file: %v", err)
+		return fmt.Errorf("failed to chown keepalived config file: %w", err)
 	}
 	if err = os.Chmod(k.K0sVars.KeepalivedConfigFile, fs.FileMode(0400)); err != nil {
-		return fmt.Errorf("failed to chmod keepalived config file: %v", err)
+		return fmt.Errorf("failed to chmod keepalived config file: %w", err)
 	}
 	return nil
 }

pkg/component/controller/workerconfig/reconciler_test.go

@@ -583,7 +583,7 @@ func TestReconciler_runReconcileLoop(t *testing.T) {
 
 	underTest.runReconcileLoop(ctx, updates, nil)
 
-	switch ctx.Err() {
+	switch ctx.Err() { //nolint:errorlint // as per context contract
 	case context.Canceled:
 		break // this is the good case
 	case context.DeadlineExceeded:

pkg/component/status/status.go

@@ -19,6 +19,7 @@ package status
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -104,7 +105,7 @@ func removeLeftovers(socket string) {
 // Start runs the component
 func (s *Status) Start(_ context.Context) error {
 	go func() {
-		if err := s.httpserver.Serve(s.listener); err != nil && err != http.ErrServerClosed {
+		if err := s.httpserver.Serve(s.listener); err != nil && !errors.Is(err, http.ErrServerClosed) {
 			s.L.Errorf("failed to start status server at %s: %s", s.Socket, err)
 		}
 	}()
@@ -115,7 +116,7 @@ func (s *Status) Start(_ context.Context) error {
 func (s *Status) Stop() error {
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
-	if err := s.httpserver.Shutdown(ctx); err != nil && err != context.Canceled {
+	if err := s.httpserver.Shutdown(ctx); err != nil && !errors.Is(err, context.Canceled) {
 		return err
 	}
 	// Unix socket doesn't need to be explicitly removed because it's hadled

pkg/component/worker/static_pods.go

@@ -362,7 +362,7 @@ func (s *staticPods) Start(ctx context.Context) error {
 	srv.Addr = addr
 
 	// Fire up the goroutine to accept HTTP connections.
-	notClosed := func(err error) bool { return err != http.ErrServerClosed }
+	notClosed := func(err error) bool { return !errors.Is(err, http.ErrServerClosed) }
 	s.stopped.Add(1)
 	go func() {
 		defer s.stopped.Done()

pkg/etcd/client.go

@@ -19,6 +19,7 @@ package etcd
 import (
 	"context"
 	"crypto/tls"
+	"errors"
 	"fmt"
 	"time"
 
@@ -145,7 +146,7 @@ func (c *Client) Health(ctx context.Context) error {
 	_, err := c.client.Get(ctx, "health")
 
 	// permission denied is OK since proposal goes through consensus to get it
-	if err == nil || err == rpctypes.ErrPermissionDenied {
+	if err == nil || errors.Is(err, rpctypes.ErrPermissionDenied) {
 		return nil
 	}
 

pkg/install/service.go

@@ -17,6 +17,7 @@ limitations under the License.
 package install
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"strings"
@@ -53,7 +54,7 @@ func InstalledService() (service.Service, error) {
 		}
 		_, err = s.Status()
 
-		if err != nil && err == service.ErrNotInstalled {
+		if err != nil && errors.Is(err, service.ErrNotInstalled) {
 			continue
 		}
 		if err != nil {
@@ -123,7 +124,7 @@ func EnsureService(args []string, envVars []string, force bool) error {
 	if force {
 		logrus.Infof("Uninstalling %s service", svcConfig.Name)
 		err = s.Uninstall()
-		if err != nil && err != service.ErrNotInstalled {
+		if err != nil && !errors.Is(err, service.ErrNotInstalled) {
 			logrus.Warnf("failed to uninstall service: %v", err)
 		}
 	}

pkg/kubernetes/watch/watcher.go

@@ -18,6 +18,7 @@ package watch
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"reflect"
 	"time"
@@ -393,9 +394,10 @@ func retry(ctx context.Context, errorCallback ErrorCallback, runWatch func(conte
 			return nil
 		}
 
-		if err, ok := err.(conditionError); ok {
+		var condErr conditionError
+		if errors.As(err, &condErr) {
 			// The user-specified condition returned an error.
-			return err.error
+			return condErr.error
 		}
 
 		if ctx.Err() != nil {