diff options
46 files changed, 324 insertions, 156 deletions
diff --git a/.cirrus.yml b/.cirrus.yml index dac41dc5f..da28bb597 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -215,7 +215,8 @@ build_each_commit_task: on_failure: failed_master_script: '$CIRRUS_WORKING_DIR/$SCRIPT_BASE/notice_master_failure.sh' -build_without_cgo: + +build_without_cgo_task: depends_on: - "gating" @@ -475,17 +476,18 @@ verify_test_built_images_task: always: <<: *standardlogs - -# Post message to IRC if everything passed +# Post message to IRC if everything passed PR testing success_task: only_if: $CIRRUS_BRANCH != 'master' - depends_on: # ignores any dependent task conditions + # ignores any dependent task conditions, include everything except 'release' + depends_on: &alltasks - "gating" - "vendor" - "varlink_api" - "build_each_commit" + - "build_without_cgo" - "meta" - "testing" - "special_testing_rootless" @@ -493,7 +495,6 @@ success_task: - "special_testing_cross" - "test_build_cache_images" - "verify_test_built_images" - - "build_without_cgo" env: CIRRUS_WORKING_DIR: "/usr/src/libpod" @@ -514,8 +515,7 @@ release_task: # allow_failures: $CI == "true" # skip_notifications: $CI == "true" - depends_on: - - "success" + depends_on: *alltasks gce_instance: image_name: "${IMAGE_BUILDER_CACHE_IMAGE_NAME}" diff --git a/cmd/podman/pod_ps.go b/cmd/podman/pod_ps.go index a525857de..fd8da53fb 100644 --- a/cmd/podman/pod_ps.go +++ b/cmd/podman/pod_ps.go @@ -552,9 +552,6 @@ func generatePodPsOutput(pods []*adapter.Pod, opts podPsOptions) error { switch opts.Format { case formats.JSONString: - if err != nil { - return errors.Wrapf(err, "unable to create JSON for output") - } out = formats.JSONStructArray{Output: podPsToGeneric([]podPsTemplateParams{}, psOutput)} default: psOutput, err := getPodTemplateOutput(psOutput, opts) diff --git a/cmd/podman/ps.go b/cmd/podman/ps.go index 26cc55e5f..9fad0ea65 100644 --- a/cmd/podman/ps.go +++ b/cmd/podman/ps.go @@ -319,6 +319,9 @@ func psDisplay(c *cliconfig.PsValues, runtime *adapter.LocalRuntime) error { } pss, err := runtime.Ps(c, opts) + if err != nil { + return err + } // Here and down if opts.Sort != "" { pss, err = sortPsOutput(opts.Sort, pss) @@ -376,8 +379,8 @@ func psDisplay(c *cliconfig.PsValues, runtime *adapter.LocalRuntime) error { size = units.HumanSizeWithPrecision(0, 0) } else { size = units.HumanSizeWithPrecision(float64(container.Size.RwSize), 3) + " (virtual " + units.HumanSizeWithPrecision(float64(container.Size.RootFsSize), 3) + ")" - fmt.Fprintf(w, "\t%s", size) } + fmt.Fprintf(w, "\t%s", size) } } else { diff --git a/cmd/podman/shared/container.go b/cmd/podman/shared/container.go index df4583be6..3c68a29b4 100644 --- a/cmd/podman/shared/container.go +++ b/cmd/podman/shared/container.go @@ -305,7 +305,7 @@ func generateContainerFilterFuncs(filter, filterValue string, r *libpod.Runtime) } return func(c *libpod.Container) bool { ec, exited, err := c.ExitCode() - if ec == int32(exitCode) && err == nil && exited == true { + if ec == int32(exitCode) && err == nil && exited { return true } return false @@ -611,7 +611,7 @@ func getNamespaceInfo(path string) (string, error) { // getStrFromSquareBrackets gets the string inside [] from a string func getStrFromSquareBrackets(cmd string) string { - reg, err := regexp.Compile(".*\\[|\\].*") + reg, err := regexp.Compile(`.*\[|\].*`) if err != nil { return "" } diff --git a/cmd/podman/shared/create.go b/cmd/podman/shared/create.go index 736a682eb..be1a731cc 100644 --- a/cmd/podman/shared/create.go +++ b/cmd/podman/shared/create.go @@ -93,9 +93,8 @@ func CreateContainer(ctx context.Context, c *GenericCLIResults, runtime *libpod. imageName = newImage.ID() } - var healthCheckCommandInput string // if the user disabled the healthcheck with "none", we skip adding it - healthCheckCommandInput = c.String("healthcheck-command") + healthCheckCommandInput := c.String("healthcheck-command") // the user didnt disable the healthcheck but did pass in a healthcheck command // now we need to make a healthcheck from the commandline input diff --git a/cmd/podman/shared/create_cli.go b/cmd/podman/shared/create_cli.go index 4bfef8b62..08a40b206 100644 --- a/cmd/podman/shared/create_cli.go +++ b/cmd/podman/shared/create_cli.go @@ -133,7 +133,7 @@ func verifyContainerResources(config *cc.CreateConfig, update bool) ([]string, e if config.Resources.KernelMemory > 0 && config.Resources.KernelMemory < linuxMinMemory { return warnings, fmt.Errorf("minimum kernel memory limit allowed is 4MB") } - if config.Resources.DisableOomKiller == true && !sysInfo.OomKillDisable { + if config.Resources.DisableOomKiller && !sysInfo.OomKillDisable { // only produce warnings if the setting wasn't to *disable* the OOM Kill; no point // warning the caller if they already wanted the feature to be off warnings = addWarning(warnings, "Your kernel does not support OomKillDisable. OomKillDisable discarded.") diff --git a/cmd/podman/stats.go b/cmd/podman/stats.go index a1ec20b37..05e30f95f 100644 --- a/cmd/podman/stats.go +++ b/cmd/podman/stats.go @@ -101,9 +101,8 @@ func statsCmd(c *cliconfig.StatsValues) error { } var ctrs []*libpod.Container - var containerFunc func() ([]*libpod.Container, error) - containerFunc = runtime.GetRunningContainers + containerFunc := runtime.GetRunningContainers if len(c.InputArgs) > 0 { containerFunc = func() ([]*libpod.Container, error) { return runtime.GetContainersByList(c.InputArgs) } } else if latest { diff --git a/cmd/podman/tree.go b/cmd/podman/tree.go index c13dffd6e..904a0d375 100644 --- a/cmd/podman/tree.go +++ b/cmd/podman/tree.go @@ -107,7 +107,7 @@ func printImageChildren(layerMap map[string]*image.LayerInfo, layerID string, pr if !ok { return fmt.Errorf("lookup error: layerid %s, not found", layerID) } - fmt.Printf(prefix) + fmt.Print(prefix) //initialize intend with middleItem to reduce middleItem checks. intend := middleItem diff --git a/docs/podman-create.1.md b/docs/podman-create.1.md index 67fd653c1..87e18dbb9 100644 --- a/docs/podman-create.1.md +++ b/docs/podman-create.1.md @@ -727,6 +727,8 @@ The following example maps uids 0-2000 in the container to the uids 30000-31999 Ulimit options +You can pass `host` to copy the current configuration from the host. + **--user**, **-u**=*user* Sets the username or UID used and optionally the groupname or GID for the specified command. diff --git a/docs/podman-run.1.md b/docs/podman-run.1.md index 2a2d04b58..95499edd6 100644 --- a/docs/podman-run.1.md +++ b/docs/podman-run.1.md @@ -763,6 +763,8 @@ The example maps uids 0-2000 in the container to the uids 30000-31999 on the hos Ulimit options +You can pass `host` to copy the current configuration from the host. + **--user**, **-u**=*user* Sets the username or UID used and optionally the groupname or GID for the specified command. diff --git a/libpod/container.go b/libpod/container.go index a9b512de9..b71c0b2be 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -639,10 +639,7 @@ func (c *Container) HostsAdd() []string { // trigger some OCI hooks. func (c *Container) UserVolumes() []string { volumes := make([]string, 0, len(c.config.UserVolumes)) - for _, vol := range c.config.UserVolumes { - volumes = append(volumes, vol) - } - + volumes = append(volumes, c.config.UserVolumes...) return volumes } @@ -650,10 +647,7 @@ func (c *Container) UserVolumes() []string { // This is not added to the spec, but is instead used during image commit. func (c *Container) Entrypoint() []string { entrypoint := make([]string, 0, len(c.config.Entrypoint)) - for _, str := range c.config.Entrypoint { - entrypoint = append(entrypoint, str) - } - + entrypoint = append(entrypoint, c.config.Entrypoint...) return entrypoint } @@ -661,10 +655,7 @@ func (c *Container) Entrypoint() []string { // This is not added to the spec, but is instead used during image commit func (c *Container) Command() []string { command := make([]string, 0, len(c.config.Command)) - for _, str := range c.config.Command { - command = append(command, str) - } - + command = append(command, c.config.Command...) return command } diff --git a/libpod/container_api.go b/libpod/container_api.go index 3dd84b02c..3577b8e8c 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -277,7 +277,7 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user, workDir break } } - if found == true { + if found { sessionID = stringid.GenerateNonCryptoID() } } diff --git a/libpod/container_graph.go b/libpod/container_graph.go index 50dbdfbe4..5aa51bc2f 100644 --- a/libpod/container_graph.go +++ b/libpod/container_graph.go @@ -264,6 +264,4 @@ func startNode(ctx context.Context, node *containerNode, setError bool, ctrError for _, successor := range node.dependedOn { startNode(ctx, successor, ctrErrored, ctrErrors, ctrsVisited, restart) } - - return } diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go index 2de78254c..de0027414 100644 --- a/libpod/container_inspect.go +++ b/libpod/container_inspect.go @@ -454,9 +454,7 @@ func (c *Container) generateInspectContainerConfig(spec *spec.Spec) (*InspectCon if spec.Process != nil { ctrConfig.Tty = spec.Process.Terminal ctrConfig.Env = []string{} - for _, val := range spec.Process.Env { - ctrConfig.Env = append(ctrConfig.Env, val) - } + ctrConfig.Env = append(ctrConfig.Env, spec.Process.Env...) ctrConfig.WorkingDir = spec.Process.Cwd } @@ -466,9 +464,7 @@ func (c *Container) generateInspectContainerConfig(spec *spec.Spec) (*InspectCon // Leave empty is not explicitly overwritten by user if len(c.config.Command) != 0 { ctrConfig.Cmd = []string{} - for _, val := range c.config.Command { - ctrConfig.Cmd = append(ctrConfig.Cmd, val) - } + ctrConfig.Cmd = append(ctrConfig.Cmd, c.config.Command...) } // Leave empty if not explicitly overwritten by user diff --git a/libpod/container_internal.go b/libpod/container_internal.go index c409da96a..47b425c0a 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -815,34 +815,6 @@ func (c *Container) checkDependenciesRunning() ([]string, error) { return notRunning, nil } -// Check if a container's dependencies are running -// Returns a []string containing the IDs of dependencies that are not running -// Assumes depencies are already locked, and will be passed in -// Accepts a map[string]*Container containing, at a minimum, the locked -// dependency containers -// (This must be a map from container ID to container) -func (c *Container) checkDependenciesRunningLocked(depCtrs map[string]*Container) ([]string, error) { - deps := c.Dependencies() - notRunning := []string{} - - for _, dep := range deps { - depCtr, ok := depCtrs[dep] - if !ok { - return nil, errors.Wrapf(define.ErrNoSuchCtr, "container %s depends on container %s but it is not on containers passed to checkDependenciesRunning", c.ID(), dep) - } - - if err := c.syncContainer(); err != nil { - return nil, err - } - - if depCtr.state.State != define.ContainerStateRunning { - notRunning = append(notRunning, dep) - } - } - - return notRunning, nil -} - func (c *Container) completeNetworkSetup() error { netDisabled, err := c.NetworkDisabled() if err != nil { diff --git a/libpod/events/filters.go b/libpod/events/filters.go index 9a64082d1..b3c5eda6e 100644 --- a/libpod/events/filters.go +++ b/libpod/events/filters.go @@ -1,7 +1,6 @@ package events import ( - "fmt" "strings" "time" @@ -23,7 +22,7 @@ func generateEventFilter(filter, filterValue string) (func(e *Event) bool, error }, nil case "EVENT", "STATUS": return func(e *Event) bool { - return fmt.Sprintf("%s", e.Status) == filterValue + return string(e.Status) == filterValue }, nil case "IMAGE": return func(e *Event) bool { @@ -54,7 +53,7 @@ func generateEventFilter(filter, filterValue string) (func(e *Event) bool, error }, nil case "TYPE": return func(e *Event) bool { - return fmt.Sprintf("%s", e.Type) == filterValue + return string(e.Type) == filterValue }, nil } return nil, errors.Errorf("%s is an invalid filter", filter) diff --git a/libpod/events/nullout.go b/libpod/events/nullout.go index 7d811a9c7..b11afcf80 100644 --- a/libpod/events/nullout.go +++ b/libpod/events/nullout.go @@ -17,7 +17,6 @@ func (e EventToNull) Read(options ReadOptions) error { // NewNullEventer returns a new null eventer. You should only do this for // the purposes on internal libpod testing. func NewNullEventer() Eventer { - var e Eventer - e = EventToNull{} + e := EventToNull{} return e } diff --git a/libpod/healthcheck_linux.go b/libpod/healthcheck_linux.go index 53fb271d1..dca72430d 100644 --- a/libpod/healthcheck_linux.go +++ b/libpod/healthcheck_linux.go @@ -62,7 +62,7 @@ func (c *Container) createTimer() error { if rootless.IsRootless() { cmd = append(cmd, "--user") } - cmd = append(cmd, "--unit", fmt.Sprintf("%s", c.ID()), fmt.Sprintf("--on-unit-inactive=%s", c.HealthCheckConfig().Interval.String()), "--timer-property=AccuracySec=1s", podman, "healthcheck", "run", c.ID()) + cmd = append(cmd, "--unit", c.ID(), fmt.Sprintf("--on-unit-inactive=%s", c.HealthCheckConfig().Interval.String()), "--timer-property=AccuracySec=1s", podman, "healthcheck", "run", c.ID()) conn, err := getConnection() if err != nil { diff --git a/libpod/image/image.go b/libpod/image/image.go index 6509134ac..f9879b85b 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -461,7 +461,11 @@ func getImageDigest(ctx context.Context, src types.ImageReference, sc *types.Sys if err != nil { return "", err } - defer newImg.Close() + defer func() { + if err := newImg.Close(); err != nil { + logrus.Errorf("failed to close image: %q", err) + } + }() imageDigest := newImg.ConfigInfo().Digest if err = imageDigest.Validate(); err != nil { return "", errors.Wrapf(err, "error getting config info") @@ -513,7 +517,7 @@ func (i *Image) TagImage(tag string) error { if err := i.reloadImage(); err != nil { return err } - defer i.newImageEvent(events.Tag) + i.newImageEvent(events.Tag) return nil } @@ -538,7 +542,7 @@ func (i *Image) UntagImage(tag string) error { if err := i.reloadImage(); err != nil { return err } - defer i.newImageEvent(events.Untag) + i.newImageEvent(events.Untag) return nil } @@ -574,7 +578,11 @@ func (i *Image) PushImageToReference(ctx context.Context, dest types.ImageRefere if err != nil { return err } - defer policyContext.Destroy() + defer func() { + if err := policyContext.Destroy(); err != nil { + logrus.Errorf("failed to destroy policy context: %q", err) + } + }() // Look up the source image, expecting it to be in local storage src, err := is.Transport.ParseStoreReference(i.imageruntime.store, i.ID()) @@ -588,7 +596,7 @@ func (i *Image) PushImageToReference(ctx context.Context, dest types.ImageRefere if err != nil { return errors.Wrapf(err, "Error copying image to the remote destination") } - defer i.newImageEvent(events.Push) + i.newImageEvent(events.Push) return nil } @@ -984,11 +992,15 @@ func (ir *Runtime) Import(ctx context.Context, path, reference string, writer io if err != nil { return nil, err } - defer policyContext.Destroy() + defer func() { + if err := policyContext.Destroy(); err != nil { + logrus.Errorf("failed to destroy policy context: %q", err) + } + }() copyOptions := getCopyOptions(sc, writer, nil, nil, signingOptions, "", nil) dest, err := is.Transport.ParseStoreReference(ir.store, reference) if err != nil { - errors.Wrapf(err, "error getting image reference for %q", reference) + return nil, errors.Wrapf(err, "error getting image reference for %q", reference) } _, err = cp.Image(ctx, policyContext, dest, src, copyOptions) if err != nil { @@ -996,7 +1008,7 @@ func (ir *Runtime) Import(ctx context.Context, path, reference string, writer io } newImage, err := ir.NewFromLocal(reference) if err == nil { - defer newImage.newImageEvent(events.Import) + newImage.newImageEvent(events.Import) } return newImage, err } @@ -1339,7 +1351,7 @@ func (i *Image) Save(ctx context.Context, source, format, output string, moreTag if err := i.PushImageToReference(ctx, destRef, manifestType, "", "", writer, compress, SigningOptions{}, &DockerRegistryOptions{}, additionaltags); err != nil { return errors.Wrapf(err, "unable to save %q", source) } - defer i.newImageEvent(events.Save) + i.newImageEvent(events.Save) return nil } diff --git a/libpod/image/pull.go b/libpod/image/pull.go index ce8a19fbc..581beb538 100644 --- a/libpod/image/pull.go +++ b/libpod/image/pull.go @@ -249,7 +249,11 @@ func (ir *Runtime) doPullImage(ctx context.Context, sc *types.SystemContext, goa if err != nil { return nil, err } - defer policyContext.Destroy() + defer func() { + if err := policyContext.Destroy(); err != nil { + logrus.Errorf("failed to destroy policy context: %q", err) + } + }() systemRegistriesConfPath := registries.SystemRegistriesConfPath() @@ -304,7 +308,7 @@ func (ir *Runtime) doPullImage(ctx context.Context, sc *types.SystemContext, goa return nil, pullErrors } if len(images) > 0 { - defer ir.newImageEvent(events.Pull, images[0]) + ir.newImageEvent(events.Pull, images[0]) } return images, nil } diff --git a/libpod/kube.go b/libpod/kube.go index 409937010..b114cda72 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -1,7 +1,6 @@ package libpod import ( - "fmt" "math/rand" "os" "strconv" @@ -179,7 +178,7 @@ func addContainersAndVolumesToPodObject(containers []v1.Container, volumes []v1. labels["app"] = removeUnderscores(podName) om := v12.ObjectMeta{ // The name of the pod is container_name-libpod - Name: fmt.Sprintf("%s", removeUnderscores(podName)), + Name: removeUnderscores(podName), Labels: labels, // CreationTimestamp seems to be required, so adding it; in doing so, the timestamp // will reflect time this is run (not container create time) because the conversion diff --git a/libpod/logs/log.go b/libpod/logs/log.go index 488291cfe..0b1703567 100644 --- a/libpod/logs/log.go +++ b/libpod/logs/log.go @@ -156,8 +156,5 @@ func NewLogLine(line string) (*LogLine, error) { // Partial returns a bool if the log line is a partial log type func (l *LogLine) Partial() bool { - if l.ParseLogType == PartialLogType { - return true - } - return false + return l.ParseLogType == PartialLogType } diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 987c1fc5b..bef3f7739 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -5,7 +5,6 @@ package libpod import ( "crypto/rand" "fmt" - "github.com/containers/libpod/pkg/errorhandling" "net" "os" "os/exec" @@ -17,6 +16,7 @@ import ( cnitypes "github.com/containernetworking/cni/pkg/types/current" "github.com/containernetworking/plugins/pkg/ns" + "github.com/containers/libpod/pkg/errorhandling" "github.com/containers/libpod/pkg/firewall" "github.com/containers/libpod/pkg/netns" "github.com/containers/libpod/pkg/rootless" @@ -151,8 +151,8 @@ func checkSlirpFlags(path string) (bool, bool, error) { // Configure the network namespace for a rootless container func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) { - defer ctr.rootlessSlirpSyncR.Close() - defer ctr.rootlessSlirpSyncW.Close() + defer errorhandling.CloseQuiet(ctr.rootlessSlirpSyncR) + defer errorhandling.CloseQuiet(ctr.rootlessSlirpSyncW) path := r.config.NetworkCmdPath @@ -201,7 +201,11 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) { if err := cmd.Start(); err != nil { return errors.Wrapf(err, "failed to start slirp4netns process") } - defer cmd.Process.Release() + defer func() { + if err := cmd.Process.Release(); err != nil { + logrus.Errorf("unable to release comman process: %q", err) + } + }() b := make([]byte, 16) for { @@ -268,7 +272,11 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) { if err != nil { return errors.Wrapf(err, "cannot open connection to %s", apiSocket) } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + logrus.Errorf("unable to close connection: %q", err) + } + }() hostIP := i.HostIP if hostIP == "" { hostIP = "0.0.0.0" diff --git a/libpod/oci.go b/libpod/oci.go index 6aad79cdf..566cbd821 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -273,7 +273,9 @@ func (r *OCIRuntime) updateContainerStatus(ctr *Container, useRuntime bool) erro } return errors.Wrapf(err, "error getting container %s state. stderr/out: %s", ctr.ID(), out) } - defer cmd.Wait() + defer func() { + _ = cmd.Wait() + }() if err := errPipe.Close(); err != nil { return err diff --git a/libpod/oci_linux.go b/libpod/oci_linux.go index ca13d5517..044373ec5 100644 --- a/libpod/oci_linux.go +++ b/libpod/oci_linux.go @@ -124,7 +124,11 @@ func (r *OCIRuntime) createContainer(ctr *Container, cgroupParent string, restor if err = unix.Unshare(unix.CLONE_NEWNS); err != nil { return err } - defer unix.Setns(int(fd.Fd()), unix.CLONE_NEWNS) + defer func() { + if err := unix.Setns(int(fd.Fd()), unix.CLONE_NEWNS); err != nil { + logrus.Errorf("unable to clone new namespace: %q", err) + } + }() // don't spread our mounts around. We are setting only /sys to be slave // so that the cleanup process is still able to umount the storage and the @@ -376,7 +380,9 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, res errorhandling.CloseQuiet(childPipe) return err } - defer cmd.Wait() + defer func() { + _ = cmd.Wait() + }() // We don't need childPipe on the parent side if err := childPipe.Close(); err != nil { diff --git a/libpod/options.go b/libpod/options.go index 4f8bb42df..8d41764a9 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1152,10 +1152,7 @@ func WithUserVolumes(volumes []string) CtrCreateOption { } ctr.config.UserVolumes = make([]string, 0, len(volumes)) - for _, vol := range volumes { - ctr.config.UserVolumes = append(ctr.config.UserVolumes, vol) - } - + ctr.config.UserVolumes = append(ctr.config.UserVolumes, volumes...) return nil } } @@ -1172,10 +1169,7 @@ func WithEntrypoint(entrypoint []string) CtrCreateOption { } ctr.config.Entrypoint = make([]string, 0, len(entrypoint)) - for _, str := range entrypoint { - ctr.config.Entrypoint = append(ctr.config.Entrypoint, str) - } - + ctr.config.Entrypoint = append(ctr.config.Entrypoint, entrypoint...) return nil } } @@ -1192,10 +1186,7 @@ func WithCommand(command []string) CtrCreateOption { } ctr.config.Command = make([]string, 0, len(command)) - for _, str := range command { - ctr.config.Command = append(ctr.config.Command, str) - } - + ctr.config.Command = append(ctr.config.Command, command...) return nil } } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 760a07daf..4b3aeaa37 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -431,10 +431,12 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool, // If we're removing the pod, the container will be evicted // from the state elsewhere if !removePod { - if cleanupErr == nil { - cleanupErr = err - } else { - logrus.Errorf("removing container from pod: %v", err) + if err := r.state.RemoveContainerFromPod(pod, c); err != nil { + if cleanupErr == nil { + cleanupErr = err + } else { + logrus.Errorf("removing container from pod: %v", err) + } } } } else { diff --git a/pkg/adapter/checkpoint_restore.go b/pkg/adapter/checkpoint_restore.go index 97ba5ecf7..ec1464fb1 100644 --- a/pkg/adapter/checkpoint_restore.go +++ b/pkg/adapter/checkpoint_restore.go @@ -4,16 +4,19 @@ package adapter import ( "context" + "io" + "io/ioutil" + "os" + "path/filepath" + "github.com/containers/libpod/libpod" "github.com/containers/libpod/libpod/image" + "github.com/containers/libpod/pkg/errorhandling" "github.com/containers/storage/pkg/archive" jsoniter "github.com/json-iterator/go" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" - "io" - "io/ioutil" - "os" - "path/filepath" + "github.com/sirupsen/logrus" ) // Prefixing the checkpoint/restore related functions with 'cr' @@ -25,7 +28,7 @@ func crImportFromJSON(filePath string, v interface{}) error { if err != nil { return errors.Wrapf(err, "Failed to open container definition %s for restore", filePath) } - defer jsonFile.Close() + defer errorhandling.CloseQuiet(jsonFile) content, err := ioutil.ReadAll(jsonFile) if err != nil { @@ -48,7 +51,7 @@ func crImportCheckpoint(ctx context.Context, runtime *libpod.Runtime, input stri if err != nil { return nil, errors.Wrapf(err, "Failed to open checkpoint archive %s for import", input) } - defer archiveFile.Close() + defer errorhandling.CloseQuiet(archiveFile) options := &archive.TarOptions{ // Here we only need the files config.dump and spec.dump ExcludePatterns: []string{ @@ -62,15 +65,19 @@ func crImportCheckpoint(ctx context.Context, runtime *libpod.Runtime, input stri if err != nil { return nil, err } - defer os.RemoveAll(dir) + defer func() { + if err := os.RemoveAll(dir); err != nil { + logrus.Errorf("could not recursively remove %s: %q", dir, err) + } + }() err = archive.Untar(archiveFile, dir, options) if err != nil { return nil, errors.Wrapf(err, "Unpacking of checkpoint archive %s failed", input) } // Load spec.dump from temporary directory - spec := new(spec.Spec) - if err := crImportFromJSON(filepath.Join(dir, "spec.dump"), spec); err != nil { + dumpSpec := new(spec.Spec) + if err := crImportFromJSON(filepath.Join(dir, "spec.dump"), dumpSpec); err != nil { return nil, err } @@ -112,7 +119,7 @@ func crImportCheckpoint(ctx context.Context, runtime *libpod.Runtime, input stri } // Now create a new container from the just loaded information - container, err := runtime.RestoreContainer(ctx, spec, config) + container, err := runtime.RestoreContainer(ctx, dumpSpec, config) if err != nil { return nil, err } @@ -127,7 +134,7 @@ func crImportCheckpoint(ctx context.Context, runtime *libpod.Runtime, input stri return nil, errors.Errorf("Name of restored container (%s) does not match requested name (%s)", containerConfig.Name, ctrName) } - if newName == false { + if !newName { // Only check ID for a restore with the same name. // Using -n to request a new name for the restored container, will also create a new ID if containerConfig.ID != ctrID { diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go index 1cf9d686a..86e9c0266 100644 --- a/pkg/adapter/containers.go +++ b/pkg/adapter/containers.go @@ -213,8 +213,8 @@ func (r *LocalRuntime) RemoveContainers(ctx context.Context, cli *cliconfig.RmVa c := c pool.Add(shared.Job{ - c.ID(), - func() error { + ID: c.ID(), + Fn: func() error { err := r.RemoveContainer(ctx, c, cli.Force, cli.Volumes) if err != nil { logrus.Debugf("Failed to remove container %s: %s", c.ID(), err.Error()) diff --git a/pkg/adapter/pods.go b/pkg/adapter/pods.go index a28e1ab4b..b45b02d09 100644 --- a/pkg/adapter/pods.go +++ b/pkg/adapter/pods.go @@ -70,8 +70,9 @@ func (r *LocalRuntime) PrunePods(ctx context.Context, cli *cliconfig.PodPruneVal for _, p := range pods { p := p - pool.Add(shared.Job{p.ID(), - func() error { + pool.Add(shared.Job{ + ID: p.ID(), + Fn: func() error { err := r.Runtime.RemovePod(ctx, p, cli.Force, cli.Force) if err != nil { logrus.Debugf("Failed to remove pod %s: %s", p.ID(), err.Error()) diff --git a/pkg/adapter/runtime.go b/pkg/adapter/runtime.go index 8ef88f36b..e65f07898 100644 --- a/pkg/adapter/runtime.go +++ b/pkg/adapter/runtime.go @@ -359,9 +359,6 @@ func (r *LocalRuntime) Events(c *cliconfig.EventValues) error { if eventsError != nil { return eventsError } - if err != nil { - return errors.Wrapf(err, "unable to tail the events log") - } w := bufio.NewWriter(os.Stdout) for event := range eventChannel { if len(c.Format) > 0 { diff --git a/pkg/adapter/terminal_linux.go b/pkg/adapter/terminal_linux.go index be7dc0cb6..e3255ecb6 100644 --- a/pkg/adapter/terminal_linux.go +++ b/pkg/adapter/terminal_linux.go @@ -39,7 +39,11 @@ func StartAttachCtr(ctx context.Context, ctr *libpod.Container, stdout, stderr, return err } - defer restoreTerminal(oldTermState) + defer func() { + if err := restoreTerminal(oldTermState); err != nil { + logrus.Errorf("unable to restore terminal: %q", err) + } + }() } streams := new(libpod.AttachStreams) diff --git a/pkg/cgroups/blkio.go b/pkg/cgroups/blkio.go index 9c2a811d9..bacd4eb93 100644 --- a/pkg/cgroups/blkio.go +++ b/pkg/cgroups/blkio.go @@ -37,7 +37,7 @@ func (c *blkioHandler) Create(ctr *CgroupControl) (bool, error) { // Destroy the cgroup func (c *blkioHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(Blkio)) + return rmDirRecursively(ctr.getCgroupv1Path(Blkio)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/pkg/cgroups/cgroups.go b/pkg/cgroups/cgroups.go index 1dad45d7f..081db772f 100644 --- a/pkg/cgroups/cgroups.go +++ b/pkg/cgroups/cgroups.go @@ -328,6 +328,13 @@ func Load(path string) (*CgroupControl, error) { systemd: false, } if !cgroup2 { + controllers, err := getAvailableControllers(handlers, false) + if err != nil { + return nil, err + } + control.additionalControllers = controllers + } + if !cgroup2 { for name := range handlers { p := control.getCgroupv1Path(name) if _, err := os.Stat(p); err != nil { @@ -355,11 +362,40 @@ func (c *CgroupControl) Delete() error { return c.DeleteByPath(c.path) } +// rmDirRecursively delete recursively a cgroup directory. +// It differs from os.RemoveAll as it doesn't attempt to unlink files. +// On cgroupfs we are allowed only to rmdir empty directories. +func rmDirRecursively(path string) error { + if err := os.Remove(path); err == nil || os.IsNotExist(err) { + return nil + } + entries, err := ioutil.ReadDir(path) + if err != nil { + return errors.Wrapf(err, "read %s", path) + } + for _, i := range entries { + if i.IsDir() { + if err := rmDirRecursively(filepath.Join(path, i.Name())); err != nil { + return err + } + } + } + if os.Remove(path); err != nil { + if !os.IsNotExist(err) { + return errors.Wrapf(err, "remove %s", path) + } + } + return nil +} + // DeleteByPath deletes the specified cgroup path func (c *CgroupControl) DeleteByPath(path string) error { if c.systemd { return systemdDestroy(path) } + if c.cgroup2 { + return rmDirRecursively(filepath.Join(cgroupRoot, c.path)) + } var lastError error for _, h := range handlers { if err := h.Destroy(c); err != nil { @@ -368,8 +404,11 @@ func (c *CgroupControl) DeleteByPath(path string) error { } for _, ctr := range c.additionalControllers { + if ctr.symlink { + continue + } p := c.getCgroupv1Path(ctr.name) - if err := os.Remove(p); err != nil { + if err := rmDirRecursively(p); err != nil { lastError = errors.Wrapf(err, "remove %s", p) } } diff --git a/pkg/cgroups/cpu.go b/pkg/cgroups/cpu.go index 1c8610cc4..03677f1ef 100644 --- a/pkg/cgroups/cpu.go +++ b/pkg/cgroups/cpu.go @@ -68,7 +68,7 @@ func (c *cpuHandler) Create(ctr *CgroupControl) (bool, error) { // Destroy the cgroup func (c *cpuHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(CPU)) + return rmDirRecursively(ctr.getCgroupv1Path(CPU)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/pkg/cgroups/cpuset.go b/pkg/cgroups/cpuset.go index 25d2f7f76..46d0484f2 100644 --- a/pkg/cgroups/cpuset.go +++ b/pkg/cgroups/cpuset.go @@ -3,7 +3,6 @@ package cgroups import ( "fmt" "io/ioutil" - "os" "path/filepath" "strings" @@ -77,7 +76,7 @@ func (c *cpusetHandler) Create(ctr *CgroupControl) (bool, error) { // Destroy the cgroup func (c *cpusetHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(CPUset)) + return rmDirRecursively(ctr.getCgroupv1Path(CPUset)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/pkg/cgroups/memory.go b/pkg/cgroups/memory.go index 80e88d17c..b3991f7e3 100644 --- a/pkg/cgroups/memory.go +++ b/pkg/cgroups/memory.go @@ -2,7 +2,6 @@ package cgroups import ( "fmt" - "os" "path/filepath" spec "github.com/opencontainers/runtime-spec/specs-go" @@ -33,7 +32,7 @@ func (c *memHandler) Create(ctr *CgroupControl) (bool, error) { // Destroy the cgroup func (c *memHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(Memory)) + return rmDirRecursively(ctr.getCgroupv1Path(Memory)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/pkg/cgroups/pids.go b/pkg/cgroups/pids.go index ffbde100d..65b9b5b34 100644 --- a/pkg/cgroups/pids.go +++ b/pkg/cgroups/pids.go @@ -3,7 +3,6 @@ package cgroups import ( "fmt" "io/ioutil" - "os" "path/filepath" spec "github.com/opencontainers/runtime-spec/specs-go" @@ -40,7 +39,7 @@ func (c *pidHandler) Create(ctr *CgroupControl) (bool, error) { // Destroy the cgroup func (c *pidHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(Pids)) + return rmDirRecursively(ctr.getCgroupv1Path(Pids)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/pkg/netns/netns_linux.go b/pkg/netns/netns_linux.go index 4a515c72a..1d6fb873c 100644 --- a/pkg/netns/netns_linux.go +++ b/pkg/netns/netns_linux.go @@ -28,6 +28,7 @@ import ( "sync" "github.com/containernetworking/plugins/pkg/ns" + "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -90,7 +91,9 @@ func NewNS() (ns.NetNS, error) { // Ensure the mount point is cleaned up on errors; if the namespace // was successfully mounted this will have no effect because the file // is in-use - defer os.RemoveAll(nsPath) + defer func() { + _ = os.RemoveAll(nsPath) + }() var wg sync.WaitGroup wg.Add(1) @@ -109,7 +112,11 @@ func NewNS() (ns.NetNS, error) { if err != nil { return } - defer origNS.Close() + defer func() { + if err := origNS.Close(); err != nil { + logrus.Errorf("unable to close namespace: %q", err) + } + }() // create a new netns on the current thread err = unix.Unshare(unix.CLONE_NEWNET) @@ -118,7 +125,11 @@ func NewNS() (ns.NetNS, error) { } // Put this thread back to the orig ns, since it might get reused (pre go1.10) - defer origNS.Set() + defer func() { + if err := origNS.Set(); err != nil { + logrus.Errorf("unable to set namespace: %q", err) + } + }() // bind mount the netns from the current thread (from /proc) onto the // mount point. This causes the namespace to persist, even when there diff --git a/pkg/rootless/rootless_linux.go b/pkg/rootless/rootless_linux.go index d7c2de81d..99a0eb729 100644 --- a/pkg/rootless/rootless_linux.go +++ b/pkg/rootless/rootless_linux.go @@ -220,7 +220,11 @@ func EnableLinger() (string, error) { conn, err := dbus.SystemBus() if err == nil { - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + logrus.Errorf("unable to close dbus connection: %q", err) + } + }() } lingerEnabled := false @@ -310,13 +314,21 @@ func joinUserAndMountNS(pid uint, pausePid string) (bool, int, error) { if err != nil { return false, -1, err } - defer userNS.Close() + defer func() { + if err := userNS.Close(); err != nil { + logrus.Errorf("unable to close namespace: %q", err) + } + }() mountNS, err := os.Open(fmt.Sprintf("/proc/%d/ns/mnt", pid)) if err != nil { return false, -1, err } - defer userNS.Close() + defer func() { + if err := mountNS.Close(); err != nil { + logrus.Errorf("unable to close namespace: %q", err) + } + }() fd, err := getUserNSFirstChild(userNS.Fd()) if err != nil { @@ -364,7 +376,11 @@ func becomeRootInUserNS(pausePid, fileToRead string, fileOutput *os.File) (bool, defer errorhandling.CloseQuiet(r) defer errorhandling.CloseQuiet(w) - defer w.Write([]byte("0")) + defer func() { + if _, err := w.Write([]byte("0")); err != nil { + logrus.Errorf("failed to write byte 0: %q", err) + } + }() pidC := C.reexec_in_user_namespace(C.int(r.Fd()), cPausePid, cFileToRead, fileOutputFD) pid := int(pidC) diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index 5cc021bf5..d44beb3e4 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -20,6 +20,12 @@ import ( const cpuPeriod = 100000 +type systemUlimit struct { + name string + max uint64 + cur uint64 +} + func getAvailableGids() (int64, error) { idMap, err := user.ParseIDMapFile("/proc/self/gid_map") if err != nil { @@ -557,6 +563,20 @@ func addRlimits(config *CreateConfig, g *generate.Generator) error { ) for _, u := range config.Resources.Ulimit { + if u == "host" { + if len(config.Resources.Ulimit) != 1 { + return errors.New("ulimit can use host only once") + } + hostLimits, err := getHostRlimits() + if err != nil { + return err + } + for _, i := range hostLimits { + g.AddProcessRlimits(i.name, i.max, i.cur) + } + break + } + ul, err := units.ParseUlimit(u) if err != nil { return errors.Wrapf(err, "ulimit option %q requires name=SOFT:HARD, failed to be parsed", u) diff --git a/pkg/spec/spec_linux.go b/pkg/spec/spec_linux.go new file mode 100644 index 000000000..fcdfc5c4e --- /dev/null +++ b/pkg/spec/spec_linux.go @@ -0,0 +1,42 @@ +//+build linux + +package createconfig + +import ( + "syscall" + + "github.com/pkg/errors" +) + +type systemRlimit struct { + name string + value int +} + +var systemLimits = []systemRlimit{ + {"RLIMIT_AS", syscall.RLIMIT_AS}, + {"RLIMIT_CORE", syscall.RLIMIT_CORE}, + {"RLIMIT_CPU", syscall.RLIMIT_CPU}, + {"RLIMIT_DATA", syscall.RLIMIT_DATA}, + {"RLIMIT_FSIZE", syscall.RLIMIT_FSIZE}, + {"RLIMIT_NOFILE", syscall.RLIMIT_NOFILE}, + {"RLIMIT_STACK", syscall.RLIMIT_STACK}, +} + +func getHostRlimits() ([]systemUlimit, error) { + ret := []systemUlimit{} + for _, i := range systemLimits { + var l syscall.Rlimit + if err := syscall.Getrlimit(i.value, &l); err != nil { + return nil, errors.Wrapf(err, "cannot read limits for %s", i.name) + } + s := systemUlimit{ + name: i.name, + max: l.Max, + cur: l.Cur, + } + ret = append(ret, s) + } + return ret, nil + +} diff --git a/pkg/spec/spec_unsupported.go b/pkg/spec/spec_unsupported.go new file mode 100644 index 000000000..0f6a9acdc --- /dev/null +++ b/pkg/spec/spec_unsupported.go @@ -0,0 +1,7 @@ +//+build !linux + +package createconfig + +func getHostRlimits() ([]systemUlimit, error) { + return nil, nil +} diff --git a/test/README.md b/test/README.md index 4e61a0774..9bea679dc 100644 --- a/test/README.md +++ b/test/README.md @@ -110,19 +110,30 @@ make shell This will run a container and give you a shell and you can follow the instructions above. -# System test +# System tests System tests are used for testing the *podman* CLI in the context of a complete system. It requires that *podman*, all dependencies, and configurations are in place. The intention of system testing is to match as closely as possible with real-world user/developer use-cases and environments. The orchestration of the environments and tests is left to external tooling. -* `PodmanTestSystem`: System test *struct* as a composite of `PodmanTest`. It will not add any -options to the command by default. When you run system test, you can set GLOBALOPTIONS, -PODMAN_SUBCMD_OPTIONS or PODMAN_BINARY in ENV to run the test suite for different test matrices. +System tests use Bash Automated Testing System (`bats`) as a testing framework. +Install it via your package manager or get latest stable version +[directly from the repository](https://github.com/bats-core/bats-core), e.g.: -## Run system test -You can run the test with following command: +``` +mkdir -p ~/tools/bats +git clone --single-branch --branch v1.1.0 https://github.com/bats-core/bats-core.git ~/tools/bats +``` + +Make sure that `bats` binary (`bin/bats` in the repository) is in your `PATH`, if not - add it: + +``` +PATH=$PATH:~/tools/bats/bin +``` + +## Running system tests +When `bats` is installed and is in your `PATH`, you can run the test suite with following command: ``` make localsystem diff --git a/test/e2e/pod_rm_test.go b/test/e2e/pod_rm_test.go index 0d3f47f30..f0689f152 100644 --- a/test/e2e/pod_rm_test.go +++ b/test/e2e/pod_rm_test.go @@ -3,6 +3,8 @@ package integration import ( "fmt" "os" + "path/filepath" + "strings" . "github.com/containers/libpod/test/utils" . "github.com/onsi/ginkgo" @@ -40,6 +42,21 @@ var _ = Describe("Podman pod rm", func() { result := podmanTest.Podman([]string{"pod", "rm", podid}) result.WaitWithDefaultTimeout() Expect(result.ExitCode()).To(Equal(0)) + + // Also check that we don't leak cgroups + err := filepath.Walk("/sys/fs/cgroup", func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if !info.IsDir() { + Expect(err).To(BeNil()) + } + if strings.Contains(info.Name(), podid) { + return fmt.Errorf("leaking cgroup path %s", path) + } + return nil + }) + Expect(err).To(BeNil()) }) It("podman pod rm latest pod", func() { diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 623e08c2a..8c7830204 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -8,7 +8,9 @@ import ( "net" "os" "path/filepath" + "strconv" "strings" + "syscall" "time" . "github.com/containers/libpod/test/utils" @@ -266,6 +268,25 @@ var _ = Describe("Podman run", func() { Expect(session.OutputToString()).To(ContainSubstring("100")) }) + It("podman run limits host test", func() { + SkipIfRemote() + + var l syscall.Rlimit + + err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &l) + Expect(err).To(BeNil()) + + session := podmanTest.Podman([]string{"run", "--rm", "--ulimit", "host", fedoraMinimal, "ulimit", "-Hn"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + ulimitCtrStr := strings.TrimSpace(session.OutputToString()) + ulimitCtr, err := strconv.ParseUint(ulimitCtrStr, 10, 0) + Expect(err).To(BeNil()) + + Expect(ulimitCtr).Should(BeNumerically(">=", l.Max)) + }) + It("podman run with cidfile", func() { session := podmanTest.Podman([]string{"run", "--cidfile", tempdir + "cidfile", ALPINE, "ls"}) session.WaitWithDefaultTimeout() |