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 ++++ 3 files changed, 17 insertions(+), 11 deletions(-) (limited to 'libpod') 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 } -- cgit v1.2.3-54-g00ecf From 7bbf774e858fa3e535d9d22d72836459608eb095 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 6 Oct 2021 09:25:24 -0400 Subject: Allow `podman stop` to be run on Stopping containers This allows you to stop a container after a `podman stop` process started, but did not finish, stopping the container (probably an ignored stop signal, with no time to SIGKILL?). This is a very narrow case, but once you're in it the only way to recover is a `podman rm -f` of the container or extensive manual remediation (you'd have to kill the container yourself, manually, and then force a `podman ps --all --sync` to update its status from the OCI runtime). [NO NEW TESTS NEEDED] I have no idea how to verify this one - we need to test that it actually started *during* the other stop command, and that's nontrivial. Signed-off-by: Matthew Heon --- libpod/container_api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/container_api.go b/libpod/container_api.go index 637f5b686..cc845fe2e 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -184,7 +184,7 @@ func (c *Container) StopWithTimeout(timeout uint) error { return define.ErrCtrStopped } - if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning) { + if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStateStopping) { return errors.Wrapf(define.ErrCtrStateInvalid, "can only stop created or running containers. %s is in state %s", c.ID(), c.state.State.String()) } -- cgit v1.2.3-54-g00ecf From ca33df1468b79dc6dc27798a242b22926c220e6d Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 6 Oct 2021 11:17:43 -0400 Subject: Ensure `podman ps --sync` functions The backend for `ps --sync` has been nonfunctional for a long while now - probably since v2.0. It's questionable how useful the flag is in modern Podman (the original case it was intended to catch, Conmon gone via SIGKILL, should be handled now via pinging the process with a signal to ensure it's still alive) but having the ability to force a refresh of container state from the OCI runtime is still useful. Signed-off-by: Matthew Heon --- libpod/container_api.go | 2 +- libpod/oci_conmon_linux.go | 6 ++++++ pkg/ps/ps.go | 6 ++++++ 3 files changed, 13 insertions(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/container_api.go b/libpod/container_api.go index cc845fe2e..c6f459fbd 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -686,7 +686,7 @@ func (c *Container) Sync() error { // If runtime knows about the container, update its status in runtime // And then save back to disk - if c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStatePaused, define.ContainerStateStopped) { + if c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStatePaused, define.ContainerStateStopped, define.ContainerStateStopping) { oldState := c.state.State if err := c.ociRuntime.UpdateContainerStatus(c); err != nil { return err diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 58aa3557a..ea0ef842d 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -351,6 +351,12 @@ func (r *ConmonOCIRuntime) UpdateContainerStatus(ctr *Container) error { return ctr.handleExitFile(exitFile, fi) } + // Handle ContainerStateStopping - keep it unless the container + // transitioned to no longer running. + if oldState == define.ContainerStateStopping && (ctr.state.State == define.ContainerStatePaused || ctr.state.State == define.ContainerStateRunning) { + ctr.state.State = define.ContainerStateStopping + } + return nil } diff --git a/pkg/ps/ps.go b/pkg/ps/ps.go index e65400555..21ce818cb 100644 --- a/pkg/ps/ps.go +++ b/pkg/ps/ps.go @@ -127,6 +127,12 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities ) batchErr := ctr.Batch(func(c *libpod.Container) error { + if opts.Sync { + if err := c.Sync(); err != nil { + return errors.Wrapf(err, "unable to update container state from OCI runtime") + } + } + conConfig = c.Config() conState, err = c.State() if err != nil { -- cgit v1.2.3-54-g00ecf From 3082ba8b7b4489ec0be3886450b2681e16aca17e Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Tue, 12 Oct 2021 12:35:55 -0500 Subject: No space in kube annotations for bind mounts Kubernetes fails to deal with an annotation that has a space in it. Trim these strings to remove spaces. Fixes: #11929 Signed-off-by: Brent Baude [NO TESTS NEEDED] --- libpod/kube.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'libpod') diff --git a/libpod/kube.go b/libpod/kube.go index cb97eb5ab..9e1e3ddfe 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -246,7 +246,7 @@ func (p *Pod) podWithContainers(ctx context.Context, containers []*Container, po return nil, err } for k, v := range annotations { - podAnnotations[define.BindMountPrefix+k] = v + podAnnotations[define.BindMountPrefix+k] = strings.TrimSpace(v) } // Since port bindings for the pod are handled by the // infra container, wipe them here. @@ -366,7 +366,7 @@ func simplePodWithV1Containers(ctx context.Context, ctrs []*Container) (*v1.Pod, return nil, err } for k, v := range annotations { - kubeAnnotations[define.BindMountPrefix+k] = v + kubeAnnotations[define.BindMountPrefix+k] = strings.TrimSpace(v) } if isInit { kubeInitCtrs = append(kubeInitCtrs, kubeCtr) -- 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 'libpod') 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 'libpod') 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 b5dd62f31558caeb3c8cb4b83c0563db9f07cf20 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Thu, 14 Oct 2021 10:24:02 -0400 Subject: Don't use docker/pkg/archive, use containers/storage/pkg/archive [NO NEW TESTS NEEDED] Signed-off-by: Daniel J Walsh --- cmd/podman/diff/diff.go | 2 +- libpod/container_copy_linux.go | 2 +- pkg/machine/pull.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'libpod') diff --git a/cmd/podman/diff/diff.go b/cmd/podman/diff/diff.go index 81bbb6c43..fba4ea540 100644 --- a/cmd/podman/diff/diff.go +++ b/cmd/podman/diff/diff.go @@ -8,7 +8,7 @@ import ( "github.com/containers/common/pkg/report" "github.com/containers/podman/v3/cmd/podman/registry" "github.com/containers/podman/v3/pkg/domain/entities" - "github.com/docker/docker/pkg/archive" + "github.com/containers/storage/pkg/archive" "github.com/pkg/errors" "github.com/spf13/cobra" ) diff --git a/libpod/container_copy_linux.go b/libpod/container_copy_linux.go index 7d4dd0d46..954d54a1d 100644 --- a/libpod/container_copy_linux.go +++ b/libpod/container_copy_linux.go @@ -15,8 +15,8 @@ import ( "github.com/containers/buildah/util" "github.com/containers/podman/v3/libpod/define" "github.com/containers/podman/v3/pkg/rootless" + "github.com/containers/storage/pkg/archive" "github.com/containers/storage/pkg/idtools" - "github.com/docker/docker/pkg/archive" "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" "github.com/sirupsen/logrus" diff --git a/pkg/machine/pull.go b/pkg/machine/pull.go index f79ac6ec4..3c8422a30 100644 --- a/pkg/machine/pull.go +++ b/pkg/machine/pull.go @@ -15,7 +15,7 @@ import ( "time" "github.com/containers/image/v5/pkg/compression" - "github.com/docker/docker/pkg/archive" + "github.com/containers/storage/pkg/archive" "github.com/sirupsen/logrus" "github.com/vbauerster/mpb/v6" "github.com/vbauerster/mpb/v6/decor" -- 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 'libpod') 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 c135ff76d2665b48133c6813d0302e12939d4d9e Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Thu, 14 Oct 2021 14:08:51 -0400 Subject: Don't include ctr.log if not using file logging Checkpoint is blowing up when you use --log-driver=none [NO NEW TESTS NEEDED] No way currently to test checkpoint restore. Fixes: https://github.com/containers/podman/issues/11974 Signed-off-by: Daniel J Walsh --- libpod/container_internal_linux.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 89287efc9..310110679 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -974,12 +974,15 @@ func (c *Container) exportCheckpoint(options ContainerCheckpointOptions) error { includeFiles := []string{ "artifacts", - "ctr.log", metadata.ConfigDumpFile, metadata.SpecDumpFile, metadata.NetworkStatusFile, } + if c.LogDriver() == define.KubernetesLogging || + c.LogDriver() == define.JSONLogging { + includeFiles = append(includeFiles, "ctr.log") + } if options.PreCheckPoint { includeFiles = append(includeFiles, preCheckpointDir) } else { -- 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 'libpod') 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