diff --git a/pkg/odo/cli/dev/dev_test.go b/pkg/odo/cli/dev/dev_test.go index 6a02f0c4304..f6c7e0d9352 100644 --- a/pkg/odo/cli/dev/dev_test.go +++ b/pkg/odo/cli/dev/dev_test.go @@ -280,7 +280,7 @@ func Test_parsePortForwardFlag(t *testing.T) { return } if diff := cmp.Diff(gotForwardedPorts, tt.wantForwardedPorts); diff != "" { - t.Errorf("parsePortForwardFlag() gotForwardedPorts = %v, want %v", gotForwardedPorts, tt.wantForwardedPorts) + t.Errorf("parsePortForwardFlag() (got vs want) diff = %v", diff) } }) } diff --git a/pkg/portForward/kubeportforward/portForward.go b/pkg/portForward/kubeportforward/portForward.go index 91bb33b5f8e..0afc248a075 100644 --- a/pkg/portForward/kubeportforward/portForward.go +++ b/pkg/portForward/kubeportforward/portForward.go @@ -172,8 +172,11 @@ func (o *PFClient) GetForwardedPorts() map[string][]v1alpha2.Endpoint { // if not, it assigns a port starting from 20001 as done in portPairsFromContainerEndpoints func getCustomPortPairs(definedPorts []api.ForwardedPort, ceMapping map[string][]v1alpha2.Endpoint) map[string][]string { portPairs := make(map[string][]string) - - // getCustomLocalPort analyzes the definedPorts i.e. custom portforwarding to see if a containerPort has a custom localPort, if a container name is provided, it also takes that into account. + usedPorts := make(map[int]struct{}) + for _, dPort := range definedPorts { + usedPorts[dPort.LocalPort] = struct{}{} + } + // getCustomLocalPort analyzes the definedPorts i.e. custom port forwarding to see if a containerPort has a custom localPort, if a container name is provided, it also takes that into account. getCustomLocalPort := func(containerPort int, container string) int { for _, dp := range definedPorts { if dp.ContainerPort == containerPort { @@ -194,11 +197,19 @@ func getCustomPortPairs(definedPorts []api.ForwardedPort, ceMapping map[string][ for _, p := range ports { freePort := getCustomLocalPort(p.TargetPort, name) if freePort == 0 { - var err error - freePort, err = util.NextFreePort(startPort, endPort, nil) - if err != nil { - klog.Infof("%s", err) - continue + for { + var err error + freePort, err = util.NextFreePort(startPort, endPort, nil) + if err != nil { + klog.Infof("%s", err) + continue + } + // if the free port matches any of the custom local port, try again + if _, isPortUsed := usedPorts[freePort]; isPortUsed { + startPort = freePort + 1 + continue + } + break } startPort = freePort + 1 } diff --git a/pkg/portForward/kubeportforward/portForward_test.go b/pkg/portForward/kubeportforward/portForward_test.go index f31886c3718..bf4a4332f40 100644 --- a/pkg/portForward/kubeportforward/portForward_test.go +++ b/pkg/portForward/kubeportforward/portForward_test.go @@ -51,12 +51,30 @@ func Test_getCompleteCustomPortPairs(t *testing.T) { "tools": {"5000:5000"}, }, }, + { + name: "local ports in range [20001-30001] are provided as custom forward ports", + args: args{ + definedPorts: []api.ForwardedPort{ + {LocalPort: 20001, ContainerPort: 8000}, + {LocalPort: 20002, ContainerPort: 9000}, + {LocalPort: 5000, ContainerPort: 5000}, + }, + ceMapping: map[string][]v1alpha2.Endpoint{ + "runtime": {{TargetPort: 8000}, {TargetPort: 9000}}, + "tools": {{TargetPort: 5000}, {TargetPort: 8080}}, + }, + }, + wantPortPairs: map[string][]string{ + "runtime": {"20001:8000", "20002:9000"}, + "tools": {"5000:5000", "20003:8080"}, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { gotPortPairs := getCustomPortPairs(tt.args.definedPorts, tt.args.ceMapping) if diff := cmp.Diff(gotPortPairs, tt.wantPortPairs); diff != "" { - t.Errorf("getCompleteCustomPortPairs() = %v, want %v", gotPortPairs, tt.wantPortPairs) + t.Errorf("getCompleteCustomPortPairs() (got vs want) diff = %v", diff) } }) }