From 26b3357661ea2360b27439a2e0748b0c66d8ac87 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 8 Dec 2020 15:57:19 -0500 Subject: [PATCH 1/2] Correct port range logic for port generation The existing logic (Range > 0) always triggered, because range is guaranteed to be at least 1 (a single port has a range of 1, a two port range (e.g. 80-81) has a range of 2, and so on). As such this could cause ports that had a host port assigned to them by the user to randomly assign one instead. Fixes #8650 Fixes #8651 Signed-off-by: Matthew Heon --- pkg/specgen/generate/ports.go | 12 ++++--- test/e2e/run_networking_test.go | 63 +++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/pkg/specgen/generate/ports.go b/pkg/specgen/generate/ports.go index 5c13c95b2c..83ded059fd 100644 --- a/pkg/specgen/generate/ports.go +++ b/pkg/specgen/generate/ports.go @@ -107,7 +107,11 @@ func parsePortMapping(portMappings []specgen.PortMapping) ([]ocicni.PortMapping, var index uint16 for index = 0; index < len; index++ { cPort := containerPort + index - hPort := hostPort + index + hPort := hostPort + // Only increment host port if it's not 0. + if hostPort != 0 { + hPort += index + } if cPort == 0 { return nil, nil, nil, errors.Errorf("container port cannot be 0") @@ -162,8 +166,8 @@ func parsePortMapping(portMappings []specgen.PortMapping) ([]ocicni.PortMapping, tempMappings, tempMapping{ mapping: cniPort, - startOfRange: port.Range > 0 && index == 0, - isInRange: port.Range > 0, + startOfRange: port.Range > 1 && index == 0, + isInRange: port.Range > 1, }, ) } @@ -183,7 +187,7 @@ func parsePortMapping(portMappings []specgen.PortMapping) ([]ocicni.PortMapping, for _, tmp := range tempMappings { p := tmp.mapping - if p.HostPort != 0 && !tmp.isInRange { + if p.HostPort != 0 { remadeMappings = append(remadeMappings, p) continue } diff --git a/test/e2e/run_networking_test.go b/test/e2e/run_networking_test.go index 3fb00a28be..b8e14530cb 100644 --- a/test/e2e/run_networking_test.go +++ b/test/e2e/run_networking_test.go @@ -97,6 +97,69 @@ var _ = Describe("Podman run networking", func() { Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostIP).To(Equal("")) }) + It("podman run -p 80-82 -p 8080:8080", func() { + name := "testctr" + session := podmanTest.Podman([]string{"create", "-t", "-p", "80-82", "-p", "8080:8080", "--name", name, ALPINE, "/bin/sh"}) + session.WaitWithDefaultTimeout() + inspectOut := podmanTest.InspectContainer(name) + Expect(len(inspectOut)).To(Equal(1)) + Expect(len(inspectOut[0].NetworkSettings.Ports)).To(Equal(4)) + Expect(len(inspectOut[0].NetworkSettings.Ports["80/tcp"])).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostPort).To(Not(Equal("80"))) + Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostIP).To(Equal("")) + Expect(len(inspectOut[0].NetworkSettings.Ports["81/tcp"])).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports["81/tcp"][0].HostPort).To(Not(Equal("81"))) + Expect(inspectOut[0].NetworkSettings.Ports["81/tcp"][0].HostIP).To(Equal("")) + Expect(len(inspectOut[0].NetworkSettings.Ports["82/tcp"])).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports["82/tcp"][0].HostPort).To(Not(Equal("82"))) + Expect(inspectOut[0].NetworkSettings.Ports["82/tcp"][0].HostIP).To(Equal("")) + Expect(len(inspectOut[0].NetworkSettings.Ports["8080/tcp"])).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports["8080/tcp"][0].HostPort).To(Equal("8080")) + Expect(inspectOut[0].NetworkSettings.Ports["8080/tcp"][0].HostIP).To(Equal("")) + }) + + It("podman run -p 80-81 -p 8080-8081", func() { + name := "testctr" + session := podmanTest.Podman([]string{"create", "-t", "-p", "80-81", "-p", "8080-8081", "--name", name, ALPINE, "/bin/sh"}) + session.WaitWithDefaultTimeout() + inspectOut := podmanTest.InspectContainer(name) + Expect(len(inspectOut)).To(Equal(1)) + Expect(len(inspectOut[0].NetworkSettings.Ports)).To(Equal(4)) + Expect(len(inspectOut[0].NetworkSettings.Ports["80/tcp"])).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostPort).To(Not(Equal("80"))) + Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostIP).To(Equal("")) + Expect(len(inspectOut[0].NetworkSettings.Ports["81/tcp"])).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports["81/tcp"][0].HostPort).To(Not(Equal("81"))) + Expect(inspectOut[0].NetworkSettings.Ports["81/tcp"][0].HostIP).To(Equal("")) + Expect(len(inspectOut[0].NetworkSettings.Ports["8080/tcp"])).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports["8080/tcp"][0].HostPort).To(Not(Equal("8080"))) + Expect(inspectOut[0].NetworkSettings.Ports["8080/tcp"][0].HostIP).To(Equal("")) + Expect(len(inspectOut[0].NetworkSettings.Ports["8081/tcp"])).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports["8081/tcp"][0].HostPort).To(Not(Equal("8081"))) + Expect(inspectOut[0].NetworkSettings.Ports["8081/tcp"][0].HostIP).To(Equal("")) + }) + + It("podman run -p 80 -p 8080-8082:8080-8082", func() { + name := "testctr" + session := podmanTest.Podman([]string{"create", "-t", "-p", "80", "-p", "8080-8082:8080-8082", "--name", name, ALPINE, "/bin/sh"}) + session.WaitWithDefaultTimeout() + inspectOut := podmanTest.InspectContainer(name) + Expect(len(inspectOut)).To(Equal(1)) + Expect(len(inspectOut[0].NetworkSettings.Ports)).To(Equal(4)) + Expect(len(inspectOut[0].NetworkSettings.Ports["80/tcp"])).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostPort).To(Not(Equal("80"))) + Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostIP).To(Equal("")) + Expect(len(inspectOut[0].NetworkSettings.Ports["8080/tcp"])).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports["8080/tcp"][0].HostPort).To(Equal("8080")) + Expect(inspectOut[0].NetworkSettings.Ports["8080/tcp"][0].HostIP).To(Equal("")) + Expect(len(inspectOut[0].NetworkSettings.Ports["8081/tcp"])).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports["8081/tcp"][0].HostPort).To(Equal("8081")) + Expect(inspectOut[0].NetworkSettings.Ports["8081/tcp"][0].HostIP).To(Equal("")) + Expect(len(inspectOut[0].NetworkSettings.Ports["8082/tcp"])).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports["8082/tcp"][0].HostPort).To(Equal("8082")) + Expect(inspectOut[0].NetworkSettings.Ports["8082/tcp"][0].HostIP).To(Equal("")) + }) + It("podman run -p 8080:80", func() { name := "testctr" session := podmanTest.Podman([]string{"create", "-t", "-p", "8080:80", "--name", name, ALPINE, "/bin/sh"}) From 4007554f97553055131df12f9a8d7b162d094fb8 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 9 Dec 2020 14:24:51 -0500 Subject: [PATCH 2/2] Do not pull rootless CNI infra image Instead, we want to advise users to manually build the image. We cannot distribute the existing image for RHEL 8.3.1, and the feature will be tech preview, so this degraded user experience will have to be sufficient until we can get a better solution in place. Ref: https://issues.redhat.com/browse/RUN-1127 Please note that this is a RHEL only change and should not be included in non-RHEL branches. Signed-off-by: Matthew Heon --- contrib/rootless-cni-infra/README.md | 7 ++++++- libpod/rootless_cni_linux.go | 19 +++++-------------- test/e2e/network_create_test.go | 1 + test/e2e/network_test.go | 2 ++ test/e2e/run_networking_test.go | 1 + test/system/500-networking.bats | 1 + 6 files changed, 16 insertions(+), 15 deletions(-) diff --git a/contrib/rootless-cni-infra/README.md b/contrib/rootless-cni-infra/README.md index c43b4cf491..fe4cf587dd 100644 --- a/contrib/rootless-cni-infra/README.md +++ b/contrib/rootless-cni-infra/README.md @@ -1,6 +1,11 @@ # rootless-cni-infra -Infra container for CNI-in-slirp4netns. +Infra container for CNI-in-slirp4netns. This is required for rootless CNI networking. + +To build the rootless CNI infra container image, please download both the Containerfile and `rootless-cni-infra` files to an otherwise empty directory. +Then, run `podman build -t rootless-cni-infra .` on that directory as the user who will be running rootless Podman. + +Once the image has been built, Podman will automatically use it as required to create CNI networks. ## How it works diff --git a/libpod/rootless_cni_linux.go b/libpod/rootless_cni_linux.go index 2c2977f9fb..f3ff790396 100644 --- a/libpod/rootless_cni_linux.go +++ b/libpod/rootless_cni_linux.go @@ -7,14 +7,11 @@ import ( "context" "io" "path/filepath" - "runtime" cnitypes "github.com/containernetworking/cni/pkg/types/current" "github.com/containernetworking/plugins/pkg/ns" "github.com/containers/podman/v2/libpod/define" - "github.com/containers/podman/v2/libpod/image" "github.com/containers/podman/v2/pkg/env" - "github.com/containers/podman/v2/pkg/util" "github.com/containers/storage/pkg/lockfile" "github.com/hashicorp/go-multierror" spec "github.com/opencontainers/runtime-spec/specs-go" @@ -23,11 +20,6 @@ import ( "github.com/sirupsen/logrus" ) -// Built from ../contrib/rootless-cni-infra. -var rootlessCNIInfraImage = map[string]string{ - "amd64": "quay.io/libpod/rootless-cni-infra@sha256:304742d5d221211df4ec672807a5842ff11e3729c50bc424ea0cea858f69d7b7", // 3-amd64 -} - const ( rootlessCNIInfraContainerNamespace = "podman-system" rootlessCNIInfraContainerName = "rootless-cni-infra" @@ -233,14 +225,13 @@ func ensureRootlessCNIInfraContainerRunning(ctx context.Context, r *Runtime) (*C } func startRootlessCNIInfraContainer(ctx context.Context, r *Runtime) (*Container, error) { - imageName, ok := rootlessCNIInfraImage[runtime.GOARCH] - if !ok { - return nil, errors.Errorf("cannot find rootless-podman-network-sandbox image for %s", runtime.GOARCH) - } + imageName := "rootless-cni-infra" logrus.Debugf("rootless CNI: ensuring image %q to exist", imageName) - newImage, err := r.ImageRuntime().New(ctx, imageName, "", "", nil, nil, - image.SigningOptions{}, nil, util.PullImageMissing) + newImage, err := r.ImageRuntime().NewFromLocal(imageName) if err != nil { + if errors.Cause(err) == define.ErrNoSuchImage { + return nil, errors.Errorf("rootless CNI infra image not present - please build image from https://github.com/containers/podman/blob/v2.2.1-rhel/contrib/rootless-cni-infra/ and tag as %q", imageName) + } return nil, err } logrus.Debugf("rootless CNI: image %q is ready", imageName) diff --git a/test/e2e/network_create_test.go b/test/e2e/network_create_test.go index cb997d10ae..660f6886e8 100644 --- a/test/e2e/network_create_test.go +++ b/test/e2e/network_create_test.go @@ -74,6 +74,7 @@ var _ = Describe("Podman network create", func() { ) BeforeEach(func() { + SkipIfRootless("rootless CNI is tech preview in RHEL 8.3.1") tempdir, err = CreateTempDirInTempDir() if err != nil { os.Exit(1) diff --git a/test/e2e/network_test.go b/test/e2e/network_test.go index adcf74f7ec..586d1efc11 100644 --- a/test/e2e/network_test.go +++ b/test/e2e/network_test.go @@ -21,6 +21,8 @@ var _ = Describe("Podman network", func() { ) BeforeEach(func() { + SkipIfRootless("rootless CNI is tech preview in RHEL 8.3.1") + tempdir, err = CreateTempDirInTempDir() if err != nil { os.Exit(1) diff --git a/test/e2e/run_networking_test.go b/test/e2e/run_networking_test.go index b8e14530cb..b85bf83157 100644 --- a/test/e2e/run_networking_test.go +++ b/test/e2e/run_networking_test.go @@ -714,6 +714,7 @@ var _ = Describe("Podman run networking", func() { }) It("podman run check dnsname plugin", func() { + SkipIfRootless("rootless CNI is tech preview in RHEL 8.3.1") pod := "testpod" session := podmanTest.Podman([]string{"pod", "create", "--name", pod}) session.WaitWithDefaultTimeout() diff --git a/test/system/500-networking.bats b/test/system/500-networking.bats index 44cc731cfe..e6189c32ec 100644 --- a/test/system/500-networking.bats +++ b/test/system/500-networking.bats @@ -83,6 +83,7 @@ load helpers # "network create" now works rootless, with the help of a special container @test "podman network create" { skip_if_remote "FIXME: pending #7808" + skip_if_rootless "Rootless CNI is tech preview in RHEL 8.2.1" local mynetname=testnet-$(random_string 10) local mysubnet=$(random_rfc1918_subnet)