Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add logic for finding next available port for gRPC if provided one is in use #1742

Closed
wants to merge 1 commit into from

Conversation

nkubala
Copy link
Contributor

@nkubala nkubala commented Mar 6, 2019

Fixes #1733

this adds a small function that makes skaffold a bit smarter about the port it uses for starting the gRPC event server. if the provided port is in use, it will attempt to parse out the port number and increment it until it finds a free port, or if the parsing fails, simply choose a random (valid) port and make sure it's free.

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.


// getOpenPort tests the provided port for availability,
// and if it's already in use, finds another open port.
func getAvailablePort(port string) string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may be able to reuse some of the port-forwarding functions here --
getAvailablePort
portAvailable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call. I did some refactoring that will have a slight functional change on the way we select ports when forwarding, so gonna close this PR and open that one first, then reopen this on top of those changes.

@codecov-io
Copy link

Codecov Report

Merging #1742 into master will increase coverage by 0.51%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1742      +/-   ##
==========================================
+ Coverage   47.02%   47.53%   +0.51%     
==========================================
  Files         126      126              
  Lines        6180     6254      +74     
==========================================
+ Hits         2906     2973      +67     
- Misses       2981     2986       +5     
- Partials      293      295       +2
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/cmd.go 0% <0%> (ø) ⬆️
pkg/skaffold/build/cache.go 56.18% <0%> (+12.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24f7382...9db9925. Read the comment docs.

@nkubala nkubala closed this Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants