fix a number of errcheck issues

Numerous issues remain, especially in tests/e2e.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>

cmd/podman/images/scp.go

@@ -267,7 +267,9 @@ func saveToRemote(image, localFile string, tag string, uri *urlP.URL, iden strin
 		return err
 	}
 	n, err := scpD.CopyFrom(dial, remoteFile, localFile)
-	connection.ExecRemoteCommand(dial, "rm "+remoteFile)
+	if _, conErr := connection.ExecRemoteCommand(dial, "rm "+remoteFile); conErr != nil {
+		logrus.Errorf("Error removing file on endpoint: %v", conErr)
+	}
 	if err != nil {
 		errOut := strconv.Itoa(int(n)) + " Bytes copied before error"
 		return errors.Wrapf(err, errOut)

cmd/podman/machine/init.go

@@ -83,7 +83,7 @@ func init() {
 		"reexec", false,
 		"process was rexeced",
 	)
-	flags.MarkHidden("reexec")
+	_ = flags.MarkHidden("reexec")
 
 	ImagePathFlagName := "image-path"
 	flags.StringVar(&initOpts.ImagePath, ImagePathFlagName, cfg.Machine.Image, "Path to qcow image")

cmd/podman/networks/create.go

@@ -60,7 +60,7 @@ func networkCreateFlags(cmd *cobra.Command) {
 	macvlanFlagName := "macvlan"
 	flags.StringVar(&networkCreateOptions.MacVLAN, macvlanFlagName, "", "create a Macvlan connection based on this device")
 	// This option is deprecated
-	flags.MarkHidden(macvlanFlagName)
+	_ = flags.MarkHidden(macvlanFlagName)
 
 	labelFlagName := "label"
 	flags.StringArrayVar(&labels, labelFlagName, nil, "set metadata on a network")

cmd/podman/root.go

@@ -402,7 +402,7 @@ func rootFlags(cmd *cobra.Command, opts *entities.PodmanConfig) {
 		networkBackendFlagName := "network-backend"
 		pFlags.StringVar(&cfg.Network.NetworkBackend, networkBackendFlagName, cfg.Network.NetworkBackend, `Network backend to use ("cni"|"netavark")`)
 		_ = cmd.RegisterFlagCompletionFunc(networkBackendFlagName, common.AutocompleteNetworkBackend)
-		pFlags.MarkHidden(networkBackendFlagName)
+		_ = pFlags.MarkHidden(networkBackendFlagName)
 
 		rootFlagName := "root"
 		pFlags.StringVar(&cfg.Engine.StaticDir, rootFlagName, "", "Path to the root directory in which data, including images, is stored")

cmd/podman/system/service.go

@@ -67,7 +67,7 @@ func init() {
 
 	flags.StringVarP(&srvArgs.PProfAddr, "pprof-address", "", "",
 		"Binding network address for pprof profile endpoints, default: do not expose endpoints")
-	flags.MarkHidden("pprof-address")
+	_ = flags.MarkHidden("pprof-address")
 }
 
 func aliasTimeoutFlag(_ *pflag.FlagSet, name string) pflag.NormalizedName {

cmd/podman/validate/args.go

@@ -27,7 +27,7 @@ func SubCommandExists(cmd *cobra.Command, args []string) error {
 		}
 		return errors.Errorf("unrecognized command `%[1]s %[2]s`\n\nDid you mean this?\n\t%[3]s\n\nTry '%[1]s --help' for more information.", cmd.CommandPath(), args[0], strings.Join(suggestions, "\n\t"))
 	}
-	cmd.Help()
+	cmd.Help() // nolint: errcheck
 	return errors.Errorf("missing command '%[1]s COMMAND'", cmd.CommandPath())
 }
 

cmd/rootlessport/main.go

@@ -253,9 +253,9 @@ func serve(listener net.Listener, pm rkport.Manager) {
 		ctx := context.TODO()
 		err = handler(ctx, conn, pm)
 		if err != nil {
-			conn.Write([]byte(err.Error()))
+			_, _ = conn.Write([]byte(err.Error()))
 		} else {
-			conn.Write([]byte("OK"))
+			_, _ = conn.Write([]byte("OK"))
 		}
 		conn.Close()
 	}

libpod/boltdb_state.go

@@ -366,8 +366,7 @@ func (s *BoltState) GetDBConfig() (*DBConfig, error) {
 	err = db.View(func(tx *bolt.Tx) error {
 		configBucket, err := getRuntimeConfigBucket(tx)
 		if err != nil {
-			// FIXME: this error should probably be returned
+			return err
-			return nil // nolint: nilerr
 		}
 
 		// Some of these may be nil

libpod/container_api.go

@@ -921,7 +921,11 @@ func (c *Container) Stat(ctx context.Context, containerPath string) (*define.Fil
 		if err != nil {
 			return nil, err
 		}
-		defer c.unmount(false)
+		defer func() {
+			if err := c.unmount(false); err != nil {
+				logrus.Errorf("Unmounting container %s: %v", c.ID(), err)
+			}
+		}()
 	}
 
 	info, _, _, err := c.stat(ctx, mountPoint, containerPath)

libpod/container_internal_linux.go

@@ -2740,7 +2740,6 @@ func (c *Container) generateUserPasswdEntry(addedUID int) (string, int, int, err
 	// If a non numeric User, then don't generate passwd
 	uid, err := strconv.ParseUint(userspec, 10, 32)
 	if err != nil {
-		// FIXME: this error should probably be returned
 		return "", 0, 0, nil // nolint: nilerr
 	}
 

libpod/events/logfile.go

@@ -9,6 +9,7 @@ import (
 	"github.com/containers/podman/v4/pkg/util"
 	"github.com/containers/storage/pkg/lockfile"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // EventLogFile is the structure for event writing to a logfile. It contains the eventer
@@ -59,7 +60,9 @@ func (e EventLogFile) Read(ctx context.Context, options ReadOptions) error {
 		}
 		go func() {
 			time.Sleep(time.Until(untilTime))
-			t.Stop()
+			if err := t.Stop(); err != nil {
+				logrus.Errorf("Stopping logger: %v", err)
+			}
 		}()
 	}
 	funcDone := make(chan bool)

pkg/bindings/test/attach_test.go

@@ -44,7 +44,8 @@ var _ = Describe("Podman containers attach", func() {
 			timeout := uint(5)
 			err := containers.Stop(bt.conn, id, new(containers.StopOptions).WithTimeout(timeout))
 			if err != nil {
-				GinkgoWriter.Write([]byte(err.Error()))
+				_, writeErr := GinkgoWriter.Write([]byte(err.Error()))
+				Expect(writeErr).ShouldNot(HaveOccurred())
 			}
 		}()
 

pkg/bindings/test/auth_test.go

@@ -40,7 +40,8 @@ var _ = Describe("Podman images", func() {
 	AfterEach(func() {
 		s.Kill()
 		bt.cleanup()
-		registry.Stop()
+		err := registry.Stop()
+		Expect(err).To(BeNil())
 	})
 
 	// Test using credentials.

pkg/bindings/test/containers_test.go

@@ -322,7 +322,8 @@ var _ = Describe("Podman containers ", func() {
 
 		// a container that has no healthcheck should be a 409
 		var name = "top"
-		bt.RunTopContainer(&name, nil)
+		_, err = bt.RunTopContainer(&name, nil)
+		Expect(err).To(BeNil())
 		_, err = containers.RunHealthCheck(bt.conn, name, nil)
 		Expect(err).ToNot(BeNil())
 		code, _ = bindings.CheckResponseCode(err)

pkg/checkpoint/crutils/checkpoint_restore_utils.go

@@ -102,7 +102,7 @@ func CRApplyRootFsDiffTar(baseDirectory, containerRootDirectory string) error {
 	// Only do this if a rootfs-diff.tar actually exists
 	rootfsDiffFile, err := os.Open(rootfsDiffPath)
 	if err != nil {
-		if os.IsNotExist(err) {
+		if errors.Is(err, os.ErrNotExist) {
 			return nil
 		}
 		return errors.Wrap(err, "failed to open root file-system diff file")

pkg/domain/filters/containers.go

@@ -213,7 +213,7 @@ func GenerateContainerFilterFuncs(filter string, filterValues []string, r *libpo
 		for _, val := range filterValues {
 			net, err := r.Network().NetworkInspect(val)
 			if err != nil {
-				if errors.Cause(err) == define.ErrNoSuchNetwork {
+				if errors.Is(err, define.ErrNoSuchNetwork) {
 					continue
 				}
 				return nil, err

pkg/domain/filters/pods.go

@@ -131,7 +131,7 @@ func GeneratePodFilterFunc(filter string, filterValues []string, r *libpod.Runti
 		for _, val := range filterValues {
 			net, err := r.Network().NetworkInspect(val)
 			if err != nil {
-				if errors.Cause(err) == define.ErrNoSuchNetwork {
+				if errors.Is(err, define.ErrNoSuchNetwork) {
 					continue
 				}
 				return nil, err

pkg/domain/infra/abi/images.go

@@ -848,13 +848,12 @@ func execPodman(execUser *user.User, command []string) error {
 	if err != nil {
 		return err
 	}
-	defer func() error {
+
-		err := cmdLogin.Process.Kill()
+	defer func() {
-		if err != nil {
+		_ = cmdLogin.Process.Kill()
-			return err
+		_ = cmdLogin.Wait()
-		}
-		return cmdLogin.Wait()
 	}()
+
 	cmd := exec.Command(command[0], command[1:]...)
 	cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "TERM=" + os.Getenv("TERM")}
 	cmd.Stderr = os.Stderr

pkg/domain/infra/abi/play.go

@@ -215,7 +215,7 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
 	// FIXME This is very hard to support properly with a good ux
 	if len(options.StaticIPs) > *ipIndex {
 		if !podOpt.Net.Network.IsBridge() {
-			errors.Wrap(define.ErrInvalidArg, "static ip addresses can only be set when the network mode is bridge")
+			return nil, errors.Wrap(define.ErrInvalidArg, "static ip addresses can only be set when the network mode is bridge")
 		}
 		if len(podOpt.Net.Networks) != 1 {
 			return nil, errors.Wrap(define.ErrInvalidArg, "cannot set static ip addresses for more than network, use netname:ip=<ip> syntax to specify ips for more than network")
@@ -230,7 +230,7 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
 	}
 	if len(options.StaticMACs) > *ipIndex {
 		if !podOpt.Net.Network.IsBridge() {
-			errors.Wrap(define.ErrInvalidArg, "static mac address can only be set when the network mode is bridge")
+			return nil, errors.Wrap(define.ErrInvalidArg, "static mac address can only be set when the network mode is bridge")
 		}
 		if len(podOpt.Net.Networks) != 1 {
 			return nil, errors.Wrap(define.ErrInvalidArg, "cannot set static mac address for more than network, use netname:mac=<mac> syntax to specify mac for more than network")

pkg/domain/infra/abi/terminal/sigproxy_linux.go

@@ -20,7 +20,7 @@ const signalBufferSize = 2048
 func ProxySignals(ctr *libpod.Container) {
 	// Stop catching the shutdown signals (SIGINT, SIGTERM) - they're going
 	// to the container now.
-	shutdown.Stop()
+	shutdown.Stop() // nolint: errcheck
 
 	sigBuffer := make(chan os.Signal, signalBufferSize)
 	signal.CatchAll(sigBuffer)

pkg/machine/fedora.go

@@ -59,7 +59,7 @@ func (f FedoraDownload) Get() *Download {
 func (f FedoraDownload) HasUsableCache() (bool, error) {
 	info, err := os.Stat(f.LocalPath)
 	if err != nil {
-		if os.IsNotExist(err) {
+		if errors.Is(err, os.ErrNotExist) {
 			return false, nil
 		}
 		return false, err

pkg/machine/qemu/machine.go

@@ -278,7 +278,9 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 		fmt.Println("An ignition path was provided.  No SSH connection was added to Podman")
 	}
 	// Write the JSON file
-	v.writeConfig()
+	if err := v.writeConfig(); err != nil {
+		return false, fmt.Errorf("writing JSON file: %w", err)
+	}
 
 	// User has provided ignition file so keygen
 	// will be skipped.
@@ -1099,10 +1101,13 @@ func waitAndPingAPI(sock string) {
 		Transport: &http.Transport{
 			DialContext: func(context.Context, string, string) (net.Conn, error) {
 				con, err := net.DialTimeout("unix", sock, apiUpTimeout)
-				if err == nil {
+				if err != nil {
-					con.SetDeadline(time.Now().Add(apiUpTimeout))
+					return nil, err
 				}
-				return con, err
+				if err := con.SetDeadline(time.Now().Add(apiUpTimeout)); err != nil {
+					return nil, err
+				}
+				return con, nil
 			},
 		},
 	}

pkg/rootless/rootless.go

@@ -1,6 +1,8 @@
 package rootless
 
 import (
+	"errors"
+	"fmt"
 	"os"
 	"sort"
 	"sync"
@@ -8,7 +10,6 @@ import (
 	"github.com/containers/storage/pkg/lockfile"
 	"github.com/opencontainers/runc/libcontainer/user"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/pkg/errors"
 )
 
 // TryJoinPauseProcess attempts to join the namespaces of the pause PID via
@@ -16,7 +17,7 @@ import (
 // file.
 func TryJoinPauseProcess(pausePidPath string) (bool, int, error) {
 	if _, err := os.Stat(pausePidPath); err != nil {
-		if os.IsNotExist(err) {
+		if errors.Is(err, os.ErrNotExist) {
 			return false, -1, nil
 		}
 		return false, -1, err
@@ -34,7 +35,7 @@ func TryJoinPauseProcess(pausePidPath string) (bool, int, error) {
 		if os.IsNotExist(err) {
 			return false, -1, nil
 		}
-		return false, -1, errors.Wrapf(err, "error acquiring lock on %s", pausePidPath)
+		return false, -1, fmt.Errorf("error acquiring lock on %s: %w", pausePidPath, err)
 	}
 
 	pidFileLock.Lock()

pkg/specgen/generate/ports_bench_test.go

@@ -9,7 +9,7 @@ import (
 
 func benchmarkParsePortMapping(b *testing.B, ports []types.PortMapping) {
 	for n := 0; n < b.N; n++ {
-		ParsePortMapping(ports, nil)
+		_, _ = ParsePortMapping(ports, nil)
 	}
 }
 

pkg/systemd/generate/containers.go

@@ -362,7 +362,9 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 		fs.StringArrayP("env", "e", nil, "")
 		fs.String("sdnotify", "", "")
 		fs.String("restart", "", "")
-		fs.Parse(remainingCmd)
+		if err := fs.Parse(remainingCmd); err != nil {
+			return "", fmt.Errorf("parsing remaining command-line arguments: %w", err)
+		}
 
 		remainingCmd = filterCommonContainerFlags(remainingCmd, fs.NArg())
 		// If the container is in a pod, make sure that the

pkg/systemd/generate/pods.go

@@ -335,7 +335,9 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 		fs.SetInterspersed(false)
 		fs.String("name", "", "")
 		fs.Bool("replace", false, "")
-		fs.Parse(podCreateArgs)
+		if err := fs.Parse(podCreateArgs); err != nil {
+			return "", fmt.Errorf("parsing remaining command-line arguments: %w", err)
+		}
 
 		hasNameParam := fs.Lookup("name").Changed
 		hasReplaceParam, err := fs.GetBool("replace")

test/e2e/attach_test.go

@@ -1,7 +1,6 @@
 package integration
 
 import (
-	"os"
 	"syscall"
 	"time"
 
@@ -20,12 +19,11 @@ var _ = Describe("Podman attach", func() {
 
 	BeforeEach(func() {
 		tempdir, err = CreateTempDirInTempDir()
-		if err != nil {
+		Expect(err).To(BeNil())
-			os.Exit(1)
-		}
 		podmanTest = PodmanTestCreate(tempdir)
 		podmanTest.Setup()
-		podmanTest.SeedImages()
+		err = podmanTest.SeedImages()
+		Expect(err).To(BeNil())
 	})
 
 	AfterEach(func() {

test/e2e/checkpoint_test.go

@@ -37,12 +37,12 @@ var _ = Describe("Podman checkpoint", func() {
 	BeforeEach(func() {
 		SkipIfRootless("checkpoint not supported in rootless mode")
 		tempdir, err = CreateTempDirInTempDir()
-		if err != nil {
+		Expect(err).To(BeNil())
-			os.Exit(1)
+
-		}
 		podmanTest = PodmanTestCreate(tempdir)
 		podmanTest.Setup()
-		podmanTest.SeedImages()
+		err = podmanTest.SeedImages()
+		Expect(err).To(BeNil())
 		// Check if the runtime implements checkpointing. Currently only
 		// runc's checkpoint/restore implementation is supported.
 		cmd := exec.Command(podmanTest.OCIRuntime, "checkpoint", "--help")

test/e2e/commit_test.go

@@ -21,12 +21,11 @@ var _ = Describe("Podman commit", func() {
 
 	BeforeEach(func() {
 		tempdir, err = CreateTempDirInTempDir()
-		if err != nil {
+		Expect(err).To(BeNil())
-			os.Exit(1)
-		}
 		podmanTest = PodmanTestCreate(tempdir)
 		podmanTest.Setup()
-		podmanTest.SeedImages()
+		err = podmanTest.SeedImages()
+		Expect(err).To(BeNil())
 	})
 
 	AfterEach(func() {

test/e2e/common_test.go

@@ -809,7 +809,8 @@ func (p *PodmanTestIntegration) RestoreArtifactToCache(image string) error {
 
 func populateCache(podman *PodmanTestIntegration) {
 	for _, image := range CACHE_IMAGES {
-		podman.RestoreArtifactToCache(image)
+		err := podman.RestoreArtifactToCache(image)
+		Expect(err).To(BeNil())
 	}
 	// logformatter uses this to recognize the first test
 	fmt.Printf("-----------------------------\n")

test/e2e/create_test.go

@@ -24,12 +24,11 @@ var _ = Describe("Podman create", func() {
 
 	BeforeEach(func() {
 		tempdir, err = CreateTempDirInTempDir()
-		if err != nil {
+		Expect(err).To(BeNil())
-			os.Exit(1)
-		}
 		podmanTest = PodmanTestCreate(tempdir)
 		podmanTest.Setup()
-		podmanTest.SeedImages()
+		err = podmanTest.SeedImages()
+		Expect(err).To(BeNil())
 	})
 
 	AfterEach(func() {

test/utils/common_function_test.go

@@ -51,7 +51,8 @@ var _ = Describe("Common functions test", func() {
 			txt := fmt.Sprintf("ID=%s\nVERSION_ID=%s", id, ver)
 			if !empty {
 				f, _ := os.Create(path)
-				f.WriteString(txt)
+				_, err := f.WriteString(txt)
+				Expect(err).To(BeNil(), "Failed to write data.")
 				f.Close()
 			}
 
@@ -136,7 +137,8 @@ var _ = Describe("Common functions test", func() {
 			}
 			if createFile {
 				f, _ := os.Create(path)
-				f.WriteString(txt)
+				_, err := f.WriteString(txt)
+				Expect(err).To(BeNil(), "Failed to write data.")
 				f.Close()
 			}
 			ProcessOneCgroupPath = path