From 41457b5a28532d410517b1afb1759e2724d03cab Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 13 Jul 2020 14:22:43 -0400 Subject: [PATCH 1/3] Include infra container information in `pod inspect` We had a field for this in the inspect data, but it was never being populated. Because of this, `podman pod inspect` stopped showing port bindings (and other infra container settings). Add code to populate the infra container inspect data, and add a test to ensure we don't regress again. Signed-off-by: Matthew Heon --- libpod/container_inspect.go | 17 +++---------- libpod/define/pod_inspect.go | 6 ++--- libpod/networking_linux.go | 16 +----------- libpod/pod_api.go | 47 +++++++++++++++++++++++++++++++++++- libpod/util.go | 19 +++++++++++++++ test/e2e/pod_inspect_test.go | 20 +++++++++++++++ 6 files changed, 92 insertions(+), 33 deletions(-) diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go index 94d5dc93bc..dad69311fd 100644 --- a/libpod/container_inspect.go +++ b/libpod/container_inspect.go @@ -612,22 +612,11 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named // Port bindings. // Only populate if we're using CNI to configure the network. - portBindings := make(map[string][]define.InspectHostPort) if c.config.CreateNetNS { - for _, port := range c.config.PortMappings { - key := fmt.Sprintf("%d/%s", port.ContainerPort, port.Protocol) - hostPorts := portBindings[key] - if hostPorts == nil { - hostPorts = []define.InspectHostPort{} - } - hostPorts = append(hostPorts, define.InspectHostPort{ - HostIP: port.HostIP, - HostPort: fmt.Sprintf("%d", port.HostPort), - }) - portBindings[key] = hostPorts - } + hostConfig.PortBindings = makeInspectPortBindings(c.config.PortMappings) + } else { + hostConfig.PortBindings = make(map[string][]define.InspectHostPort) } - hostConfig.PortBindings = portBindings // Cap add and cap drop. // We need a default set of capabilities to compare against. diff --git a/libpod/define/pod_inspect.go b/libpod/define/pod_inspect.go index 7f06e16fc3..634cbb728e 100644 --- a/libpod/define/pod_inspect.go +++ b/libpod/define/pod_inspect.go @@ -3,8 +3,6 @@ package define import ( "net" "time" - - "github.com/cri-o/ocicni/pkg/ocicni" ) // InspectPodData contains detailed information on a pod's configuration and @@ -60,7 +58,7 @@ type InspectPodData struct { type InspectPodInfraConfig struct { // PortBindings are ports that will be forwarded to the infra container // and then shared with the pod. - PortBindings []ocicni.PortMapping + PortBindings map[string][]InspectHostPort // HostNetwork is whether the infra container (and thus the whole pod) // will use the host's network and not create a network namespace. HostNetwork bool @@ -89,6 +87,8 @@ type InspectPodInfraConfig struct { // HostAdd adds a number of hosts to the infra container's resolv.conf // which will be shared with the rest of the pod. HostAdd []string + // Networks is a list of CNI networks te pod will join. + Networks []string } // InspectPodContainerInfo contains information on a container in a pod. diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 5a8faa7a44..1e79e8732b 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -587,21 +587,7 @@ func getContainerNetIO(ctr *Container) (*netlink.LinkStatistics, error) { // network. func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, error) { settings := new(define.InspectNetworkSettings) - settings.Ports = make(map[string][]define.InspectHostPort) - if c.config.PortMappings != nil { - for _, port := range c.config.PortMappings { - key := fmt.Sprintf("%d/%s", port.ContainerPort, port.Protocol) - mapping := settings.Ports[key] - if mapping == nil { - mapping = []define.InspectHostPort{} - } - mapping = append(mapping, define.InspectHostPort{ - HostIP: port.HostIP, - HostPort: fmt.Sprintf("%d", port.HostPort), - }) - settings.Ports[key] = mapping - } - } + settings.Ports = makeInspectPortBindings(c.config.PortMappings) // We can't do more if the network is down. if c.state.NetNS == nil { diff --git a/libpod/pod_api.go b/libpod/pod_api.go index a02b171e18..cc10a33555 100644 --- a/libpod/pod_api.go +++ b/libpod/pod_api.go @@ -481,6 +481,51 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) { } } + // Infra config contains detailed information on the pod's infra + // container. + var infraConfig *define.InspectPodInfraConfig + if p.config.InfraContainer != nil && p.config.InfraContainer.HasInfraContainer { + infraConfig = new(define.InspectPodInfraConfig) + infraConfig.HostNetwork = p.config.InfraContainer.HostNetwork + infraConfig.StaticIP = p.config.InfraContainer.StaticIP + infraConfig.StaticMAC = p.config.InfraContainer.StaticMAC + infraConfig.NoManageResolvConf = p.config.InfraContainer.UseImageResolvConf + infraConfig.NoManageHosts = p.config.InfraContainer.UseImageHosts + + if len(p.config.InfraContainer.DNSServer) > 0 { + infraConfig.DNSServer = make([]string, 0, len(p.config.InfraContainer.DNSServer)) + for _, i := range p.config.InfraContainer.DNSServer { + infraConfig.DNSServer = append(infraConfig.DNSServer, i) + } + } + if len(p.config.InfraContainer.DNSSearch) > 0 { + infraConfig.DNSSearch = make([]string, 0, len(p.config.InfraContainer.DNSSearch)) + for _, i := range p.config.InfraContainer.DNSSearch { + infraConfig.DNSSearch = append(infraConfig.DNSSearch, i) + } + } + if len(p.config.InfraContainer.DNSOption) > 0 { + infraConfig.DNSOption = make([]string, 0, len(p.config.InfraContainer.DNSOption)) + for _, i := range p.config.InfraContainer.DNSOption { + infraConfig.DNSOption = append(infraConfig.DNSOption, i) + } + } + if len(p.config.InfraContainer.HostAdd) > 0 { + infraConfig.HostAdd = make([]string, 0, len(p.config.InfraContainer.HostAdd)) + for _, i := range p.config.InfraContainer.HostAdd { + infraConfig.HostAdd = append(infraConfig.HostAdd, i) + } + } + if len(p.config.InfraContainer.Networks) > 0 { + infraConfig.Networks = make([]string, 0, len(p.config.InfraContainer.Networks)) + for _, i := range p.config.InfraContainer.Networks { + infraConfig.Networks = append(infraConfig.Networks, i) + } + } + + infraConfig.PortBindings = makeInspectPortBindings(p.config.InfraContainer.PortBindings) + } + inspectData := define.InspectPodData{ ID: p.ID(), Name: p.Name(), @@ -495,7 +540,7 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) { CgroupPath: p.state.CgroupPath, CreateInfra: false, InfraContainerID: p.state.InfraContainerID, - InfraConfig: nil, + InfraConfig: infraConfig, SharedNamespaces: sharesNS, NumContainers: uint(len(containers)), Containers: ctrs, diff --git a/libpod/util.go b/libpod/util.go index 7504295f08..8c2d946bad 100644 --- a/libpod/util.go +++ b/libpod/util.go @@ -15,6 +15,7 @@ import ( "github.com/containers/common/pkg/config" "github.com/containers/libpod/v2/libpod/define" "github.com/containers/libpod/v2/utils" + "github.com/cri-o/ocicni/pkg/ocicni" "github.com/fsnotify/fsnotify" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" @@ -254,3 +255,21 @@ func makeHTTPAttachHeader(stream byte, length uint32) []byte { binary.BigEndian.PutUint32(header[4:], length) return header } + +// Convert OCICNI port bindings into Inspect-formatted port bindings. +func makeInspectPortBindings(bindings []ocicni.PortMapping) map[string][]define.InspectHostPort { + portBindings := make(map[string][]define.InspectHostPort) + for _, port := range bindings { + key := fmt.Sprintf("%d/%s", port.ContainerPort, port.Protocol) + hostPorts := portBindings[key] + if hostPorts == nil { + hostPorts = []define.InspectHostPort{} + } + hostPorts = append(hostPorts, define.InspectHostPort{ + HostIP: port.HostIP, + HostPort: fmt.Sprintf("%d", port.HostPort), + }) + portBindings[key] = hostPorts + } + return portBindings +} diff --git a/test/e2e/pod_inspect_test.go b/test/e2e/pod_inspect_test.go index 5e3634435e..64f943c80b 100644 --- a/test/e2e/pod_inspect_test.go +++ b/test/e2e/pod_inspect_test.go @@ -3,6 +3,8 @@ package integration import ( "os" + "github.com/containers/libpod/v2/libpod/define" + . "github.com/containers/libpod/v2/test/utils" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -79,4 +81,22 @@ var _ = Describe("Podman pod inspect", func() { index := len(inspectCreateCommand) - len(createCommand) Expect(inspectCreateCommand[index:]).To(Equal(createCommand)) }) + + It("podman pod inspect outputs port bindings", func() { + podName := "testPod" + create := podmanTest.Podman([]string{"pod", "create", "--name", testPod, "-p", "8080:80"}) + create.WaitWithDefaultTimeout() + Expect(create.ExitCode()).To(Equal(0)) + + inspectOut := podmanTest.Podman([]string{"pod", "inspect", podName}) + inspectOut.WaitWithDefaultTimeout() + Expect(inspectOut.ExitCode()).To(Equal(0)) + + inspectJSON := new(define.InspectPodData) + err := json.Unmarshal(inspectOut.Out.Contents(), inspectJSON) + Expect(err).To(BeNil()) + Expect(inspectJSON.InfraConfig).To(Not(BeNil())) + Expect(len(inspectJSON.PortBindings["80/tcp"])).To(Equal(1)) + Expect(inspectJSON.PortBindings["80/tcp"].HostPort).To(Equal("8080")) + }) }) From 1fdfd52eeb513c8d6fad11f53baeef97151775a6 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 14 Jul 2020 09:07:08 -0400 Subject: [PATCH 2/3] Populate remaining unused fields in `pod inspect` We were hard-coding two fields to false, instead of grabbing their value from the pod config, which means that `pod inspect` would print the wrong value always. Fixes #6968 Signed-off-by: Matthew Heon --- libpod/pod_api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libpod/pod_api.go b/libpod/pod_api.go index cc10a33555..f889ba7894 100644 --- a/libpod/pod_api.go +++ b/libpod/pod_api.go @@ -535,10 +535,10 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) { State: podState, Hostname: p.config.Hostname, Labels: p.Labels(), - CreateCgroup: false, + CreateCgroup: p.config.UsePodCgroup, CgroupParent: p.CgroupParent(), CgroupPath: p.state.CgroupPath, - CreateInfra: false, + CreateInfra: infraConfig != nil, InfraContainerID: p.state.InfraContainerID, InfraConfig: infraConfig, SharedNamespaces: sharesNS, From fbc1167c4d7861013001d0c2460c6e1c1e1ad66d Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 14 Jul 2020 09:44:56 -0400 Subject: [PATCH 3/3] Fix lint Signed-off-by: Matthew Heon --- libpod/pod_api.go | 20 +++++--------------- test/e2e/pod_inspect_test.go | 7 ++++--- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/libpod/pod_api.go b/libpod/pod_api.go index f889ba7894..f2ef81bec2 100644 --- a/libpod/pod_api.go +++ b/libpod/pod_api.go @@ -494,33 +494,23 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) { if len(p.config.InfraContainer.DNSServer) > 0 { infraConfig.DNSServer = make([]string, 0, len(p.config.InfraContainer.DNSServer)) - for _, i := range p.config.InfraContainer.DNSServer { - infraConfig.DNSServer = append(infraConfig.DNSServer, i) - } + infraConfig.DNSServer = append(infraConfig.DNSServer, p.config.InfraContainer.DNSServer...) } if len(p.config.InfraContainer.DNSSearch) > 0 { infraConfig.DNSSearch = make([]string, 0, len(p.config.InfraContainer.DNSSearch)) - for _, i := range p.config.InfraContainer.DNSSearch { - infraConfig.DNSSearch = append(infraConfig.DNSSearch, i) - } + infraConfig.DNSSearch = append(infraConfig.DNSSearch, p.config.InfraContainer.DNSSearch...) } if len(p.config.InfraContainer.DNSOption) > 0 { infraConfig.DNSOption = make([]string, 0, len(p.config.InfraContainer.DNSOption)) - for _, i := range p.config.InfraContainer.DNSOption { - infraConfig.DNSOption = append(infraConfig.DNSOption, i) - } + infraConfig.DNSOption = append(infraConfig.DNSOption, p.config.InfraContainer.DNSOption...) } if len(p.config.InfraContainer.HostAdd) > 0 { infraConfig.HostAdd = make([]string, 0, len(p.config.InfraContainer.HostAdd)) - for _, i := range p.config.InfraContainer.HostAdd { - infraConfig.HostAdd = append(infraConfig.HostAdd, i) - } + infraConfig.HostAdd = append(infraConfig.HostAdd, p.config.InfraContainer.HostAdd...) } if len(p.config.InfraContainer.Networks) > 0 { infraConfig.Networks = make([]string, 0, len(p.config.InfraContainer.Networks)) - for _, i := range p.config.InfraContainer.Networks { - infraConfig.Networks = append(infraConfig.Networks, i) - } + infraConfig.Networks = append(infraConfig.Networks, p.config.InfraContainer.Networks...) } infraConfig.PortBindings = makeInspectPortBindings(p.config.InfraContainer.PortBindings) diff --git a/test/e2e/pod_inspect_test.go b/test/e2e/pod_inspect_test.go index 64f943c80b..16bf1c4c96 100644 --- a/test/e2e/pod_inspect_test.go +++ b/test/e2e/pod_inspect_test.go @@ -1,6 +1,7 @@ package integration import ( + "encoding/json" "os" "github.com/containers/libpod/v2/libpod/define" @@ -84,7 +85,7 @@ var _ = Describe("Podman pod inspect", func() { It("podman pod inspect outputs port bindings", func() { podName := "testPod" - create := podmanTest.Podman([]string{"pod", "create", "--name", testPod, "-p", "8080:80"}) + create := podmanTest.Podman([]string{"pod", "create", "--name", podName, "-p", "8080:80"}) create.WaitWithDefaultTimeout() Expect(create.ExitCode()).To(Equal(0)) @@ -96,7 +97,7 @@ var _ = Describe("Podman pod inspect", func() { err := json.Unmarshal(inspectOut.Out.Contents(), inspectJSON) Expect(err).To(BeNil()) Expect(inspectJSON.InfraConfig).To(Not(BeNil())) - Expect(len(inspectJSON.PortBindings["80/tcp"])).To(Equal(1)) - Expect(inspectJSON.PortBindings["80/tcp"].HostPort).To(Equal("8080")) + Expect(len(inspectJSON.InfraConfig.PortBindings["80/tcp"])).To(Equal(1)) + Expect(inspectJSON.InfraConfig.PortBindings["80/tcp"][0].HostPort).To(Equal("8080")) }) })