-
Notifications
You must be signed in to change notification settings - Fork 617
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 networking support in agent #258
Conversation
Current coverage is
|
return | ||
} | ||
|
||
for _, na := range task.Networks { |
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.
This should be done in the executor, not the agent.
Updated engine-api godeps to `mrjana/engine-api agent` Signed-off-by: Jana Radhakrishnan <[email protected]>
Updated the PR. @stevvooe PTAL |
@@ -413,6 +413,7 @@ type EndpointResource struct { | |||
// NetworkCreate is the expected body of the "create network" http request message | |||
type NetworkCreate struct { |
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 for vendor updates - this will likely break next time one of us updates godep :/
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.
Unrelated to this PR but we should have a chat regarding the network API endpoints we'll need from engine
/cc @icecrime
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, we don't need to merge this if we are close to making a decision on swarm/engine integration. But are we?
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, that's why I mentioned "unrelated to this PR". It's not a blocker, it just reminded me we should discuss this
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.
We cannot fork vendored dependencies. I will not go through that madness... 🐹
Minor comments, LGTM |
return &network.NetworkingConfig{EndpointsConfig: epConfig} | ||
} | ||
|
||
func (c *containerConfig) networkOptions() []types.NetworkCreate { |
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.
networkCreate
.
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.
Ok. Will update.
Missing an error check on remove but otherwise, LGTM. |
This commit adds networking support in agent by processing task downloads and detecting network references in the task. When a network reference is detected, a network is created on the first reference and removed when the last task with a reference to that network is DEAD. Also, injected task side networking configuration so that containers are properly plumbed into the appropriate network. Signed-off-by: Jana Radhakrishnan <[email protected]>
See commit messages for more information
Signed-off-by: Jana Radhakrishnan [email protected]