Add logic for finding next available port for gRPC if provided one is in use #1752
Conversation
nkubala
requested review from
balopat,
dgageot,
priyawadhwa and
r2d4
as code owners
Codecov Report
@@ Coverage Diff @@
## master #1752 +/- ##
=======================================
Coverage 47.03% 47.03%
=======================================
Files 128 128
Lines 6213 6213
=======================================
Hits 2922 2922
Misses 2993 2993
Partials 298 298
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #1752 +/- ##
=======================================
Coverage 47.03% 47.03%
=======================================
Files 128 128
Lines 6213 6213
=======================================
Hits 2922 2922
Misses 2993 2993
Partials 298 298
Continue to review full report at Codecov.
|
| func newStatusServer(port string) (func() error, error) { | ||
| if port == "" { | ||
| func newStatusServer(originalPort int) (func() error, error) { | ||
| if originalPort == -1 { |
There was a problem hiding this comment.
I don't see why this would be -1? i see its called here with default port 50051?
skaffold/pkg/skaffold/event/event.go
Line 103 in 80d0347
There was a problem hiding this comment.
this was for testing as a way to bypass creating an RPC server in some cases. I probably should have removed it.
| return func() error { return nil }, nil | ||
| } | ||
| l, err := net.Listen("tcp", port) | ||
| port := util.GetAvailablePort(originalPort) | ||
| if port != originalPort && originalPort != constants.DefaultRPCPort { |
There was a problem hiding this comment.
wouldn't port != constants.DefaultRPCPort suffice?
is this to guard against condition where util.GetAvailbalePort returns -1?
Maybe we shd define an constant PORT_NOT_FOUND and assign it to -1 just to make this readable.
There was a problem hiding this comment.
this logic is basically "if the user provided a different port than the skaffold default, but we couldn't use that port because it was already in use, warn them that we're using a different port than they might expect".
if the user didn't provide a port, they probably don't care that skaffold didn't use its own default. the port being used will still be in the logs.
Fixes #1733
this adds functionality in the gRPC event server logic to be smarter about finding an available port if the provided one is already in use.
this also changes the CLI flag to be an int instead of a string, for clarity.
this will also issue a small warning to the user that their specified port is in use, if they provided a non-default port through the CLI.