-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Pass a combination of ip and port to the task environment. #704
Conversation
The different labels can be on different IPs.
@@ -347,9 +347,8 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, dri | |||
// TODO refactor the implementation in GetTaskEnv to match | |||
// the 0.2 ports world view. Docker seems to be the only place where | |||
// this is actually needed, but this is kinda hacky. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what to do with this comment. What is the 0.2 ports world view? A port map is now passed to the taskEnv so that it can use this when building out the ports env.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think you can delete that. Your PR is the proper handling
This PR requires some updates to documentation. @dadgar: do you want to add a general description of this change to the docs? I think replacing NOMAD_PORT_"label" = port with NOMAD_IP_"label" = "ip:port" requires an introduction or general description? |
// E.g. $NOMAD_PORT_1 or $NOMAD_PORT_http | ||
PortPrefix = "NOMAD_PORT_" | ||
// E.g. $NOMAD_IP_1=127.0.0.1:1 or $NOMAD_IP_http=127.0.0.1:80 | ||
IPPortPrefix = "NOMAD_IP_" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iverberk Can we name this NOMAD_ADDR? I think addr sounds better since the value would be a ip:port and not just the IP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I agree
All comments are addressed. |
LGTM. Can we add documentation here: https://github.com/hashicorp/nomad/blob/master/website/source/docs/jobspec/networking.html.md |
Sure, I'll update the docs. |
👍 |
Pass a combination of ip and port to the task environment.
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
The different labels can be on different IPs so pass in a combination of the two.