From 15da607d137a6b219d08b21f305de335f94d34d7 Mon Sep 17 00:00:00 2001 From: Anders F Björklund Date: Mon, 22 Mar 2021 22:09:16 +0100 Subject: Fix swapped dimensions from terminal.GetSize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Anders F Björklund Signed-off-by: Matthew Heon --- pkg/bindings/containers/attach.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/bindings/containers/attach.go b/pkg/bindings/containers/attach.go index f48b99a95..ecae22a1b 100644 --- a/pkg/bindings/containers/attach.go +++ b/pkg/bindings/containers/attach.go @@ -336,7 +336,7 @@ func attachHandleResize(ctx, winCtx context.Context, winChange chan os.Signal, i case <-winCtx.Done(): return case <-winChange: - h, w, err := terminal.GetSize(int(file.Fd())) + w, h, err := terminal.GetSize(int(file.Fd())) if err != nil { logrus.Warnf("failed to obtain TTY size: %v", err) } -- cgit v1.2.3-54-g00ecf From 505f43c084a710c36195b8fe979c2ccb673c6e0c Mon Sep 17 00:00:00 2001 From: Robb Manes Date: Tue, 16 Mar 2021 12:42:02 -0400 Subject: Add RequiresMountsFor= to systemd generate It is rare but possible that storage locations for the graphroot and the runroot are not mounted at boot time, and therefore might race when doing container operations. An example we've seen in the wild is that a slow tmpfs mount for the runroot would suddenly mount over /run, causing the container to lose all currently-running data, requiring a system refresh to get it back. This patch adds RequiresMountsFor= to the systemd.unit header to ensure the paths for both the graphroot and runroot are mounted prior to starting any generated unit files. Signed-off-by: Robb Manes --- docs/source/markdown/podman-generate-systemd.1.md | 9 +++- pkg/systemd/generate/common.go | 1 + pkg/systemd/generate/containers.go | 23 ++++++++++ pkg/systemd/generate/containers_test.go | 54 +++++++++++++++++++++++ pkg/systemd/generate/pods.go | 6 +++ pkg/systemd/generate/pods_test.go | 19 ++++++++ 6 files changed, 111 insertions(+), 1 deletion(-) (limited to 'pkg') diff --git a/docs/source/markdown/podman-generate-systemd.1.md b/docs/source/markdown/podman-generate-systemd.1.md index 00b13669a..43cb30f59 100644 --- a/docs/source/markdown/podman-generate-systemd.1.md +++ b/docs/source/markdown/podman-generate-systemd.1.md @@ -61,7 +61,7 @@ Set the systemd unit name separator between the name/id of a container/pod and t ### Generate and print a systemd unit file for a container -Generate a systemd unit file for a container running nginx with an *always* restart policy and 1-second timeout to stdout. +Generate a systemd unit file for a container running nginx with an *always* restart policy and 1-second timeout to stdout. Note that the **RequiresMountsFor** option in the **Unit** section ensures that the container storage for both the GraphRoot and the RunRoot are mounted prior to starting the service. For systems with container storage on disks like iSCSI or other remote block protocols, this ensures that Podman is not executed prior to any necessary storage operations coming online. ``` $ podman create --name nginx nginx:latest @@ -73,6 +73,9 @@ $ podman generate systemd --restart-policy=always -t 1 nginx [Unit] Description=Podman container-de1e3223b1b888bc02d0962dd6cb5855eb00734061013ffdd3479d225abacdc6.service Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/container/storage [Service] Restart=always @@ -101,6 +104,7 @@ Description=Podman container-busy_moser.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/container/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -140,6 +144,9 @@ Description=Podman pod-systemd-pod.service Documentation=man:podman-generate-systemd(1) Requires=container-amazing_chandrasekhar.service container-jolly_shtern.service Before=container-amazing_chandrasekhar.service container-jolly_shtern.service +Wants=network.target +After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/container/storage [Service] Restart=on-failure diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go index 94a6f4cb5..19d468403 100644 --- a/pkg/systemd/generate/common.go +++ b/pkg/systemd/generate/common.go @@ -36,6 +36,7 @@ Description=Podman {{{{.ServiceName}}}}.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor={{{{.GraphRoot}}}} {{{{.RunRoot}}}} ` // filterPodFlags removes --pod and --pod-id-file from the specified command. diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index 9343a5067..bc13a6116 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -71,6 +71,12 @@ type containerInfo struct { // If not nil, the container is part of the pod. We can use the // podInfo to extract the relevant data. Pod *podInfo + // Location of the GraphRoot for the container. Required for ensuring the + // volume has finished mounting when coming online at boot. + GraphRoot string + // Location of the RunRoot for the container. Required for ensuring the tmpfs + // or volume exists and is mounted when coming online at boot. + RunRoot string } const containerTemplate = headerTemplate + ` @@ -132,6 +138,21 @@ func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSyste nameOrID, serviceName := containerServiceName(ctr, options) + store := ctr.Runtime().GetStore() + if store == nil { + return nil, errors.Errorf("could not determine storage store for container") + } + + graphRoot := store.GraphRoot() + if graphRoot == "" { + return nil, errors.Errorf("could not lookup container's graphroot: got empty string") + } + + runRoot := store.RunRoot() + if runRoot == "" { + return nil, errors.Errorf("could not lookup container's runroot: got empty string") + } + info := containerInfo{ ServiceName: serviceName, ContainerNameOrID: nameOrID, @@ -140,6 +161,8 @@ func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSyste StopTimeout: timeout, GenerateTimestamp: true, CreateCommand: createCommand, + GraphRoot: graphRoot, + RunRoot: runRoot, } return &info, nil diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go index ebbbdb786..1359c1a37 100644 --- a/pkg/systemd/generate/containers_test.go +++ b/pkg/systemd/generate/containers_test.go @@ -48,6 +48,7 @@ Description=Podman container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -73,6 +74,7 @@ Description=Podman container-foobar.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -96,6 +98,7 @@ Description=Podman container-foobar.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage BindsTo=a.service b.service c.service pod.service After=a.service b.service c.service pod.service @@ -121,6 +124,7 @@ Description=Podman jadda-jadda.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -145,6 +149,7 @@ Description=Podman jadda-jadda.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -169,6 +174,7 @@ Description=Podman jadda-jadda.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -193,6 +199,7 @@ Description=Podman jadda-jadda.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -217,6 +224,7 @@ Description=Podman container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -242,6 +250,7 @@ Description=Podman jadda-jadda.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -270,6 +279,7 @@ Description=Podman jadda-jadda.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -294,6 +304,7 @@ Description=Podman jadda-jadda.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -318,6 +329,7 @@ Description=Podman jadda-jadda.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -342,6 +354,7 @@ Description=Podman jadda-jadda.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -366,6 +379,7 @@ Description=Podman jadda-jadda.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage [Service] Environment=PODMAN_SYSTEMD_UNIT=%n @@ -400,6 +414,8 @@ WantedBy=multi-user.target default.target StopTimeout: 22, PodmanVersion: "CI", EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodID, false, @@ -416,6 +432,8 @@ WantedBy=multi-user.target default.target StopTimeout: 22, PodmanVersion: "CI", EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodIDNoHeaderInfo, false, @@ -432,6 +450,8 @@ WantedBy=multi-user.target default.target StopTimeout: 10, PodmanVersion: "CI", EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodName, false, @@ -449,6 +469,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", BoundToServices: []string{"pod", "a", "b", "c"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodNameBoundTo, false, @@ -464,6 +486,8 @@ WantedBy=multi-user.target default.target StopTimeout: 10, PodmanVersion: "CI", EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, "", false, @@ -481,6 +505,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "container", "run", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN", "foo=arg \"with \" space"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodWithNameAndGeneric, true, @@ -498,6 +524,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodWithExplicitShortDetachParam, true, @@ -515,6 +543,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", Pod: &podInfo{ PodIDFile: "%t/pod-foobar.pod-id-file", }, @@ -535,6 +565,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "run", "--detach", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodNameNewDetach, true, @@ -552,6 +584,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "run", "awesome-image:latest"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodIDNew, true, @@ -569,6 +603,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "run", "--detach=true", "awesome-image:latest"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, genGoodNewDetach("--detach=true"), true, @@ -586,6 +622,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "run", "--detach=false", "awesome-image:latest"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, genGoodNewDetach("-d"), true, @@ -603,6 +641,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "run", "--name", "test", "-p", "80:80", "--detach=false", "awesome-image:latest", "somecmd", "--detach=false"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodNameNewDetachFalseWithCmd, true, @@ -620,6 +660,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "run", "--name", "test", "-p", "80:80", "--detach=false", "--detach=false", "awesome-image:latest", "somecmd", "--detach=false"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodNameNewDetachFalseWithCmd, true, @@ -637,6 +679,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "run", "-dti", "awesome-image:latest"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, genGoodNewDetach("-dti"), true, @@ -654,6 +698,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "run", "-tid", "awesome-image:latest"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, genGoodNewDetach("-tid"), true, @@ -671,6 +717,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "--events-backend", "none", "--runroot", "/root", "run", "awesome-image:latest"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodNewRootFlags, true, @@ -688,6 +736,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "container", "create", "awesome-image:latest"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodContainerCreate, true, @@ -705,6 +755,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "create", "--name", "test", "--log-driver=journald", "--log-opt=tag={{.Name}}", "awesome-image:latest"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodNewWithJournaldTag, true, @@ -722,6 +774,8 @@ WantedBy=multi-user.target default.target PodmanVersion: "CI", CreateCommand: []string{"I'll get stripped", "create", "--name", "test", "awesome-image:latest", "sh", "-c", "kill $$ && echo %\\"}, EnvVariable: define.EnvVariable, + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", }, goodNewWithSpecialChars, true, diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go index f96058d36..a76979ecf 100644 --- a/pkg/systemd/generate/pods.go +++ b/pkg/systemd/generate/pods.go @@ -73,6 +73,12 @@ type podInfo struct { ExecStopPost string // Removes autogenerated by Podman and timestamp if set to true GenerateNoHeader bool + // Location of the GraphRoot for the pod. Required for ensuring the + // volume has finished mounting when coming online at boot. + GraphRoot string + // Location of the RunRoot for the pod. Required for ensuring the tmpfs + // or volume exists and is mounted when coming online at boot. + RunRoot string } const podTemplate = headerTemplate + `Requires={{{{- range $index, $value := .RequiredServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}} diff --git a/pkg/systemd/generate/pods_test.go b/pkg/systemd/generate/pods_test.go index 50c8d4556..559f7365f 100644 --- a/pkg/systemd/generate/pods_test.go +++ b/pkg/systemd/generate/pods_test.go @@ -47,6 +47,7 @@ Description=Podman pod-123abc.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage Requires=container-1.service container-2.service Before=container-1.service container-2.service @@ -74,6 +75,7 @@ Description=Podman pod-123abc.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage Requires=container-1.service container-2.service Before=container-1.service container-2.service @@ -101,6 +103,7 @@ Description=Podman pod-123abc.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage Requires=container-1.service container-2.service Before=container-1.service container-2.service @@ -128,6 +131,7 @@ Description=Podman pod-123abc.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage Requires=container-1.service container-2.service Before=container-1.service container-2.service @@ -155,6 +159,7 @@ Description=Podman pod-123abc.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage Requires=container-1.service container-2.service Before=container-1.service container-2.service @@ -191,6 +196,8 @@ WantedBy=multi-user.target default.target PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 42, PodmanVersion: "CI", + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", RequiredServices: []string{"container-1", "container-2"}, CreateCommand: []string{"podman", "pod", "create", "--name", "foo", "bar=arg with space"}, }, @@ -208,6 +215,8 @@ WantedBy=multi-user.target default.target PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 42, PodmanVersion: "CI", + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", RequiredServices: []string{"container-1", "container-2"}, CreateCommand: []string{"podman", "pod", "create", "--name", "foo", "bar=arg with space"}, }, @@ -225,6 +234,8 @@ WantedBy=multi-user.target default.target PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 42, PodmanVersion: "CI", + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", RequiredServices: []string{"container-1", "container-2"}, CreateCommand: []string{"podman", "--events-backend", "none", "--runroot", "/root", "pod", "create", "--name", "foo", "bar=arg with space"}, }, @@ -242,6 +253,8 @@ WantedBy=multi-user.target default.target PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 10, PodmanVersion: "CI", + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", RequiredServices: []string{"container-1", "container-2"}, CreateCommand: []string{"podman", "pod", "create", "--name", "foo", "bar=arg with space"}, }, @@ -259,6 +272,8 @@ WantedBy=multi-user.target default.target PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 10, PodmanVersion: "CI", + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", RequiredServices: []string{"container-1", "container-2"}, CreateCommand: []string{"podman", "--events-backend", "none", "--runroot", "/root", "pod", "create", "--name", "foo", "bar=arg with space"}, }, @@ -276,6 +291,8 @@ WantedBy=multi-user.target default.target PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 10, PodmanVersion: "CI", + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", RequiredServices: []string{"container-1", "container-2"}, CreateCommand: []string{"podman", "pod", "create", "--name", "foo", "--replace=false"}, }, @@ -293,6 +310,8 @@ WantedBy=multi-user.target default.target PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 10, PodmanVersion: "CI", + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", RequiredServices: []string{"container-1", "container-2"}, CreateCommand: []string{"podman", "pod", "create", "--name", "foo", "--label", "key={{someval}}"}, }, -- cgit v1.2.3-54-g00ecf From b5573018073b22b923d88efc853f8f1c35ca8119 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Fri, 26 Mar 2021 10:54:33 -0400 Subject: Remove resize race condition Since podman-remote resize requests can come in at random times, this generates a real potential for race conditions. We should only be attempting to resize TTY on running containers, but the containers can go from running to stopped at any time, and returning an error to the caller is just causing noice. This change will basically ignore requests to resize terminals if the container is not running and return the caller to success. All other callers will still return failure. Fixes: https://github.com/containers/podman/issues/9831 Signed-off-by: Daniel J Walsh Signed-off-by: Matthew Heon --- pkg/api/handlers/compat/resize.go | 22 ++++++++++++++-------- pkg/api/server/register_containers.go | 5 +++++ pkg/api/server/register_exec.go | 5 +++++ pkg/bindings/containers/attach.go | 1 + pkg/bindings/containers/types.go | 5 +++-- pkg/bindings/containers/types_resizetty_options.go | 16 ++++++++++++++++ 6 files changed, 44 insertions(+), 10 deletions(-) (limited to 'pkg') diff --git a/pkg/api/handlers/compat/resize.go b/pkg/api/handlers/compat/resize.go index 1bf7ad460..23ed33a22 100644 --- a/pkg/api/handlers/compat/resize.go +++ b/pkg/api/handlers/compat/resize.go @@ -19,8 +19,9 @@ func ResizeTTY(w http.ResponseWriter, r *http.Request) { // /containers/{id}/resize query := struct { - Height uint16 `schema:"h"` - Width uint16 `schema:"w"` + Height uint16 `schema:"h"` + Width uint16 `schema:"w"` + IgnoreNotRunning bool `schema:"running"` }{ // override any golang type defaults } @@ -48,14 +49,17 @@ func ResizeTTY(w http.ResponseWriter, r *http.Request) { if state, err := ctnr.State(); err != nil { utils.InternalServerError(w, errors.Wrapf(err, "cannot obtain container state")) return - } else if state != define.ContainerStateRunning { + } else if state != define.ContainerStateRunning && !query.IgnoreNotRunning { utils.Error(w, "Container not running", http.StatusConflict, fmt.Errorf("container %q in wrong state %q", name, state.String())) return } + // If container is not running, ignore since this can be a race condition, and is expected if err := ctnr.AttachResize(sz); err != nil { - utils.InternalServerError(w, errors.Wrapf(err, "cannot resize container")) - return + if errors.Cause(err) != define.ErrCtrStateInvalid || !query.IgnoreNotRunning { + utils.InternalServerError(w, errors.Wrapf(err, "cannot resize container")) + return + } } // This is not a 204, even though we write nothing, for compatibility // reasons. @@ -70,14 +74,16 @@ func ResizeTTY(w http.ResponseWriter, r *http.Request) { if state, err := ctnr.State(); err != nil { utils.InternalServerError(w, errors.Wrapf(err, "cannot obtain session container state")) return - } else if state != define.ContainerStateRunning { + } else if state != define.ContainerStateRunning && !query.IgnoreNotRunning { utils.Error(w, "Container not running", http.StatusConflict, fmt.Errorf("container %q in wrong state %q", name, state.String())) return } if err := ctnr.ExecResize(name, sz); err != nil { - utils.InternalServerError(w, errors.Wrapf(err, "cannot resize session")) - return + if errors.Cause(err) != define.ErrCtrStateInvalid || !query.IgnoreNotRunning { + utils.InternalServerError(w, errors.Wrapf(err, "cannot resize session")) + return + } } // This is not a 204, even though we write nothing, for compatibility // reasons. diff --git a/pkg/api/server/register_containers.go b/pkg/api/server/register_containers.go index 31196aa9e..b379d52ce 100644 --- a/pkg/api/server/register_containers.go +++ b/pkg/api/server/register_containers.go @@ -587,6 +587,11 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error { // type: integer // required: false // description: Width to set for the terminal, in characters + // - in: query + // name: running + // type: boolean + // required: false + // description: Ignore containers not running errors // produces: // - application/json // responses: diff --git a/pkg/api/server/register_exec.go b/pkg/api/server/register_exec.go index 0f8c827c8..de437ab1a 100644 --- a/pkg/api/server/register_exec.go +++ b/pkg/api/server/register_exec.go @@ -136,6 +136,11 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error { // name: w // type: integer // description: Width of the TTY session in characters + // - in: query + // name: running + // type: boolean + // required: false + // description: Ignore containers not running errors // produces: // - application/json // responses: diff --git a/pkg/bindings/containers/attach.go b/pkg/bindings/containers/attach.go index ecae22a1b..fd8a7011d 100644 --- a/pkg/bindings/containers/attach.go +++ b/pkg/bindings/containers/attach.go @@ -307,6 +307,7 @@ func resizeTTY(ctx context.Context, endpoint string, height *int, width *int) er if width != nil { params.Set("w", strconv.Itoa(*width)) } + params.Set("running", "true") rsp, err := conn.DoRequest(nil, http.MethodPost, endpoint, params, nil) if err != nil { return err diff --git a/pkg/bindings/containers/types.go b/pkg/bindings/containers/types.go index 2d0e65bb4..f63e35bf1 100644 --- a/pkg/bindings/containers/types.go +++ b/pkg/bindings/containers/types.go @@ -210,8 +210,9 @@ type RenameOptions struct { // ResizeTTYOptions are optional options for resizing // container TTYs type ResizeTTYOptions struct { - Height *int - Width *int + Height *int + Width *int + Running *bool } //go:generate go run ../generator/generator.go ResizeExecTTYOptions diff --git a/pkg/bindings/containers/types_resizetty_options.go b/pkg/bindings/containers/types_resizetty_options.go index 68527b330..94946692f 100644 --- a/pkg/bindings/containers/types_resizetty_options.go +++ b/pkg/bindings/containers/types_resizetty_options.go @@ -51,3 +51,19 @@ func (o *ResizeTTYOptions) GetWidth() int { } return *o.Width } + +// WithRunning +func (o *ResizeTTYOptions) WithRunning(value bool) *ResizeTTYOptions { + v := &value + o.Running = v + return o +} + +// GetRunning +func (o *ResizeTTYOptions) GetRunning() bool { + var running bool + if o.Running == nil { + return running + } + return *o.Running +} -- cgit v1.2.3-54-g00ecf From d498022fdde96ef1eb028ea89bb68bf01ca495c5 Mon Sep 17 00:00:00 2001 From: Jakub Guzik Date: Fri, 26 Mar 2021 09:34:34 +0100 Subject: Fix list pods filter handling in libpod api Signed-off-by: Jakub Guzik --- pkg/api/handlers/libpod/pods.go | 12 ++++-------- test/apiv2/40-pods.at | 6 ++++++ 2 files changed, 10 insertions(+), 8 deletions(-) (limited to 'pkg') diff --git a/pkg/api/handlers/libpod/pods.go b/pkg/api/handlers/libpod/pods.go index 89a4a451f..4dc8740e2 100644 --- a/pkg/api/handlers/libpod/pods.go +++ b/pkg/api/handlers/libpod/pods.go @@ -44,13 +44,9 @@ func PodCreate(w http.ResponseWriter, r *http.Request) { func Pods(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) - decoder := r.Context().Value("decoder").(*schema.Decoder) - query := struct { - Filters map[string][]string `schema:"filters"` - }{ - // override any golang type defaults - } - if err := decoder.Decode(&query, r.URL.Query()); err != nil { + + filterMap, err := util.PrepareFilters(r) + if err != nil { utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return @@ -58,7 +54,7 @@ func Pods(w http.ResponseWriter, r *http.Request) { containerEngine := abi.ContainerEngine{Libpod: runtime} podPSOptions := entities.PodPSOptions{ - Filters: query.Filters, + Filters: *filterMap, } pods, err := containerEngine.PodPs(r.Context(), podPSOptions) if err != nil { diff --git a/test/apiv2/40-pods.at b/test/apiv2/40-pods.at index f3272c41e..94c72dbaa 100644 --- a/test/apiv2/40-pods.at +++ b/test/apiv2/40-pods.at @@ -116,6 +116,12 @@ t GET libpod/pods/foo/top?ps_args=args,pid 200 \ .Titles[0]="COMMAND" \ .Titles[1]="PID" \ +#api list pods sanity checks +t GET libpod/pods/json?filters='garb1age}' 400 \ + .cause="invalid character 'g' looking for beginning of value" +t GET libpod/pods/json?filters='{"label":["testl' 400 \ + .cause="unexpected end of JSON input" + # FIXME: I'm not sure what 'prune' is supposed to do; as of 20200224 it # just returns 200 (ok) with empty result list. #t POST libpod/pods/prune 200 # FIXME: 2020-02-24 returns 200 {} -- cgit v1.2.3-54-g00ecf From 3d1aaffdbb2fafae5f27467c59efc7ec7314c154 Mon Sep 17 00:00:00 2001 From: Jordan Christiansen Date: Fri, 26 Mar 2021 22:10:43 -0500 Subject: Add problematic volume name to kube play error messages When kube play fails to create a volume, it should say which volume had the problem so the user doesn't have to guess. For the following pod spec: apiVersion: v1 kind: Pod metadata: name: mypod spec: containers: - name: myfrontend image: nginx volumeMounts: - mountPath: "/var/www/html" name: mypd volumes: - name: mypd hostPath: path: /var/blah podman will now report: Error: failed to create volume "mypd": error in parsing HostPath in YAML: error checking path "/var/blah": stat /var/blah: no such file or directory Signed-off-by: Jordan Christiansen --- pkg/specgen/generate/kube/volume.go | 2 +- test/e2e/play_kube_test.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/specgen/generate/kube/volume.go b/pkg/specgen/generate/kube/volume.go index e4f3eb196..a8042b532 100644 --- a/pkg/specgen/generate/kube/volume.go +++ b/pkg/specgen/generate/kube/volume.go @@ -116,7 +116,7 @@ func InitializeVolumes(specVolumes []v1.Volume) (map[string]*KubeVolume, error) for _, specVolume := range specVolumes { volume, err := VolumeFromSource(specVolume.VolumeSource) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "failed to create volume %q", specVolume.Name) } volumes[specVolume.Name] = volume diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 9260d6cd2..433a8158c 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -1426,6 +1426,7 @@ spec: kube := podmanTest.Podman([]string{"play", "kube", kubeYaml}) kube.WaitWithDefaultTimeout() Expect(kube.ExitCode()).NotTo(Equal(0)) + Expect(kube.ErrorToString()).To(ContainSubstring(defaultVolName)) }) It("podman play kube test with empty HostPath type volume", func() { -- cgit v1.2.3-54-g00ecf From 62b49e176f5d10b8c7bfa6e293bb8b2a423db3ea Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Fri, 26 Mar 2021 07:02:58 -0400 Subject: [NO TESTS NEEDED] Use same function podman-remote rmi as podman Make sure fixes that go into local podman commands also work in podman-remote, by using the same function. Since this is just a rewrite of existing code, existing tests should handle it. Signed-off-by: Daniel J Walsh --- pkg/api/handlers/compat/images_remove.go | 35 +++++++++++++++++++------------- 1 file changed, 21 insertions(+), 14 deletions(-) (limited to 'pkg') diff --git a/pkg/api/handlers/compat/images_remove.go b/pkg/api/handlers/compat/images_remove.go index 874c57f16..e89558a86 100644 --- a/pkg/api/handlers/compat/images_remove.go +++ b/pkg/api/handlers/compat/images_remove.go @@ -4,7 +4,10 @@ import ( "net/http" "github.com/containers/podman/v3/libpod" + "github.com/containers/podman/v3/libpod/define" "github.com/containers/podman/v3/pkg/api/handlers/utils" + "github.com/containers/podman/v3/pkg/domain/entities" + "github.com/containers/podman/v3/pkg/domain/infra/abi" "github.com/gorilla/schema" "github.com/pkg/errors" ) @@ -30,28 +33,32 @@ func RemoveImage(w http.ResponseWriter, r *http.Request) { } } name := utils.GetName(r) - newImage, err := runtime.ImageRuntime().NewFromLocal(name) - if err != nil { - utils.ImageNotFound(w, name, errors.Wrapf(err, "failed to find image %s", name)) - return + imageEngine := abi.ImageEngine{Libpod: runtime} + + options := entities.ImageRemoveOptions{ + Force: query.Force, } + report, rmerrors := imageEngine.Remove(r.Context(), []string{name}, options) + if len(rmerrors) > 0 && rmerrors[0] != nil { + err := rmerrors[0] + if errors.Cause(err) == define.ErrNoSuchImage { + utils.ImageNotFound(w, name, errors.Wrapf(err, "failed to find image %s", name)) + return + } - results, err := runtime.RemoveImage(r.Context(), newImage, query.Force) - if err != nil { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err) return } - - response := make([]map[string]string, 0, len(results.Untagged)+1) - deleted := make(map[string]string, 1) - deleted["Deleted"] = results.Deleted - response = append(response, deleted) - - for _, u := range results.Untagged { + response := make([]map[string]string, 0, len(report.Untagged)+1) + for _, d := range report.Deleted { + deleted := make(map[string]string, 1) + deleted["Deleted"] = d + response = append(response, deleted) + } + for _, u := range report.Untagged { untagged := make(map[string]string, 1) untagged["Untagged"] = u response = append(response, untagged) } - utils.WriteResponse(w, http.StatusOK, response) } -- cgit v1.2.3-54-g00ecf From 1089f83a40af084d4a3c0a03f2279ff3f58c2b4c Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Fri, 26 Mar 2021 15:44:02 -0400 Subject: Fix podman build --pull-never Currently pull policy is set incorrectly when users set --pull-never. Also pull-policy is not being translated correctly when using podman-remote. Fixes: #9573 Signed-off-by: Daniel J Walsh Signed-off-by: Matthew Heon --- cmd/podman/images/build.go | 17 +++++++++++- contrib/cirrus/setup_environment.sh | 3 +++ pkg/api/handlers/compat/images_build.go | 14 +++++++--- pkg/bindings/images/build.go | 7 +++-- test/e2e/build/basicalpine/Containerfile | 2 +- test/e2e/build/basicalpine/Containerfile.path | 2 +- test/e2e/build/basicalpine/Containerfile.volume | 2 +- test/e2e/build/squash/Dockerfile.squash-a | 2 +- test/e2e/build/squash/Dockerfile.squash-c | 2 +- test/e2e/build_test.go | 35 +++++++++++++------------ test/e2e/containers_conf_test.go | 2 +- test/e2e/exec_test.go | 4 +-- test/e2e/prune_test.go | 7 ++--- test/e2e/ps_test.go | 4 +-- test/e2e/rmi_test.go | 19 ++++++++------ test/e2e/run_passwd_test.go | 9 ++++--- test/e2e/run_privileged_test.go | 16 +++++------ test/e2e/run_security_labels_test.go | 7 ++--- test/e2e/run_test.go | 12 ++++----- test/e2e/run_volume_test.go | 9 +++---- test/e2e/run_working_dir_test.go | 5 ++-- test/e2e/runlabel_test.go | 20 +++++++------- test/e2e/system_df_test.go | 2 +- test/system/070-build.bats | 27 +++++++++++++++++++ 24 files changed, 143 insertions(+), 86 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/images/build.go b/cmd/podman/images/build.go index f24e9a8f6..7db927e55 100644 --- a/cmd/podman/images/build.go +++ b/cmd/podman/images/build.go @@ -304,6 +304,21 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil return nil, err } + pullFlagsCount := 0 + if c.Flag("pull").Changed { + pullFlagsCount++ + } + if c.Flag("pull-always").Changed { + pullFlagsCount++ + } + if c.Flag("pull-never").Changed { + pullFlagsCount++ + } + + if pullFlagsCount > 1 { + return nil, errors.Errorf("can only set one of 'pull' or 'pull-always' or 'pull-never'") + } + pullPolicy := define.PullIfMissing if c.Flags().Changed("pull") && flags.Pull { pullPolicy = define.PullAlways @@ -313,7 +328,7 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil } if flags.PullNever { - pullPolicy = define.PullIfMissing + pullPolicy = define.PullNever } args := make(map[string]string) diff --git a/contrib/cirrus/setup_environment.sh b/contrib/cirrus/setup_environment.sh index 64ea3b7b4..429621024 100755 --- a/contrib/cirrus/setup_environment.sh +++ b/contrib/cirrus/setup_environment.sh @@ -36,6 +36,9 @@ do fi done +# Make sure cni network plugins directory exists +mkdir -p /etc/cni/net.d + # Ensure that all lower-level contexts and child-processes have # ready access to higher level orchestration (e.g Cirrus-CI) # variables. diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index 7751b91a7..9aa035212 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -13,6 +13,7 @@ import ( "time" "github.com/containers/buildah" + "github.com/containers/buildah/define" "github.com/containers/buildah/imagebuildah" "github.com/containers/buildah/util" "github.com/containers/image/v5/types" @@ -98,6 +99,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { OutputFormat string `schema:"outputformat"` Platform string `schema:"platform"` Pull bool `schema:"pull"` + PullPolicy string `schema:"pullpolicy"` Quiet bool `schema:"q"` Registry string `schema:"registry"` Rm bool `schema:"rm"` @@ -273,10 +275,14 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { jobs = query.Jobs } - pullPolicy := buildah.PullIfMissing - if _, found := r.URL.Query()["pull"]; found { - if query.Pull { - pullPolicy = buildah.PullAlways + pullPolicy := define.PullIfMissing + if utils.IsLibpodRequest(r) { + pullPolicy = define.PolicyMap[query.PullPolicy] + } else { + if _, found := r.URL.Query()["pull"]; found { + if query.Pull { + pullPolicy = define.PullAlways + } } } diff --git a/pkg/bindings/images/build.go b/pkg/bindings/images/build.go index 9d77883f9..17095b84b 100644 --- a/pkg/bindings/images/build.go +++ b/pkg/bindings/images/build.go @@ -15,7 +15,6 @@ import ( "strconv" "strings" - "github.com/containers/buildah" "github.com/containers/podman/v3/pkg/auth" "github.com/containers/podman/v3/pkg/bindings" "github.com/containers/podman/v3/pkg/domain/entities" @@ -175,9 +174,9 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO if len(platform) > 0 { params.Set("platform", platform) } - if options.PullPolicy == buildah.PullAlways { - params.Set("pull", "1") - } + + params.Set("pullpolicy", options.PullPolicy.String()) + if options.Quiet { params.Set("q", "1") } diff --git a/test/e2e/build/basicalpine/Containerfile b/test/e2e/build/basicalpine/Containerfile index 67fd37901..f6e07066c 100644 --- a/test/e2e/build/basicalpine/Containerfile +++ b/test/e2e/build/basicalpine/Containerfile @@ -1 +1 @@ -FROM alpine +FROM quay.io/libpod/alpine:latest diff --git a/test/e2e/build/basicalpine/Containerfile.path b/test/e2e/build/basicalpine/Containerfile.path index d2b03a6b8..a1349eb05 100644 --- a/test/e2e/build/basicalpine/Containerfile.path +++ b/test/e2e/build/basicalpine/Containerfile.path @@ -1,2 +1,2 @@ -FROM alpine +FROM quay.io/libpod/alpine:latest ENV PATH=/tmp:/bin:/usr/bin:/usr/sbin diff --git a/test/e2e/build/basicalpine/Containerfile.volume b/test/e2e/build/basicalpine/Containerfile.volume index 6a4fc8242..283d6376e 100644 --- a/test/e2e/build/basicalpine/Containerfile.volume +++ b/test/e2e/build/basicalpine/Containerfile.volume @@ -1,2 +1,2 @@ -FROM alpine +FROM quay.io/libpod/alpine:latest VOLUME "/volume0" diff --git a/test/e2e/build/squash/Dockerfile.squash-a b/test/e2e/build/squash/Dockerfile.squash-a index f084e093d..ade3eafce 100644 --- a/test/e2e/build/squash/Dockerfile.squash-a +++ b/test/e2e/build/squash/Dockerfile.squash-a @@ -1,2 +1,2 @@ -FROM busybox:latest +FROM quay.io/libpod/busybox:latest ADD alpinetest.tgz /data diff --git a/test/e2e/build/squash/Dockerfile.squash-c b/test/e2e/build/squash/Dockerfile.squash-c index df9c90388..63bf84e69 100644 --- a/test/e2e/build/squash/Dockerfile.squash-c +++ b/test/e2e/build/squash/Dockerfile.squash-c @@ -1,3 +1,3 @@ -FROM busybox:latest +FROM quay.io/libpod/busybox:latest ADD alpinetest.tgz /data RUN rm -rf /data diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go index e061a2154..95ed23313 100644 --- a/test/e2e/build_test.go +++ b/test/e2e/build_test.go @@ -1,6 +1,7 @@ package integration import ( + "fmt" "io/ioutil" "os" "path/filepath" @@ -150,7 +151,7 @@ var _ = Describe("Podman build", func() { } fakeFile := filepath.Join(os.TempDir(), "Containerfile") - Expect(ioutil.WriteFile(fakeFile, []byte("FROM alpine"), 0755)).To(BeNil()) + Expect(ioutil.WriteFile(fakeFile, []byte(fmt.Sprintf("FROM %s", ALPINE)), 0755)).To(BeNil()) targetFile := filepath.Join(targetPath, "Containerfile") Expect(ioutil.WriteFile(targetFile, []byte("FROM scratch"), 0755)).To(BeNil()) @@ -219,8 +220,8 @@ var _ = Describe("Podman build", func() { podmanTest.StartRemoteService() } podmanTest.AddImageToRWStore(ALPINE) - dockerfile := `FROM quay.io/libpod/alpine:latest -RUN printenv http_proxy` + dockerfile := fmt.Sprintf(`FROM %s +RUN printenv http_proxy`, ALPINE) dockerfilePath := filepath.Join(podmanTest.TempDir, "Dockerfile") err := ioutil.WriteFile(dockerfilePath, []byte(dockerfile), 0755) @@ -263,9 +264,9 @@ RUN printenv http_proxy` err = ioutil.WriteFile(dummyFile, []byte("dummy"), 0644) Expect(err).To(BeNil()) - containerfile := `FROM quay.io/libpod/alpine:latest + containerfile := fmt.Sprintf(`FROM %s ADD . /test -RUN find /test` +RUN find /test`, ALPINE) containerfilePath := filepath.Join(targetPath, "Containerfile") err = ioutil.WriteFile(containerfilePath, []byte(containerfile), 0644) @@ -307,7 +308,7 @@ RUN find /test` err = os.Mkdir(targetSubPath, 0755) Expect(err).To(BeNil()) - containerfile := `FROM quay.io/libpod/alpine:latest` + containerfile := fmt.Sprintf("FROM %s", ALPINE) containerfilePath := filepath.Join(targetSubPath, "Containerfile") err = ioutil.WriteFile(containerfilePath, []byte(containerfile), 0644) @@ -344,9 +345,9 @@ RUN find /test` targetPath, err := CreateTempDirInTempDir() Expect(err).To(BeNil()) - containerfile := `FROM quay.io/libpod/alpine:latest + containerfile := fmt.Sprintf(`FROM %s ADD . /testfilter/ -RUN find /testfilter/` +RUN find /testfilter/`, ALPINE) containerfilePath := filepath.Join(targetPath, "Containerfile") err = ioutil.WriteFile(containerfilePath, []byte(containerfile), 0644) @@ -428,10 +429,10 @@ subdir**` Expect(os.Chdir(targetSubPath)).To(BeNil()) Expect(os.Symlink("dummy", "dummy-symlink")).To(BeNil()) - containerfile := `FROM quay.io/libpod/alpine:latest + containerfile := fmt.Sprintf(`FROM %s ADD . /test RUN find /test -RUN [[ -L /test/dummy-symlink ]] && echo SYMLNKOK || echo SYMLNKERR` +RUN [[ -L /test/dummy-symlink ]] && echo SYMLNKOK || echo SYMLNKERR`, ALPINE) containerfilePath := filepath.Join(targetSubPath, "Containerfile") err = ioutil.WriteFile(containerfilePath, []byte(containerfile), 0644) @@ -475,14 +476,14 @@ RUN grep CapEff /proc/self/status` // When session := podmanTest.Podman([]string{ - "build", "--pull-never", "--cap-drop=all", "--cap-add=net_bind_service", "--add-host", "testhost:1.2.3.4", "--from", "alpine", targetPath, + "build", "--pull-never", "--cap-drop=all", "--cap-add=net_bind_service", "--add-host", "testhost:1.2.3.4", "--from", ALPINE, targetPath, }) session.WaitWithDefaultTimeout() // Then Expect(session.ExitCode()).To(Equal(0)) Expect(strings.Fields(session.OutputToString())). - To(ContainElement("alpine")) + To(ContainElement(ALPINE)) Expect(strings.Fields(session.OutputToString())). To(ContainElement("testhost")) Expect(strings.Fields(session.OutputToString())). @@ -494,7 +495,7 @@ RUN grep CapEff /proc/self/status` Expect(err).To(BeNil()) containerFile := filepath.Join(targetPath, "Containerfile") - Expect(ioutil.WriteFile(containerFile, []byte("FROM alpine"), 0755)).To(BeNil()) + Expect(ioutil.WriteFile(containerFile, []byte(fmt.Sprintf("FROM %s", ALPINE)), 0755)).To(BeNil()) defer func() { Expect(os.RemoveAll(containerFile)).To(BeNil()) @@ -502,7 +503,7 @@ RUN grep CapEff /proc/self/status` // When session := podmanTest.Podman([]string{ - "build", "--pull-never", "--isolation", "oci", "--arch", "arm64", targetPath, + "build", "--isolation", "oci", "--arch", "arm64", targetPath, }) session.WaitWithDefaultTimeout() // Then @@ -510,7 +511,7 @@ RUN grep CapEff /proc/self/status` // When session = podmanTest.Podman([]string{ - "build", "--pull-never", "--isolation", "chroot", "--arch", "arm64", targetPath, + "build", "--isolation", "chroot", "--arch", "arm64", targetPath, }) session.WaitWithDefaultTimeout() // Then @@ -534,8 +535,8 @@ RUN grep CapEff /proc/self/status` }) It("podman build --timestamp flag", func() { - containerfile := `FROM quay.io/libpod/alpine:latest -RUN echo hello` + containerfile := fmt.Sprintf(`FROM %s +RUN echo hello`, ALPINE) containerfilePath := filepath.Join(podmanTest.TempDir, "Containerfile") err := ioutil.WriteFile(containerfilePath, []byte(containerfile), 0755) diff --git a/test/e2e/containers_conf_test.go b/test/e2e/containers_conf_test.go index aa2380c51..803124de1 100644 --- a/test/e2e/containers_conf_test.go +++ b/test/e2e/containers_conf_test.go @@ -91,7 +91,7 @@ var _ = Describe("Podman run", func() { if IsRemote() { podmanTest.RestartRemoteService() } - session := podmanTest.Podman([]string{"run", "busybox", "grep", "CapEff", "/proc/self/status"}) + session := podmanTest.Podman([]string{"run", BB, "grep", "CapEff", "/proc/self/status"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) Expect(session.OutputToString()).ToNot(Equal(cap.OutputToString())) diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go index 2ffb5cd2e..df86eab15 100644 --- a/test/e2e/exec_test.go +++ b/test/e2e/exec_test.go @@ -475,10 +475,10 @@ var _ = Describe("Podman exec", func() { }) It("podman exec preserves container groups with --user and --group-add", func() { - dockerfile := `FROM registry.fedoraproject.org/fedora-minimal + dockerfile := fmt.Sprintf(`FROM %s RUN groupadd -g 4000 first RUN groupadd -g 4001 second -RUN useradd -u 1000 auser` +RUN useradd -u 1000 auser`, fedoraMinimal) imgName := "testimg" podmanTest.BuildImage(dockerfile, imgName, "false") diff --git a/test/e2e/prune_test.go b/test/e2e/prune_test.go index 73da77417..cbe38fc76 100644 --- a/test/e2e/prune_test.go +++ b/test/e2e/prune_test.go @@ -1,6 +1,7 @@ package integration import ( + "fmt" "os" . "github.com/containers/podman/v3/test/utils" @@ -8,11 +9,11 @@ import ( . "github.com/onsi/gomega" ) -var pruneImage = ` -FROM alpine:latest +var pruneImage = fmt.Sprintf(` +FROM %s LABEL RUN podman --version RUN apk update -RUN apk add bash` +RUN apk add bash`, ALPINE) var _ = Describe("Podman prune", func() { var ( diff --git a/test/e2e/ps_test.go b/test/e2e/ps_test.go index ac0910a83..37b6516c1 100644 --- a/test/e2e/ps_test.go +++ b/test/e2e/ps_test.go @@ -351,7 +351,7 @@ var _ = Describe("Podman ps", func() { }) It("podman --format by size", func() { - session := podmanTest.Podman([]string{"create", "busybox", "ls"}) + session := podmanTest.Podman([]string{"create", BB, "ls"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) @@ -366,7 +366,7 @@ var _ = Describe("Podman ps", func() { }) It("podman --sort by size", func() { - session := podmanTest.Podman([]string{"create", "busybox", "ls"}) + session := podmanTest.Podman([]string{"create", BB, "ls"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) diff --git a/test/e2e/rmi_test.go b/test/e2e/rmi_test.go index d8367d636..5e6d66d53 100644 --- a/test/e2e/rmi_test.go +++ b/test/e2e/rmi_test.go @@ -184,19 +184,20 @@ var _ = Describe("Podman rmi", func() { It("podman rmi with cached images", func() { podmanTest.AddImageToRWStore(cirros) - dockerfile := `FROM quay.io/libpod/cirros:latest + dockerfile := fmt.Sprintf(`FROM %s RUN mkdir hello RUN touch test.txt ENV foo=bar - ` + `, cirros) podmanTest.BuildImage(dockerfile, "test", "true") - dockerfile = `FROM quay.io/libpod/cirros:latest + dockerfile = fmt.Sprintf(`FROM %s RUN mkdir hello RUN touch test.txt RUN mkdir blah ENV foo=bar - ` + `, cirros) + podmanTest.BuildImage(dockerfile, "test2", "true") session := podmanTest.Podman([]string{"images", "-q", "-a"}) @@ -249,14 +250,15 @@ var _ = Describe("Podman rmi", func() { }) It("podman rmi -a with parent|child images", func() { - dockerfile := `FROM quay.io/libpod/cirros:latest AS base + podmanTest.AddImageToRWStore(cirros) + dockerfile := fmt.Sprintf(`FROM %s AS base RUN touch /1 ENV LOCAL=/1 RUN find $LOCAL FROM base RUN find $LOCAL -` +`, cirros) podmanTest.BuildImage(dockerfile, "test", "true") session := podmanTest.Podman([]string{"rmi", "-a"}) session.WaitWithDefaultTimeout() @@ -284,14 +286,15 @@ RUN find $LOCAL // a race, we may not hit the condition a 100 percent of times // but ocal reproducers hit it all the time. + podmanTest.AddImageToRWStore(cirros) var wg sync.WaitGroup buildAndRemove := func(i int) { defer GinkgoRecover() defer wg.Done() imageName := fmt.Sprintf("rmtest:%d", i) - containerfile := `FROM quay.io/libpod/cirros:latest -RUN ` + fmt.Sprintf("touch %s", imageName) + containerfile := fmt.Sprintf(`FROM %s +RUN touch %s`, cirros, imageName) podmanTest.BuildImage(containerfile, imageName, "false") session := podmanTest.Podman([]string{"rmi", "-f", imageName}) diff --git a/test/e2e/run_passwd_test.go b/test/e2e/run_passwd_test.go index 12b6c64df..0d5dd5f3b 100644 --- a/test/e2e/run_passwd_test.go +++ b/test/e2e/run_passwd_test.go @@ -1,6 +1,7 @@ package integration import ( + "fmt" "os" . "github.com/containers/podman/v3/test/utils" @@ -60,9 +61,9 @@ var _ = Describe("Podman run passwd", func() { }) It("podman can run container without /etc/passwd", func() { - dockerfile := `FROM alpine + dockerfile := fmt.Sprintf(`FROM %s RUN rm -f /etc/passwd /etc/shadow /etc/group -USER 1000` +USER 1000`, ALPINE) imgName := "testimg" podmanTest.BuildImage(dockerfile, imgName, "false") session := podmanTest.Podman([]string{"run", "--rm", imgName, "ls", "/etc/"}) @@ -113,9 +114,9 @@ USER 1000` }) It("podman run numeric group from image and no group file", func() { - dockerfile := `FROM alpine + dockerfile := fmt.Sprintf(`FROM %s RUN rm -f /etc/passwd /etc/shadow /etc/group -USER 1000` +USER 1000`, ALPINE) imgName := "testimg" podmanTest.BuildImage(dockerfile, imgName, "false") session := podmanTest.Podman([]string{"run", "--rm", imgName, "ls", "/etc/"}) diff --git a/test/e2e/run_privileged_test.go b/test/e2e/run_privileged_test.go index 33b3b85c5..0bf68e20b 100644 --- a/test/e2e/run_privileged_test.go +++ b/test/e2e/run_privileged_test.go @@ -59,7 +59,7 @@ var _ = Describe("Podman privileged container tests", func() { }) It("podman privileged make sure sys is mounted rw", func() { - session := podmanTest.Podman([]string{"run", "--privileged", "busybox", "mount"}) + session := podmanTest.Podman([]string{"run", "--privileged", BB, "mount"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) ok, lines := session.GrepString("sysfs") @@ -71,7 +71,7 @@ var _ = Describe("Podman privileged container tests", func() { hostCap := SystemExec("awk", []string{"/^CapEff/ { print $2 }", "/proc/self/status"}) Expect(hostCap.ExitCode()).To(Equal(0)) - session := podmanTest.Podman([]string{"run", "--privileged", "busybox", "awk", "/^CapEff/ { print $2 }", "/proc/self/status"}) + session := podmanTest.Podman([]string{"run", "--privileged", BB, "awk", "/^CapEff/ { print $2 }", "/proc/self/status"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) @@ -83,7 +83,7 @@ var _ = Describe("Podman privileged container tests", func() { hostCap := SystemExec("awk", []string{"/^CapEff/ { print $2 }", "/proc/self/status"}) Expect(hostCap.ExitCode()).To(Equal(0)) - session := podmanTest.Podman([]string{"run", "--cap-add", "all", "busybox", "awk", "/^CapEff/ { print $2 }", "/proc/self/status"}) + session := podmanTest.Podman([]string{"run", "--cap-add", "all", BB, "awk", "/^CapEff/ { print $2 }", "/proc/self/status"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) @@ -95,7 +95,7 @@ var _ = Describe("Podman privileged container tests", func() { hostCap := SystemExec("awk", []string{"/^CapEff/ { print $2 }", "/proc/self/status"}) Expect(hostCap.ExitCode()).To(Equal(0)) - session := podmanTest.Podman([]string{"run", "--user=bin", "--cap-add", "all", "busybox", "awk", "/^CapEff/ { print $2 }", "/proc/self/status"}) + session := podmanTest.Podman([]string{"run", "--user=bin", "--cap-add", "all", BB, "awk", "/^CapEff/ { print $2 }", "/proc/self/status"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) @@ -103,7 +103,7 @@ var _ = Describe("Podman privileged container tests", func() { }) It("podman cap-drop CapEff", func() { - session := podmanTest.Podman([]string{"run", "--cap-drop", "all", "busybox", "grep", "CapEff", "/proc/self/status"}) + session := podmanTest.Podman([]string{"run", "--cap-drop", "all", BB, "grep", "CapEff", "/proc/self/status"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) capEff := strings.Split(session.OutputToString(), " ") @@ -120,7 +120,7 @@ var _ = Describe("Podman privileged container tests", func() { }) It("podman non-privileged should have very few devices", func() { - session := podmanTest.Podman([]string{"run", "-t", "busybox", "ls", "-l", "/dev"}) + session := podmanTest.Podman([]string{"run", "-t", BB, "ls", "-l", "/dev"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) Expect(len(session.OutputToStringArray())).To(Equal(17)) @@ -147,12 +147,12 @@ var _ = Describe("Podman privileged container tests", func() { Skip("Can't determine NoNewPrivs") } - session := podmanTest.Podman([]string{"run", "busybox", "grep", "NoNewPrivs", "/proc/self/status"}) + session := podmanTest.Podman([]string{"run", BB, "grep", "NoNewPrivs", "/proc/self/status"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) privs := strings.Split(session.OutputToString(), ":") - session = podmanTest.Podman([]string{"run", "--security-opt", "no-new-privileges", "busybox", "grep", "NoNewPrivs", "/proc/self/status"}) + session = podmanTest.Podman([]string{"run", "--security-opt", "no-new-privileges", BB, "grep", "NoNewPrivs", "/proc/self/status"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) diff --git a/test/e2e/run_security_labels_test.go b/test/e2e/run_security_labels_test.go index a2e0b2aab..b714df323 100644 --- a/test/e2e/run_security_labels_test.go +++ b/test/e2e/run_security_labels_test.go @@ -1,6 +1,7 @@ package integration import ( + "fmt" "os" "strings" @@ -128,9 +129,9 @@ var _ = Describe("Podman generate kube", func() { It("podman container runlabel (podman --version)", func() { SkipIfRemote("runlabel not supported on podman-remote") - PodmanDockerfile := ` -FROM alpine:latest -LABEL io.containers.capabilities=chown,kill` + PodmanDockerfile := fmt.Sprintf(` +FROM %s +LABEL io.containers.capabilities=chown,kill`, ALPINE) image := "podman-caps:podman" podmanTest.BuildImage(PodmanDockerfile, image, "false") diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 4e5106731..53a304aec 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -489,8 +489,8 @@ var _ = Describe("Podman run", func() { if IsRemote() { podmanTest.RestartRemoteService() } - dockerfile := `FROM busybox -USER bin` + dockerfile := fmt.Sprintf(`FROM %s +USER bin`, BB) podmanTest.BuildImage(dockerfile, "test", "false") session := podmanTest.Podman([]string{"run", "--rm", "--user", "bin", "test", "grep", "CapBnd", "/proc/self/status"}) session.WaitWithDefaultTimeout() @@ -898,10 +898,10 @@ USER bin` session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) - dockerfile := `FROM busybox + dockerfile := fmt.Sprintf(`FROM %s RUN mkdir -p /myvol/data && chown -R mail.0 /myvol VOLUME ["/myvol/data"] -USER mail` +USER mail`, BB) podmanTest.BuildImage(dockerfile, "test", "false") session = podmanTest.Podman([]string{"run", "--rm", "test", "ls", "-al", "/myvol/data"}) @@ -1478,8 +1478,8 @@ USER mail` It("podman run makes workdir from image", func() { // BuildImage does not seem to work remote - dockerfile := `FROM busybox -WORKDIR /madethis` + dockerfile := fmt.Sprintf(`FROM %s +WORKDIR /madethis`, BB) podmanTest.BuildImage(dockerfile, "test", "false") session := podmanTest.Podman([]string{"run", "--rm", "test", "pwd"}) session.WaitWithDefaultTimeout() diff --git a/test/e2e/run_volume_test.go b/test/e2e/run_volume_test.go index 454dfdc83..a0407804f 100644 --- a/test/e2e/run_volume_test.go +++ b/test/e2e/run_volume_test.go @@ -308,9 +308,9 @@ var _ = Describe("Podman run with volumes", func() { It("podman named volume copyup symlink", func() { imgName := "testimg" - dockerfile := `FROM alpine + dockerfile := fmt.Sprintf(`FROM %s RUN touch /testfile -RUN sh -c "cd /etc/apk && ln -s ../../testfile"` +RUN sh -c "cd /etc/apk && ln -s ../../testfile"`, ALPINE) podmanTest.BuildImage(dockerfile, imgName, "false") baselineSession := podmanTest.Podman([]string{"run", "--rm", "-t", "-i", imgName, "ls", "/etc/apk/"}) @@ -479,9 +479,8 @@ RUN sh -c "cd /etc/apk && ln -s ../../testfile"` It("Podman mount over image volume with trailing /", func() { image := "podman-volume-test:trailing" - dockerfile := ` -FROM alpine:latest -VOLUME /test/` + dockerfile := fmt.Sprintf(`FROM %s +VOLUME /test/`, ALPINE) podmanTest.BuildImage(dockerfile, image, "false") ctrName := "testCtr" diff --git a/test/e2e/run_working_dir_test.go b/test/e2e/run_working_dir_test.go index 2d16cdc18..de0f55134 100644 --- a/test/e2e/run_working_dir_test.go +++ b/test/e2e/run_working_dir_test.go @@ -1,6 +1,7 @@ package integration import ( + "fmt" "os" . "github.com/containers/podman/v3/test/utils" @@ -46,9 +47,9 @@ var _ = Describe("Podman run", func() { }) It("podman run a container on an image with a workdir", func() { - dockerfile := `FROM alpine + dockerfile := fmt.Sprintf(`FROM %s RUN mkdir -p /home/foobar /etc/foobar; chown bin:bin /etc/foobar -WORKDIR /etc/foobar` +WORKDIR /etc/foobar`, ALPINE) podmanTest.BuildImage(dockerfile, "test", "false") session := podmanTest.Podman([]string{"run", "test", "pwd"}) diff --git a/test/e2e/runlabel_test.go b/test/e2e/runlabel_test.go index 2eec15c62..54fa7e2f6 100644 --- a/test/e2e/runlabel_test.go +++ b/test/e2e/runlabel_test.go @@ -1,6 +1,7 @@ package integration import ( + "fmt" "os" . "github.com/containers/podman/v3/test/utils" @@ -8,18 +9,17 @@ import ( . "github.com/onsi/gomega" ) -var PodmanDockerfile = ` -FROM alpine:latest -LABEL RUN podman --version` +var PodmanDockerfile = fmt.Sprintf(` +FROM %s +LABEL RUN podman --version`, ALPINE) -var LsDockerfile = ` -FROM alpine:latest -LABEL RUN ls -la` +var LsDockerfile = fmt.Sprintf(` +FROM %s +LABEL RUN ls -la`, ALPINE) -var GlobalDockerfile = ` -FROM alpine:latest -LABEL RUN echo \$GLOBAL_OPTS -` +var GlobalDockerfile = fmt.Sprintf(` +FROM %s +LABEL RUN echo \$GLOBAL_OPTS`, ALPINE) var _ = Describe("podman container runlabel", func() { var ( diff --git a/test/e2e/system_df_test.go b/test/e2e/system_df_test.go index 9daf3f8f9..9aee85ca3 100644 --- a/test/e2e/system_df_test.go +++ b/test/e2e/system_df_test.go @@ -44,7 +44,7 @@ var _ = Describe("podman system df", func() { session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) - session = podmanTest.Podman([]string{"create", "-v", "data:/data", "--name", "container1", "busybox"}) + session = podmanTest.Podman([]string{"create", "-v", "data:/data", "--name", "container1", BB}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) diff --git a/test/system/070-build.bats b/test/system/070-build.bats index 8f6cdb46b..e5b68a0d8 100644 --- a/test/system/070-build.bats +++ b/test/system/070-build.bats @@ -668,6 +668,33 @@ EOF run_podman image prune -f } +@test "podman build --pull-never" { + local tmpdir=$PODMAN_TMPDIR/build-test + mkdir -p $tmpdir + + # First, confirm that --pull-never is a NOP if image exists locally + local random_string=$(random_string 15) + + cat >$tmpdir/Containerfile <$tmpdir/Containerfile < Date: Sun, 28 Mar 2021 16:26:31 +0200 Subject: podman generate systemd --new do not duplicate params podman generate systemd --new inserts extra idfile arguments. The generated unit can break when the user did provide their own idfile arguments as they overwrite the arguments added by generate systemd. This also happens when a user tries to generate the systemd unit on a container already create with a --new unit. This should now create a identical unit. The solution is to remove all user provided idfile arguments. This commit also ensures that we do not remove arguments that are part off the containers entrypoint. Fixes #9776 Signed-off-by: Paul Holzinger --- pkg/systemd/generate/common.go | 36 ++++++-- pkg/systemd/generate/common_test.go | 147 +++++++++++++++++++++++++++++--- pkg/systemd/generate/containers.go | 21 ++--- pkg/systemd/generate/containers_test.go | 91 ++++++++++++++++++++ pkg/systemd/generate/pods.go | 8 +- pkg/systemd/generate/pods_test.go | 19 +++++ 6 files changed, 290 insertions(+), 32 deletions(-) (limited to 'pkg') diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go index 19d468403..eafd45528 100644 --- a/pkg/systemd/generate/common.go +++ b/pkg/systemd/generate/common.go @@ -39,20 +39,46 @@ After=network-online.target RequiresMountsFor={{{{.GraphRoot}}}} {{{{.RunRoot}}}} ` -// filterPodFlags removes --pod and --pod-id-file from the specified command. -func filterPodFlags(command []string) []string { +// filterPodFlags removes --pod, --pod-id-file and --infra-conmon-pidfile from the specified command. +// argCount is the number of last arguments which should not be filtered, e.g. the container entrypoint. +func filterPodFlags(command []string, argCount int) []string { processed := []string{} - for i := 0; i < len(command); i++ { + for i := 0; i < len(command)-argCount; i++ { s := command[i] - if s == "--pod" || s == "--pod-id-file" { + if s == "--pod" || s == "--pod-id-file" || s == "--infra-conmon-pidfile" { i++ continue } - if strings.HasPrefix(s, "--pod=") || strings.HasPrefix(s, "--pod-id-file=") { + if strings.HasPrefix(s, "--pod=") || + strings.HasPrefix(s, "--pod-id-file=") || + strings.HasPrefix(s, "--infra-conmon-pidfile=") { continue } processed = append(processed, s) } + processed = append(processed, command[len(command)-argCount:]...) + return processed +} + +// filterCommonContainerFlags removes --conmon-pidfile, --cidfile and --cgroups from the specified command. +// argCount is the number of last arguments which should not be filtered, e.g. the container entrypoint. +func filterCommonContainerFlags(command []string, argCount int) []string { + processed := []string{} + for i := 0; i < len(command)-argCount; i++ { + s := command[i] + + switch { + case s == "--conmon-pidfile", s == "--cidfile", s == "--cgroups": + i++ + continue + case strings.HasPrefix(s, "--conmon-pidfile="), + strings.HasPrefix(s, "--cidfile="), + strings.HasPrefix(s, "--cgroups="): + continue + } + processed = append(processed, s) + } + processed = append(processed, command[len(command)-argCount:]...) return processed } diff --git a/pkg/systemd/generate/common_test.go b/pkg/systemd/generate/common_test.go index 3787e461e..30e758127 100644 --- a/pkg/systemd/generate/common_test.go +++ b/pkg/systemd/generate/common_test.go @@ -1,7 +1,6 @@ package generate import ( - "strings" "testing" "github.com/stretchr/testify/assert" @@ -9,22 +8,144 @@ import ( func TestFilterPodFlags(t *testing.T) { tests := []struct { - input []string + input []string + output []string + argCount int }{ - {[]string{"podman", "pod", "create"}}, - {[]string{"podman", "pod", "create", "--name", "foo"}}, - {[]string{"podman", "pod", "create", "--pod-id-file", "foo"}}, - {[]string{"podman", "pod", "create", "--pod-id-file=foo"}}, - {[]string{"podman", "run", "--pod", "foo"}}, - {[]string{"podman", "run", "--pod=foo"}}, + { + []string{"podman", "pod", "create"}, + []string{"podman", "pod", "create"}, + 0, + }, + { + []string{"podman", "pod", "create", "--name", "foo"}, + []string{"podman", "pod", "create", "--name", "foo"}, + 0, + }, + { + []string{"podman", "pod", "create", "--pod-id-file", "foo"}, + []string{"podman", "pod", "create"}, + 0, + }, + { + []string{"podman", "pod", "create", "--pod-id-file=foo"}, + []string{"podman", "pod", "create"}, + 0, + }, + { + []string{"podman", "pod", "create", "--pod-id-file", "foo", "--infra-conmon-pidfile", "foo"}, + []string{"podman", "pod", "create"}, + 0, + }, + { + []string{"podman", "pod", "create", "--pod-id-file", "foo", "--infra-conmon-pidfile=foo"}, + []string{"podman", "pod", "create"}, + 0, + }, + { + []string{"podman", "run", "--pod", "foo"}, + []string{"podman", "run"}, + 0, + }, + { + []string{"podman", "run", "--pod=foo"}, + []string{"podman", "run"}, + 0, + }, + { + []string{"podman", "run", "--pod=foo", "fedora", "podman", "run", "--pod=test", "alpine"}, + []string{"podman", "run", "fedora", "podman", "run", "--pod=test", "alpine"}, + 5, + }, + { + []string{"podman", "run", "--pod", "foo", "fedora", "podman", "run", "--pod", "test", "alpine"}, + []string{"podman", "run", "fedora", "podman", "run", "--pod", "test", "alpine"}, + 6, + }, + { + []string{"podman", "run", "--pod-id-file=foo", "fedora", "podman", "run", "--pod-id-file=test", "alpine"}, + []string{"podman", "run", "fedora", "podman", "run", "--pod-id-file=test", "alpine"}, + 5, + }, + { + []string{"podman", "run", "--pod-id-file", "foo", "fedora", "podman", "run", "--pod-id-file", "test", "alpine"}, + []string{"podman", "run", "fedora", "podman", "run", "--pod-id-file", "test", "alpine"}, + 6, + }, + } + + for _, test := range tests { + processed := filterPodFlags(test.input, test.argCount) + assert.Equal(t, test.output, processed) + } +} + +func TestFilterCommonContainerFlags(t *testing.T) { + tests := []struct { + input []string + output []string + argCount int + }{ + { + []string{"podman", "run", "alpine"}, + []string{"podman", "run", "alpine"}, + 1, + }, + { + []string{"podman", "run", "--conmon-pidfile", "foo", "alpine"}, + []string{"podman", "run", "alpine"}, + 1, + }, + { + []string{"podman", "run", "--conmon-pidfile=foo", "alpine"}, + []string{"podman", "run", "alpine"}, + 1, + }, + { + []string{"podman", "run", "--cidfile", "foo", "alpine"}, + []string{"podman", "run", "alpine"}, + 1, + }, + { + []string{"podman", "run", "--cidfile=foo", "alpine"}, + []string{"podman", "run", "alpine"}, + 1, + }, + { + []string{"podman", "run", "--cgroups", "foo", "alpine"}, + []string{"podman", "run", "alpine"}, + 1, + }, + { + []string{"podman", "run", "--cgroups=foo", "alpine"}, + []string{"podman", "run", "alpine"}, + 1, + }, + { + []string{"podman", "run", "--cgroups", "foo", "--conmon-pidfile", "foo", "--cidfile", "foo", "alpine"}, + []string{"podman", "run", "alpine"}, + 1, + }, + { + []string{"podman", "run", "--cgroups=foo", "--conmon-pidfile=foo", "--cidfile=foo", "alpine"}, + []string{"podman", "run", "alpine"}, + 1, + }, + { + []string{"podman", "run", "--cgroups", "foo", "--conmon-pidfile", "foo", "--cidfile", "foo", "alpine", "--cgroups", "foo", "--conmon-pidfile", "foo", "--cidfile", "foo"}, + []string{"podman", "run", "alpine", "--cgroups", "foo", "--conmon-pidfile", "foo", "--cidfile", "foo"}, + 7, + }, + { + []string{"podman", "run", "--cgroups=foo", "--conmon-pidfile=foo", "--cidfile=foo", "alpine", "--cgroups=foo", "--conmon-pidfile=foo", "--cidfile=foo"}, + []string{"podman", "run", "alpine", "--cgroups=foo", "--conmon-pidfile=foo", "--cidfile=foo"}, + 4, + }, } for _, test := range tests { - processed := filterPodFlags(test.input) - for _, s := range processed { - assert.False(t, strings.HasPrefix(s, "--pod-id-file")) - assert.False(t, strings.HasPrefix(s, "--pod")) - } + processed := filterCommonContainerFlags(test.input, test.argCount) + assert.Equal(t, test.output, processed) } } diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index bc13a6116..e06655a8d 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -238,13 +238,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst "--cidfile", "{{{{.ContainerIDFile}}}}", "--cgroups=no-conmon", ) - // If the container is in a pod, make sure that the - // --pod-id-file is set correctly. - if info.Pod != nil { - podFlags := []string{"--pod-id-file", "{{{{.Pod.PodIDFile}}}}"} - startCommand = append(startCommand, podFlags...) - info.CreateCommand = filterPodFlags(info.CreateCommand) - } + remainingCmd := info.CreateCommand[index:] // Presence check for certain flags/options. fs := pflag.NewFlagSet("args", pflag.ContinueOnError) @@ -254,7 +248,16 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst fs.BoolP("detach", "d", false, "") fs.String("name", "", "") fs.Bool("replace", false, "") - fs.Parse(info.CreateCommand[index:]) + fs.Parse(remainingCmd) + + remainingCmd = filterCommonContainerFlags(remainingCmd, fs.NArg()) + // If the container is in a pod, make sure that the + // --pod-id-file is set correctly. + if info.Pod != nil { + podFlags := []string{"--pod-id-file", "{{{{.Pod.PodIDFile}}}}"} + startCommand = append(startCommand, podFlags...) + remainingCmd = filterPodFlags(remainingCmd, fs.NArg()) + } hasDetachParam, err := fs.GetBool("detach") if err != nil { @@ -266,8 +269,6 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst return "", err } - remainingCmd := info.CreateCommand[index:] - if !hasDetachParam { // Enforce detaching // diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go index 1359c1a37..899ba6bfa 100644 --- a/pkg/systemd/generate/containers_test.go +++ b/pkg/systemd/generate/containers_test.go @@ -392,6 +392,56 @@ ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id PIDFile=%t/jadda-jadda.pid Type=forking +[Install] +WantedBy=multi-user.target default.target +` + + goodNewWithIDFiles := `# jadda-jadda.service +# autogenerated by Podman CI + +[Unit] +Description=Podman jadda-jadda.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +TimeoutStopSec=70 +ExecStartPre=/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon -d awesome-image:latest podman run --cgroups=foo --conmon-pidfile=foo --cidfile=foo alpine +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 10 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id +PIDFile=%t/jadda-jadda.pid +Type=forking + +[Install] +WantedBy=multi-user.target default.target +` + + goodNewWithPodIDFiles := `# jadda-jadda.service +# autogenerated by Podman CI + +[Unit] +Description=Podman jadda-jadda.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +TimeoutStopSec=70 +ExecStartPre=/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon --pod-id-file %t/pod-foobar.pod-id-file -d awesome-image:latest podman run --cgroups=foo --conmon-pidfile=foo --cidfile=foo --pod-id-file /tmp/pod-foobar.pod-id-file alpine +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 10 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id +PIDFile=%t/jadda-jadda.pid +Type=forking + [Install] WantedBy=multi-user.target default.target ` @@ -782,6 +832,47 @@ WantedBy=multi-user.target default.target false, false, }, + {"good with ID files", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", + CreateCommand: []string{"I'll get stripped", "create", "--cgroups=foo", "--conmon-pidfile=foo", "--cidfile=foo", "awesome-image:latest", "podman", "run", "--cgroups=foo", "--conmon-pidfile=foo", "--cidfile=foo", "alpine"}, + EnvVariable: define.EnvVariable, + }, + goodNewWithIDFiles, + true, + false, + false, + }, + {"good with pod ID files", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", + CreateCommand: []string{"I'll get stripped", "create", "--cgroups=foo", "--conmon-pidfile=foo", "--cidfile=foo", "--pod", "test", "awesome-image:latest", "podman", "run", "--cgroups=foo", "--conmon-pidfile=foo", "--cidfile=foo", "--pod-id-file", "/tmp/pod-foobar.pod-id-file", "alpine"}, + EnvVariable: define.EnvVariable, + Pod: &podInfo{ + PodIDFile: "%t/pod-foobar.pod-id-file", + }, + }, + goodNewWithPodIDFiles, + true, + false, + false, + }, } for _, tt := range tests { test := tt diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go index a76979ecf..1b92649e8 100644 --- a/pkg/systemd/generate/pods.go +++ b/pkg/systemd/generate/pods.go @@ -279,16 +279,16 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions) } podRootArgs = info.CreateCommand[1 : podCreateIndex-1] info.RootFlags = strings.Join(escapeSystemdArguments(podRootArgs), " ") - podCreateArgs = filterPodFlags(info.CreateCommand[podCreateIndex+1:]) + podCreateArgs = filterPodFlags(info.CreateCommand[podCreateIndex+1:], 0) } // We're hard-coding the first five arguments and append the // CreateCommand with a stripped command and subcommand. startCommand := []string{info.Executable} startCommand = append(startCommand, podRootArgs...) startCommand = append(startCommand, - []string{"pod", "create", - "--infra-conmon-pidfile", "{{{{.PIDFile}}}}", - "--pod-id-file", "{{{{.PodIDFile}}}}"}...) + "pod", "create", + "--infra-conmon-pidfile", "{{{{.PIDFile}}}}", + "--pod-id-file", "{{{{.PodIDFile}}}}") // Presence check for certain flags/options. fs := pflag.NewFlagSet("args", pflag.ContinueOnError) diff --git a/pkg/systemd/generate/pods_test.go b/pkg/systemd/generate/pods_test.go index 559f7365f..0e4d92c50 100644 --- a/pkg/systemd/generate/pods_test.go +++ b/pkg/systemd/generate/pods_test.go @@ -320,6 +320,25 @@ WantedBy=multi-user.target default.target false, false, }, + {"pod --new with ID files", + podInfo{ + Executable: "/usr/bin/podman", + ServiceName: "pod-123abc", + InfraNameOrID: "jadda-jadda-infra", + RestartPolicy: "on-failure", + PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", + RequiredServices: []string{"container-1", "container-2"}, + CreateCommand: []string{"podman", "pod", "create", "--infra-conmon-pidfile", "/tmp/pod-123abc.pid", "--pod-id-file", "/tmp/pod-123abc.pod-id", "--name", "foo", "bar=arg with space"}, + }, + podGoodNamedNew, + true, + false, + false, + }, } for _, tt := range tests { -- cgit v1.2.3-54-g00ecf From 573ed9220b9b295eacdd017dc87bbb592043cff4 Mon Sep 17 00:00:00 2001 From: Jakub Guzik Date: Thu, 25 Mar 2021 01:32:00 +0100 Subject: Fix filters in image http compat/libpod api endpoints Signed-off-by: Jakub Guzik --- libpod/image/prune.go | 7 +++++ pkg/api/handlers/compat/images_prune.go | 19 ++++--------- pkg/api/handlers/libpod/images.go | 18 ++++++------ pkg/api/handlers/utils/images.go | 14 ++++++---- test/apiv2/10-images.at | 49 +++++++++++++++++++++++++++++++++ 5 files changed, 80 insertions(+), 27 deletions(-) (limited to 'pkg') diff --git a/libpod/image/prune.go b/libpod/image/prune.go index d6ae5feaf..21867698a 100644 --- a/libpod/image/prune.go +++ b/libpod/image/prune.go @@ -2,6 +2,7 @@ package image import ( "context" + "strconv" "strings" "time" @@ -52,6 +53,12 @@ func generatePruneFilterFuncs(filter, filterValue string) (ImageFilter, error) { } return false }, nil + case "dangling": + danglingImages, err := strconv.ParseBool(filterValue) + if err != nil { + return nil, errors.Wrapf(err, "invalid filter dangling=%s", filterValue) + } + return ImageFilter(DanglingFilter(danglingImages)), nil } return nil, nil } diff --git a/pkg/api/handlers/compat/images_prune.go b/pkg/api/handlers/compat/images_prune.go index 63daaa780..ddf559ec6 100644 --- a/pkg/api/handlers/compat/images_prune.go +++ b/pkg/api/handlers/compat/images_prune.go @@ -8,8 +8,8 @@ import ( "github.com/containers/podman/v3/libpod" "github.com/containers/podman/v3/pkg/api/handlers" "github.com/containers/podman/v3/pkg/api/handlers/utils" + "github.com/containers/podman/v3/pkg/util" "github.com/docker/docker/api/types" - "github.com/gorilla/schema" "github.com/pkg/errors" ) @@ -17,27 +17,20 @@ func PruneImages(w http.ResponseWriter, r *http.Request) { var ( filters []string ) - decoder := r.Context().Value("decoder").(*schema.Decoder) runtime := r.Context().Value("runtime").(*libpod.Runtime) - query := struct { - All bool - Filters map[string][]string `schema:"filters"` - }{ - // This is where you can override the golang default value for one of fields - } - - if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) + filterMap, err := util.PrepareFilters(r) + if err != nil { + utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } - for k, v := range query.Filters { + for k, v := range *filterMap { for _, val := range v { filters = append(filters, fmt.Sprintf("%s=%s", k, val)) } } - imagePruneReports, err := runtime.ImageRuntime().PruneImages(r.Context(), query.All, filters) + imagePruneReports, err := runtime.ImageRuntime().PruneImages(r.Context(), false, filters) if err != nil { utils.InternalServerError(w, err) return diff --git a/pkg/api/handlers/libpod/images.go b/pkg/api/handlers/libpod/images.go index 1f306a533..158babcdc 100644 --- a/pkg/api/handlers/libpod/images.go +++ b/pkg/api/handlers/libpod/images.go @@ -22,6 +22,7 @@ import ( "github.com/containers/podman/v3/pkg/domain/entities" "github.com/containers/podman/v3/pkg/domain/infra/abi" "github.com/containers/podman/v3/pkg/errorhandling" + "github.com/containers/podman/v3/pkg/util" utils2 "github.com/containers/podman/v3/utils" "github.com/gorilla/schema" "github.com/pkg/errors" @@ -125,31 +126,32 @@ func PruneImages(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) decoder := r.Context().Value("decoder").(*schema.Decoder) query := struct { - All bool `schema:"all"` - Filters map[string][]string `schema:"filters"` + All bool `schema:"all"` }{ // override any golang type defaults } - if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + filterMap, err := util.PrepareFilters(r) + + if dErr := decoder.Decode(&query, r.URL.Query()); dErr != nil || err != nil { + utils.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } var libpodFilters = []string{} if _, found := r.URL.Query()["filters"]; found { - dangling := query.Filters["all"] + dangling := (*filterMap)["all"] if len(dangling) > 0 { - query.All, err = strconv.ParseBool(query.Filters["all"][0]) + query.All, err = strconv.ParseBool((*filterMap)["all"][0]) if err != nil { utils.InternalServerError(w, err) return } } // dangling is special and not implemented in the libpod side of things - delete(query.Filters, "dangling") - for k, v := range query.Filters { + delete(*filterMap, "dangling") + for k, v := range *filterMap { libpodFilters = append(libpodFilters, fmt.Sprintf("%s=%s", k, v[0])) } } diff --git a/pkg/api/handlers/utils/images.go b/pkg/api/handlers/utils/images.go index 743629db8..da3c9e985 100644 --- a/pkg/api/handlers/utils/images.go +++ b/pkg/api/handlers/utils/images.go @@ -11,6 +11,7 @@ import ( "github.com/containers/image/v5/types" "github.com/containers/podman/v3/libpod" "github.com/containers/podman/v3/libpod/image" + "github.com/containers/podman/v3/pkg/util" "github.com/gorilla/schema" "github.com/pkg/errors" ) @@ -58,13 +59,17 @@ func GetImages(w http.ResponseWriter, r *http.Request) ([]*image.Image, error) { runtime := r.Context().Value("runtime").(*libpod.Runtime) query := struct { All bool - Filters map[string][]string `schema:"filters"` Digests bool Filter string // Docker 1.24 compatibility }{ // This is where you can override the golang default value for one of fields } + filterMap, err := util.PrepareFilters(r) + if err != nil { + return nil, err + } + if err := decoder.Decode(&query, r.URL.Query()); err != nil { return nil, err } @@ -72,12 +77,9 @@ func GetImages(w http.ResponseWriter, r *http.Request) ([]*image.Image, error) { if _, found := r.URL.Query()["digests"]; found && query.Digests { UnSupportedParameter("digests") } - var ( - images []*image.Image - err error - ) + var images []*image.Image - queryFilters := query.Filters + queryFilters := *filterMap if !IsLibpodRequest(r) && len(query.Filter) > 0 { // Docker 1.24 compatibility if queryFilters == nil { queryFilters = make(map[string][]string) diff --git a/test/apiv2/10-images.at b/test/apiv2/10-images.at index 4ebaeff45..f854d38ab 100644 --- a/test/apiv2/10-images.at +++ b/test/apiv2/10-images.at @@ -77,6 +77,55 @@ for i in $iid ${iid:0:12} $PODMAN_TEST_IMAGE_NAME; do t GET "libpod/images/$i/get?compress=false" 200 '[POSIX tar archive]' done +#compat api list images sanity checks +t GET images/json?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" +t GET images/json?filters='{"label":["testl' 500 \ + .cause="unexpected end of JSON input" + +#libpod api list images sanity checks +t GET libpod/images/json?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" +t GET libpod/images/json?filters='{"label":["testl' 500 \ + .cause="unexpected end of JSON input" + +# Prune images - bad filter input +t POST images/prune?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" +t POST libpod/images/prune?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" + +## Prune images with illformed label +t POST images/prune?filters='{"label":["tes' 500 \ + .cause="unexpected end of JSON input" +t POST libpod/images/prune?filters='{"label":["tes' 500 \ + .cause="unexpected end of JSON input" + + +#create, list and remove dangling image +podman image build -t test:test -<file1 +EOF + +podman image build -t test:test --label xyz -<file2 +EOF + +t GET images/json?filters='{"dangling":["true"]}' 200 length=1 +t POST images/prune?filters='{"dangling":["true"]}' 200 +t GET images/json?filters='{"dangling":["true"]}' 200 length=0 + +#label filter check in libpod and compat +t GET images/json?filters='{"label":["xyz"]}' 200 length=1 +t GET libpod/images/json?filters='{"label":["xyz"]}' 200 length=1 + +t DELETE libpod/images/test:test 200 + +t GET images/json?filters='{"label":["xyz"]}' 200 length=0 +t GET libpod/images/json?filters='{"label":["xyz"]}' 200 length=0 + # Export more than one image # FIXME FIXME FIXME, this doesn't work: # not ok 64 [10-images] GET images/get?names=alpine,busybox : status -- cgit v1.2.3-54-g00ecf From d560f168f60ecdb25165332a871d4ff352cd5e00 Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Thu, 25 Mar 2021 07:51:16 -0700 Subject: Correct json field name [NO TESTS NEEDED] * When using the Namespace type, the field Value was json encoded with the name "string" vs "value". Signed-off-by: Jhon Honce --- pkg/specgen/namespaces.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/specgen/namespaces.go b/pkg/specgen/namespaces.go index fb7d65da4..f665fc0be 100644 --- a/pkg/specgen/namespaces.go +++ b/pkg/specgen/namespaces.go @@ -54,7 +54,7 @@ const ( // Namespace describes the namespace type Namespace struct { NSMode NamespaceMode `json:"nsmode,omitempty"` - Value string `json:"string,omitempty"` + Value string `json:"value,omitempty"` } // IsDefault returns whether the namespace is set to the default setting (which -- cgit v1.2.3-54-g00ecf From 6b6989206430721368351c04f73cf0e80f5fc339 Mon Sep 17 00:00:00 2001 From: Eduardo Vega Date: Thu, 11 Mar 2021 19:16:41 -0600 Subject: Support multi doc yaml for generate/play kube Signed-off-by: Eduardo Vega Signed-off-by: Matthew Heon --- go.mod | 2 +- pkg/domain/infra/abi/generate.go | 100 ++++++++++++++++++++--------- pkg/domain/infra/abi/play.go | 124 +++++++++++++++++++++++++++++------- pkg/domain/infra/abi/play_test.go | 97 ++++++++++++++++++++++++++++ test/e2e/generate_kube_test.go | 36 ++++++----- test/e2e/play_kube_test.go | 129 ++++++++++++++++++++++++++++++++++++-- 6 files changed, 411 insertions(+), 77 deletions(-) (limited to 'pkg') diff --git a/go.mod b/go.mod index b820ea40e..9545251fe 100644 --- a/go.mod +++ b/go.mod @@ -66,7 +66,7 @@ require ( golang.org/x/sync v0.0.0-20201207232520-09787c993a3a golang.org/x/sys v0.0.0-20210324051608-47abb6519492 gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect - gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 // indirect + gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 k8s.io/api v0.20.1 k8s.io/apimachinery v0.20.5 ) diff --git a/pkg/domain/infra/abi/generate.go b/pkg/domain/infra/abi/generate.go index 161becbfa..94f649e15 100644 --- a/pkg/domain/infra/abi/generate.go +++ b/pkg/domain/infra/abi/generate.go @@ -44,11 +44,10 @@ func (ic *ContainerEngine) GenerateSystemd(ctx context.Context, nameOrID string, func (ic *ContainerEngine) GenerateKube(ctx context.Context, nameOrIDs []string, options entities.GenerateKubeOptions) (*entities.GenerateKubeReport, error) { var ( pods []*libpod.Pod - podYAML *k8sAPI.Pod - err error ctrs []*libpod.Container - servicePorts []k8sAPI.ServicePort - serviceYAML k8sAPI.Service + kubePods []*k8sAPI.Pod + kubeServices []k8sAPI.Service + content []byte ) for _, nameOrID := range nameOrIDs { // Get the container in question @@ -59,9 +58,6 @@ func (ic *ContainerEngine) GenerateKube(ctx context.Context, nameOrIDs []string, return nil, err } pods = append(pods, pod) - if len(pods) > 1 { - return nil, errors.New("can only generate single pod at a time") - } } else { if len(ctr.Dependencies()) > 0 { return nil, errors.Wrapf(define.ErrNotImplemented, "containers with dependencies") @@ -79,20 +75,29 @@ func (ic *ContainerEngine) GenerateKube(ctx context.Context, nameOrIDs []string, return nil, errors.New("cannot generate pods and containers at the same time") } - if len(pods) == 1 { - podYAML, servicePorts, err = pods[0].GenerateForKube() + if len(pods) >= 1 { + pos, svcs, err := getKubePods(pods, options.Service) + if err != nil { + return nil, err + } + + kubePods = append(kubePods, pos...) + if options.Service { + kubeServices = append(kubeServices, svcs...) + } } else { - podYAML, err = libpod.GenerateForKube(ctrs) - } - if err != nil { - return nil, err - } + po, err := libpod.GenerateForKube(ctrs) + if err != nil { + return nil, err + } - if options.Service { - serviceYAML = libpod.GenerateKubeServiceFromV1Pod(podYAML, servicePorts) + kubePods = append(kubePods, po) + if options.Service { + kubeServices = append(kubeServices, libpod.GenerateKubeServiceFromV1Pod(po, []k8sAPI.ServicePort{})) + } } - content, err := generateKubeOutput(podYAML, &serviceYAML, options.Service) + content, err := generateKubeOutput(kubePods, kubeServices, options.Service) if err != nil { return nil, err } @@ -100,24 +105,56 @@ func (ic *ContainerEngine) GenerateKube(ctx context.Context, nameOrIDs []string, return &entities.GenerateKubeReport{Reader: bytes.NewReader(content)}, nil } -func generateKubeOutput(podYAML *k8sAPI.Pod, serviceYAML *k8sAPI.Service, hasService bool) ([]byte, error) { - var ( - output []byte - marshalledPod []byte - marshalledService []byte - err error - ) +func getKubePods(pods []*libpod.Pod, getService bool) ([]*k8sAPI.Pod, []k8sAPI.Service, error) { + kubePods := make([]*k8sAPI.Pod, 0) + kubeServices := make([]k8sAPI.Service, 0) - marshalledPod, err = yaml.Marshal(podYAML) - if err != nil { - return nil, err + for _, p := range pods { + po, svc, err := p.GenerateForKube() + if err != nil { + return nil, nil, err + } + + kubePods = append(kubePods, po) + if getService { + kubeServices = append(kubeServices, libpod.GenerateKubeServiceFromV1Pod(po, svc)) + } } - if hasService { - marshalledService, err = yaml.Marshal(serviceYAML) + return kubePods, kubeServices, nil +} + +func generateKubeOutput(kubePods []*k8sAPI.Pod, kubeServices []k8sAPI.Service, hasService bool) ([]byte, error) { + output := make([]byte, 0) + marshalledPods := make([]byte, 0) + marshalledServices := make([]byte, 0) + + for i, p := range kubePods { + if i != 0 { + marshalledPods = append(marshalledPods, []byte("---\n")...) + } + + b, err := yaml.Marshal(p) if err != nil { return nil, err } + + marshalledPods = append(marshalledPods, b...) + } + + if hasService { + for i, s := range kubeServices { + if i != 0 { + marshalledServices = append(marshalledServices, []byte("---\n")...) + } + + b, err := yaml.Marshal(s) + if err != nil { + return nil, err + } + + marshalledServices = append(marshalledServices, b...) + } } header := `# Generation of Kubernetes YAML is still under development! @@ -133,11 +170,12 @@ func generateKubeOutput(podYAML *k8sAPI.Pod, serviceYAML *k8sAPI.Service, hasSer } output = append(output, []byte(fmt.Sprintf(header, podmanVersion.Version))...) - output = append(output, marshalledPod...) + // kube generate order is based on helm install order (service, pod...) if hasService { + output = append(output, marshalledServices...) output = append(output, []byte("---\n")...) - output = append(output, marshalledService...) } + output = append(output, marshalledPods...) return output, nil } diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index efc7c86e3..7d87fc83a 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -1,6 +1,7 @@ package abi import ( + "bytes" "context" "fmt" "io" @@ -20,46 +21,79 @@ import ( "github.com/ghodss/yaml" "github.com/pkg/errors" "github.com/sirupsen/logrus" + yamlv3 "gopkg.in/yaml.v3" v1apps "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" ) func (ic *ContainerEngine) PlayKube(ctx context.Context, path string, options entities.PlayKubeOptions) (*entities.PlayKubeReport, error) { - var ( - kubeObject v1.ObjectReference - ) + report := &entities.PlayKubeReport{} + validKinds := 0 + // read yaml document content, err := ioutil.ReadFile(path) if err != nil { return nil, err } - if err := yaml.Unmarshal(content, &kubeObject); err != nil { - return nil, errors.Wrapf(err, "unable to read %q as YAML", path) + // split yaml document + documentList, err := splitMultiDocYAML(content) + if err != nil { + return nil, err } - // NOTE: pkg/bindings/play is also parsing the file. - // A pkg/kube would be nice to refactor and abstract - // parts of the K8s-related code. - switch kubeObject.Kind { - case "Pod": - var podYAML v1.Pod - var podTemplateSpec v1.PodTemplateSpec - if err := yaml.Unmarshal(content, &podYAML); err != nil { - return nil, errors.Wrapf(err, "unable to read YAML %q as Kube Pod", path) + // create pod on each document if it is a pod or deployment + // any other kube kind will be skipped + for _, document := range documentList { + kind, err := getKubeKind(document) + if err != nil { + return nil, errors.Wrapf(err, "unable to read %q as kube YAML", path) } - podTemplateSpec.ObjectMeta = podYAML.ObjectMeta - podTemplateSpec.Spec = podYAML.Spec - return ic.playKubePod(ctx, podTemplateSpec.ObjectMeta.Name, &podTemplateSpec, options) - case "Deployment": - var deploymentYAML v1apps.Deployment - if err := yaml.Unmarshal(content, &deploymentYAML); err != nil { - return nil, errors.Wrapf(err, "unable to read YAML %q as Kube Deployment", path) + + switch kind { + case "Pod": + var podYAML v1.Pod + var podTemplateSpec v1.PodTemplateSpec + + if err := yaml.Unmarshal(document, &podYAML); err != nil { + return nil, errors.Wrapf(err, "unable to read YAML %q as Kube Pod", path) + } + + podTemplateSpec.ObjectMeta = podYAML.ObjectMeta + podTemplateSpec.Spec = podYAML.Spec + + r, err := ic.playKubePod(ctx, podTemplateSpec.ObjectMeta.Name, &podTemplateSpec, options) + if err != nil { + return nil, err + } + + report.Pods = append(report.Pods, r.Pods...) + validKinds++ + case "Deployment": + var deploymentYAML v1apps.Deployment + + if err := yaml.Unmarshal(document, &deploymentYAML); err != nil { + return nil, errors.Wrapf(err, "unable to read YAML %q as Kube Deployment", path) + } + + r, err := ic.playKubeDeployment(ctx, &deploymentYAML, options) + if err != nil { + return nil, err + } + + report.Pods = append(report.Pods, r.Pods...) + validKinds++ + default: + logrus.Infof("kube kind %s not supported", kind) + continue } - return ic.playKubeDeployment(ctx, &deploymentYAML, options) - default: - return nil, errors.Errorf("invalid YAML kind: %q. [Pod|Deployment] are the only supported Kubernetes Kinds", kubeObject.Kind) } + + if validKinds == 0 { + return nil, fmt.Errorf("YAML document does not contain any supported kube kind") + } + + return report, nil } func (ic *ContainerEngine) playKubeDeployment(ctx context.Context, deploymentYAML *v1apps.Deployment, options entities.PlayKubeOptions) (*entities.PlayKubeReport, error) { @@ -290,3 +324,45 @@ func readConfigMapFromFile(r io.Reader) (v1.ConfigMap, error) { return cm, nil } + +// splitMultiDocYAML reads mutiple documents in a YAML file and +// returns them as a list. +func splitMultiDocYAML(yamlContent []byte) ([][]byte, error) { + var documentList [][]byte + + d := yamlv3.NewDecoder(bytes.NewReader(yamlContent)) + for { + var o interface{} + // read individual document + err := d.Decode(&o) + if err == io.EOF { + break + } + if err != nil { + return nil, errors.Wrapf(err, "multi doc yaml could not be split") + } + + if o != nil { + // back to bytes + document, err := yamlv3.Marshal(o) + if err != nil { + return nil, errors.Wrapf(err, "individual doc yaml could not be marshalled") + } + + documentList = append(documentList, document) + } + } + + return documentList, nil +} + +// getKubeKind unmarshals a kube YAML document and returns its kind. +func getKubeKind(obj []byte) (string, error) { + var kubeObject v1.ObjectReference + + if err := yaml.Unmarshal(obj, &kubeObject); err != nil { + return "", err + } + + return kubeObject.Kind, nil +} diff --git a/pkg/domain/infra/abi/play_test.go b/pkg/domain/infra/abi/play_test.go index 4354a3835..bbc7c3493 100644 --- a/pkg/domain/infra/abi/play_test.go +++ b/pkg/domain/infra/abi/play_test.go @@ -89,3 +89,100 @@ data: }) } } + +func TestGetKubeKind(t *testing.T) { + tests := []struct { + name string + kubeYAML string + expectError bool + expectedErrorMsg string + expected string + }{ + { + "ValidKubeYAML", + ` +apiVersion: v1 +kind: Pod +`, + false, + "", + "Pod", + }, + { + "InvalidKubeYAML", + "InvalidKubeYAML", + true, + "cannot unmarshal", + "", + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + kind, err := getKubeKind([]byte(test.kubeYAML)) + if test.expectError { + assert.Error(t, err) + assert.Contains(t, err.Error(), test.expectedErrorMsg) + } else { + assert.NoError(t, err) + assert.Equal(t, test.expected, kind) + } + }) + } +} + +func TestSplitMultiDocYAML(t *testing.T) { + tests := []struct { + name string + kubeYAML string + expectError bool + expectedErrorMsg string + expected int + }{ + { + "ValidNumberOfDocs", + ` +apiVersion: v1 +kind: Pod +--- +apiVersion: v1 +kind: Pod +--- +apiVersion: v1 +kind: Pod +`, + false, + "", + 3, + }, + { + "InvalidMultiDocYAML", + ` +apiVersion: v1 +kind: Pod +--- +apiVersion: v1 +kind: Pod +- +`, + true, + "multi doc yaml could not be split", + 0, + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + docs, err := splitMultiDocYAML([]byte(test.kubeYAML)) + if test.expectError { + assert.Error(t, err) + assert.Contains(t, err.Error(), test.expectedErrorMsg) + } else { + assert.NoError(t, err) + assert.Equal(t, test.expected, len(docs)) + } + }) + } +} diff --git a/test/e2e/generate_kube_test.go b/test/e2e/generate_kube_test.go index bc7c21785..e74192746 100644 --- a/test/e2e/generate_kube_test.go +++ b/test/e2e/generate_kube_test.go @@ -155,6 +155,23 @@ var _ = Describe("Podman generate kube", func() { Expect(numContainers).To(Equal(1)) }) + It("podman generate kube multiple pods", func() { + pod1 := podmanTest.Podman([]string{"run", "-dt", "--pod", "new:pod1", ALPINE, "top"}) + pod1.WaitWithDefaultTimeout() + Expect(pod1.ExitCode()).To(Equal(0)) + + pod2 := podmanTest.Podman([]string{"run", "-dt", "--pod", "new:pod2", ALPINE, "top"}) + pod2.WaitWithDefaultTimeout() + Expect(pod2.ExitCode()).To(Equal(0)) + + kube := podmanTest.Podman([]string{"generate", "kube", "pod1", "pod2"}) + kube.WaitWithDefaultTimeout() + Expect(kube.ExitCode()).To(Equal(0)) + + Expect(string(kube.Out.Contents())).To(ContainSubstring(`name: pod1`)) + Expect(string(kube.Out.Contents())).To(ContainSubstring(`name: pod2`)) + }) + It("podman generate kube on pod with host network", func() { podSession := podmanTest.Podman([]string{"pod", "create", "--name", "testHostNetwork", "--network", "host"}) podSession.WaitWithDefaultTimeout() @@ -507,21 +524,6 @@ var _ = Describe("Podman generate kube", func() { Expect(inspect.OutputToString()).To(ContainSubstring(`"pid"`)) }) - It("podman generate kube multiple pods should fail", func() { - SkipIfRootlessCgroupsV1("Not supported for rootless + CGroupsV1") - pod1 := podmanTest.Podman([]string{"run", "-dt", "--pod", "new:pod1", ALPINE, "top"}) - pod1.WaitWithDefaultTimeout() - Expect(pod1.ExitCode()).To(Equal(0)) - - pod2 := podmanTest.Podman([]string{"run", "-dt", "--pod", "new:pod2", ALPINE, "top"}) - pod2.WaitWithDefaultTimeout() - Expect(pod2.ExitCode()).To(Equal(0)) - - kube := podmanTest.Podman([]string{"generate", "kube", "pod1", "pod2"}) - kube.WaitWithDefaultTimeout() - Expect(kube.ExitCode()).ToNot(Equal(0)) - }) - It("podman generate kube with pods and containers should fail", func() { pod1 := podmanTest.Podman([]string{"run", "-dt", "--pod", "new:pod1", ALPINE, "top"}) pod1.WaitWithDefaultTimeout() @@ -564,7 +566,7 @@ var _ = Describe("Podman generate kube", func() { Expect(kube.ExitCode()).To(Equal(0)) }) - It("podman generate kube with containers in a pod should fail", func() { + It("podman generate kube with containers in pods should fail", func() { pod1 := podmanTest.Podman([]string{"run", "-dt", "--pod", "new:pod1", "--name", "top1", ALPINE, "top"}) pod1.WaitWithDefaultTimeout() Expect(pod1.ExitCode()).To(Equal(0)) @@ -573,7 +575,7 @@ var _ = Describe("Podman generate kube", func() { pod2.WaitWithDefaultTimeout() Expect(pod2.ExitCode()).To(Equal(0)) - kube := podmanTest.Podman([]string{"generate", "kube", "pod1", "pod2"}) + kube := podmanTest.Podman([]string{"generate", "kube", "top1", "top2"}) kube.WaitWithDefaultTimeout() Expect(kube.ExitCode()).ToNot(Equal(0)) }) diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 433a8158c..a4c738f17 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -357,7 +357,8 @@ func writeYaml(content string, fileName string) error { return nil } -func generateKubeYaml(kind string, object interface{}, pathname string) error { +// getKubeYaml returns a kubernetes YAML document. +func getKubeYaml(kind string, object interface{}) (string, error) { var yamlTemplate string templateBytes := &bytes.Buffer{} @@ -369,19 +370,41 @@ func generateKubeYaml(kind string, object interface{}, pathname string) error { case "deployment": yamlTemplate = deploymentYamlTemplate default: - return fmt.Errorf("unsupported kubernetes kind") + return "", fmt.Errorf("unsupported kubernetes kind") } t, err := template.New(kind).Parse(yamlTemplate) if err != nil { - return err + return "", err } if err := t.Execute(templateBytes, object); err != nil { + return "", err + } + + return templateBytes.String(), nil +} + +// generateKubeYaml writes a kubernetes YAML document. +func generateKubeYaml(kind string, object interface{}, pathname string) error { + k, err := getKubeYaml(kind, object) + if err != nil { return err } - return writeYaml(templateBytes.String(), pathname) + return writeYaml(k, pathname) +} + +// generateMultiDocKubeYaml writes multiple kube objects in one Yaml document. +func generateMultiDocKubeYaml(kubeObjects []string, pathname string) error { + var multiKube string + + for _, k := range kubeObjects { + multiKube += "---\n" + multiKube += k + } + + return writeYaml(multiKube, pathname) } // ConfigMap describes the options a kube yaml can be configured at configmap level @@ -1699,4 +1722,102 @@ MemoryReservation: {{ .HostConfig.MemoryReservation }}`}) Expect(inspect.ExitCode()).To(Equal(0)) Expect(inspect.OutputToString()).To(Equal("true")) }) + + // Multi doc related tests + It("podman play kube multi doc yaml", func() { + yamlDocs := []string{} + podNames := []string{} + + serviceTemplate := `apiVersion: v1 +kind: Service +metadata: + name: %s +spec: + ports: + - port: 80 + protocol: TCP + targetPort: 9376 + selector: + app: %s +` + // generate servies, pods and deployments + for i := 0; i < 2; i++ { + podName := fmt.Sprintf("testPod%d", i) + deploymentName := fmt.Sprintf("testDeploy%d", i) + deploymentPodName := fmt.Sprintf("%s-pod-0", deploymentName) + + podNames = append(podNames, podName) + podNames = append(podNames, deploymentPodName) + + pod := getPod(withPodName(podName)) + podDeployment := getPod(withPodName(deploymentName)) + deployment := getDeployment(withPod(podDeployment)) + deployment.Name = deploymentName + + // add services + yamlDocs = append([]string{ + fmt.Sprintf(serviceTemplate, podName, podName), + fmt.Sprintf(serviceTemplate, deploymentPodName, deploymentPodName)}, yamlDocs...) + + // add pods + k, err := getKubeYaml("pod", pod) + Expect(err).To(BeNil()) + yamlDocs = append(yamlDocs, k) + + // add deployments + k, err = getKubeYaml("deployment", deployment) + Expect(err).To(BeNil()) + yamlDocs = append(yamlDocs, k) + } + + // generate multi doc yaml + err = generateMultiDocKubeYaml(yamlDocs, kubeYaml) + Expect(err).To(BeNil()) + + kube := podmanTest.Podman([]string{"play", "kube", kubeYaml}) + kube.WaitWithDefaultTimeout() + Expect(kube.ExitCode()).To(Equal(0)) + + for _, n := range podNames { + inspect := podmanTest.Podman([]string{"inspect", n, "--format", "'{{ .State }}'"}) + inspect.WaitWithDefaultTimeout() + Expect(inspect.ExitCode()).To(Equal(0)) + Expect(inspect.OutputToString()).To(ContainSubstring(`Running`)) + } + }) + + It("podman play kube invalid multi doc yaml", func() { + yamlDocs := []string{} + + serviceTemplate := `apiVersion: v1 +kind: Service +metadata: + name: %s +spec: + ports: + - port: 80 + protocol: TCP + targetPort: 9376 + selector: + app: %s +--- +invalid kube kind +` + // add invalid multi doc yaml + yamlDocs = append(yamlDocs, fmt.Sprintf(serviceTemplate, "foo", "foo")) + + // add pod + pod := getPod() + k, err := getKubeYaml("pod", pod) + Expect(err).To(BeNil()) + yamlDocs = append(yamlDocs, k) + + // generate multi doc yaml + err = generateMultiDocKubeYaml(yamlDocs, kubeYaml) + Expect(err).To(BeNil()) + + kube := podmanTest.Podman([]string{"play", "kube", kubeYaml}) + kube.WaitWithDefaultTimeout() + Expect(kube.ExitCode()).To(Not(Equal(0))) + }) }) -- cgit v1.2.3-54-g00ecf From 31351ad94bf961a1e2341abbdf66145073445526 Mon Sep 17 00:00:00 2001 From: Matej Vasek Date: Tue, 23 Mar 2021 16:36:23 +0100 Subject: fix: build endpoint for compat API Signed-off-by: Matej Vasek --- pkg/api/handlers/compat/images_build.go | 48 +++++++++++++++++++++++++++++--- test/python/docker/compat/test_images.py | 2 +- 2 files changed, 45 insertions(+), 5 deletions(-) (limited to 'pkg') diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index 9aa035212..fb759b7aa 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -70,7 +70,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { BuildArgs string `schema:"buildargs"` CacheFrom string `schema:"cachefrom"` Compression uint64 `schema:"compression"` - ConfigureNetwork int64 `schema:"networkmode"` + ConfigureNetwork string `schema:"networkmode"` CpuPeriod uint64 `schema:"cpuperiod"` // nolint CpuQuota int64 `schema:"cpuquota"` // nolint CpuSetCpus string `schema:"cpusetcpus"` // nolint @@ -85,7 +85,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { ForceRm bool `schema:"forcerm"` From string `schema:"from"` HTTPProxy bool `schema:"httpproxy"` - Isolation int64 `schema:"isolation"` + Isolation string `schema:"isolation"` Ignore bool `schema:"ignore"` Jobs int `schema:"jobs"` // nolint Labels string `schema:"labels"` @@ -207,9 +207,15 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { isolation := buildah.IsolationDefault */ if utils.IsLibpodRequest(r) { - // isolation = buildah.Isolation(query.Isolation) + // isolation = parseLibPodIsolation(query.Isolation) registry = "" format = query.OutputFormat + } else { + if _, found := r.URL.Query()["isolation"]; found { + if query.Isolation != "" && query.Isolation != "default" { + logrus.Debugf("invalid `isolation` parameter: %q", query.Isolation) + } + } } var additionalTags []string if len(query.Tag) > 1 { @@ -335,7 +341,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { CNIConfigDir: rtc.Network.CNIPluginDirs[0], CNIPluginPath: util.DefaultCNIPluginPath, Compression: compression, - ConfigureNetwork: buildah.NetworkConfigurationPolicy(query.ConfigureNetwork), + ConfigureNetwork: parseNetworkConfigurationPolicy(query.ConfigureNetwork), ContextDirectory: contextDirectory, Devices: devices, DropCapabilities: dropCaps, @@ -465,6 +471,40 @@ loop: } } +func parseNetworkConfigurationPolicy(network string) buildah.NetworkConfigurationPolicy { + if val, err := strconv.Atoi(network); err == nil { + return buildah.NetworkConfigurationPolicy(val) + } + switch network { + case "NetworkDefault": + return buildah.NetworkDefault + case "NetworkDisabled": + return buildah.NetworkDisabled + case "NetworkEnabled": + return buildah.NetworkEnabled + default: + return buildah.NetworkDefault + } +} + +func parseLibPodIsolation(isolation string) buildah.Isolation { + if val, err := strconv.Atoi(isolation); err == nil { + return buildah.Isolation(val) + } + switch isolation { + case "IsolationDefault", "default": + return buildah.IsolationDefault + case "IsolationOCI": + return buildah.IsolationOCI + case "IsolationChroot": + return buildah.IsolationChroot + case "IsolationOCIRootless": + return buildah.IsolationOCIRootless + default: + return buildah.IsolationDefault + } +} + func extractTarFile(r *http.Request) (string, error) { // build a home for the request body anchorDir, err := ioutil.TempDir("", "libpod_builder") diff --git a/test/python/docker/compat/test_images.py b/test/python/docker/compat/test_images.py index 571b31881..1e2b531b7 100644 --- a/test/python/docker/compat/test_images.py +++ b/test/python/docker/compat/test_images.py @@ -158,7 +158,7 @@ class TestImages(unittest.TestCase): def test_build_image(self): labels = {"apple": "red", "grape": "green"} _ = self.client.images.build( - path="test/python/docker/build_labels", labels=labels, tag="labels" + path="test/python/docker/build_labels", labels=labels, tag="labels", isolation="default" ) image = self.client.images.get("labels") self.assertEqual(image.labels["apple"], labels["apple"]) -- cgit v1.2.3-54-g00ecf From d1589f280474dd90236a59f8b32bb2a4a04fd683 Mon Sep 17 00:00:00 2001 From: Matej Vasek Date: Tue, 23 Mar 2021 18:04:18 +0100 Subject: fixup Signed-off-by: Matej Vasek --- pkg/api/handlers/compat/images_build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index fb759b7aa..20331600d 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -487,7 +487,7 @@ func parseNetworkConfigurationPolicy(network string) buildah.NetworkConfiguratio } } -func parseLibPodIsolation(isolation string) buildah.Isolation { +func parseLibPodIsolation(isolation string) buildah.Isolation { // nolint if val, err := strconv.Atoi(isolation); err == nil { return buildah.Isolation(val) } -- cgit v1.2.3-54-g00ecf From 183a68a817208821a9fc79a3c9c19202a63dfb44 Mon Sep 17 00:00:00 2001 From: Jakub Guzik Date: Tue, 23 Mar 2021 00:13:44 +0100 Subject: Unification of label filter across list/prune endpoints Signed-off-by: Jakub Guzik --- libpod/image/filters.go | 14 ++------ libpod/image/prune.go | 15 ++------ libpod/network/netconflist.go | 27 ++------------ pkg/domain/filters/containers.go | 22 +----------- pkg/domain/filters/pods.go | 21 +---------- pkg/domain/filters/volumes.go | 17 ++------- pkg/util/filters.go | 23 +++++++++++- pkg/util/filters_test.go | 77 ++++++++++++++++++++++++++++++++++++++++ 8 files changed, 111 insertions(+), 105 deletions(-) create mode 100644 pkg/util/filters_test.go (limited to 'pkg') diff --git a/libpod/image/filters.go b/libpod/image/filters.go index 37d3cb6a5..d316c6956 100644 --- a/libpod/image/filters.go +++ b/libpod/image/filters.go @@ -9,6 +9,7 @@ import ( "time" "github.com/containers/podman/v3/pkg/inspect" + "github.com/containers/podman/v3/pkg/util" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -78,23 +79,14 @@ func ReadOnlyFilter(readOnly bool) ResultFilter { } // LabelFilter allows you to filter by images labels key and/or value -func LabelFilter(ctx context.Context, labelfilter string) ResultFilter { +func LabelFilter(ctx context.Context, filter string) ResultFilter { // We need to handle both label=key and label=key=value return func(i *Image) bool { - var value string - splitFilter := strings.SplitN(labelfilter, "=", 2) - key := splitFilter[0] - if len(splitFilter) > 1 { - value = splitFilter[1] - } labels, err := i.Labels(ctx) if err != nil { return false } - if len(strings.TrimSpace(labels[key])) > 0 && len(strings.TrimSpace(value)) == 0 { - return true - } - return labels[key] == value + return util.MatchLabelFilters([]string{filter}, labels) } } diff --git a/libpod/image/prune.go b/libpod/image/prune.go index 21867698a..1ef35ef83 100644 --- a/libpod/image/prune.go +++ b/libpod/image/prune.go @@ -9,6 +9,7 @@ import ( "github.com/containers/podman/v3/libpod/events" "github.com/containers/podman/v3/pkg/domain/entities/reports" "github.com/containers/podman/v3/pkg/timetype" + "github.com/containers/podman/v3/pkg/util" "github.com/containers/storage" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -17,24 +18,12 @@ import ( func generatePruneFilterFuncs(filter, filterValue string) (ImageFilter, error) { switch filter { case "label": - var filterArray = strings.SplitN(filterValue, "=", 2) - var filterKey = filterArray[0] - if len(filterArray) > 1 { - filterValue = filterArray[1] - } else { - filterValue = "" - } return func(i *Image) bool { labels, err := i.Labels(context.Background()) if err != nil { return false } - for labelKey, labelValue := range labels { - if labelKey == filterKey && (filterValue == "" || labelValue == filterValue) { - return true - } - } - return false + return util.MatchLabelFilters([]string{filterValue}, labels) }, nil case "until": diff --git a/libpod/network/netconflist.go b/libpod/network/netconflist.go index b358bc530..179620a0d 100644 --- a/libpod/network/netconflist.go +++ b/libpod/network/netconflist.go @@ -225,7 +225,7 @@ func IfPassesFilter(netconf *libcni.NetworkConfigList, filters map[string][]stri case "label": // matches all labels - result = matchPruneLabelFilters(netconf, filterValues) + result = util.MatchLabelFilters(filterValues, GetNetworkLabels(netconf)) case "driver": // matches only for the DefaultNetworkDriver @@ -260,7 +260,7 @@ func IfPassesPruneFilter(config *config.Config, netconf *libcni.NetworkConfigLis for key, filterValues := range f { switch strings.ToLower(key) { case "label": - return matchPruneLabelFilters(netconf, filterValues), nil + return util.MatchLabelFilters(filterValues, GetNetworkLabels(netconf)), nil case "until": until, err := util.ComputeUntilTimestamp(key, filterValues) if err != nil { @@ -280,29 +280,6 @@ func IfPassesPruneFilter(config *config.Config, netconf *libcni.NetworkConfigLis return false, nil } -func matchPruneLabelFilters(netconf *libcni.NetworkConfigList, filterValues []string) bool { - labels := GetNetworkLabels(netconf) - result := true -outer: - for _, filterValue := range filterValues { - filterArray := strings.SplitN(filterValue, "=", 2) - filterKey := filterArray[0] - if len(filterArray) > 1 { - filterValue = filterArray[1] - } else { - filterValue = "" - } - for labelKey, labelValue := range labels { - if labelKey == filterKey && (filterValue == "" || labelValue == filterValue) { - result = true - continue outer - } - } - result = false - } - return result -} - func getCreatedTimestamp(config *config.Config, netconf *libcni.NetworkConfigList) (*time.Time, error) { networkConfigPath, err := GetCNIConfigPathByNameOrID(config, netconf.Name) if err != nil { diff --git a/pkg/domain/filters/containers.go b/pkg/domain/filters/containers.go index 02727e841..9cae01dc0 100644 --- a/pkg/domain/filters/containers.go +++ b/pkg/domain/filters/containers.go @@ -23,27 +23,7 @@ func GenerateContainerFilterFuncs(filter string, filterValues []string, r *libpo case "label": // we have to match that all given labels exits on that container return func(c *libpod.Container) bool { - labels := c.Labels() - for _, filterValue := range filterValues { - matched := false - filterArray := strings.SplitN(filterValue, "=", 2) - filterKey := filterArray[0] - if len(filterArray) > 1 { - filterValue = filterArray[1] - } else { - filterValue = "" - } - for labelKey, labelValue := range labels { - if labelKey == filterKey && (filterValue == "" || labelValue == filterValue) { - matched = true - break - } - } - if !matched { - return false - } - } - return true + return util.MatchLabelFilters(filterValues, c.Labels()) }, nil case "name": // we only have to match one name diff --git a/pkg/domain/filters/pods.go b/pkg/domain/filters/pods.go index 0490a4848..9a1c7d19d 100644 --- a/pkg/domain/filters/pods.go +++ b/pkg/domain/filters/pods.go @@ -114,26 +114,7 @@ func GeneratePodFilterFunc(filter string, filterValues []string) ( case "label": return func(p *libpod.Pod) bool { labels := p.Labels() - for _, filterValue := range filterValues { - matched := false - filterArray := strings.SplitN(filterValue, "=", 2) - filterKey := filterArray[0] - if len(filterArray) > 1 { - filterValue = filterArray[1] - } else { - filterValue = "" - } - for labelKey, labelValue := range labels { - if labelKey == filterKey && (filterValue == "" || labelValue == filterValue) { - matched = true - break - } - } - if !matched { - return false - } - } - return true + return util.MatchLabelFilters(filterValues, labels) }, nil case "network": return func(p *libpod.Pod) bool { diff --git a/pkg/domain/filters/volumes.go b/pkg/domain/filters/volumes.go index bc1756cf5..9b2fc5280 100644 --- a/pkg/domain/filters/volumes.go +++ b/pkg/domain/filters/volumes.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/containers/podman/v3/libpod" + "github.com/containers/podman/v3/pkg/util" "github.com/pkg/errors" ) @@ -29,21 +30,9 @@ func GenerateVolumeFilters(filters url.Values) ([]libpod.VolumeFilter, error) { return v.Scope() == scopeVal }) case "label": - filterArray := strings.SplitN(val, "=", 2) - filterKey := filterArray[0] - var filterVal string - if len(filterArray) > 1 { - filterVal = filterArray[1] - } else { - filterVal = "" - } + filter := val vf = append(vf, func(v *libpod.Volume) bool { - for labelKey, labelValue := range v.Labels() { - if labelKey == filterKey && (filterVal == "" || labelValue == filterVal) { - return true - } - } - return false + return util.MatchLabelFilters([]string{filter}, v.Labels()) }) case "opt": filterArray := strings.SplitN(val, "=", 2) diff --git a/pkg/util/filters.go b/pkg/util/filters.go index 51b2c5331..90daef402 100644 --- a/pkg/util/filters.go +++ b/pkg/util/filters.go @@ -11,7 +11,7 @@ import ( "github.com/pkg/errors" ) -// ComputeUntilTimestamp extracts unitil timestamp from filters +// ComputeUntilTimestamp extracts until timestamp from filters func ComputeUntilTimestamp(filter string, filterValues []string) (time.Time, error) { invalid := time.Time{} if len(filterValues) != 1 { @@ -93,3 +93,24 @@ func PrepareFilters(r *http.Request) (*map[string][]string, error) { } return &filterMap, nil } + +// MatchLabelFilters matches labels and returs true if they are valid +func MatchLabelFilters(filterValues []string, labels map[string]string) bool { +outer: + for _, filterValue := range filterValues { + filterArray := strings.SplitN(filterValue, "=", 2) + filterKey := filterArray[0] + if len(filterArray) > 1 { + filterValue = filterArray[1] + } else { + filterValue = "" + } + for labelKey, labelValue := range labels { + if labelKey == filterKey && (filterValue == "" || labelValue == filterValue) { + continue outer + } + } + return false + } + return true +} diff --git a/pkg/util/filters_test.go b/pkg/util/filters_test.go new file mode 100644 index 000000000..4771f682b --- /dev/null +++ b/pkg/util/filters_test.go @@ -0,0 +1,77 @@ +package util + +import "testing" + +func TestMatchLabelFilters(t *testing.T) { + testLabels := map[string]string{ + "label1": "", + "label2": "test", + "label3": "", + } + type args struct { + filterValues []string + labels map[string]string + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "Match when all filters the same as labels", + args: args{ + filterValues: []string{"label1", "label3", "label2=test"}, + labels: testLabels, + }, + want: true, + }, + { + name: "Match when filter value not provided in args", + args: args{ + filterValues: []string{"label2"}, + labels: testLabels, + }, + want: true, + }, + { + name: "Match when no filter value is given", + args: args{ + filterValues: []string{"label2="}, + labels: testLabels, + }, + want: true, + }, + { + name: "Do not match when filter value differs", + args: args{ + filterValues: []string{"label2=differs"}, + labels: testLabels, + }, + want: false, + }, + { + name: "Do not match when filter value not listed in labels", + args: args{ + filterValues: []string{"label1=xyz"}, + labels: testLabels, + }, + want: false, + }, + { + name: "Do not match when one from many not ok", + args: args{ + filterValues: []string{"label1=xyz", "invalid=valid"}, + labels: testLabels, + }, + want: false, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + if got := MatchLabelFilters(tt.args.filterValues, tt.args.labels); got != tt.want { + t.Errorf("MatchLabelFilters() = %v, want %v", got, tt.want) + } + }) + } +} -- cgit v1.2.3-54-g00ecf From a01b387e8251bef5fc73f3e8ac72cc5db9ecea9e Mon Sep 17 00:00:00 2001 From: Jakub Guzik Date: Wed, 24 Mar 2021 00:42:42 +0100 Subject: Unification of until filter across list/prune endpoints Signed-off-by: Jakub Guzik --- libpod/image/prune.go | 9 +-------- libpod/network/netconflist.go | 2 +- pkg/domain/filters/containers.go | 2 +- pkg/util/filters.go | 4 ++-- pkg/util/filters_test.go | 38 +++++++++++++++++++++++++++++++++++++- 5 files changed, 42 insertions(+), 13 deletions(-) (limited to 'pkg') diff --git a/libpod/image/prune.go b/libpod/image/prune.go index 1ef35ef83..12727901a 100644 --- a/libpod/image/prune.go +++ b/libpod/image/prune.go @@ -4,11 +4,9 @@ import ( "context" "strconv" "strings" - "time" "github.com/containers/podman/v3/libpod/events" "github.com/containers/podman/v3/pkg/domain/entities/reports" - "github.com/containers/podman/v3/pkg/timetype" "github.com/containers/podman/v3/pkg/util" "github.com/containers/storage" "github.com/pkg/errors" @@ -27,15 +25,10 @@ func generatePruneFilterFuncs(filter, filterValue string) (ImageFilter, error) { }, nil case "until": - ts, err := timetype.GetTimestamp(filterValue, time.Now()) + until, err := util.ComputeUntilTimestamp([]string{filterValue}) if err != nil { return nil, err } - seconds, nanoseconds, err := timetype.ParseTimestamps(ts, 0) - if err != nil { - return nil, err - } - until := time.Unix(seconds, nanoseconds) return func(i *Image) bool { if !until.IsZero() && i.Created().After((until)) { return true diff --git a/libpod/network/netconflist.go b/libpod/network/netconflist.go index 179620a0d..08816f2bd 100644 --- a/libpod/network/netconflist.go +++ b/libpod/network/netconflist.go @@ -262,7 +262,7 @@ func IfPassesPruneFilter(config *config.Config, netconf *libcni.NetworkConfigLis case "label": return util.MatchLabelFilters(filterValues, GetNetworkLabels(netconf)), nil case "until": - until, err := util.ComputeUntilTimestamp(key, filterValues) + until, err := util.ComputeUntilTimestamp(filterValues) if err != nil { return false, err } diff --git a/pkg/domain/filters/containers.go b/pkg/domain/filters/containers.go index 9cae01dc0..84cf03764 100644 --- a/pkg/domain/filters/containers.go +++ b/pkg/domain/filters/containers.go @@ -165,7 +165,7 @@ func GenerateContainerFilterFuncs(filter string, filterValues []string, r *libpo return false }, nil case "until": - until, err := util.ComputeUntilTimestamp(filter, filterValues) + until, err := util.ComputeUntilTimestamp(filterValues) if err != nil { return nil, err } diff --git a/pkg/util/filters.go b/pkg/util/filters.go index 90daef402..43bf646f1 100644 --- a/pkg/util/filters.go +++ b/pkg/util/filters.go @@ -12,10 +12,10 @@ import ( ) // ComputeUntilTimestamp extracts until timestamp from filters -func ComputeUntilTimestamp(filter string, filterValues []string) (time.Time, error) { +func ComputeUntilTimestamp(filterValues []string) (time.Time, error) { invalid := time.Time{} if len(filterValues) != 1 { - return invalid, errors.Errorf("specify exactly one timestamp for %s", filter) + return invalid, errors.Errorf("specify exactly one timestamp for until") } ts, err := timetype.GetTimestamp(filterValues[0], time.Now()) if err != nil { diff --git a/pkg/util/filters_test.go b/pkg/util/filters_test.go index 4771f682b..47259013e 100644 --- a/pkg/util/filters_test.go +++ b/pkg/util/filters_test.go @@ -1,6 +1,8 @@ package util -import "testing" +import ( + "testing" +) func TestMatchLabelFilters(t *testing.T) { testLabels := map[string]string{ @@ -75,3 +77,37 @@ func TestMatchLabelFilters(t *testing.T) { }) } } + +func TestComputeUntilTimestamp(t *testing.T) { + tests := []struct { + name string + args []string + wantErr bool + }{ + { + name: "Return error when more values in list", + args: []string{"5h", "6s"}, + wantErr: true, + }, + { + name: "Return error when invalid time", + args: []string{"invalidTime"}, + wantErr: true, + }, + { + name: "Do not return error when correct time format supplied", + args: []string{"44m"}, + wantErr: false, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + _, err := ComputeUntilTimestamp(tt.args) + if (err != nil) != tt.wantErr { + t.Errorf("ComputeUntilTimestamp() error = %v, wantErr %v", err, tt.wantErr) + return + } + }) + } +} -- cgit v1.2.3-54-g00ecf From 96ffce2198785c9d5717e3104f0afd8a0c38b6af Mon Sep 17 00:00:00 2001 From: Jakub Guzik Date: Sat, 20 Mar 2021 22:56:01 +0100 Subject: Fix containers list/prune http api filter behaviour The problem described in #9711 and followed by #9758 affects containers as well. When user provides wrong filter input, error message should occur, not fallback to full list/prune command. This change fixes the issue. Additionally, there are error message fixes for docker http api compat. Signed-off-by: Jakub Guzik --- pkg/api/handlers/compat/containers.go | 22 ++++++++++++---------- pkg/api/handlers/compat/containers_prune.go | 17 +++++++---------- pkg/api/handlers/libpod/containers.go | 24 +++++++++++++----------- test/apiv2/20-containers.at | 26 ++++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 31 deletions(-) (limited to 'pkg') diff --git a/pkg/api/handlers/compat/containers.go b/pkg/api/handlers/compat/containers.go index d3277b815..e7146a5d8 100644 --- a/pkg/api/handlers/compat/containers.go +++ b/pkg/api/handlers/compat/containers.go @@ -19,6 +19,7 @@ import ( "github.com/containers/podman/v3/pkg/domain/infra/abi" "github.com/containers/podman/v3/pkg/ps" "github.com/containers/podman/v3/pkg/signal" + "github.com/containers/podman/v3/pkg/util" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/go-connections/nat" @@ -92,23 +93,24 @@ func ListContainers(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) decoder := r.Context().Value("decoder").(*schema.Decoder) query := struct { - All bool `schema:"all"` - Limit int `schema:"limit"` - Size bool `schema:"size"` - Filters map[string][]string `schema:"filters"` + All bool `schema:"all"` + Limit int `schema:"limit"` + Size bool `schema:"size"` }{ // override any golang type defaults } - if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) + filterMap, err := util.PrepareFilters(r) + + if dErr := decoder.Decode(&query, r.URL.Query()); dErr != nil || err != nil { + utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } - filterFuncs := make([]libpod.ContainerFilter, 0, len(query.Filters)) + filterFuncs := make([]libpod.ContainerFilter, 0, len(*filterMap)) all := query.All || query.Limit > 0 - if len(query.Filters) > 0 { - for k, v := range query.Filters { + if len((*filterMap)) > 0 { + for k, v := range *filterMap { generatedFunc, err := filters.GenerateContainerFilterFuncs(k, v, runtime) if err != nil { utils.InternalServerError(w, err) @@ -120,7 +122,7 @@ func ListContainers(w http.ResponseWriter, r *http.Request) { // Docker thinks that if status is given as an input, then we should override // the all setting and always deal with all containers. - if len(query.Filters["status"]) > 0 { + if len((*filterMap)["status"]) > 0 { all = true } if !all { diff --git a/pkg/api/handlers/compat/containers_prune.go b/pkg/api/handlers/compat/containers_prune.go index dc4d53af6..e37929d27 100644 --- a/pkg/api/handlers/compat/containers_prune.go +++ b/pkg/api/handlers/compat/containers_prune.go @@ -9,23 +9,20 @@ import ( "github.com/containers/podman/v3/pkg/api/handlers/utils" "github.com/containers/podman/v3/pkg/domain/entities/reports" "github.com/containers/podman/v3/pkg/domain/filters" - "github.com/gorilla/schema" + "github.com/containers/podman/v3/pkg/util" "github.com/pkg/errors" ) func PruneContainers(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) - decoder := r.Context().Value("decoder").(*schema.Decoder) - - query := struct { - Filters map[string][]string `schema:"filters"` - }{} - if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) + filtersMap, err := util.PrepareFilters(r) + if err != nil { + utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } - filterFuncs := make([]libpod.ContainerFilter, 0, len(query.Filters)) - for k, v := range query.Filters { + + filterFuncs := make([]libpod.ContainerFilter, 0, len(*filtersMap)) + for k, v := range *filtersMap { generatedFunc, err := filters.GenerateContainerFilterFuncs(k, v, runtime) if err != nil { utils.InternalServerError(w, err) diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go index 01b9ec101..77269db8b 100644 --- a/pkg/api/handlers/libpod/containers.go +++ b/pkg/api/handlers/libpod/containers.go @@ -11,6 +11,7 @@ import ( "github.com/containers/podman/v3/pkg/api/handlers/utils" "github.com/containers/podman/v3/pkg/domain/entities" "github.com/containers/podman/v3/pkg/domain/infra/abi" + "github.com/containers/podman/v3/pkg/util" "github.com/gorilla/schema" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -59,20 +60,21 @@ func ContainerExists(w http.ResponseWriter, r *http.Request) { func ListContainers(w http.ResponseWriter, r *http.Request) { decoder := r.Context().Value("decoder").(*schema.Decoder) query := struct { - All bool `schema:"all"` - External bool `schema:"external"` - Filters map[string][]string `schema:"filters"` - Last int `schema:"last"` // alias for limit - Limit int `schema:"limit"` - Namespace bool `schema:"namespace"` - Size bool `schema:"size"` - Sync bool `schema:"sync"` + All bool `schema:"all"` + External bool `schema:"external"` + Last int `schema:"last"` // alias for limit + Limit int `schema:"limit"` + Namespace bool `schema:"namespace"` + Size bool `schema:"size"` + Sync bool `schema:"sync"` }{ // override any golang type defaults } - if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + filterMap, err := util.PrepareFilters(r) + + if dErr := decoder.Decode(&query, r.URL.Query()); dErr != nil || err != nil { + utils.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } @@ -94,7 +96,7 @@ func ListContainers(w http.ResponseWriter, r *http.Request) { opts := entities.ContainerListOptions{ All: query.All, External: query.External, - Filters: query.Filters, + Filters: *filterMap, Last: limit, Namespace: query.Namespace, // Always return Pod, should not be part of the API. diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at index 478717700..9030f0095 100644 --- a/test/apiv2/20-containers.at +++ b/test/apiv2/20-containers.at @@ -280,6 +280,32 @@ t GET containers/json 200 \ podman stop bar +#compat api list containers sanity checks +t GET containers/json?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" +t GET containers/json?filters='{"label":["testl' 500 \ + .cause="unexpected end of JSON input" + +#libpod api list containers sanity checks +t GET libpod/containers/json?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" +t GET libpod/containers/json?filters='{"label":["testl' 500 \ + .cause="unexpected end of JSON input" + +# Prune containers - bad filter input +t POST containers/prune?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" +t POST libpod/containers/prune?filters='garb1age}' 500 \ + .cause="invalid character 'g' looking for beginning of value" + +## Prune containers with illformed label +t POST containers/prune?filters='{"label":["tes' 500 \ + .cause="unexpected end of JSON input" +t POST libpod/containers/prune?filters='{"label":["tes' 500 \ + .cause="unexpected end of JSON input" + +t GET libpod/containers/json?filters='{"label":["testlabel"]}' 200 length=0 + # Test CPU limit (NanoCPUs) t POST containers/create Image=$IMAGE HostConfig='{"NanoCpus":500000}' 201 \ .Id~[0-9a-f]\\{64\\} -- cgit v1.2.3-54-g00ecf From 670e1ac67c3e0afbd583dd7fda5fa94f373c1a86 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Fri, 26 Mar 2021 15:47:48 -0400 Subject: [NO TESTS NEEDED] Turn on podman-remote build --isolation Currently podman only works with --isolation chroot. This PR fixes this by allowing the isolation mode to default to OCI and to also allow users to pass the isolation mode into the containers. The current tests for --isolation should cause this code to be tested. Signed-off-by: Daniel J Walsh --- cmd/podman/images/build.go | 17 ++++++++--------- pkg/api/handlers/compat/images_build.go | 8 ++------ 2 files changed, 10 insertions(+), 15 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/images/build.go b/cmd/podman/images/build.go index 7db927e55..da6d556b1 100644 --- a/cmd/podman/images/build.go +++ b/cmd/podman/images/build.go @@ -8,7 +8,6 @@ import ( "strings" "time" - "github.com/containers/buildah" "github.com/containers/buildah/define" buildahCLI "github.com/containers/buildah/pkg/cli" "github.com/containers/buildah/pkg/parse" @@ -159,11 +158,11 @@ func buildFlags(cmd *cobra.Command) { flags.SetNormalizeFunc(buildahCLI.AliasFlags) if registry.IsRemote() { flag = flags.Lookup("isolation") - buildOpts.Isolation = buildah.OCI - if err := flag.Value.Set(buildah.OCI); err != nil { - logrus.Errorf("unable to set --isolation to %v: %v", buildah.OCI, err) + buildOpts.Isolation = define.OCI + if err := flag.Value.Set(define.OCI); err != nil { + logrus.Errorf("unable to set --isolation to %v: %v", define.OCI, err) } - flag.DefValue = buildah.OCI + flag.DefValue = define.OCI _ = flags.MarkHidden("disable-content-trust") _ = flags.MarkHidden("cache-from") _ = flags.MarkHidden("sign-by") @@ -427,10 +426,10 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil format := "" flags.Format = strings.ToLower(flags.Format) switch { - case strings.HasPrefix(flags.Format, buildah.OCI): - format = buildah.OCIv1ImageManifest - case strings.HasPrefix(flags.Format, buildah.DOCKER): - format = buildah.Dockerv2ImageManifest + case strings.HasPrefix(flags.Format, define.OCI): + format = define.OCIv1ImageManifest + case strings.HasPrefix(flags.Format, define.DOCKER): + format = define.Dockerv2ImageManifest default: return nil, errors.Errorf("unrecognized image type %q", flags.Format) } diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index 20331600d..36785a362 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -201,13 +201,9 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { } format := buildah.Dockerv2ImageManifest registry := query.Registry - isolation := buildah.IsolationChroot - /* - // FIXME, This is very broken. Buildah will only work with chroot - isolation := buildah.IsolationDefault - */ + isolation := buildah.IsolationDefault if utils.IsLibpodRequest(r) { - // isolation = parseLibPodIsolation(query.Isolation) + isolation = parseLibPodIsolation(query.Isolation) registry = "" format = query.OutputFormat } else { -- cgit v1.2.3-54-g00ecf