diff --git a/pkg/skaffold/deploy/docker/port.go b/pkg/skaffold/deploy/docker/port.go index 62ada4ea95e..1baa56eacb4 100644 --- a/pkg/skaffold/deploy/docker/port.go +++ b/pkg/skaffold/deploy/docker/port.go @@ -35,6 +35,8 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" ) +var GetAvailablePort = util.GetAvailablePort // For testing + type containerPortForwardEntry struct { container string resourceName string @@ -85,12 +87,15 @@ func (pm *PortManager) getPorts(containerName string, pf []*v1.PortForwardResour log.Entry(context.TODO()).Debugf("skipping non-container port forward resource in Docker deploy: %s\n", p.Name) continue } - localPort := util.GetAvailablePort(p.Address, p.LocalPort, &pm.portSet) + localPort := GetAvailablePort(p.Address, p.LocalPort, &pm.portSet) ports = append(ports, localPort) port, err := nat.NewPort("tcp", p.Port.String()) if err != nil { return nil, err } + if cfg.ExposedPorts == nil { + cfg.ExposedPorts = nat.PortSet{} + } cfg.ExposedPorts[port] = struct{}{} m[port] = []nat.PortBinding{ {HostIP: p.Address, HostPort: fmt.Sprintf("%d", localPort)}, diff --git a/pkg/skaffold/deploy/docker/port_test.go b/pkg/skaffold/deploy/docker/port_test.go index f8d37fa02c2..8310dc265bc 100644 --- a/pkg/skaffold/deploy/docker/port_test.go +++ b/pkg/skaffold/deploy/docker/port_test.go @@ -17,12 +17,14 @@ limitations under the License. package docker import ( + "strconv" "testing" "github.com/docker/docker/api/types/container" v1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" + schemautil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/GoogleContainerTools/skaffold/testutil" ) @@ -35,7 +37,8 @@ func TestGetPorts(t *testing.T) { name: "one port, one resource", resources: map[int]*v1.PortForwardResource{ 9000: { - Port: util.IntOrString{ + Type: "container", + Port: schemautil.IntOrString{ IntVal: 9000, StrVal: "9000", }, @@ -48,7 +51,8 @@ func TestGetPorts(t *testing.T) { name: "two ports, two resources", resources: map[int]*v1.PortForwardResource{ 1234: { - Port: util.IntOrString{ + Type: "container", + Port: schemautil.IntOrString{ IntVal: 20, StrVal: "20", }, @@ -56,7 +60,8 @@ func TestGetPorts(t *testing.T) { LocalPort: 1234, }, 4321: { - Port: util.IntOrString{ + Type: "container", + Port: schemautil.IntOrString{ IntVal: 8080, StrVal: "8080", }, @@ -69,15 +74,19 @@ func TestGetPorts(t *testing.T) { for _, test := range tests { testutil.Run(t, test.name, func(t *testutil.T) { + t.Override(&GetAvailablePort, func(_ string, port int, _ *util.PortSet) int { + return port + }) pm := NewPortManager() cfg := container.Config{} m, err := pm.getPorts(test.name, collectResources(test.resources), &cfg) - for port := range cfg.ExposedPorts { // the image config's PortSet contains the local ports, so we grab the bindings keyed off these - bindings := m[port] + for containerPort := range cfg.ExposedPorts { // the image config's PortSet contains the local ports, so we grab the bindings keyed off these + bindings := m[containerPort] t.CheckDeepEqual(len(bindings), 1) // we always have a 1-1 mapping of resource to binding t.CheckError(false, err) // shouldn't error, unless GetAvailablePort is broken - t.CheckDeepEqual(bindings[0].HostIP, test.resources[port.Int()].Address) - t.CheckDeepEqual(bindings[0].HostPort, test.resources[port.Int()].Port.StrVal) + resource := resourceFromContainerPort(test.resources, containerPort.Int()) + t.CheckDeepEqual(bindings[0].HostIP, resource.Address) + t.CheckDeepEqual(bindings[0].HostPort, strconv.Itoa(resource.LocalPort)) } }) } @@ -90,3 +99,12 @@ func collectResources(resourceMap map[int]*v1.PortForwardResource) []*v1.PortFor } return resources } + +func resourceFromContainerPort(resourceMap map[int]*v1.PortForwardResource, containerPort int) *v1.PortForwardResource { + for _, resource := range resourceMap { + if resource.Port.IntVal == containerPort { + return resource + } + } + return nil +}