From e053e0e05ecd884067125627f0006d1b6e19226e Mon Sep 17 00:00:00 2001 From: baude Date: Mon, 8 Jul 2019 13:20:17 -0500 Subject: first pass of corrections for golangci-lint Signed-off-by: baude --- libpod/boltdb_state.go | 66 ++++++++++++++++++++-------------------- libpod/boltdb_state_internal.go | 11 ++++++- libpod/container_attach_linux.go | 7 +++-- libpod/image/image.go | 30 +++++++++++------- libpod/image/pull.go | 6 ++-- libpod/image/search.go | 4 ++- libpod/networking_linux.go | 5 +-- libpod/oci_linux.go | 21 ++++++++----- libpod/runtime.go | 7 +++++ 9 files changed, 98 insertions(+), 59 deletions(-) (limited to 'libpod') diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index 4dda3a7f0..176781f07 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -66,7 +66,7 @@ func NewBoltState(path string, runtime *Runtime) (State, error) { if err != nil { return nil, errors.Wrapf(err, "error opening database %s", path) } - // Everywhere else, we use s.closeDBCon(db) to ensure the state's DB + // Everywhere else, we use s.deferredCloseDBCon(db) to ensure the state's DB // mutex is also unlocked. // However, here, the mutex has not been locked, since we just created // the DB connection, and it hasn't left this function yet - no risk of @@ -141,7 +141,7 @@ func (s *BoltState) Refresh() error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.Update(func(tx *bolt.Tx) error { idBucket, err := getIDBucket(tx) @@ -253,7 +253,7 @@ func (s *BoltState) GetDBConfig() (*DBConfig, error) { if err != nil { return nil, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { configBucket, err := getRuntimeConfigBucket(tx) @@ -298,7 +298,7 @@ func (s *BoltState) ValidateDBConfig(runtime *Runtime) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) // Check runtime configuration if err := checkRuntimeConfig(db, runtime); err != nil { @@ -342,7 +342,7 @@ func (s *BoltState) Container(id string) (*Container, error) { if err != nil { return nil, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { ctrBucket, err := getCtrBucket(tx) @@ -378,7 +378,7 @@ func (s *BoltState) LookupContainer(idOrName string) (*Container, error) { if err != nil { return nil, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { ctrBucket, err := getCtrBucket(tx) @@ -484,7 +484,7 @@ func (s *BoltState) HasContainer(id string) (bool, error) { if err != nil { return false, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) exists := false @@ -549,7 +549,7 @@ func (s *BoltState) RemoveContainer(ctr *Container) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.Update(func(tx *bolt.Tx) error { return s.removeContainer(ctr, nil, tx) @@ -580,7 +580,7 @@ func (s *BoltState) UpdateContainer(ctr *Container) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { ctrBucket, err := getCtrBucket(tx) @@ -651,7 +651,7 @@ func (s *BoltState) SaveContainer(ctr *Container) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.Update(func(tx *bolt.Tx) error { ctrBucket, err := getCtrBucket(tx) @@ -708,7 +708,7 @@ func (s *BoltState) ContainerInUse(ctr *Container) ([]string, error) { if err != nil { return nil, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { ctrBucket, err := getCtrBucket(tx) @@ -759,7 +759,7 @@ func (s *BoltState) AllContainers() ([]*Container, error) { if err != nil { return nil, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { allCtrsBucket, err := getAllCtrsBucket(tx) @@ -833,7 +833,7 @@ func (s *BoltState) RewriteContainerConfig(ctr *Container, newCfg *ContainerConf if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.Update(func(tx *bolt.Tx) error { ctrBkt, err := getCtrBucket(tx) @@ -877,7 +877,7 @@ func (s *BoltState) RewritePodConfig(pod *Pod, newCfg *PodConfig) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.Update(func(tx *bolt.Tx) error { podBkt, err := getPodBucket(tx) @@ -920,7 +920,7 @@ func (s *BoltState) Pod(id string) (*Pod, error) { if err != nil { return nil, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { podBkt, err := getPodBucket(tx) @@ -955,7 +955,7 @@ func (s *BoltState) LookupPod(idOrName string) (*Pod, error) { if err != nil { return nil, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { podBkt, err := getPodBucket(tx) @@ -1062,7 +1062,7 @@ func (s *BoltState) HasPod(id string) (bool, error) { if err != nil { return false, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { podBkt, err := getPodBucket(tx) @@ -1118,7 +1118,7 @@ func (s *BoltState) PodHasContainer(pod *Pod, id string) (bool, error) { if err != nil { return false, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { podBkt, err := getPodBucket(tx) @@ -1180,7 +1180,7 @@ func (s *BoltState) PodContainersByID(pod *Pod) ([]string, error) { if err != nil { return nil, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { podBkt, err := getPodBucket(tx) @@ -1242,7 +1242,7 @@ func (s *BoltState) PodContainers(pod *Pod) ([]*Container, error) { if err != nil { return nil, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { podBkt, err := getPodBucket(tx) @@ -1312,7 +1312,7 @@ func (s *BoltState) AddVolume(volume *Volume) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.Update(func(tx *bolt.Tx) error { volBkt, err := getVolBucket(tx) @@ -1369,7 +1369,7 @@ func (s *BoltState) RemoveVolume(volume *Volume) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.Update(func(tx *bolt.Tx) error { volBkt, err := getVolBucket(tx) @@ -1451,7 +1451,7 @@ func (s *BoltState) AllVolumes() ([]*Volume, error) { if err != nil { return nil, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { allVolsBucket, err := getAllVolsBucket(tx) @@ -1512,7 +1512,7 @@ func (s *BoltState) Volume(name string) (*Volume, error) { if err != nil { return nil, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { volBkt, err := getVolBucket(tx) @@ -1547,7 +1547,7 @@ func (s *BoltState) HasVolume(name string) (bool, error) { if err != nil { return false, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { volBkt, err := getVolBucket(tx) @@ -1587,7 +1587,7 @@ func (s *BoltState) VolumeInUse(volume *Volume) ([]string, error) { if err != nil { return nil, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { volBucket, err := getVolBucket(tx) @@ -1673,7 +1673,7 @@ func (s *BoltState) AddPod(pod *Pod) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.Update(func(tx *bolt.Tx) error { podBkt, err := getPodBucket(tx) @@ -1782,7 +1782,7 @@ func (s *BoltState) RemovePod(pod *Pod) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.Update(func(tx *bolt.Tx) error { podBkt, err := getPodBucket(tx) @@ -1877,7 +1877,7 @@ func (s *BoltState) RemovePodContainers(pod *Pod) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.Update(func(tx *bolt.Tx) error { podBkt, err := getPodBucket(tx) @@ -2038,7 +2038,7 @@ func (s *BoltState) RemoveContainerFromPod(pod *Pod, ctr *Container) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.Update(func(tx *bolt.Tx) error { return s.removeContainer(ctr, pod, tx) @@ -2066,7 +2066,7 @@ func (s *BoltState) UpdatePod(pod *Pod) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) podID := []byte(pod.ID()) @@ -2126,7 +2126,7 @@ func (s *BoltState) SavePod(pod *Pod) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) podID := []byte(pod.ID()) @@ -2168,7 +2168,7 @@ func (s *BoltState) AllPods() ([]*Pod, error) { if err != nil { return nil, err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.View(func(tx *bolt.Tx) error { allPodsBucket, err := getAllPodsBucket(tx) diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index ee2784cdd..408ef7224 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -247,6 +247,15 @@ func (s *BoltState) getDBCon() (*bolt.DB, error) { return db, nil } +// deferredCloseDBCon closes the bolt db but instead of returning an +// error it logs the error. it is meant to be used within the confines +// of a defer statement only +func (s *BoltState) deferredCloseDBCon(db *bolt.DB) { + if err := s.closeDBCon(db); err != nil { + logrus.Errorf("failed to close libpod db: %q", err) + } +} + // Close a connection to the database. // MUST be used in place of `db.Close()` to ensure proper unlocking of the // state. @@ -479,7 +488,7 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error { if err != nil { return err } - defer s.closeDBCon(db) + defer s.deferredCloseDBCon(db) err = db.Update(func(tx *bolt.Tx) error { idsBucket, err := getIDBucket(tx) diff --git a/libpod/container_attach_linux.go b/libpod/container_attach_linux.go index 17b09fccc..43dd7d579 100644 --- a/libpod/container_attach_linux.go +++ b/libpod/container_attach_linux.go @@ -10,6 +10,7 @@ import ( "path/filepath" "github.com/containers/libpod/libpod/define" + "github.com/containers/libpod/pkg/errorhandling" "github.com/containers/libpod/pkg/kubeutils" "github.com/containers/libpod/utils" "github.com/docker/docker/pkg/term" @@ -66,7 +67,7 @@ func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSi logrus.Debugf("Could not open ctl file: %v", err) return } - defer controlFile.Close() + defer errorhandling.CloseQuiet(controlFile) logrus.Debugf("Received a resize event: %+v", size) if _, err = fmt.Fprintf(controlFile, "%d %d %d\n", 1, size.Height, size.Width); err != nil { @@ -108,7 +109,9 @@ func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSi var err error if streams.AttachInput { _, err = utils.CopyDetachable(conn, streams.InputStream, detachKeys) - conn.CloseWrite() + if err := conn.CloseWrite(); err != nil { + logrus.Error("failed to close write in attach") + } } stdinDone <- err }() diff --git a/libpod/image/image.go b/libpod/image/image.go index 76e46f74f..6509134ac 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -323,7 +323,7 @@ func (i *Image) Names() []string { // RepoDigests returns a string array of repodigests associated with the image func (i *Image) RepoDigests() ([]string, error) { var repoDigests []string - digest := i.Digest() + imageDigest := i.Digest() for _, name := range i.Names() { named, err := reference.ParseNormalizedNamed(name) @@ -331,7 +331,7 @@ func (i *Image) RepoDigests() ([]string, error) { return nil, err } - canonical, err := reference.WithDigest(reference.TrimNamed(named), digest) + canonical, err := reference.WithDigest(reference.TrimNamed(named), imageDigest) if err != nil { return nil, err } @@ -462,11 +462,11 @@ func getImageDigest(ctx context.Context, src types.ImageReference, sc *types.Sys return "", err } defer newImg.Close() - digest := newImg.ConfigInfo().Digest - if err = digest.Validate(); err != nil { + imageDigest := newImg.ConfigInfo().Digest + if err = imageDigest.Validate(); err != nil { return "", errors.Wrapf(err, "error getting config info") } - return "@" + digest.Hex(), nil + return "@" + imageDigest.Hex(), nil } // normalizedTag returns the canonical version of tag for use in Image.Names() @@ -495,7 +495,9 @@ func normalizedTag(tag string) (reference.Named, error) { // TagImage adds a tag to the given image func (i *Image) TagImage(tag string) error { - i.reloadImage() + if err := i.reloadImage(); err != nil { + return err + } ref, err := normalizedTag(tag) if err != nil { return err @@ -508,14 +510,18 @@ func (i *Image) TagImage(tag string) error { if err := i.imageruntime.store.SetNames(i.ID(), tags); err != nil { return err } - i.reloadImage() + if err := i.reloadImage(); err != nil { + return err + } defer i.newImageEvent(events.Tag) return nil } // UntagImage removes a tag from the given image func (i *Image) UntagImage(tag string) error { - i.reloadImage() + if err := i.reloadImage(); err != nil { + return err + } var newTags []string tags := i.Names() if !util.StringInSlice(tag, tags) { @@ -529,7 +535,9 @@ func (i *Image) UntagImage(tag string) error { if err := i.imageruntime.store.SetNames(i.ID(), newTags); err != nil { return err } - i.reloadImage() + if err := i.reloadImage(); err != nil { + return err + } defer i.newImageEvent(events.Untag) return nil } @@ -825,7 +833,7 @@ func (i *Image) GetLabel(ctx context.Context, label string) (string, error) { // Annotations returns the annotations of an image func (i *Image) Annotations(ctx context.Context) (map[string]string, error) { - manifest, manifestType, err := i.Manifest(ctx) + imageManifest, manifestType, err := i.Manifest(ctx) if err != nil { return nil, err } @@ -833,7 +841,7 @@ func (i *Image) Annotations(ctx context.Context) (map[string]string, error) { switch manifestType { case ociv1.MediaTypeImageManifest: var m ociv1.Manifest - if err := json.Unmarshal(manifest, &m); err == nil { + if err := json.Unmarshal(imageManifest, &m); err == nil { for k, v := range m.Annotations { annotations[k] = v } diff --git a/libpod/image/pull.go b/libpod/image/pull.go index e5765febc..ce8a19fbc 100644 --- a/libpod/image/pull.go +++ b/libpod/image/pull.go @@ -263,7 +263,9 @@ func (ir *Runtime) doPullImage(ctx context.Context, sc *types.SystemContext, goa copyOptions.SourceCtx.SystemRegistriesConfPath = systemRegistriesConfPath // FIXME: Set this more globally. Probably no reason not to have it in every types.SystemContext, and to compute the value just once in one place. // Print the following statement only when pulling from a docker or atomic registry if writer != nil && (imageInfo.srcRef.Transport().Name() == DockerTransport || imageInfo.srcRef.Transport().Name() == AtomicTransport) { - io.WriteString(writer, fmt.Sprintf("Trying to pull %s...", imageInfo.image)) + if _, err := io.WriteString(writer, fmt.Sprintf("Trying to pull %s...", imageInfo.image)); err != nil { + return nil, err + } } // If the label is not nil, check if the label exists and if not, return err if label != nil { @@ -277,7 +279,7 @@ func (ir *Runtime) doPullImage(ctx context.Context, sc *types.SystemContext, goa pullErrors = multierror.Append(pullErrors, err) logrus.Errorf("Error pulling image ref %s: %v", imageInfo.srcRef.StringWithinTransport(), err) if writer != nil { - io.WriteString(writer, "Failed\n") + _, _ = io.WriteString(writer, "Failed\n") } } else { if !goal.pullAllPairs { diff --git a/libpod/image/search.go b/libpod/image/search.go index 03a67636b..9984e5234 100644 --- a/libpod/image/search.go +++ b/libpod/image/search.go @@ -99,7 +99,9 @@ func SearchImages(term string, options SearchOptions) ([]SearchResult, error) { ctx := context.Background() for i := range registries { - sem.Acquire(ctx, 1) + if err := sem.Acquire(ctx, 1); err != nil { + return nil, err + } go searchImageInRegistryHelper(i, registries[i]) } diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index d978bceed..987c1fc5b 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -5,6 +5,7 @@ package libpod import ( "crypto/rand" "fmt" + "github.com/containers/libpod/pkg/errorhandling" "net" "os" "os/exec" @@ -168,8 +169,8 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) { if err != nil { return errors.Wrapf(err, "failed to open pipe") } - defer syncR.Close() - defer syncW.Close() + defer errorhandling.CloseQuiet(syncR) + defer errorhandling.CloseQuiet(syncW) havePortMapping := len(ctr.Config().PortMappings) > 0 apiSocket := filepath.Join(ctr.ociRuntime.tmpDir, fmt.Sprintf("%s.net", ctr.config.ID)) diff --git a/libpod/oci_linux.go b/libpod/oci_linux.go index 802f4311b..ca13d5517 100644 --- a/libpod/oci_linux.go +++ b/libpod/oci_linux.go @@ -17,6 +17,7 @@ import ( "github.com/containers/libpod/libpod/define" "github.com/containers/libpod/pkg/cgroups" + "github.com/containers/libpod/pkg/errorhandling" "github.com/containers/libpod/pkg/rootless" "github.com/containers/libpod/pkg/util" "github.com/containers/libpod/utils" @@ -117,7 +118,7 @@ func (r *OCIRuntime) createContainer(ctr *Container, cgroupParent string, restor if err != nil { return err } - defer fd.Close() + defer errorhandling.CloseQuiet(fd) // create a new mountns on the current thread if err = unix.Unshare(unix.CLONE_NEWNS); err != nil { @@ -207,8 +208,8 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, res return errors.Wrapf(err, "error creating socket pair for start pipe") } - defer parentPipe.Close() - defer parentStartPipe.Close() + defer errorhandling.CloseQuiet(parentPipe) + defer errorhandling.CloseQuiet(parentStartPipe) ociLog := filepath.Join(ctr.state.RunDir, "oci-log") logLevel := logrus.GetLevel() @@ -364,20 +365,26 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, res err = cmd.Start() // Ignore error returned from SetProcessLabel("") call, // can't recover. - label.SetProcessLabel("") + if err := label.SetProcessLabel(""); err != nil { + _ = err + } runtime.UnlockOSThread() } else { err = cmd.Start() } if err != nil { - childPipe.Close() + errorhandling.CloseQuiet(childPipe) return err } defer cmd.Wait() // We don't need childPipe on the parent side - childPipe.Close() - childStartPipe.Close() + if err := childPipe.Close(); err != nil { + return err + } + if err := childStartPipe.Close(); err != nil { + return err + } // Move conmon to specified cgroup if err := r.moveConmonToCgroup(ctr, cgroupParent, cmd); err != nil { diff --git a/libpod/runtime.go b/libpod/runtime.go index 53c9a1209..9196547a2 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -1162,6 +1162,13 @@ func (r *Runtime) GetConfig() (*RuntimeConfig, error) { return config, nil } +// DeferredShutdown shuts down the runtime without exposing any +// errors. This is only meant to be used when the runtime is being +// shutdown within a defer statement; else use Shutdown +func (r *Runtime) DeferredShutdown(force bool) { + _ = r.Shutdown(force) +} + // Shutdown shuts down the runtime and associated containers and storage // If force is true, containers and mounted storage will be shut down before // cleaning up; if force is false, an error will be returned if there are -- cgit v1.2.3-54-g00ecf