From 22c8270135620bbbeb988076378173e452ec04af Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Sat, 3 Oct 2020 06:56:42 -0400 Subject: fix podman container exists and diff for storage containers Current these commands only check if a container exists in libpod. With this fix, the commands will also check if they are in containers/storage. This allows users to look at differences within a buildah or CRI-O container. Currently buildah diff does not exists, so this helps out in that situation as well as in CRI-O since the cri does not implement a diff command. Signed-off-by: Daniel J Walsh --- pkg/api/handlers/libpod/containers.go | 13 +++++++++++-- pkg/domain/infra/abi/containers.go | 12 +++++++++--- 2 files changed, 20 insertions(+), 5 deletions(-) (limited to 'pkg') diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go index 7dde51102..8068ea1b8 100644 --- a/pkg/api/handlers/libpod/containers.go +++ b/pkg/api/handlers/libpod/containers.go @@ -11,6 +11,7 @@ import ( "github.com/containers/podman/v2/pkg/api/handlers/compat" "github.com/containers/podman/v2/pkg/api/handlers/utils" "github.com/containers/podman/v2/pkg/domain/entities" + "github.com/containers/podman/v2/pkg/domain/infra/abi" "github.com/containers/podman/v2/pkg/ps" "github.com/gorilla/schema" "github.com/pkg/errors" @@ -19,8 +20,12 @@ import ( func ContainerExists(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) + // Now use the ABI implementation to prevent us from having duplicate + // code. + containerEngine := abi.ContainerEngine{Libpod: runtime} + name := utils.GetName(r) - _, err := runtime.LookupContainer(name) + report, err := containerEngine.ContainerExists(r.Context(), name) if err != nil { if errors.Cause(err) == define.ErrNoSuchCtr { utils.ContainerNotFound(w, name, err) @@ -30,7 +35,11 @@ func ContainerExists(w http.ResponseWriter, r *http.Request) { return } - utils.WriteResponse(w, http.StatusNoContent, "") + if report.Value { + utils.WriteResponse(w, http.StatusNoContent, "") + } else { + utils.ContainerNotFound(w, name, define.ErrNoSuchCtr) + } } func ListContainers(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 614fd5fe0..191ad8eec 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -74,11 +74,17 @@ func getContainersByContext(all, latest bool, names []string, runtime *libpod.Ru return } -// TODO: Should return *entities.ContainerExistsReport, error +// ContainerExists returns whether the container exists in container storage func (ic *ContainerEngine) ContainerExists(ctx context.Context, nameOrID string) (*entities.BoolReport, error) { _, err := ic.Libpod.LookupContainer(nameOrID) - if err != nil && errors.Cause(err) != define.ErrNoSuchCtr { - return nil, err + if err != nil { + if errors.Cause(err) != define.ErrNoSuchCtr { + return nil, err + } + // Check if container exists in storage + if _, storageErr := ic.Libpod.StorageContainer(nameOrID); storageErr == nil { + err = nil + } } return &entities.BoolReport{Value: err == nil}, nil } -- cgit v1.2.3-54-g00ecf From db23e1261165bae3dda38bc1f3b6a11573e36417 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Wed, 7 Oct 2020 15:27:54 -0400 Subject: Add support for external container External containers are containers created outside of Podman. For example Buildah and CRI-O Containers. $ buildah from alpine alpine-working-container $ buildah run alpine-working-container touch /test $ podman container exists --external alpine-working-container $ podman container diff alpine-working-container C /etc A /test Added --external flag to refer to external containers, rather then --storage. Added --external for podman container exists and modified podman ps to use --external rather then --storage. It was felt that --storage would confuse the user into thinking about changing the storage driver or options. --storage is still supported through the use of aliases. Finally podman contianer diff, does not require the --external flag, since it there is little change of users making the mistake, and would just be a pain for the user to remember the flag. podman container exists --external is required because it could fool scripts that rely on the existance of a Podman container, and there is a potential for a partial deletion of a container, which could mess up existing users. Signed-off-by: Daniel J Walsh --- cmd/podman/containers/exists.go | 15 ++++- cmd/podman/containers/ps.go | 2 +- cmd/podman/diff.go | 5 +- cmd/podman/utils/alias.go | 2 + completions/bash/podman | 9 ++- docs/source/markdown/podman-container-exists.1.md | 12 +++- docs/source/markdown/podman-ps.1.md | 78 +++++++++++------------ pkg/api/handlers/libpod/containers.go | 19 +++++- pkg/bindings/containers/containers.go | 6 +- pkg/bindings/test/containers_test.go | 12 ++-- pkg/domain/entities/containers.go | 5 ++ pkg/domain/entities/engine_container.go | 2 +- pkg/domain/infra/abi/containers.go | 10 +-- pkg/domain/infra/tunnel/containers.go | 4 +- test/system/030-run.bats | 5 +- 15 files changed, 121 insertions(+), 65 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/containers/exists.go b/cmd/podman/containers/exists.go index 283f6df18..1d79b684d 100644 --- a/cmd/podman/containers/exists.go +++ b/cmd/podman/containers/exists.go @@ -12,10 +12,10 @@ var ( containerExistsDescription = `If the named container exists in local storage, podman container exists exits with 0, otherwise the exit code will be 1.` existsCommand = &cobra.Command{ - Use: "exists CONTAINER", + Use: "exists [flags] CONTAINER", Short: "Check if a container exists in local storage", Long: containerExistsDescription, - Example: `podman container exists containerID + Example: `podman container exists --external containerID podman container exists myctr || podman run --name myctr [etc...]`, RunE: exists, Args: cobra.ExactArgs(1), @@ -29,10 +29,19 @@ func init() { Command: existsCommand, Parent: containerCmd, }) + flags := existsCommand.Flags() + flags.Bool("external", false, "Check external storage containers as well as Podman containers") } func exists(cmd *cobra.Command, args []string) error { - response, err := registry.ContainerEngine().ContainerExists(context.Background(), args[0]) + external, err := cmd.Flags().GetBool("external") + if err != nil { + return err + } + options := entities.ContainerExistsOptions{ + External: external, + } + response, err := registry.ContainerEngine().ContainerExists(context.Background(), args[0], options) if err != nil { return err } diff --git a/cmd/podman/containers/ps.go b/cmd/podman/containers/ps.go index 65bfc97da..41d309f51 100644 --- a/cmd/podman/containers/ps.go +++ b/cmd/podman/containers/ps.go @@ -58,7 +58,7 @@ func init() { func listFlagSet(flags *pflag.FlagSet) { flags.BoolVarP(&listOpts.All, "all", "a", false, "Show all the containers, default is only running containers") flags.StringSliceVarP(&filters, "filter", "f", []string{}, "Filter output based on conditions given") - flags.BoolVar(&listOpts.Storage, "storage", false, "Show containers in storage not controlled by Podman") + flags.BoolVar(&listOpts.Storage, "external", false, "Show containers in storage not controlled by Podman") flags.StringVar(&listOpts.Format, "format", "", "Pretty-print containers to JSON or using a Go template") flags.IntVarP(&listOpts.Last, "last", "n", -1, "Print the n last created containers (all states)") flags.BoolVar(&listOpts.Namespace, "ns", false, "Display namespace information") diff --git a/cmd/podman/diff.go b/cmd/podman/diff.go index 51a04bf46..9d2236abe 100644 --- a/cmd/podman/diff.go +++ b/cmd/podman/diff.go @@ -48,7 +48,10 @@ func diff(cmd *cobra.Command, args []string) error { return containers.Diff(cmd, args, diffOpts) } - if found, err := registry.ContainerEngine().ContainerExists(registry.GetContext(), args[0]); err != nil { + options := entities.ContainerExistsOptions{ + External: true, + } + if found, err := registry.ContainerEngine().ContainerExists(registry.GetContext(), args[0], options); err != nil { return err } else if found.Value { return containers.Diff(cmd, args, diffOpts) diff --git a/cmd/podman/utils/alias.go b/cmd/podman/utils/alias.go index ff31e82ea..10b96fa98 100644 --- a/cmd/podman/utils/alias.go +++ b/cmd/podman/utils/alias.go @@ -21,6 +21,8 @@ func AliasFlags(f *pflag.FlagSet, name string) pflag.NormalizedName { name = "time" case "namespace": name = "ns" + case "storage": + name = "external" } return pflag.NormalizedName(name) } diff --git a/completions/bash/podman b/completions/bash/podman index 564d35f67..9ed64ce3a 100644 --- a/completions/bash/podman +++ b/completions/bash/podman @@ -2468,7 +2468,6 @@ _podman_rm() { -i --latest -l - --storage --volumes -v " @@ -2696,7 +2695,7 @@ _podman_ps() { --pod -p --quiet -q --size -s - --storage + --external --namespace --ns --sync " @@ -3168,7 +3167,11 @@ _podman_container_exists() { " local boolean_options=" - " + --external + -h + --help + " + case "$cur" in -*) COMPREPLY=($(compgen -W "$boolean_options $options_with_args" -- "$cur")) diff --git a/docs/source/markdown/podman-container-exists.1.md b/docs/source/markdown/podman-container-exists.1.md index b8be07cb9..3d151cfce 100644 --- a/docs/source/markdown/podman-container-exists.1.md +++ b/docs/source/markdown/podman-container-exists.1.md @@ -4,7 +4,7 @@ podman-container-exists - Check if a container exists in local storage ## SYNOPSIS -**podman container exists** *container* +**podman container exists** [*options*] *container* ## DESCRIPTION **podman container exists** checks if a container exists in local storage. The **ID** or **Name** @@ -14,6 +14,9 @@ was an issue accessing the local storage. ## OPTIONS +**--external**=*true|false* +Check for external containers as well as Podman containers. These external containers are generally created via other container technology such as Buildah or CRI-O. + **-h**, **--help** Print usage statement @@ -33,6 +36,13 @@ $ echo $? 1 ``` +Check if an container called `ubi8-working-container` created via Buildah exists in local storage (the container does not actually exist). +``` +$ podman container exists --external ubi8-working-container +$ echo $? +1 +``` + ## SEE ALSO podman(1) diff --git a/docs/source/markdown/podman-ps.1.md b/docs/source/markdown/podman-ps.1.md index 58d3358e5..90f147222 100644 --- a/docs/source/markdown/podman-ps.1.md +++ b/docs/source/markdown/podman-ps.1.md @@ -34,23 +34,33 @@ all the containers information. By default it lists: Show all the containers created by Podman, default is only running containers. -Note: Podman shares containers storage with other tools such as Buildah and CRI-O. In some cases these `external` containers might also exist in the same storage. Use the `--storage` option to see these external containers. External containers show the 'storage' status. +Note: Podman shares containers storage with other tools such as Buildah and CRI-O. In some cases these `external` containers might also exist in the same storage. Use the `--external` option to see these external containers. External containers show the 'storage' status. -**--pod**, **-p** - -Display the pods the containers are associated with - -**--storage** +**--external** Display external containers that are not controlled by Podman but are stored in containers storage. These external containers are generally created via other container technology such as Buildah or CRI-O and may depend on the same container images that Podman is also using. External containers are denoted with either a 'buildah' or 'storage' in the COMMAND and STATUS column of the ps output. Only used with the --all option. -**--no-trunc** +**--filter**, **-f** -Display the extended information +Filter what containers are shown in the output. +Multiple filters can be given with multiple uses of the --filter flag. +If multiple filters are given, only containers which match all of the given filters will be shown. +Results will be drawn from all containers, regardless of whether --all was given. -**--quiet**, **-q** +Valid filters are listed below: -Print the numeric IDs of the containers only +| **Filter** | **Description** | +| --------------- | -------------------------------------------------------------------------------- | +| id | [ID] Container's ID | +| name | [Name] Container's name | +| label | [Key] or [Key=Value] Label assigned to a container | +| exited | [Int] Container's exit code | +| status | [Status] Container's status: 'created', 'exited', 'paused', 'running', 'unknown' | +| ancestor | [ImageName] Image or descendant used to create container | +| before | [ID] or [Name] Containers created before this container | +| since | [ID] or [Name] Containers created since this container | +| volume | [VolumeName] or [MountpointDestination] Volume mounted in container | +| health | [Status] healthy or unhealthy | **--format**=*format* @@ -74,15 +84,9 @@ Valid placeholders for the Go template are listed below: | .Labels | All the labels assigned to the container | | .Mounts | Volumes mounted in the container | -**--sort** - -Sort by command, created, id, image, names, runningfor, size, or status", -Note: Choosing size will sort by size of rootFs, not alphabetically like the rest of the options -Default: created - -**--size**, **-s** +**--help**, **-h** -Display the total file size +Print usage statement **--last**, **-n** @@ -98,31 +102,27 @@ The latest option is not supported on the remote client. Display namespace information -**--filter**, **-f** +**--no-trunc** -Filter what containers are shown in the output. -Multiple filters can be given with multiple uses of the --filter flag. -If multiple filters are given, only containers which match all of the given filters will be shown. -Results will be drawn from all containers, regardless of whether --all was given. +Display the extended information -Valid filters are listed below: +**--pod**, **-p** -| **Filter** | **Description** | -| --------------- | -------------------------------------------------------------------------------- | -| id | [ID] Container's ID | -| name | [Name] Container's name | -| label | [Key] or [Key=Value] Label assigned to a container | -| exited | [Int] Container's exit code | -| status | [Status] Container's status: 'created', 'exited', 'paused', 'running', 'unknown' | -| ancestor | [ImageName] Image or descendant used to create container | -| before | [ID] or [Name] Containers created before this container | -| since | [ID] or [Name] Containers created since this container | -| volume | [VolumeName] or [MountpointDestination] Volume mounted in container | -| health | [Status] healthy or unhealthy | +Display the pods the containers are associated with -**--help**, **-h** +**--quiet**, **-q** -Print usage statement +Print the numeric IDs of the containers only + +**--sort** + +Sort by command, created, id, image, names, runningfor, size, or status", +Note: Choosing size will sort by size of rootFs, not alphabetically like the rest of the options +Default: created + +**--size**, **-s** + +Display the total file size **--sync** @@ -181,7 +181,7 @@ CONTAINER ID IMAGE COMMAND CREATED STATUS ``` ``` -$ podman ps --storage -a +$ podman ps --external -a CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES 69ed779d8ef9f redis:alpine "redis-server" 25 hours ago Created 6379/tcp k8s_container1_podsandbox1_redhat.test.crio_redhat-test-crio_1 38a8a78596f9 docker.io/library/busybox:latest buildah 2 hours ago storage busybox-working-container diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go index 8068ea1b8..7e6481321 100644 --- a/pkg/api/handlers/libpod/containers.go +++ b/pkg/api/handlers/libpod/containers.go @@ -19,13 +19,30 @@ import ( ) func ContainerExists(w http.ResponseWriter, r *http.Request) { + decoder := r.Context().Value("decoder").(*schema.Decoder) runtime := r.Context().Value("runtime").(*libpod.Runtime) // Now use the ABI implementation to prevent us from having duplicate // code. containerEngine := abi.ContainerEngine{Libpod: runtime} name := utils.GetName(r) - report, err := containerEngine.ContainerExists(r.Context(), name) + query := struct { + External bool `schema:"external"` + }{ + // override any golang type defaults + } + + if err := decoder.Decode(&query, r.URL.Query()); err != nil { + utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) + return + } + + options := entities.ContainerExistsOptions{ + External: query.External, + } + + report, err := containerEngine.ContainerExists(r.Context(), name, options) if err != nil { if errors.Cause(err) == define.ErrNoSuchCtr { utils.ContainerNotFound(w, name, err) diff --git a/pkg/bindings/containers/containers.go b/pkg/bindings/containers/containers.go index 708ad06cb..b5cd2128b 100644 --- a/pkg/bindings/containers/containers.go +++ b/pkg/bindings/containers/containers.go @@ -322,12 +322,14 @@ func Wait(ctx context.Context, nameOrID string, condition *define.ContainerStatu // Exists is a quick, light-weight way to determine if a given container // exists in local storage. The nameOrID can be a container name // or a partial/full ID. -func Exists(ctx context.Context, nameOrID string) (bool, error) { +func Exists(ctx context.Context, nameOrID string, external bool) (bool, error) { conn, err := bindings.GetClient(ctx) if err != nil { return false, err } - response, err := conn.DoRequest(nil, http.MethodGet, "/containers/%s/exists", nil, nil, nameOrID) + params := url.Values{} + params.Set("external", strconv.FormatBool(external)) + response, err := conn.DoRequest(nil, http.MethodGet, "/containers/%s/exists", params, nil, nameOrID) if err != nil { return false, err } diff --git a/pkg/bindings/test/containers_test.go b/pkg/bindings/test/containers_test.go index 408b4769d..0fb677768 100644 --- a/pkg/bindings/test/containers_test.go +++ b/pkg/bindings/test/containers_test.go @@ -405,7 +405,7 @@ var _ = Describe("Podman containers ", func() { It("podman bogus container does not exist in local storage", func() { // Bogus container existence check should fail - containerExists, err := containers.Exists(bt.conn, "foobar") + containerExists, err := containers.Exists(bt.conn, "foobar", false) Expect(err).To(BeNil()) Expect(containerExists).To(BeFalse()) }) @@ -415,7 +415,7 @@ var _ = Describe("Podman containers ", func() { var name = "top" _, err := bt.RunTopContainer(&name, bindings.PFalse, nil) Expect(err).To(BeNil()) - containerExists, err := containers.Exists(bt.conn, name) + containerExists, err := containers.Exists(bt.conn, name, false) Expect(err).To(BeNil()) Expect(containerExists).To(BeTrue()) }) @@ -425,7 +425,7 @@ var _ = Describe("Podman containers ", func() { var name = "top" cid, err := bt.RunTopContainer(&name, bindings.PFalse, nil) Expect(err).To(BeNil()) - containerExists, err := containers.Exists(bt.conn, cid) + containerExists, err := containers.Exists(bt.conn, cid, false) Expect(err).To(BeNil()) Expect(containerExists).To(BeTrue()) }) @@ -435,7 +435,7 @@ var _ = Describe("Podman containers ", func() { var name = "top" cid, err := bt.RunTopContainer(&name, bindings.PFalse, nil) Expect(err).To(BeNil()) - containerExists, err := containers.Exists(bt.conn, cid[0:12]) + containerExists, err := containers.Exists(bt.conn, cid[0:12], false) Expect(err).To(BeNil()) Expect(containerExists).To(BeTrue()) }) @@ -455,7 +455,7 @@ var _ = Describe("Podman containers ", func() { Expect(err).To(BeNil()) err = containers.Kill(bt.conn, name, "SIGINT") Expect(err).To(BeNil()) - _, err = containers.Exists(bt.conn, name) + _, err = containers.Exists(bt.conn, name, false) Expect(err).To(BeNil()) }) @@ -466,7 +466,7 @@ var _ = Describe("Podman containers ", func() { Expect(err).To(BeNil()) err = containers.Kill(bt.conn, cid, "SIGTERM") Expect(err).To(BeNil()) - _, err = containers.Exists(bt.conn, cid) + _, err = containers.Exists(bt.conn, cid, false) Expect(err).To(BeNil()) }) diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index 3b6dd106f..46b169284 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -246,6 +246,11 @@ type ExecOptions struct { WorkDir string } +// ContainerExistsOptions describes the cli values to check if a container exists +type ContainerExistsOptions struct { + External bool +} + // ContainerStartOptions describes the val from the // CLI needed to start a container type ContainerStartOptions struct { diff --git a/pkg/domain/entities/engine_container.go b/pkg/domain/entities/engine_container.go index 803a59932..a20d3b404 100644 --- a/pkg/domain/entities/engine_container.go +++ b/pkg/domain/entities/engine_container.go @@ -21,7 +21,7 @@ type ContainerEngine interface { ContainerDiff(ctx context.Context, nameOrID string, options DiffOptions) (*DiffReport, error) ContainerExec(ctx context.Context, nameOrID string, options ExecOptions, streams define.AttachStreams) (int, error) ContainerExecDetached(ctx context.Context, nameOrID string, options ExecOptions) (string, error) - ContainerExists(ctx context.Context, nameOrID string) (*BoolReport, error) + ContainerExists(ctx context.Context, nameOrID string, options ContainerExistsOptions) (*BoolReport, error) ContainerExport(ctx context.Context, nameOrID string, options ContainerExportOptions) error ContainerInit(ctx context.Context, namesOrIds []string, options ContainerInitOptions) ([]*ContainerInitReport, error) ContainerInspect(ctx context.Context, namesOrIds []string, options InspectOptions) ([]*ContainerInspectReport, []error, error) diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 191ad8eec..60dbbce6c 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -75,15 +75,17 @@ func getContainersByContext(all, latest bool, names []string, runtime *libpod.Ru } // ContainerExists returns whether the container exists in container storage -func (ic *ContainerEngine) ContainerExists(ctx context.Context, nameOrID string) (*entities.BoolReport, error) { +func (ic *ContainerEngine) ContainerExists(ctx context.Context, nameOrID string, options entities.ContainerExistsOptions) (*entities.BoolReport, error) { _, err := ic.Libpod.LookupContainer(nameOrID) if err != nil { if errors.Cause(err) != define.ErrNoSuchCtr { return nil, err } - // Check if container exists in storage - if _, storageErr := ic.Libpod.StorageContainer(nameOrID); storageErr == nil { - err = nil + if options.External { + // Check if container exists in storage + if _, storageErr := ic.Libpod.StorageContainer(nameOrID); storageErr == nil { + err = nil + } } } return &entities.BoolReport{Value: err == nil}, nil diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 1bb4e68ac..98e30c83d 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -29,8 +29,8 @@ func (ic *ContainerEngine) ContainerRunlabel(ctx context.Context, label string, return errors.New("not implemented") } -func (ic *ContainerEngine) ContainerExists(ctx context.Context, nameOrID string) (*entities.BoolReport, error) { - exists, err := containers.Exists(ic.ClientCxt, nameOrID) +func (ic *ContainerEngine) ContainerExists(ctx context.Context, nameOrID string, options entities.ContainerExistsOptions) (*entities.BoolReport, error) { + exists, err := containers.Exists(ic.ClientCxt, nameOrID, options.External) return &entities.BoolReport{Value: exists}, err } diff --git a/test/system/030-run.bats b/test/system/030-run.bats index a82743c70..f7c48da8d 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -471,8 +471,11 @@ json-file | f run buildah from $IMAGE cid="$output" + # exists should fail + run_podman 1 container exists $cid + # exists should succeed - run_podman container exists $cid + run_podman container exists --external $cid run buildah rm $cid } -- cgit v1.2.3-54-g00ecf