Create port map on container config if empty (#6621)

GoogleContainerTools · Sep 22, 2021 · f102665 · f102665
1 parent c105a18
commit f102665
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 9 deletions.
diff --git 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
@@ -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,15 +51,17 @@ 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",
 					},
 					Address:   "192.168.999.999",
 					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
+}