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

validate that hostports are not configured twice with the same protocol #2975

Closed
BenTheElder opened this issue Oct 21, 2022 · 6 comments · Fixed by #3175
Closed

validate that hostports are not configured twice with the same protocol #2975

BenTheElder opened this issue Oct 21, 2022 · 6 comments · Fixed by #3175
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@BenTheElder
Copy link
Member

cc @aojea (thinking of that kubernetes issue)

@BenTheElder BenTheElder added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 21, 2022
@aojea
Copy link
Contributor

aojea commented Oct 21, 2022

👁️

@aroradaman
Copy link
Member

aroradaman commented Jan 11, 2023

Hi @BenTheElder @aojea

(thinking of that kubernetes issue)

Can you guys share the link to the issue, I would like to take this up.

@BenTheElder
Copy link
Member Author

The issue doesn't really add more context than this title, it just brought the topic to attention: hostports should not be configured twice with the same protocol.

You can't listen to say port 80 on the host on TCP twice to different container Ports. It's not possible, both port forwards have to create a listener on the host on the (port, protocol) and if there's duplicate entries covering the same (hostPort, protocol) that should be an error.

Sorry I've been drowning in notifications :-)

@BenTheElder BenTheElder added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Apr 18, 2023
@aroradaman
Copy link
Member

@BenTheElder It is possible to bind a socket to the same port and protocol on different interfaces (listenAddress).
Should we also consider this then? Would need to handle 0.0.0.0 separately.

@aroradaman
Copy link
Member

aroradaman commented Apr 18, 2023

@BenTheElder @aojea While going through the code I found that we don't check if the user-provided port is free/available, is this intentional or should I create an issue for this?

func PortOrGetFreePort(port int32, listenAddr string) (int32, error) {
// in the case of -1 we actually want to pass 0 to the backend to let it pick
if port == -1 {
return 0, nil
}
// in the case of 0 (unset) we want kind to pick one and supply it to the backend
if port == 0 {
return GetFreePort(listenAddr)
}
// otherwise keep the port
return port, nil
}

@BenTheElder
Copy link
Member Author

Yes we should consider listenAddress.

No we should not attempt to check if the port is free.

  1. That can change between checking and actually creating the cluster
  2. The cluster nodes may be created on a remote machine via DOCKER_HOST or similar

The linked utility is used for obtaining a semi-randomized port, because if we don't explicitly set one it will be re-randomized when the container is recreated on restart. In the case of a remote host -1 may be configured to avoid the local check at the cost of the reboot behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants