From 18315db025780fbc250155a82c7b2dcc61a247d1 Mon Sep 17 00:00:00 2001 From: Harry Zhang Date: Sun, 30 Aug 2015 16:13:33 +0000 Subject: [PATCH 1/2] Allow multiple host ports map to the same port in container --- pkg/kubelet/dockertools/docker_test.go | 13 +++++++++++++ pkg/kubelet/dockertools/manager.go | 20 +++++++++++++++----- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index 22161b13d0c..2eb2ea7db11 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -698,6 +698,11 @@ func TestMakePortsAndBindings(t *testing.T) { HostPort: 445, Protocol: "foobar", }, + { + ContainerPort: 443, + HostPort: 446, + Protocol: "tcp", + }, } exposedPorts, bindings := makePortsAndBindings(ports) if len(ports) != len(exposedPorts) || @@ -734,6 +739,14 @@ func TestMakePortsAndBindings(t *testing.T) { if value[0].HostIP != "" { t.Errorf("Unexpected host IP: %s", value[0].HostIP) } + case "446": + // allow multiple host ports bind to same container port + if !reflect.DeepEqual(docker.Port("443/tcp"), key) { + t.Errorf("Unexpected docker port: %#v", key) + } + if value[0].HostIP != "" { + t.Errorf("Unexpected host IP: %s", value[0].HostIP) + } } } } diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 4f51b492f2b..2bf3dfd0eab 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -573,11 +573,21 @@ func makePortsAndBindings(portMappings []kubecontainer.PortMapping) (map[docker. } dockerPort := docker.Port(strconv.Itoa(interiorPort) + protocol) exposedPorts[dockerPort] = struct{}{} - portBindings[dockerPort] = []docker.PortBinding{ - { - HostPort: strconv.Itoa(exteriorPort), - HostIP: port.HostIP, - }, + + hostBinding := docker.PortBinding{ + HostPort: strconv.Itoa(exteriorPort), + HostIP: port.HostIP, + } + + // Allow multiple host ports bind to same container port + if existedBindings := portBindings[dockerPort]; len(existedBindings) != 0 { + // If a container port already map to a host port, append to the host ports + portBindings[dockerPort] = append(existedBindings, hostBinding) + } else { + // Otherwise, it's fresh new port binding + portBindings[dockerPort] = []docker.PortBinding{ + hostBinding, + } } } return exposedPorts, portBindings From 7b2e2e56493f0bae8a2a36e4509719151efa24bc Mon Sep 17 00:00:00 2001 From: Harry Zhang Date: Mon, 31 Aug 2015 20:53:02 +0800 Subject: [PATCH 2/2] Fix tests and clear fmt mess in manager.go --- pkg/kubelet/dockertools/docker_test.go | 107 ++++++++++++++++--------- pkg/kubelet/dockertools/manager.go | 11 +-- 2 files changed, 73 insertions(+), 45 deletions(-) diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index 2eb2ea7db11..ceaf474344f 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -22,6 +22,7 @@ import ( "hash/adler32" "reflect" "sort" + "strconv" "strings" "testing" @@ -698,55 +699,81 @@ func TestMakePortsAndBindings(t *testing.T) { HostPort: 445, Protocol: "foobar", }, - { + { ContainerPort: 443, HostPort: 446, Protocol: "tcp", }, + { + ContainerPort: 443, + HostPort: 446, + Protocol: "udp", + }, } + exposedPorts, bindings := makePortsAndBindings(ports) - if len(ports) != len(exposedPorts) || - len(ports) != len(bindings) { + + // Count the expected exposed ports and bindings + expectedExposedPorts := map[string]struct{}{} + + for _, binding := range ports { + dockerKey := strconv.Itoa(binding.ContainerPort) + "/" + string(binding.Protocol) + expectedExposedPorts[dockerKey] = struct{}{} + } + + // Should expose right ports in docker + if len(expectedExposedPorts) != len(exposedPorts) { t.Errorf("Unexpected ports and bindings, %#v %#v %#v", ports, exposedPorts, bindings) } - for key, value := range bindings { - switch value[0].HostPort { - case "8080": - if !reflect.DeepEqual(docker.Port("80/tcp"), key) { - t.Errorf("Unexpected docker port: %#v", key) - } - if value[0].HostIP != "127.0.0.1" { - t.Errorf("Unexpected host IP: %s", value[0].HostIP) - } - case "443": - if !reflect.DeepEqual(docker.Port("443/tcp"), key) { - t.Errorf("Unexpected docker port: %#v", key) - } - if value[0].HostIP != "" { - t.Errorf("Unexpected host IP: %s", value[0].HostIP) - } - case "444": - if !reflect.DeepEqual(docker.Port("444/udp"), key) { - t.Errorf("Unexpected docker port: %#v", key) - } - if value[0].HostIP != "" { - t.Errorf("Unexpected host IP: %s", value[0].HostIP) - } - case "445": - if !reflect.DeepEqual(docker.Port("445/tcp"), key) { - t.Errorf("Unexpected docker port: %#v", key) - } - if value[0].HostIP != "" { - t.Errorf("Unexpected host IP: %s", value[0].HostIP) - } - case "446": - // allow multiple host ports bind to same container port - if !reflect.DeepEqual(docker.Port("443/tcp"), key) { - t.Errorf("Unexpected docker port: %#v", key) - } - if value[0].HostIP != "" { - t.Errorf("Unexpected host IP: %s", value[0].HostIP) + + // Construct expected bindings + expectPortBindings := map[string][]docker.PortBinding{ + "80/tcp": { + docker.PortBinding{ + HostPort: "8080", + HostIP: "127.0.0.1", + }, + }, + "443/tcp": { + docker.PortBinding{ + HostPort: "443", + HostIP: "", + }, + docker.PortBinding{ + HostPort: "446", + HostIP: "", + }, + }, + "443/udp": { + docker.PortBinding{ + HostPort: "446", + HostIP: "", + }, + }, + "444/udp": { + docker.PortBinding{ + HostPort: "444", + HostIP: "", + }, + }, + "445/tcp": { + docker.PortBinding{ + HostPort: "445", + HostIP: "", + }, + }, + } + + // interate the bindings by dockerPort, and check its portBindings + for dockerPort, portBindings := range bindings { + switch dockerPort { + case "80/tcp", "443/tcp", "443/udp", "444/udp", "445/tcp": + if !reflect.DeepEqual(expectPortBindings[string(dockerPort)], portBindings) { + t.Errorf("Unexpected portbindings for %#v, expected: %#v, but got: %#v", + dockerPort, expectPortBindings[string(dockerPort)], portBindings) } + default: + t.Errorf("Unexpected docker port: %#v with portbindings: %#v", dockerPort, portBindings) } } } diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 2bf3dfd0eab..5dd51ad3d90 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -571,17 +571,18 @@ func makePortsAndBindings(portMappings []kubecontainer.PortMapping) (map[docker. glog.Warningf("Unknown protocol %q: defaulting to TCP", port.Protocol) protocol = "/tcp" } + dockerPort := docker.Port(strconv.Itoa(interiorPort) + protocol) exposedPorts[dockerPort] = struct{}{} - - hostBinding := docker.PortBinding{ + + hostBinding := docker.PortBinding{ HostPort: strconv.Itoa(exteriorPort), HostIP: port.HostIP, } - // Allow multiple host ports bind to same container port - if existedBindings := portBindings[dockerPort]; len(existedBindings) != 0 { - // If a container port already map to a host port, append to the host ports + // Allow multiple host ports bind to same docker port + if existedBindings, ok := portBindings[dockerPort]; ok { + // If a docker port already map to a host port, just append the host ports portBindings[dockerPort] = append(existedBindings, hostBinding) } else { // Otherwise, it's fresh new port binding