From 835d74ac63747e4e97b134d57a81679d41295310 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Thu, 30 Sep 2021 12:09:48 -0600 Subject: sdnotify test: accept MAINPID anywhere systemd sometimes spits out lines in the wrong order. Deal with it. This fixes an infrequent flake that I haven't filed because I didn't understand it well enough. (Hence, this reduces BUGS but does not reduce BUG COUNT. Sorry!) Signed-off-by: Ed Santiago --- test/system/260-sdnotify.bats | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'test') diff --git a/test/system/260-sdnotify.bats b/test/system/260-sdnotify.bats index 0dae569a8..395e6f94f 100644 --- a/test/system/260-sdnotify.bats +++ b/test/system/260-sdnotify.bats @@ -70,7 +70,7 @@ function _stop_socat() { # Check that MAINPID=xxxxx points to a running conmon process function _assert_mainpid_is_conmon() { - local mainpid=$(expr "$1" : "MAINPID=\([0-9]\+\)") + local mainpid=$(expr "$1" : ".*MAINPID=\([0-9]\+\)") test -n "$mainpid" || die "Could not parse '$1' as 'MAINPID=nnnn'" test -d /proc/$mainpid || die "sdnotify MAINPID=$mainpid - but /proc/$mainpid does not exist" @@ -121,7 +121,7 @@ function _assert_mainpid_is_conmon() { # we look for READY=1 _anywhere_ in the output, not just the last line. is "$output" ".*READY=1.*" "sdnotify sent READY=1" - _assert_mainpid_is_conmon "${lines[0]}" + _assert_mainpid_is_conmon "$output" # Done. Stop container, clean up. run_podman exec $cid touch /stop @@ -163,7 +163,7 @@ function _assert_mainpid_is_conmon() { is "$output" ".*READY=1" "received READY=1 through notify socket" - _assert_mainpid_is_conmon "${lines[0]}" + _assert_mainpid_is_conmon "$output" # Done. Stop container, clean up. run_podman exec $cid touch /stop -- cgit v1.2.3-54-g00ecf From 2cd206d0fce477d123ab8b7acf19fdf34ca0a7aa Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 5 Oct 2021 11:13:49 +0200 Subject: libpod: fix race when closing STDIN There is a race where `conn.Close()` was called before `conn.CloseWrite()`. In this case `CloseWrite` will fail and an useless error is printed. To fix this we move the the `CloseWrite()` call to the same goroutine to remove the race. This ensures that `CloseWrite()` is called before `Close()` and never afterwards. Also fixed podman-remote run where the STDIN was never was closed. This is causing flakes in CI testing. [NO TESTS NEEDED] Fixes #11856 Signed-off-by: Paul Holzinger Signed-off-by: Matthew Heon --- libpod/oci_attach_linux.go | 17 +++++++++-------- libpod/oci_conmon_exec_linux.go | 7 ++++--- libpod/oci_conmon_linux.go | 4 ++++ pkg/bindings/containers/attach.go | 14 +++++++------- test/system/030-run.bats | 6 ++++++ 5 files changed, 30 insertions(+), 18 deletions(-) (limited to 'test') diff --git a/libpod/oci_attach_linux.go b/libpod/oci_attach_linux.go index de435b58a..702628aa8 100644 --- a/libpod/oci_attach_linux.go +++ b/libpod/oci_attach_linux.go @@ -84,7 +84,7 @@ func (c *Container) attach(streams *define.AttachStreams, keys string, resize <- if attachRdy != nil { attachRdy <- true } - return readStdio(streams, receiveStdoutError, stdinDone) + return readStdio(conn, streams, receiveStdoutError, stdinDone) } // Attach to the given container's exec session @@ -165,7 +165,7 @@ func (c *Container) attachToExec(streams *define.AttachStreams, keys *string, se return err } - return readStdio(streams, receiveStdoutError, stdinDone) + return readStdio(conn, streams, receiveStdoutError, stdinDone) } func processDetachKeys(keys string) ([]byte, error) { @@ -208,11 +208,6 @@ func setupStdioChannels(streams *define.AttachStreams, conn *net.UnixConn, detac var err error if streams.AttachInput { _, err = utils.CopyDetachable(conn, streams.InputStream, detachKeys) - if err == nil { - if connErr := conn.CloseWrite(); connErr != nil { - logrus.Errorf("unable to close conn: %q", connErr) - } - } } stdinDone <- err }() @@ -265,7 +260,7 @@ func redirectResponseToOutputStreams(outputStream, errorStream io.Writer, writeO return err } -func readStdio(streams *define.AttachStreams, receiveStdoutError, stdinDone chan error) error { +func readStdio(conn *net.UnixConn, streams *define.AttachStreams, receiveStdoutError, stdinDone chan error) error { var err error select { case err = <-receiveStdoutError: @@ -274,6 +269,12 @@ func readStdio(streams *define.AttachStreams, receiveStdoutError, stdinDone chan if err == define.ErrDetach { return err } + if err == nil { + // copy stdin is done, close it + if connErr := conn.CloseWrite(); connErr != nil { + logrus.Errorf("Unable to close conn: %v", connErr) + } + } if streams.AttachOutput || streams.AttachError { return <-receiveStdoutError } diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go index 5a7677b04..553c91833 100644 --- a/libpod/oci_conmon_exec_linux.go +++ b/libpod/oci_conmon_exec_linux.go @@ -609,9 +609,6 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp _, err := utils.CopyDetachable(conn, httpBuf, detachKeys) logrus.Debugf("STDIN copy completed") stdinChan <- err - if connErr := conn.CloseWrite(); connErr != nil { - logrus.Errorf("Unable to close conn: %v", connErr) - } }() } @@ -654,6 +651,10 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp if err != nil { return err } + // copy stdin is done, close it + if connErr := conn.CloseWrite(); connErr != nil { + logrus.Errorf("Unable to close conn: %v", connErr) + } case <-cancel: return nil } diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index c2b472f76..58aa3557a 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -699,6 +699,10 @@ func (r *ConmonOCIRuntime) HTTPAttach(ctr *Container, req *http.Request, w http. if err != nil { return err } + // copy stdin is done, close it + if connErr := conn.CloseWrite(); connErr != nil { + logrus.Errorf("Unable to close conn: %v", connErr) + } case <-cancel: return nil } diff --git a/pkg/bindings/containers/attach.go b/pkg/bindings/containers/attach.go index 6efbcb57b..725d06648 100644 --- a/pkg/bindings/containers/attach.go +++ b/pkg/bindings/containers/attach.go @@ -157,24 +157,24 @@ func Attach(ctx context.Context, nameOrID string, stdin io.Reader, stdout io.Wri } stdoutChan := make(chan error) - stdinChan := make(chan error) + stdinChan := make(chan error, 1) //stdin channel should not block if isSet.stdin { go func() { logrus.Debugf("Copying STDIN to socket") _, err := utils.CopyDetachable(socket, stdin, detachKeysInBytes) - if err != nil && err != define.ErrDetach { logrus.Error("failed to write input to service: " + err.Error()) } - stdinChan <- err - - if closeWrite, ok := socket.(CloseWriter); ok { - if err := closeWrite.CloseWrite(); err != nil { - logrus.Warnf("Failed to close STDIN for writing: %v", err) + if err == nil { + if closeWrite, ok := socket.(CloseWriter); ok { + if err := closeWrite.CloseWrite(); err != nil { + logrus.Warnf("Failed to close STDIN for writing: %v", err) + } } } + stdinChan <- err }() } diff --git a/test/system/030-run.bats b/test/system/030-run.bats index 3d9d834b3..8640d427a 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -723,4 +723,10 @@ EOF is "$output" "Error: strconv.ParseInt: parsing \"a\": invalid syntax" } +@test "podman run closes stdin" { + random_1=$(random_string 25) + run_podman run -i --rm $IMAGE cat <<<"$random_1" + is "$output" "$random_1" "output matches STDIN" +} + # vim: filetype=sh -- cgit v1.2.3-54-g00ecf From 8b87793d4878026d5d4f734409f33fe873160115 Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Mon, 11 Oct 2021 09:11:32 -0700 Subject: Use SplitN(2) when copying env variables Environment variables whose value contained an equal sign where truncated Fixes #11891 Signed-off-by: Jhon Honce Signed-off-by: Matthew Heon --- pkg/specgen/generate/kube/kube.go | 2 +- test/e2e/play_kube_test.go | 41 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) (limited to 'test') diff --git a/pkg/specgen/generate/kube/kube.go b/pkg/specgen/generate/kube/kube.go index 27a1e5a72..7072a5821 100644 --- a/pkg/specgen/generate/kube/kube.go +++ b/pkg/specgen/generate/kube/kube.go @@ -254,7 +254,7 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener // Environment Variables envs := map[string]string{} for _, env := range imageData.Config.Env { - keyval := strings.Split(env, "=") + keyval := strings.SplitN(env, "=", 2) envs[keyval[0]] = keyval[1] } diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 0d5b9d52c..c31b89650 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -18,6 +18,7 @@ import ( "github.com/containers/storage/pkg/stringid" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/onsi/gomega/format" . "github.com/onsi/gomega/gexec" "github.com/opencontainers/selinux/go-selinux" ) @@ -2722,4 +2723,44 @@ invalid kube kind exists.WaitWithDefaultTimeout() Expect(exists).To(Exit(0)) }) + + Describe("verify environment variables", func() { + var maxLength int + BeforeEach(func() { + maxLength = format.MaxLength + format.MaxLength = 0 + }) + AfterEach(func() { + format.MaxLength = maxLength + }) + + It("values containing equal sign", func() { + javaToolOptions := `-XX:+IgnoreUnrecognizedVMOptions -XX:+IdleTuningGcOnIdle -Xshareclasses:name=openj9_system_scc,cacheDir=/opt/java/.scc,readonly,nonFatal` + openj9JavaOptions := `-XX:+IgnoreUnrecognizedVMOptions -XX:+IdleTuningGcOnIdle -Xshareclasses:name=openj9_system_scc,cacheDir=/opt/java/.scc,readonly,nonFatal -Dosgi.checkConfiguration=false` + + containerfile := fmt.Sprintf(`FROM %s +ENV JAVA_TOOL_OPTIONS=%q +ENV OPENJ9_JAVA_OPTIONS=%q +`, + ALPINE, javaToolOptions, openj9JavaOptions) + + image := "podman-kube-test:env" + podmanTest.BuildImage(containerfile, image, "false") + ctnr := getCtr(withImage(image)) + pod := getPod(withCtr(ctnr)) + Expect(generateKubeYaml("pod", pod, kubeYaml)).Should(Succeed()) + + play := podmanTest.Podman([]string{"play", "kube", "--start", kubeYaml}) + play.WaitWithDefaultTimeout() + Expect(play).Should(Exit(0)) + + inspect := podmanTest.Podman([]string{"container", "inspect", "--format=json", getCtrNameInPod(pod)}) + inspect.WaitWithDefaultTimeout() + Expect(inspect).Should(Exit(0)) + + contents := string(inspect.Out.Contents()) + Expect(contents).To(ContainSubstring(javaToolOptions)) + Expect(contents).To(ContainSubstring(openj9JavaOptions)) + }) + }) }) -- cgit v1.2.3-54-g00ecf From 16fb4161a6853c047f40920adce43049cc428243 Mon Sep 17 00:00:00 2001 From: cdoern Date: Mon, 11 Oct 2021 11:01:36 -0400 Subject: Kube Gen run as user/group issues Removed the inclusion of RunAsUser or RunAsGroup unless a container is run with the --user flag. When building from an image the user will be pulled from there anyway resolves #11914 Signed-off-by: cdoern --- libpod/kube.go | 4 ++++ test/e2e/generate_kube_test.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'test') diff --git a/libpod/kube.go b/libpod/kube.go index 9e1e3ddfe..9c71c8ad6 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -485,6 +485,10 @@ func containerToV1Container(ctx context.Context, c *Container) (v1.Container, [] kubeContainer.Command = nil } + if imgData.User == c.User() { + kubeSec.RunAsGroup, kubeSec.RunAsUser = nil, nil + } + kubeContainer.WorkingDir = c.WorkingDir() kubeContainer.Ports = ports // This should not be applicable diff --git a/test/e2e/generate_kube_test.go b/test/e2e/generate_kube_test.go index cb987e139..b84977d0d 100644 --- a/test/e2e/generate_kube_test.go +++ b/test/e2e/generate_kube_test.go @@ -942,7 +942,7 @@ USER test1` pod := new(v1.Pod) err = yaml.Unmarshal(kube.Out.Contents(), pod) Expect(err).To(BeNil()) - Expect(*pod.Spec.Containers[0].SecurityContext.RunAsUser).To(Equal(int64(10001))) + Expect(pod.Spec.Containers[0].SecurityContext.RunAsUser).To(BeNil()) }) It("podman generate kube on named volume", func() { -- cgit v1.2.3-54-g00ecf From 92ed439d2252146220922bc1dd8cacfc4090a8c5 Mon Sep 17 00:00:00 2001 From: Urvashi Mohnani Date: Tue, 12 Oct 2021 16:02:47 -0400 Subject: Don't add image entrypoint to the generate kube yaml If no entrypoint or command is set in the podman create command, and the image command or entrypoint is being used as the default, then do not add the image command or entrypoint to the generated kube yaml. Kubernetes knows to default to the image command and/or entrypoint settings when not defined in the kube yaml. Add and modify tests for this case. Signed-off-by: Urvashi Mohnani --- libpod/kube.go | 4 +++- test/e2e/generate_kube_test.go | 39 ++++++++++++++++++++++++++++----------- 2 files changed, 31 insertions(+), 12 deletions(-) (limited to 'test') diff --git a/libpod/kube.go b/libpod/kube.go index 9c71c8ad6..72d2f8e96 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -481,7 +481,9 @@ func containerToV1Container(ctx context.Context, c *Container) (v1.Container, [] if err != nil { return kubeContainer, kubeVolumes, nil, annotations, err } - if reflect.DeepEqual(imgData.Config.Cmd, kubeContainer.Command) { + // If the user doesn't set a command/entrypoint when creating the container with podman and + // is using the image command or entrypoint from the image, don't add it to the generated kube yaml + if reflect.DeepEqual(imgData.Config.Cmd, kubeContainer.Command) || reflect.DeepEqual(imgData.Config.Entrypoint, kubeContainer.Command) { kubeContainer.Command = nil } diff --git a/test/e2e/generate_kube_test.go b/test/e2e/generate_kube_test.go index b84977d0d..8f3d5027d 100644 --- a/test/e2e/generate_kube_test.go +++ b/test/e2e/generate_kube_test.go @@ -803,7 +803,7 @@ var _ = Describe("Podman generate kube", func() { Expect(containers[0].Args).To(Equal([]string{"10s"})) }) - It("podman generate kube - no command", func() { + It("podman generate kube - use command from image unless explicitly set in the podman command", func() { session := podmanTest.Podman([]string{"create", "--name", "test", ALPINE}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) @@ -812,8 +812,8 @@ var _ = Describe("Podman generate kube", func() { kube.WaitWithDefaultTimeout() Expect(kube).Should(Exit(0)) - // Now make sure that the container's command is not set to the - // entrypoint and it's arguments to "10s". + // Now make sure that the container's command in the kube yaml is not set to the + // image command. pod := new(v1.Pod) err := yaml.Unmarshal(kube.Out.Contents(), pod) Expect(err).To(BeNil()) @@ -831,8 +831,8 @@ var _ = Describe("Podman generate kube", func() { kube.WaitWithDefaultTimeout() Expect(kube).Should(Exit(0)) - // Now make sure that the container's command is not set to the - // entrypoint and it's arguments to "10s". + // Now make sure that the container's command in the kube yaml is set to the + // command passed via the cli to podman create. pod = new(v1.Pod) err = yaml.Unmarshal(kube.Out.Contents(), pod) Expect(err).To(BeNil()) @@ -842,10 +842,10 @@ var _ = Describe("Podman generate kube", func() { Expect(containers[0].Command).To(Equal(cmd)) }) - It("podman generate kube - use entrypoint from image", func() { + It("podman generate kube - use entrypoint from image unless --entrypoint is set", func() { // Build an image with an entrypoint. containerfile := `FROM quay.io/libpod/alpine:latest -ENTRYPOINT /bin/sleep` +ENTRYPOINT ["sleep"]` targetPath, err := CreateTempDirInTempDir() Expect(err).To(BeNil()) @@ -866,17 +866,34 @@ ENTRYPOINT /bin/sleep` kube.WaitWithDefaultTimeout() Expect(kube).Should(Exit(0)) - // Now make sure that the container's command is set to the - // entrypoint and it's arguments to "10s". + // Now make sure that the container's command in the kube yaml is NOT set to the + // entrypoint but the arguments should be set to "10s". pod := new(v1.Pod) err = yaml.Unmarshal(kube.Out.Contents(), pod) Expect(err).To(BeNil()) containers := pod.Spec.Containers Expect(len(containers)).To(Equal(1)) - - Expect(containers[0].Command).To(Equal([]string{"/bin/sh", "-c", "/bin/sleep"})) Expect(containers[0].Args).To(Equal([]string{"10s"})) + + session = podmanTest.Podman([]string{"create", "--pod", "new:testpod-2", "--entrypoint", "echo", image, "hello"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + kube = podmanTest.Podman([]string{"generate", "kube", "testpod-2"}) + kube.WaitWithDefaultTimeout() + Expect(kube).Should(Exit(0)) + + // Now make sure that the container's command in the kube yaml is set to the + // entrypoint defined by the --entrypoint flag and the arguments should be set to "hello". + pod = new(v1.Pod) + err = yaml.Unmarshal(kube.Out.Contents(), pod) + Expect(err).To(BeNil()) + + containers = pod.Spec.Containers + Expect(len(containers)).To(Equal(1)) + Expect(containers[0].Command).To(Equal([]string{"echo"})) + Expect(containers[0].Args).To(Equal([]string{"hello"})) }) It("podman generate kube - --privileged container", func() { -- cgit v1.2.3-54-g00ecf From a213661ae7af22038465c8a61d97bde211674996 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 14 Oct 2021 12:56:44 +0200 Subject: Fix panic in container create compat api The bind and tmpfs options can be nil, we have to check that before we try to use it. Fixes #11961 Signed-off-by: Paul Holzinger --- cmd/podman/common/create_opts.go | 13 ++++++++----- test/apiv2/20-containers.at | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) (limited to 'test') diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go index 09ac61f2e..50d7c446d 100644 --- a/cmd/podman/common/create_opts.go +++ b/cmd/podman/common/create_opts.go @@ -104,15 +104,18 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, rtc *c addField(&builder, "target", m.Target) addField(&builder, "ro", strconv.FormatBool(m.ReadOnly)) addField(&builder, "consistency", string(m.Consistency)) - // Map any specialized mount options that intersect between *Options and cli options switch m.Type { case mount.TypeBind: - addField(&builder, "bind-propagation", string(m.BindOptions.Propagation)) - addField(&builder, "bind-nonrecursive", strconv.FormatBool(m.BindOptions.NonRecursive)) + if m.BindOptions != nil { + addField(&builder, "bind-propagation", string(m.BindOptions.Propagation)) + addField(&builder, "bind-nonrecursive", strconv.FormatBool(m.BindOptions.NonRecursive)) + } case mount.TypeTmpfs: - addField(&builder, "tmpfs-size", strconv.FormatInt(m.TmpfsOptions.SizeBytes, 10)) - addField(&builder, "tmpfs-mode", strconv.FormatUint(uint64(m.TmpfsOptions.Mode), 10)) + if m.TmpfsOptions != nil { + addField(&builder, "tmpfs-size", strconv.FormatInt(m.TmpfsOptions.SizeBytes, 10)) + addField(&builder, "tmpfs-mode", strconv.FormatUint(uint64(m.TmpfsOptions.Mode), 10)) + } case mount.TypeVolume: // All current VolumeOpts are handled above // See vendor/github.com/containers/common/pkg/parse/parse.go:ValidateVolumeOpts() diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at index afff68c22..748a0750f 100644 --- a/test/apiv2/20-containers.at +++ b/test/apiv2/20-containers.at @@ -379,3 +379,21 @@ t GET containers/$cid/json 200 \ .HostConfig.Tmpfs['"/mnt/scratch"']~.*mode=755.* t DELETE containers/$cid?v=true 204 + +# compat api: tmpfs without mount options +payload='{"Mounts":[{"Type":"tmpfs","Target":"/mnt/scratch"}]}' +t POST containers/create Image=$IMAGE HostConfig="$payload" 201 .Id~[0-9a-f]\\{64\\} +cid=$(jq -r '.Id' <<<"$output") +t GET containers/$cid/json 200 \ + .HostConfig.Tmpfs['"/mnt/scratch"']~.*tmpcopyup.* \ + +t DELETE containers/$cid?v=true 204 + +# compat api: bind mount without mount options +payload='{"Mounts":[{"Type":"bind","Source":"/tmp","Target":"/mnt"}]}' +t POST containers/create Image=$IMAGE HostConfig="$payload" 201 .Id~[0-9a-f]\\{64\\} +cid=$(jq -r '.Id' <<<"$output") +t GET containers/$cid/json 200 \ + .HostConfig.Binds[0]~/tmp:/mnt:.* \ + +t DELETE containers/$cid?v=true 204 -- cgit v1.2.3-54-g00ecf From 9168db8bc0b1158389801c2fa53563a4ced3aea5 Mon Sep 17 00:00:00 2001 From: Urvashi Mohnani Date: Thu, 14 Oct 2021 15:03:18 -0400 Subject: Do not add TCP to protocol in generated kube yaml As the default protocol in k8s is TCP, don't add it to the generate yaml when using protocol. Add UDP to the protocol of the generated yaml when udp is being used. Add tests for this as well. Signed-off-by: Urvashi Mohnani --- libpod/kube.go | 3 ++- test/e2e/generate_kube_test.go | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) (limited to 'test') diff --git a/libpod/kube.go b/libpod/kube.go index 72d2f8e96..ad70dd2cf 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -578,7 +578,8 @@ func ocicniPortMappingToContainerPort(portMappings []ocicni.PortMapping) ([]v1.C var protocol v1.Protocol switch strings.ToUpper(p.Protocol) { case "TCP": - protocol = v1.ProtocolTCP + // do nothing as it is the default protocol in k8s, there is no need to explicitly + // add it to the generated yaml case "UDP": protocol = v1.ProtocolUDP default: diff --git a/test/e2e/generate_kube_test.go b/test/e2e/generate_kube_test.go index 8f3d5027d..7b0ed144d 100644 --- a/test/e2e/generate_kube_test.go +++ b/test/e2e/generate_kube_test.go @@ -451,6 +451,10 @@ var _ = Describe("Podman generate kube", func() { foundOtherPort := 0 for _, ctr := range pod.Spec.Containers { for _, port := range ctr.Ports { + // Since we are using tcp here, the generated kube yaml shouldn't + // have anything for protocol under the ports as tcp is the default + // for k8s + Expect(port.Protocol).To(BeEmpty()) if port.HostPort == 4000 { foundPort4000 = foundPort4000 + 1 } else if port.HostPort == 5000 { @@ -463,6 +467,24 @@ var _ = Describe("Podman generate kube", func() { Expect(foundPort4000).To(Equal(1)) Expect(foundPort5000).To(Equal(1)) Expect(foundOtherPort).To(Equal(0)) + + // Create container with UDP port and check the generated kube yaml + ctrWithUDP := podmanTest.Podman([]string{"create", "--pod", "new:test-pod", "-p", "6666:66/udp", ALPINE, "top"}) + ctrWithUDP.WaitWithDefaultTimeout() + Expect(ctrWithUDP).Should(Exit(0)) + + kube = podmanTest.Podman([]string{"generate", "kube", "test-pod"}) + kube.WaitWithDefaultTimeout() + Expect(kube).Should(Exit(0)) + + pod = new(v1.Pod) + err = yaml.Unmarshal(kube.Out.Contents(), pod) + Expect(err).To(BeNil()) + + containers := pod.Spec.Containers + Expect(len(containers)).To(Equal(1)) + Expect(len(containers[0].Ports)).To(Equal(1)) + Expect(containers[0].Ports[0].Protocol).To(Equal(v1.ProtocolUDP)) }) It("podman generate and reimport kube on pod", func() { -- cgit v1.2.3-54-g00ecf From 7923bfcb0dcf645cf4c53124c87eaa933b7bd30c Mon Sep 17 00:00:00 2001 From: Chris Evich Date: Thu, 14 Oct 2021 14:32:20 -0400 Subject: Test-hang fix: Wait for ready + timeout on connect. It was observed during initial F35 testing, this test can cause Ginkgo to "hang" by attempting to connect before the redis is up/listening. Fix this by confirming the ready-state before attempting to connect. Also, force IPv4 and timeout on any connection fault - to allow other tests to run. Thanks to Adrian Reber for help on this and related fixes. Signed-off-by: Chris Evich --- test/e2e/checkpoint_test.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'test') diff --git a/test/e2e/checkpoint_test.go b/test/e2e/checkpoint_test.go index 403d739f0..e88198f53 100644 --- a/test/e2e/checkpoint_test.go +++ b/test/e2e/checkpoint_test.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "strings" + "time" "github.com/containers/podman/v3/pkg/checkpoint/crutils" "github.com/containers/podman/v3/pkg/criu" @@ -247,16 +248,19 @@ var _ = Describe("Podman checkpoint", func() { session := podmanTest.Podman(localRunString) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) + cid := session.OutputToString() + if !WaitContainerReady(podmanTest, cid, "Ready to accept connections", 20, 1) { + Fail("Container failed to get ready") + } IP := podmanTest.Podman([]string{"inspect", "-l", "--format={{.NetworkSettings.IPAddress}}"}) IP.WaitWithDefaultTimeout() Expect(IP).Should(Exit(0)) // Open a network connection to the redis server - conn, err := net.Dial("tcp", IP.OutputToString()+":6379") - if err != nil { - os.Exit(1) - } + conn, err := net.DialTimeout("tcp4", IP.OutputToString()+":6379", time.Duration(3)*time.Second) + Expect(err).To(BeNil()) + // This should fail as the container has established TCP connections result := podmanTest.Podman([]string{"container", "checkpoint", "-l"}) result.WaitWithDefaultTimeout() -- cgit v1.2.3-54-g00ecf From d39e4128342635a9385b29f3be64c69a4ce0ea32 Mon Sep 17 00:00:00 2001 From: Urvashi Mohnani Date: Thu, 14 Oct 2021 16:58:27 -0400 Subject: Set targetPort to the port value in the kube yaml When the targetPort is not defined, it is supposed to be set to the port value according to the k8s docs. Add tests for targetPort. Update tests to be able to check the Service yaml that is generated. Signed-off-by: Urvashi Mohnani --- libpod/kube.go | 10 +++++---- test/e2e/generate_kube_test.go | 50 ++++++++++++++++++++++++++++-------------- 2 files changed, 39 insertions(+), 21 deletions(-) (limited to 'test') diff --git a/libpod/kube.go b/libpod/kube.go index ad70dd2cf..f5291ce60 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -25,6 +25,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" v12 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" ) // GenerateForKube takes a slice of libpod containers and generates @@ -196,10 +197,11 @@ func containerPortsToServicePorts(containerPorts []v1.ContainerPort) []v1.Servic for _, cp := range containerPorts { nodePort := 30000 + rand.Intn(32767-30000+1) servicePort := v1.ServicePort{ - Protocol: cp.Protocol, - Port: cp.ContainerPort, - NodePort: int32(nodePort), - Name: strconv.Itoa(int(cp.ContainerPort)), + Protocol: cp.Protocol, + Port: cp.ContainerPort, + NodePort: int32(nodePort), + Name: strconv.Itoa(int(cp.ContainerPort)), + TargetPort: intstr.Parse(strconv.Itoa(int(cp.ContainerPort))), } sps = append(sps, servicePort) } diff --git a/test/e2e/generate_kube_test.go b/test/e2e/generate_kube_test.go index 7b0ed144d..07515fe7b 100644 --- a/test/e2e/generate_kube_test.go +++ b/test/e2e/generate_kube_test.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "strconv" + "strings" "github.com/containers/podman/v3/libpod/define" @@ -119,20 +120,28 @@ var _ = Describe("Podman generate kube", func() { Expect(kube.OutputToString()).To(ContainSubstring("type: foo_bar_t")) }) - It("podman generate service kube on container", func() { - session := podmanTest.RunTopContainer("top") + It("podman generate service kube on container - targetPort should match port name", func() { + session := podmanTest.Podman([]string{"create", "--name", "test-ctr", "-p", "3890:3890", ALPINE, "ls"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - kube := podmanTest.Podman([]string{"generate", "kube", "-s", "top"}) + kube := podmanTest.Podman([]string{"generate", "kube", "-s", "test-ctr"}) kube.WaitWithDefaultTimeout() Expect(kube).Should(Exit(0)) - // TODO - test generated YAML - service produces multiple - // structs. - // pod := new(v1.Pod) - // err := yaml.Unmarshal([]byte(kube.OutputToString()), pod) - // Expect(err).To(BeNil()) + // Separate out the Service and Pod yaml + arr := strings.Split(string(kube.Out.Contents()), "---") + Expect(len(arr)).To(Equal(2)) + + svc := new(v1.Service) + err := yaml.Unmarshal([]byte(arr[0]), svc) + Expect(err).To(BeNil()) + Expect(len(svc.Spec.Ports)).To(Equal(1)) + Expect(svc.Spec.Ports[0].TargetPort.IntValue()).To(Equal(3890)) + + pod := new(v1.Pod) + err = yaml.Unmarshal([]byte(arr[1]), pod) + Expect(err).To(BeNil()) }) It("podman generate kube on pod", func() { @@ -315,21 +324,28 @@ var _ = Describe("Podman generate kube", func() { }) It("podman generate service kube on pod", func() { - _, rc, _ := podmanTest.CreatePod(map[string][]string{"--name": {"toppod"}}) - Expect(rc).To(Equal(0)) - - session := podmanTest.RunTopContainerInPod("topcontainer", "toppod") + session := podmanTest.Podman([]string{"create", "--pod", "new:test-pod", "-p", "4000:4000/udp", ALPINE, "ls"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - kube := podmanTest.Podman([]string{"generate", "kube", "-s", "toppod"}) + kube := podmanTest.Podman([]string{"generate", "kube", "-s", "test-pod"}) kube.WaitWithDefaultTimeout() Expect(kube).Should(Exit(0)) - // TODO: How do we test unmarshal with a service? We have two - // structs that need to be unmarshalled... - // _, err := yaml.Marshal(kube.OutputToString()) - // Expect(err).To(BeNil()) + // Separate out the Service and Pod yaml + arr := strings.Split(string(kube.Out.Contents()), "---") + Expect(len(arr)).To(Equal(2)) + + svc := new(v1.Service) + err := yaml.Unmarshal([]byte(arr[0]), svc) + Expect(err).To(BeNil()) + Expect(len(svc.Spec.Ports)).To(Equal(1)) + Expect(svc.Spec.Ports[0].TargetPort.IntValue()).To(Equal(4000)) + Expect(svc.Spec.Ports[0].Protocol).To(Equal(v1.ProtocolUDP)) + + pod := new(v1.Pod) + err = yaml.Unmarshal([]byte(arr[1]), pod) + Expect(err).To(BeNil()) }) It("podman generate kube on pod with restartPolicy", func() { -- cgit v1.2.3-54-g00ecf From 5aa89c88f9da4d0076196990a3b9b627fe7c2d60 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Wed, 13 Oct 2021 12:59:01 +0000 Subject: Checkpoint/Restore test fixes Moving to Fedora 35 showed test failures (time outs) in the test "podman checkpoint and restore container with different port mappings" The test starts a container and maps the internal port 6379 to the local port 1234 ('-p 1234:6379') and then tries to connect to localhost:1234 On Fedora 35 this failed and blocked the test because the container was not yet ready. The test was trying to connect to localhost:1234 but nothing was running there. So the error was not checkpointing related. Before trying to connect to the container the test is now waiting for the container to be ready. Another problem with this test and running ginkgo in parallel was that it was possible that the port was already in use. Now for each run a random port is selected to decrease the chance of collisions. Signed-off-by: Adrian Reber --- test/e2e/checkpoint_test.go | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) (limited to 'test') diff --git a/test/e2e/checkpoint_test.go b/test/e2e/checkpoint_test.go index e88198f53..0d89772f6 100644 --- a/test/e2e/checkpoint_test.go +++ b/test/e2e/checkpoint_test.go @@ -11,6 +11,7 @@ import ( "github.com/containers/podman/v3/pkg/checkpoint/crutils" "github.com/containers/podman/v3/pkg/criu" . "github.com/containers/podman/v3/test/utils" + "github.com/containers/podman/v3/utils" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gexec" @@ -937,18 +938,23 @@ var _ = Describe("Podman checkpoint", func() { }) It("podman checkpoint and restore container with different port mappings", func() { - localRunString := getRunString([]string{"-p", "1234:6379", "--rm", redis}) + randomPort, err := utils.GetRandomPort() + Expect(err).ShouldNot(HaveOccurred()) + localRunString := getRunString([]string{"-p", fmt.Sprintf("%d:6379", randomPort), "--rm", redis}) session := podmanTest.Podman(localRunString) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) cid := session.OutputToString() fileName := "/tmp/checkpoint-" + cid + ".tar.gz" - // Open a network connection to the redis server via initial port mapping - conn, err := net.Dial("tcp", "localhost:1234") - if err != nil { - os.Exit(1) + if !WaitContainerReady(podmanTest, cid, "Ready to accept connections", 20, 1) { + Fail("Container failed to get ready") } + + fmt.Fprintf(os.Stderr, "Trying to connect to redis server at localhost:%d", randomPort) + // Open a network connection to the redis server via initial port mapping + conn, err := net.DialTimeout("tcp4", fmt.Sprintf("localhost:%d", randomPort), time.Duration(3)*time.Second) + Expect(err).ShouldNot(HaveOccurred()) conn.Close() // Checkpoint the container @@ -962,7 +968,9 @@ var _ = Describe("Podman checkpoint", func() { Expect(podmanTest.NumberOfContainers()).To(Equal(0)) // Restore container with different port mapping - result = podmanTest.Podman([]string{"container", "restore", "-p", "1235:6379", "-i", fileName}) + newRandomPort, err := utils.GetRandomPort() + Expect(err).ShouldNot(HaveOccurred()) + result = podmanTest.Podman([]string{"container", "restore", "-p", fmt.Sprintf("%d:6379", newRandomPort), "-i", fileName}) result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) @@ -971,13 +979,12 @@ var _ = Describe("Podman checkpoint", func() { // Open a network connection to the redis server via initial port mapping // This should fail - conn, err = net.Dial("tcp", "localhost:1234") + conn, err = net.DialTimeout("tcp4", fmt.Sprintf("localhost:%d", randomPort), time.Duration(3)*time.Second) Expect(err.Error()).To(ContainSubstring("connection refused")) // Open a network connection to the redis server via new port mapping - conn, err = net.Dial("tcp", "localhost:1235") - if err != nil { - os.Exit(1) - } + fmt.Fprintf(os.Stderr, "Trying to reconnect to redis server at localhost:%d", newRandomPort) + conn, err = net.DialTimeout("tcp4", fmt.Sprintf("localhost:%d", newRandomPort), time.Duration(3)*time.Second) + Expect(err).ShouldNot(HaveOccurred()) conn.Close() result = podmanTest.Podman([]string{"rm", "-fa"}) -- cgit v1.2.3-54-g00ecf From 437ec951d9c02e2b48857cad3aaba4e34c70c3e5 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Mon, 18 Oct 2021 08:47:15 -0600 Subject: system tests: socket activation: clean up Multiarch folks are seeing flakes in this test. I can't reproduce them, but I did notice that the test isn't doing the best possible job of reporting failures nor of confirming what it purports to test. Major fix here is to check the exit status of each curl: if we see the flake again, that will help us track down the failure. Other fixes are just refactoring, cleanup, and disambiguation (using the random service name consistently) Signed-off-by: Ed Santiago --- test/system/270-socket-activation.bats | 51 +++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 20 deletions(-) (limited to 'test') diff --git a/test/system/270-socket-activation.bats b/test/system/270-socket-activation.bats index dd439d3ae..6d582be18 100644 --- a/test/system/270-socket-activation.bats +++ b/test/system/270-socket-activation.bats @@ -8,14 +8,16 @@ load helpers.systemd SERVICE_NAME="podman_test_$(random_string)" -SERVICE_SOCK_ADDR="/run/podman/podman.sock" +SERVICE_SOCK_ADDR="/run/podman/$SERVICE_NAME.sock" if is_rootless; then - SERVICE_SOCK_ADDR="$XDG_RUNTIME_DIR/podman/podman.sock" + SERVICE_SOCK_ADDR="$XDG_RUNTIME_DIR/podman/$SERVICE_NAME.sock" fi SERVICE_FILE="$UNIT_DIR/$SERVICE_NAME.service" SOCKET_FILE="$UNIT_DIR/$SERVICE_NAME.socket" +# URL to use for ping +_PING=http://placeholder-hostname/libpod/_ping function setup() { skip_if_remote "systemd tests are meaningless over remote" @@ -25,8 +27,8 @@ function setup() { cat > $SERVICE_FILE < /dev/null - rm -f $pause_pid + local pause_pid_file="$XDG_RUNTIME_DIR/libpod/tmp/pause.pid" + if [ -f $pause_pid_file ]; then + kill -9 $(< $pause_pid_file) 2> /dev/null + rm -f $pause_pid_file fi fi systemctl start "$SERVICE_NAME.socket" @@ -68,7 +70,9 @@ function teardown() { } @test "podman system service - socket activation - no container" { - run curl -s --max-time 3 --unix-socket $SERVICE_SOCK_ADDR http://podman/libpod/_ping + run curl -s --max-time 3 --unix-socket $SERVICE_SOCK_ADDR $_PING + echo "curl output: $output" + is "$status" "0" "curl exit status" is "$output" "OK" "podman service responds normally" } @@ -76,29 +80,36 @@ function teardown() { run_podman run -d $IMAGE sleep 90 cid="$output" - run curl -s --max-time 3 --unix-socket $SERVICE_SOCK_ADDR http://podman/libpod/_ping + run curl -s --max-time 3 --unix-socket $SERVICE_SOCK_ADDR $_PING + echo "curl output: $output" + is "$status" "0" "curl exit status" is "$output" "OK" "podman service responds normally" - run_podman stop -t 0 $cid - run_podman rm -f $cid + run_podman rm -f -t 0 $cid } @test "podman system service - socket activation - kill rootless pause" { if ! is_rootless; then - skip "root podman no need pause process" + skip "there is no pause process when running rootful" fi run_podman run -d $IMAGE sleep 90 cid="$output" - local pause_pid="$XDG_RUNTIME_DIR/libpod/tmp/pause.pid" - if [ -f $pause_pid ]; then - kill -9 $(cat $pause_pid) 2> /dev/null + local pause_pid_file="$XDG_RUNTIME_DIR/libpod/tmp/pause.pid" + if [ ! -f $pause_pid_file ]; then + # This seems unlikely, but not impossible + die "Pause pid file does not exist: $pause_pid_file" fi - run curl -s --max-time 3 --unix-socket $SERVICE_SOCK_ADDR http://podman/libpod/_ping + + echo "kill -9 $(< pause_pid_file)" + kill -9 $(< $pause_pid_file) + + run curl -s --max-time 3 --unix-socket $SERVICE_SOCK_ADDR $_PING + echo "curl output: $output" + is "$status" "0" "curl exit status" is "$output" "OK" "podman service responds normally" - run_podman stop -t 0 $cid - run_podman rm -f $cid + run_podman rm -f -t 0 $cid } # vim: filetype=sh -- cgit v1.2.3-54-g00ecf From f05e206bd34e189cdadbfa6c59cb3c2cd5fcc84c Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 19 Oct 2021 17:13:50 -0400 Subject: Fix test failures from backports Timeouts for `podman rm` aren't in until 4.0, so we need to remove them. Signed-off-by: Matthew Heon --- test/system/270-socket-activation.bats | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'test') diff --git a/test/system/270-socket-activation.bats b/test/system/270-socket-activation.bats index 6d582be18..0bdfbdad4 100644 --- a/test/system/270-socket-activation.bats +++ b/test/system/270-socket-activation.bats @@ -85,7 +85,7 @@ function teardown() { is "$status" "0" "curl exit status" is "$output" "OK" "podman service responds normally" - run_podman rm -f -t 0 $cid + run_podman rm -f $cid } @test "podman system service - socket activation - kill rootless pause" { @@ -109,7 +109,7 @@ function teardown() { is "$status" "0" "curl exit status" is "$output" "OK" "podman service responds normally" - run_podman rm -f -t 0 $cid + run_podman rm -f $cid } # vim: filetype=sh -- cgit v1.2.3-54-g00ecf