Merge pull request #3521 from baude/golangcilint1

first pass of corrections for golangci-lint

14 files changed, 125 insertions, 43 deletions

diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go
index 0ea89a72c..1cf9d686a 100644
--- a/pkg/adapter/containers.go
+++ b/pkg/adapter/containers.go
@@ -95,8 +95,8 @@ func (r *LocalRuntime) StopContainers(ctx context.Context, cli *cliconfig.StopVa
 		}
 
 		pool.Add(shared.Job{
-			c.ID(),
-			func() error {
+			ID: c.ID(),
+			Fn: func() error {
 				err := c.StopWithTimeout(*timeout)
 				if err != nil {
 					if errors.Cause(err) == define.ErrCtrStopped {
@@ -134,8 +134,8 @@ func (r *LocalRuntime) KillContainers(ctx context.Context, cli *cliconfig.KillVa
 		c := c
 
 		pool.Add(shared.Job{
-			c.ID(),
-			func() error {
+			ID: c.ID(),
+			Fn: func() error {
 				return c.Kill(uint(signal))
 			},
 		})
@@ -163,8 +163,8 @@ func (r *LocalRuntime) InitContainers(ctx context.Context, cli *cliconfig.InitVa
 		ctr := c
 
 		pool.Add(shared.Job{
-			ctr.ID(),
-			func() error {
+			ID: ctr.ID(),
+			Fn: func() error {
 				err := ctr.Init(ctx)
 				if err != nil {
 					// If we're initializing all containers, ignore invalid state errors
diff --git a/pkg/adapter/runtime_remote.go b/pkg/adapter/runtime_remote.go
index 800ed7569..db3f23629 100644
--- a/pkg/adapter/runtime_remote.go
+++ b/pkg/adapter/runtime_remote.go
@@ -97,6 +97,14 @@ func GetRuntime(ctx context.Context, c *cliconfig.PodmanCommand) (*LocalRuntime,
 	}, nil
 }
 
+// DeferredShutdown is a bogus wrapper for compaat with the libpod
+// runtime and should only be run when a defer is being used
+func (r RemoteRuntime) DeferredShutdown(force bool) {
+	if err := r.Shutdown(force); err != nil {
+		logrus.Error("unable to shutdown runtime")
+	}
+}
+
 // Shutdown is a bogus wrapper for compat with the libpod runtime
 func (r RemoteRuntime) Shutdown(force bool) error {
 	return nil
diff --git a/pkg/adapter/sigproxy_linux.go b/pkg/adapter/sigproxy_linux.go
index af968cb89..efa6afa7b 100644
--- a/pkg/adapter/sigproxy_linux.go
+++ b/pkg/adapter/sigproxy_linux.go
@@ -27,7 +27,9 @@ func ProxySignals(ctr *libpod.Container) {
 			if err := ctr.Kill(uint(s.(syscall.Signal))); err != nil {
 				logrus.Errorf("Error forwarding signal %d to container %s: %v", s, ctr.ID(), err)
 				signal.StopCatch(sigBuffer)
-				syscall.Kill(syscall.Getpid(), s.(syscall.Signal))
+				if err := syscall.Kill(syscall.Getpid(), s.(syscall.Signal)); err != nil {
+					logrus.Errorf("failed to kill pid %d", syscall.Getpid())
+				}
 			}
 		}
 	}()
diff --git a/pkg/adapter/terminal_linux.go b/pkg/adapter/terminal_linux.go
index 3c4c3bd38..be7dc0cb6 100644
--- a/pkg/adapter/terminal_linux.go
+++ b/pkg/adapter/terminal_linux.go
@@ -35,7 +35,9 @@ func StartAttachCtr(ctx context.Context, ctr *libpod.Container, stdout, stderr,
 		}
 
 		logrus.SetFormatter(&RawTtyFormatter{})
-		term.SetRawTerminal(os.Stdin.Fd())
+		if _, err := term.SetRawTerminal(os.Stdin.Fd()); err != nil {
+			return err
+		}
 
 		defer restoreTerminal(oldTermState)
 	}
diff --git a/pkg/errorhandling/errorhandling.go b/pkg/errorhandling/errorhandling.go
new file mode 100644
index 000000000..970d47636
--- /dev/null
+++ b/pkg/errorhandling/errorhandling.go
@@ -0,0 +1,23 @@
+package errorhandling
+
+import (
+	"os"
+
+	"github.com/sirupsen/logrus"
+)
+
+// SyncQuiet syncs a file and logs any error. Should only be used within
+// a defer.
+func SyncQuiet(f *os.File) {
+	if err := f.Sync(); err != nil {
+		logrus.Errorf("unable to sync file %s: %q", f.Name(), err)
+	}
+}
+
+// CloseQuiet closes a file and logs any error. Should only be used within
+// a defer.
+func CloseQuiet(f *os.File) {
+	if err := f.Close(); err != nil {
+		logrus.Errorf("unable to close file %s: %q", f.Name(), err)
+	}
+}
diff --git a/pkg/firewall/firewalld.go b/pkg/firewall/firewalld.go
index 32c2337a0..15e845cb7 100644
--- a/pkg/firewall/firewalld.go
+++ b/pkg/firewall/firewalld.go
@@ -18,6 +18,7 @@ package firewall
 
 import (
 	"fmt"
+	"github.com/sirupsen/logrus"
 	"strings"
 
 	"github.com/godbus/dbus"
@@ -113,7 +114,9 @@ func (fb *fwdBackend) Del(conf *FirewallNetConf) error {
 		// Remove firewalld rules which assigned the given source IP to the given zone
 		firewalldObj := fb.conn.Object(firewalldName, firewalldPath)
 		var res string
-		firewalldObj.Call(firewalldZoneInterface+"."+firewalldRemoveSourceMethod, 0, getFirewalldZone(conf), ipStr).Store(&res)
+		if err := firewalldObj.Call(firewalldZoneInterface+"."+firewalldRemoveSourceMethod, 0, getFirewalldZone(conf), ipStr).Store(&res); err != nil {
+			logrus.Errorf("unable to store firewallobj")
+		}
 	}
 	return nil
 }
diff --git a/pkg/firewall/iptables.go b/pkg/firewall/iptables.go
index 59d81b287..92d249f7b 100644
--- a/pkg/firewall/iptables.go
+++ b/pkg/firewall/iptables.go
@@ -21,6 +21,7 @@ package firewall
 
 import (
 	"fmt"
+	"github.com/sirupsen/logrus"
 	"net"
 
 	"github.com/coreos/go-iptables/iptables"
@@ -53,7 +54,9 @@ func generateFilterRule(privChainName string) []string {
 
 func cleanupRules(ipt *iptables.IPTables, privChainName string, rules [][]string) {
 	for _, rule := range rules {
-		ipt.Delete("filter", privChainName, rule...)
+		if err := ipt.Delete("filter", privChainName, rule...); err != nil {
+			logrus.Errorf("failed to delete iptables rule %s", privChainName)
+		}
 	}
 }
 
@@ -185,7 +188,9 @@ func (ib *iptablesBackend) Add(conf *FirewallNetConf) error {
 
 func (ib *iptablesBackend) Del(conf *FirewallNetConf) error {
 	for proto, ipt := range ib.protos {
-		ib.delRules(conf, ipt, proto)
+		if err := ib.delRules(conf, ipt, proto); err != nil {
+			logrus.Errorf("failed to delete iptables backend rule %s", conf.IptablesAdminChainName)
+		}
 	}
 	return nil
 }
diff --git a/pkg/hooks/exec/exec.go b/pkg/hooks/exec/exec.go
index 0dd091561..4038e3d94 100644
--- a/pkg/hooks/exec/exec.go
+++ b/pkg/hooks/exec/exec.go
@@ -5,6 +5,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"github.com/sirupsen/logrus"
 	"io"
 	osexec "os/exec"
 	"time"
@@ -54,7 +55,9 @@ func Run(ctx context.Context, hook *rspec.Hook, state []byte, stdout io.Writer,
 	case err = <-exit:
 		return err, err
 	case <-ctx.Done():
-		cmd.Process.Kill()
+		if err := cmd.Process.Kill(); err != nil {
+			logrus.Errorf("failed to kill pid %v", cmd.Process)
+		}
 		timer := time.NewTimer(postKillTimeout)
 		defer timer.Stop()
 		select {
diff --git a/pkg/logs/logs.go b/pkg/logs/logs.go
index 0f684750e..89e4e5686 100644
--- a/pkg/logs/logs.go
+++ b/pkg/logs/logs.go
@@ -30,6 +30,7 @@ import (
 
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/libpod/define"
+	"github.com/containers/libpod/pkg/errorhandling"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -153,7 +154,7 @@ func ReadLogs(logPath string, ctr *libpod.Container, opts *LogOptions) error {
 	if err != nil {
 		return errors.Wrapf(err, "failed to open log file %q", logPath)
 	}
-	defer file.Close()
+	defer errorhandling.CloseQuiet(file)
 
 	msg := &logMessage{}
 	opts.bytes = -1
@@ -161,9 +162,9 @@ func ReadLogs(logPath string, ctr *libpod.Container, opts *LogOptions) error {
 	reader := bufio.NewReader(file)
 
 	if opts.Follow {
-		followLog(reader, writer, opts, ctr, msg, logPath)
+		err = followLog(reader, writer, opts, ctr, msg, logPath)
 	} else {
-		dumpLog(reader, writer, opts, msg, logPath)
+		err = dumpLog(reader, writer, opts, msg, logPath)
 	}
 	return err
 }
diff --git a/pkg/netns/netns_linux.go b/pkg/netns/netns_linux.go
index a72a2d098..4a515c72a 100644
--- a/pkg/netns/netns_linux.go
+++ b/pkg/netns/netns_linux.go
@@ -83,7 +83,9 @@ func NewNS() (ns.NetNS, error) {
 	if err != nil {
 		return nil, err
 	}
-	mountPointFd.Close()
+	if err := mountPointFd.Close(); err != nil {
+		return nil, err
+	}
 
 	// Ensure the mount point is cleaned up on errors; if the namespace
 	// was successfully mounted this will have no effect because the file
diff --git a/pkg/rootless/rootless_linux.go b/pkg/rootless/rootless_linux.go
index 8028a359c..d7c2de81d 100644
--- a/pkg/rootless/rootless_linux.go
+++ b/pkg/rootless/rootless_linux.go
@@ -17,6 +17,7 @@ import (
 	"syscall"
 	"unsafe"
 
+	"github.com/containers/libpod/pkg/errorhandling"
 	"github.com/containers/storage/pkg/idtools"
 	"github.com/docker/docker/pkg/signal"
 	"github.com/godbus/dbus"
@@ -41,8 +42,7 @@ const (
 )
 
 func runInUser() error {
-	os.Setenv("_CONTAINERS_USERNS_CONFIGURED", "done")
-	return nil
+	return os.Setenv("_CONTAINERS_USERNS_CONFIGURED", "done")
 }
 
 var (
@@ -57,9 +57,15 @@ func IsRootless() bool {
 		rootlessGIDInit := int(C.rootless_gid())
 		if rootlessUIDInit != 0 {
 			// This happens if we joined the user+mount namespace as part of
-			os.Setenv("_CONTAINERS_USERNS_CONFIGURED", "done")
-			os.Setenv("_CONTAINERS_ROOTLESS_UID", fmt.Sprintf("%d", rootlessUIDInit))
-			os.Setenv("_CONTAINERS_ROOTLESS_GID", fmt.Sprintf("%d", rootlessGIDInit))
+			if err := os.Setenv("_CONTAINERS_USERNS_CONFIGURED", "done"); err != nil {
+				logrus.Errorf("failed to set environment variable %s as %s", "_CONTAINERS_USERNS_CONFIGURED", "done")
+			}
+			if err := os.Setenv("_CONTAINERS_ROOTLESS_UID", fmt.Sprintf("%d", rootlessUIDInit)); err != nil {
+				logrus.Errorf("failed to set environment variable %s as %d", "_CONTAINERS_ROOTLESS_UID", rootlessUIDInit)
+			}
+			if err := os.Setenv("_CONTAINERS_ROOTLESS_GID", fmt.Sprintf("%d", rootlessGIDInit)); err != nil {
+				logrus.Errorf("failed to set environment variable %s as %d", "_CONTAINERS_ROOTLESS_GID", rootlessGIDInit)
+			}
 		}
 		isRootless = os.Geteuid() != 0 || os.Getenv("_CONTAINERS_USERNS_CONFIGURED") != ""
 	})
@@ -185,18 +191,24 @@ func getUserNSFirstChild(fd uintptr) (*os.File, error) {
 		}
 
 		if ns == currentNS {
-			syscall.Close(int(nextFd))
+			if err := syscall.Close(int(nextFd)); err != nil {
+				return nil, err
+			}
 
 			// Drop O_CLOEXEC for the fd.
 			_, _, errno := syscall.Syscall(syscall.SYS_FCNTL, fd, syscall.F_SETFD, 0)
 			if errno != 0 {
-				syscall.Close(int(fd))
+				if err := syscall.Close(int(fd)); err != nil {
+					logrus.Errorf("failed to close file descriptor %d", fd)
+				}
 				return nil, errno
 			}
 
 			return os.NewFile(fd, "userns child"), nil
 		}
-		syscall.Close(int(fd))
+		if err := syscall.Close(int(fd)); err != nil {
+			return nil, err
+		}
 		fd = nextFd
 	}
 }
@@ -252,7 +264,9 @@ func EnableLinger() (string, error) {
 		if lingerEnabled && lingerFile != "" {
 			f, err := os.Create(lingerFile)
 			if err == nil {
-				f.Close()
+				if err := f.Close(); err != nil {
+					logrus.Errorf("failed to close %s", f.Name())
+				}
 			} else {
 				logrus.Debugf("could not create linger file: %v", err)
 			}
@@ -348,8 +362,8 @@ func becomeRootInUserNS(pausePid, fileToRead string, fileOutput *os.File) (bool,
 	}
 	r, w := os.NewFile(uintptr(fds[0]), "sync host"), os.NewFile(uintptr(fds[1]), "sync child")
 
-	defer r.Close()
-	defer w.Close()
+	defer errorhandling.CloseQuiet(r)
+	defer errorhandling.CloseQuiet(w)
 	defer w.Write([]byte("0"))
 
 	pidC := C.reexec_in_user_namespace(C.int(r.Fd()), cPausePid, cFileToRead, fileOutputFD)
@@ -361,9 +375,9 @@ func becomeRootInUserNS(pausePid, fileToRead string, fileOutput *os.File) (bool,
 	var uids, gids []idtools.IDMap
 	username := os.Getenv("USER")
 	if username == "" {
-		user, err := user.LookupId(fmt.Sprintf("%d", os.Getuid()))
+		userID, err := user.LookupId(fmt.Sprintf("%d", os.Getuid()))
 		if err == nil {
-			username = user.Username
+			username = userID.Username
 		}
 	}
 	mappings, err := idtools.NewIDMappings(username, username)
@@ -458,7 +472,9 @@ func becomeRootInUserNS(pausePid, fileToRead string, fileOutput *os.File) (bool,
 				continue
 			}
 
-			syscall.Kill(int(pidC), s.(syscall.Signal))
+			if err := syscall.Kill(int(pidC), s.(syscall.Signal)); err != nil {
+				logrus.Errorf("failed to kill %d", int(pidC))
+			}
 		}
 	}()
 
@@ -519,17 +535,19 @@ func TryJoinFromFilePaths(pausePidPath string, needNewNamespace bool, paths []st
 
 			r, w := os.NewFile(uintptr(fds[0]), "read file"), os.NewFile(uintptr(fds[1]), "write file")
 
-			defer w.Close()
-			defer r.Close()
+			defer errorhandling.CloseQuiet(w)
+			defer errorhandling.CloseQuiet(r)
 
 			if _, _, err := becomeRootInUserNS("", path, w); err != nil {
 				lastErr = err
 				continue
 			}
 
-			w.Close()
+			if err := w.Close(); err != nil {
+				return false, 0, err
+			}
 			defer func() {
-				r.Close()
+				errorhandling.CloseQuiet(r)
 				C.reexec_in_user_namespace_wait(-1, 0)
 			}()
 
diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go
index 06d1ac12d..5cc021bf5 100644
--- a/pkg/spec/spec.go
+++ b/pkg/spec/spec.go
@@ -475,7 +475,9 @@ func addPidNS(config *CreateConfig, g *generate.Generator) error {
 
 func addUserNS(config *CreateConfig, g *generate.Generator) error {
 	if IsNS(string(config.UsernsMode)) {
-		g.AddOrReplaceLinuxNamespace(spec.UserNamespace, NS(string(config.UsernsMode)))
+		if err := g.AddOrReplaceLinuxNamespace(spec.UserNamespace, NS(string(config.UsernsMode))); err != nil {
+			return err
+		}
 
 		// runc complains if no mapping is specified, even if we join another ns.  So provide a dummy mapping
 		g.AddLinuxUIDMapping(uint32(0), uint32(0), uint32(1))
@@ -483,7 +485,9 @@ func addUserNS(config *CreateConfig, g *generate.Generator) error {
 	}
 
 	if (len(config.IDMappings.UIDMap) > 0 || len(config.IDMappings.GIDMap) > 0) && !config.UsernsMode.IsHost() {
-		g.AddOrReplaceLinuxNamespace(spec.UserNamespace, "")
+		if err := g.AddOrReplaceLinuxNamespace(spec.UserNamespace, ""); err != nil {
+			return err
+		}
 	}
 	return nil
 }
diff --git a/pkg/util/utils.go b/pkg/util/utils.go
index 9e49f08a0..fba34a337 100644
--- a/pkg/util/utils.go
+++ b/pkg/util/utils.go
@@ -12,12 +12,14 @@ import (
 	"github.com/BurntSushi/toml"
 	"github.com/containers/image/types"
 	"github.com/containers/libpod/cmd/podman/cliconfig"
+	"github.com/containers/libpod/pkg/errorhandling"
 	"github.com/containers/libpod/pkg/namespaces"
 	"github.com/containers/libpod/pkg/rootless"
 	"github.com/containers/storage"
 	"github.com/containers/storage/pkg/idtools"
 	"github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
 	"golang.org/x/crypto/ssh/terminal"
 )
@@ -272,16 +274,20 @@ func getTomlStorage(storeOptions *storage.StoreOptions) *tomlConfig {
 
 // WriteStorageConfigFile writes the configuration to a file
 func WriteStorageConfigFile(storageOpts *storage.StoreOptions, storageConf string) error {
-	os.MkdirAll(filepath.Dir(storageConf), 0755)
-	file, err := os.OpenFile(storageConf, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666)
+	if err := os.MkdirAll(filepath.Dir(storageConf), 0755); err != nil {
+		return err
+	}
+	storageFile, err := os.OpenFile(storageConf, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666)
 	if err != nil {
 		return errors.Wrapf(err, "cannot open %s", storageConf)
 	}
 	tomlConfiguration := getTomlStorage(storageOpts)
-	defer file.Close()
-	enc := toml.NewEncoder(file)
+	defer errorhandling.CloseQuiet(storageFile)
+	enc := toml.NewEncoder(storageFile)
 	if err := enc.Encode(tomlConfiguration); err != nil {
-		os.Remove(storageConf)
+		if err := os.Remove(storageConf); err != nil {
+			logrus.Errorf("unable to remove file %s", storageConf)
+		}
 		return err
 	}
 	return nil
diff --git a/pkg/util/utils_supported.go b/pkg/util/utils_supported.go
index 99c9e4f1e..af55689a6 100644
--- a/pkg/util/utils_supported.go
+++ b/pkg/util/utils_supported.go
@@ -13,6 +13,7 @@ import (
 
 	"github.com/containers/libpod/pkg/rootless"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // GetRootlessRuntimeDir returns the runtime directory when running as non root
@@ -24,7 +25,9 @@ func GetRootlessRuntimeDir() (string, error) {
 		uid := fmt.Sprintf("%d", rootless.GetRootlessUID())
 		if runtimeDir == "" {
 			tmpDir := filepath.Join("/run", "user", uid)
-			os.MkdirAll(tmpDir, 0700)
+			if err := os.MkdirAll(tmpDir, 0700); err != nil {
+				logrus.Errorf("unable to make temp dir %s", tmpDir)
+			}
 			st, err := os.Stat(tmpDir)
 			if err == nil && int(st.Sys().(*syscall.Stat_t).Uid) == os.Geteuid() && st.Mode().Perm() == 0700 {
 				runtimeDir = tmpDir
@@ -32,7 +35,9 @@ func GetRootlessRuntimeDir() (string, error) {
 		}
 		if runtimeDir == "" {
 			tmpDir := filepath.Join(os.TempDir(), fmt.Sprintf("run-%s", uid))
-			os.MkdirAll(tmpDir, 0700)
+			if err := os.MkdirAll(tmpDir, 0700); err != nil {
+				logrus.Errorf("unable to make temp dir %s", tmpDir)
+			}
 			st, err := os.Stat(tmpDir)
 			if err == nil && int(st.Sys().(*syscall.Stat_t).Uid) == os.Geteuid() && st.Mode().Perm() == 0700 {
 				runtimeDir = tmpDir