Skip to content

Commit

Permalink
Fix: random port matches a custom local port
Browse files Browse the repository at this point in the history
Signed-off-by: Parthvi Vala <[email protected]>
  • Loading branch information
valaparthvi committed Apr 10, 2023
1 parent c2d850f commit 126d81a
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 9 deletions.
2 changes: 1 addition & 1 deletion pkg/odo/cli/dev/dev_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
Expand Down
25 changes: 18 additions & 7 deletions pkg/portForward/kubeportforward/portForward.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
20 changes: 19 additions & 1 deletion pkg/portForward/kubeportforward/portForward_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
Expand Down

0 comments on commit 126d81a

Please sign in to comment.