From 667bb7c93a49c364ed87795af16e1efa233271b2 Mon Sep 17 00:00:00 2001 From: Boris Lau Date: Wed, 21 Apr 2021 15:30:08 -0700 Subject: [PATCH 01/10] Added fix for RPC port detection - fixes #5713 Test plan: `make test` Also go locally, and run multiple version of skaffold, ensure that none of them have issues with starting up their own RPC server. --- pkg/skaffold/server/server.go | 2 +- pkg/skaffold/util/port.go | 47 +++++++++++++++++++++-------------- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/pkg/skaffold/server/server.go b/pkg/skaffold/server/server.go index e0f5b6c8666..a8cd77348ba 100644 --- a/pkg/skaffold/server/server.go +++ b/pkg/skaffold/server/server.go @@ -242,7 +242,7 @@ func errorHandler(ctx context.Context, _ *runtime.ServeMux, marshaler runtime.Ma func listenOnAvailablePort(preferredPort int, usedPorts *util.PortSet) (net.Listener, int, error) { for try := 1; ; try++ { - port := util.GetAvailablePort(preferredPort, usedPorts) + port := util.GetAvailablePortWithAddreess(util.Loopback, preferredPort, usedPorts) l, err := net.Listen("tcp", fmt.Sprintf("%s:%d", util.Loopback, port)) if err != nil { diff --git a/pkg/skaffold/util/port.go b/pkg/skaffold/util/port.go index 8bebb5f7d50..7239a25bbdd 100644 --- a/pkg/skaffold/util/port.go +++ b/pkg/skaffold/util/port.go @@ -28,6 +28,9 @@ import ( // Loopback network address. Skaffold should not bind to 0.0.0.0 // unless we really want to expose something to the network. const Loopback = "127.0.0.1" +// Network address which represent any address. This is the default that +// we should use when checking if port is free. +const Any = "" type PortSet struct { ports map[int]bool @@ -87,37 +90,39 @@ func (f *PortSet) List() []int { return list } -// GetAvailablePort returns an available port that is near the requested port when possible. // First, check if the provided port is available on the specified address. If so, use it. // If not, check if any of the next 10 subsequent ports are available. // If not, check if any of ports 4503-4533 are available. // If not, return a random port, which hopefully won't collide with any future containers -// -// See https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt + +// See https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt, func GetAvailablePort(port int, usedPorts *PortSet) int { - if port > 0 { - if getPortIfAvailable(port, usedPorts) { - return port - } + // We map this to allow + return GetAvailablePortWithAddreess(Any, port, usedPorts) +} + +func GetAvailablePortWithAddreess(address string, port int, usedPorts *PortSet) int { + if getPortIfAvailable(address, port, usedPorts) { + return port + } - // try the next 10 ports after the provided one - for i := 0; i < 10; i++ { - port++ - if getPortIfAvailable(port, usedPorts) { - logrus.Debugf("found open port: %d", port) - return port - } + // try the next 10 ports after the provided one + for i := 0; i < 10; i++ { + port++ + if getPortIfAvailable(address, port, usedPorts) { + logrus.Debugf("found open port: %d", port) + return port } } for port = 4503; port <= 4533; port++ { - if getPortIfAvailable(port, usedPorts) { + if getPortIfAvailable(address, port, usedPorts) { logrus.Debugf("found open port: %d", port) return port } } - l, err := net.Listen("tcp", ":0") + l, err := net.Listen("tcp", fmt.Sprintf("%s:0", address)) if err != nil { return -1 } @@ -129,16 +134,20 @@ func GetAvailablePort(port int, usedPorts *PortSet) int { return p } -func getPortIfAvailable(p int, usedPorts *PortSet) bool { +func getPortIfAvailable(address string, p int, usedPorts *PortSet) bool { if alreadySet := usedPorts.LoadOrSet(p); alreadySet { return false } - return IsPortFree(p) + return IsPortFreeWithAddress(address, p) } func IsPortFree(p int) bool { - l, err := net.Listen("tcp", fmt.Sprintf(":%d", p)) + return IsPortFreeWithAddress(Any, p) +} + +func IsPortFreeWithAddress(address string, p int) bool { + l, err := net.Listen("tcp", fmt.Sprintf("%s:%d", address, p)) if err != nil { return false } From 49881a3388b8ac36f01bad77d59fc8bf7c4cfd61 Mon Sep 17 00:00:00 2001 From: Boris Lau Date: Wed, 21 Apr 2021 16:40:30 -0700 Subject: [PATCH 02/10] Fix comments and unnecessary revert from feedback --- pkg/skaffold/util/port.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/skaffold/util/port.go b/pkg/skaffold/util/port.go index 7239a25bbdd..727ddd11032 100644 --- a/pkg/skaffold/util/port.go +++ b/pkg/skaffold/util/port.go @@ -102,17 +102,20 @@ func GetAvailablePort(port int, usedPorts *PortSet) int { } func GetAvailablePortWithAddreess(address string, port int, usedPorts *PortSet) int { - if getPortIfAvailable(address, port, usedPorts) { - return port - } - - // try the next 10 ports after the provided one - for i := 0; i < 10; i++ { - port++ + if port > 0 { if getPortIfAvailable(address, port, usedPorts) { logrus.Debugf("found open port: %d", port) return port } + + // try the next 10 ports after the provided one + for i := 0; i < 10; i++ { + port++ + if getPortIfAvailable(address, port, usedPorts) { + logrus.Debugf("found open port: %d", port) + return port + } + } } for port = 4503; port <= 4533; port++ { From 07d230d28f66d464abbd20cc260d0a577f4eea42 Mon Sep 17 00:00:00 2001 From: Boris Lau Date: Wed, 21 Apr 2021 16:49:17 -0700 Subject: [PATCH 03/10] Really fix comments - failed update from UI --- pkg/skaffold/server/server.go | 2 +- pkg/skaffold/util/port.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/skaffold/server/server.go b/pkg/skaffold/server/server.go index a8cd77348ba..726ffe98ce3 100644 --- a/pkg/skaffold/server/server.go +++ b/pkg/skaffold/server/server.go @@ -242,7 +242,7 @@ func errorHandler(ctx context.Context, _ *runtime.ServeMux, marshaler runtime.Ma func listenOnAvailablePort(preferredPort int, usedPorts *util.PortSet) (net.Listener, int, error) { for try := 1; ; try++ { - port := util.GetAvailablePortWithAddreess(util.Loopback, preferredPort, usedPorts) + port := util.GetAvailablePortWithAddress(util.Loopback, preferredPort, usedPorts) l, err := net.Listen("tcp", fmt.Sprintf("%s:%d", util.Loopback, port)) if err != nil { diff --git a/pkg/skaffold/util/port.go b/pkg/skaffold/util/port.go index 727ddd11032..6f091e72074 100644 --- a/pkg/skaffold/util/port.go +++ b/pkg/skaffold/util/port.go @@ -95,13 +95,13 @@ func (f *PortSet) List() []int { // If not, check if any of ports 4503-4533 are available. // If not, return a random port, which hopefully won't collide with any future containers -// See https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt, +// See https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt func GetAvailablePort(port int, usedPorts *PortSet) int { // We map this to allow - return GetAvailablePortWithAddreess(Any, port, usedPorts) + return GetAvailablePortWithAddress(Any, port, usedPorts) } -func GetAvailablePortWithAddreess(address string, port int, usedPorts *PortSet) int { +func GetAvailablePortWithAddress(address string, port int, usedPorts *PortSet) int { if port > 0 { if getPortIfAvailable(address, port, usedPorts) { logrus.Debugf("found open port: %d", port) From 8897c569e55c75caa0bc17a059ccc33012015580 Mon Sep 17 00:00:00 2001 From: Boris Lau Date: Wed, 21 Apr 2021 16:51:28 -0700 Subject: [PATCH 04/10] Revert old unnecessary debug log --- pkg/skaffold/util/port.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/skaffold/util/port.go b/pkg/skaffold/util/port.go index 6f091e72074..0efa4e91889 100644 --- a/pkg/skaffold/util/port.go +++ b/pkg/skaffold/util/port.go @@ -104,7 +104,6 @@ func GetAvailablePort(port int, usedPorts *PortSet) int { func GetAvailablePortWithAddress(address string, port int, usedPorts *PortSet) int { if port > 0 { if getPortIfAvailable(address, port, usedPorts) { - logrus.Debugf("found open port: %d", port) return port } From abd44453f2fe0339b2cd096e4c94a8a1c6009b39 Mon Sep 17 00:00:00 2001 From: Boris Lau Date: Wed, 21 Apr 2021 16:53:22 -0700 Subject: [PATCH 05/10] Preserve improved comments --- pkg/skaffold/util/port.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/skaffold/util/port.go b/pkg/skaffold/util/port.go index 0efa4e91889..71dd43f2c5c 100644 --- a/pkg/skaffold/util/port.go +++ b/pkg/skaffold/util/port.go @@ -90,11 +90,12 @@ func (f *PortSet) List() []int { return list } +// GetAvailablePort returns an available port that is near the requested port when possible. // First, check if the provided port is available on the specified address. If so, use it. // If not, check if any of the next 10 subsequent ports are available. // If not, check if any of ports 4503-4533 are available. // If not, return a random port, which hopefully won't collide with any future containers - +// // See https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt func GetAvailablePort(port int, usedPorts *PortSet) int { // We map this to allow From 53777b17d4feb1365092861966592936aa11caa6 Mon Sep 17 00:00:00 2001 From: Boris Lau Date: Thu, 22 Apr 2021 04:59:03 -0700 Subject: [PATCH 06/10] Make linter happy --- pkg/skaffold/util/port.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/skaffold/util/port.go b/pkg/skaffold/util/port.go index 71dd43f2c5c..c637ac908ab 100644 --- a/pkg/skaffold/util/port.go +++ b/pkg/skaffold/util/port.go @@ -28,6 +28,7 @@ import ( // Loopback network address. Skaffold should not bind to 0.0.0.0 // unless we really want to expose something to the network. const Loopback = "127.0.0.1" + // Network address which represent any address. This is the default that // we should use when checking if port is free. const Any = "" From c26f89c5d34e81246266e339860c40fac5a67c27 Mon Sep 17 00:00:00 2001 From: Boris Lau Date: Fri, 23 Apr 2021 20:27:06 -0700 Subject: [PATCH 07/10] Add INPORT_ANY check from @briandealwis Co-authored-by: Brian de Alwis --- pkg/skaffold/util/port.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/skaffold/util/port.go b/pkg/skaffold/util/port.go index c637ac908ab..3ba3873c3c5 100644 --- a/pkg/skaffold/util/port.go +++ b/pkg/skaffold/util/port.go @@ -151,9 +151,19 @@ func IsPortFree(p int) bool { } func IsPortFreeWithAddress(address string, p int) bool { - l, err := net.Listen("tcp", fmt.Sprintf("%s:%d", address, p)) - if err != nil { + // Ensure the port is available across all interfaces + if l, err := net.Listen("tcp", fmt.Sprintf(":%d", p)); err != nil { return false + } else { + l.Close() + } + if address != Any { + // Ensure the port is available on the specific interface too + if l, err := net.Listen("tcp", fmt.Sprintf("%s:%d", address, p)); err != nil { + return false + } else { + l.Close() + } } l.Close() From eb1bf6a64fb5c271555b6938ad5cdfb35f8b3329 Mon Sep 17 00:00:00 2001 From: Boris Lau Date: Fri, 23 Apr 2021 20:43:01 -0700 Subject: [PATCH 08/10] Be more defensive on nil pointer on Listening --- pkg/skaffold/util/port.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/pkg/skaffold/util/port.go b/pkg/skaffold/util/port.go index 3ba3873c3c5..a4989b40683 100644 --- a/pkg/skaffold/util/port.go +++ b/pkg/skaffold/util/port.go @@ -152,20 +152,19 @@ func IsPortFree(p int) bool { func IsPortFreeWithAddress(address string, p int) bool { // Ensure the port is available across all interfaces - if l, err := net.Listen("tcp", fmt.Sprintf(":%d", p)); err != nil { + l, err := net.Listen("tcp", fmt.Sprintf(":%d", p)) + if err != nil || l == nil { return false - } else { - l.Close() } + l.Close() + if address != Any { // Ensure the port is available on the specific interface too - if l, err := net.Listen("tcp", fmt.Sprintf("%s:%d", address, p)); err != nil { + l, err := net.Listen("tcp", fmt.Sprintf("%s:%d", address, p)) + if err != nil || l == nil { return false - } else { - l.Close() } + l.Close() } - - l.Close() return true } From 12f35d8c53bd6f4bdd2bc35a5a5e59641a88c25a Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Tue, 27 Apr 2021 00:12:01 -0400 Subject: [PATCH 09/10] restore to pre-#5670 calling conventions of passing the address --- .../kubernetes/portforward/kubectl_forwarder.go | 2 +- .../portforward/kubectl_forwarder_test.go | 2 +- .../kubernetes/portforward/pod_forwarder.go | 2 +- .../portforward/pod_forwarder_test.go | 3 ++- .../portforward/port_forward_integration.go | 5 +++-- .../portforward/resource_forwarder.go | 2 +- .../portforward/resource_forwarder_test.go | 12 ++++++------ pkg/skaffold/server/server.go | 2 +- pkg/skaffold/util/port.go | 17 ++++------------- pkg/skaffold/util/port_test.go | 2 +- 10 files changed, 21 insertions(+), 28 deletions(-) diff --git a/pkg/skaffold/kubernetes/portforward/kubectl_forwarder.go b/pkg/skaffold/kubernetes/portforward/kubectl_forwarder.go index 07fbd29a4ac..1607e85c5b7 100644 --- a/pkg/skaffold/kubernetes/portforward/kubectl_forwarder.go +++ b/pkg/skaffold/kubernetes/portforward/kubectl_forwarder.go @@ -91,7 +91,7 @@ func (k *KubectlForwarder) forward(parentCtx context.Context, pfe *portForwardEn } pfe.terminationLock.Unlock() - if !isPortFree(pfe.localPort) { + if !isPortFree(util.Loopback, pfe.localPort) { // Assuming that Skaffold brokered ports don't overlap, this has to be an external process that started // since the dev loop kicked off. We are notifying the user in the hope that they can fix it color.Red.Fprintf(k.out, "failed to port forward %v, port %d is taken, retrying...\n", pfe, pfe.localPort) diff --git a/pkg/skaffold/kubernetes/portforward/kubectl_forwarder_test.go b/pkg/skaffold/kubernetes/portforward/kubectl_forwarder_test.go index cf11a711a5b..b241d141ce2 100644 --- a/pkg/skaffold/kubernetes/portforward/kubectl_forwarder_test.go +++ b/pkg/skaffold/kubernetes/portforward/kubectl_forwarder_test.go @@ -50,7 +50,7 @@ func TestUnavailablePort(t *testing.T) { // has been called var portFreeWG sync.WaitGroup portFreeWG.Add(1) - t.Override(&isPortFree, func(int) bool { + t.Override(&isPortFree, func(string, int) bool { portFreeWG.Done() return false }) diff --git a/pkg/skaffold/kubernetes/portforward/pod_forwarder.go b/pkg/skaffold/kubernetes/portforward/pod_forwarder.go index 1d66aaa9e8c..3bd968f6f80 100644 --- a/pkg/skaffold/kubernetes/portforward/pod_forwarder.go +++ b/pkg/skaffold/kubernetes/portforward/pod_forwarder.go @@ -148,7 +148,7 @@ func (p *WatchingPodForwarder) podForwardingEntry(resourceVersion, containerName } // retrieve an open port on the host - entry.localPort = retrieveAvailablePort(resource.Port.IntVal, &p.entryManager.forwardedPorts) + entry.localPort = retrieveAvailablePort(resource.Address, resource.Port.IntVal, &p.entryManager.forwardedPorts) return entry, nil } diff --git a/pkg/skaffold/kubernetes/portforward/pod_forwarder_test.go b/pkg/skaffold/kubernetes/portforward/pod_forwarder_test.go index 5bd322d7e45..a710e127474 100644 --- a/pkg/skaffold/kubernetes/portforward/pod_forwarder_test.go +++ b/pkg/skaffold/kubernetes/portforward/pod_forwarder_test.go @@ -31,6 +31,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" schemautil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/GoogleContainerTools/skaffold/testutil" testEvent "github.com/GoogleContainerTools/skaffold/testutil/event" ) @@ -403,7 +404,7 @@ func TestAutomaticPortForwardPod(t *testing.T) { testutil.Run(t, test.description, func(t *testutil.T) { testEvent.InitializeState([]latest.Pipeline{{}}) taken := map[int]struct{}{} - t.Override(&retrieveAvailablePort, mockRetrieveAvailablePort(taken, test.availablePorts)) + t.Override(&retrieveAvailablePort, mockRetrieveAvailablePort(util.Loopback, taken, test.availablePorts)) t.Override(&topLevelOwnerKey, func(context.Context, metav1.Object, string) string { return "owner" }) if test.forwarder == nil { diff --git a/pkg/skaffold/kubernetes/portforward/port_forward_integration.go b/pkg/skaffold/kubernetes/portforward/port_forward_integration.go index 30149cdc540..7d90d210801 100644 --- a/pkg/skaffold/kubernetes/portforward/port_forward_integration.go +++ b/pkg/skaffold/kubernetes/portforward/port_forward_integration.go @@ -28,6 +28,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" schemautil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" ) // SimulateDevCycle is used for testing a port forward + stop + restart in a simulated dev cycle @@ -37,7 +38,7 @@ func SimulateDevCycle(t *testing.T, kubectlCLI *kubectl.CLI, namespace string) { defer func() { portForwardEvent = portForwardEventHandler }() portForwardEvent = func(entry *portForwardEntry) {} ctx := context.Background() - localPort := retrieveAvailablePort(9000, &em.forwardedPorts) + localPort := retrieveAvailablePort(util.Loopback, 9000, &em.forwardedPorts) pfe := newPortForwardEntry(0, latest.PortForwardResource{ Type: "deployment", Name: "leeroy-web", @@ -50,7 +51,7 @@ func SimulateDevCycle(t *testing.T, kubectlCLI *kubectl.CLI, namespace string) { logrus.Info("waiting for the same port to become available...") if err := wait.Poll(100*time.Millisecond, 5*time.Second, func() (done bool, err error) { - nextPort := retrieveAvailablePort(localPort, &em.forwardedPorts) + nextPort := retrieveAvailablePort(util.Loopback, localPort, &em.forwardedPorts) logrus.Infof("next port %d", nextPort) diff --git a/pkg/skaffold/kubernetes/portforward/resource_forwarder.go b/pkg/skaffold/kubernetes/portforward/resource_forwarder.go index 8e568ed5824..2f6f671a520 100644 --- a/pkg/skaffold/kubernetes/portforward/resource_forwarder.go +++ b/pkg/skaffold/kubernetes/portforward/resource_forwarder.go @@ -132,7 +132,7 @@ func (p *ResourceForwarder) getCurrentEntry(resource latest.PortForwardResource) if requestPort == 0 && resource.Port.IntVal >= 1024 { requestPort = resource.Port.IntVal } - entry.localPort = retrieveAvailablePort(requestPort, &p.entryManager.forwardedPorts) + entry.localPort = retrieveAvailablePort(resource.Address, requestPort, &p.entryManager.forwardedPorts) return entry } diff --git a/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go b/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go index fe24ae36fd6..babdd594d11 100644 --- a/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go +++ b/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go @@ -64,10 +64,10 @@ func newTestForwarder() *testForwarder { return &testForwarder{} } -func mockRetrieveAvailablePort(taken map[int]struct{}, availablePorts []int) func(int, *util.PortSet) int { +func mockRetrieveAvailablePort(_ string, taken map[int]struct{}, availablePorts []int) func(string, int, *util.PortSet) int { // Return first available port in ports that isn't taken var lock sync.Mutex - return func(int, *util.PortSet) int { + return func(string, int, *util.PortSet) int { for _, p := range availablePorts { lock.Lock() if _, ok := taken[p]; ok { @@ -122,7 +122,7 @@ func TestStart(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { testEvent.InitializeState([]latest.Pipeline{{}}) - t.Override(&retrieveAvailablePort, mockRetrieveAvailablePort(map[int]struct{}{}, test.availablePorts)) + t.Override(&retrieveAvailablePort, mockRetrieveAvailablePort(util.Loopback, map[int]struct{}{}, test.availablePorts)) t.Override(&retrieveServices, func(context.Context, string, []string) ([]*latest.PortForwardResource, error) { return test.resources, nil }) @@ -201,9 +201,9 @@ func TestGetCurrentEntryFunc(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - t.Override(&retrieveAvailablePort, func(req int, ps *util.PortSet) int { + t.Override(&retrieveAvailablePort, func(addr string, req int, ps *util.PortSet) int { t.CheckDeepEqual(test.expectedReq, req) - return mockRetrieveAvailablePort(map[int]struct{}{}, test.availablePorts)(req, ps) + return mockRetrieveAvailablePort(util.Loopback, map[int]struct{}{}, test.availablePorts)(addr, req, ps) }) entryManager := NewEntryManager(ioutil.Discard, newTestForwarder()) @@ -267,7 +267,7 @@ func TestUserDefinedResources(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { testEvent.InitializeState([]latest.Pipeline{{}}) - t.Override(&retrieveAvailablePort, mockRetrieveAvailablePort(map[int]struct{}{}, []int{8080, 9000})) + t.Override(&retrieveAvailablePort, mockRetrieveAvailablePort(util.Loopback, map[int]struct{}{}, []int{8080, 9000})) t.Override(&retrieveServices, func(context.Context, string, []string) ([]*latest.PortForwardResource, error) { return []*latest.PortForwardResource{svc}, nil }) diff --git a/pkg/skaffold/server/server.go b/pkg/skaffold/server/server.go index 726ffe98ce3..920155d5a19 100644 --- a/pkg/skaffold/server/server.go +++ b/pkg/skaffold/server/server.go @@ -242,7 +242,7 @@ func errorHandler(ctx context.Context, _ *runtime.ServeMux, marshaler runtime.Ma func listenOnAvailablePort(preferredPort int, usedPorts *util.PortSet) (net.Listener, int, error) { for try := 1; ; try++ { - port := util.GetAvailablePortWithAddress(util.Loopback, preferredPort, usedPorts) + port := util.GetAvailablePort(util.Loopback, preferredPort, usedPorts) l, err := net.Listen("tcp", fmt.Sprintf("%s:%d", util.Loopback, port)) if err != nil { diff --git a/pkg/skaffold/util/port.go b/pkg/skaffold/util/port.go index a4989b40683..5cce80dcde3 100644 --- a/pkg/skaffold/util/port.go +++ b/pkg/skaffold/util/port.go @@ -92,18 +92,13 @@ func (f *PortSet) List() []int { } // GetAvailablePort returns an available port that is near the requested port when possible. -// First, check if the provided port is available on the specified address. If so, use it. +// First, check if the provided port is available on the specified address and INADDR_ANY. If so, use it. // If not, check if any of the next 10 subsequent ports are available. // If not, check if any of ports 4503-4533 are available. // If not, return a random port, which hopefully won't collide with any future containers // // See https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt -func GetAvailablePort(port int, usedPorts *PortSet) int { - // We map this to allow - return GetAvailablePortWithAddress(Any, port, usedPorts) -} - -func GetAvailablePortWithAddress(address string, port int, usedPorts *PortSet) int { +func GetAvailablePort(address string, port int, usedPorts *PortSet) int { if port > 0 { if getPortIfAvailable(address, port, usedPorts) { return port @@ -143,14 +138,10 @@ func getPortIfAvailable(address string, p int, usedPorts *PortSet) bool { return false } - return IsPortFreeWithAddress(address, p) -} - -func IsPortFree(p int) bool { - return IsPortFreeWithAddress(Any, p) + return IsPortFree(address, p) } -func IsPortFreeWithAddress(address string, p int) bool { +func IsPortFree(address string, p int) bool { // Ensure the port is available across all interfaces l, err := net.Listen("tcp", fmt.Sprintf(":%d", p)) if err != nil || l == nil { diff --git a/pkg/skaffold/util/port_test.go b/pkg/skaffold/util/port_test.go index 6bc76a7238e..2de922fe082 100644 --- a/pkg/skaffold/util/port_test.go +++ b/pkg/skaffold/util/port_test.go @@ -56,7 +56,7 @@ func TestGetAvailablePort(t *testing.T) { wg.Add(N) for i := 0; i < N; i++ { go func() { - port := GetAvailablePort(4503, &ports) + port := GetAvailablePort(Loopback, 4503, &ports) l, err := net.Listen("tcp", fmt.Sprintf("%s:%d", Loopback, port)) if err != nil { From feae9352b54027bb00b70ac1c3cc5f1b34d388fc Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Tue, 27 Apr 2021 13:18:52 -0400 Subject: [PATCH 10/10] gofmt --- pkg/skaffold/util/port_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/skaffold/util/port_test.go b/pkg/skaffold/util/port_test.go index 2de922fe082..34519390d39 100644 --- a/pkg/skaffold/util/port_test.go +++ b/pkg/skaffold/util/port_test.go @@ -56,7 +56,7 @@ func TestGetAvailablePort(t *testing.T) { wg.Add(N) for i := 0; i < N; i++ { go func() { - port := GetAvailablePort(Loopback, 4503, &ports) + port := GetAvailablePort(Loopback, 4503, &ports) l, err := net.Listen("tcp", fmt.Sprintf("%s:%d", Loopback, port)) if err != nil {