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

Fix network connect dulicate mac-address #17860

Closed
wants to merge 1 commit into from

Conversation

coolljt0725
Copy link
Contributor

when a container created with --mac-address, the newer endpoint created
by docker network connect will has the same mac address with the exists
endpoint. And fix a typo in the comment of method ConnectToNetwork.

Signed-off-by: Lei Jitang leijitang@huawei.com

when a container created with --mac-address, the newer endpoint created
by docker network connect will has the same mac address with the exists
endpoint. And fix a typo in the comment of method ConnectToNetwork.

Signed-off-by: Lei Jitang <leijitang@huawei.com>
@@ -797,7 +797,7 @@ func (daemon *Daemon) updateNetwork(container *Container) error {
return nil
}

func (container *Container) buildCreateEndpointOptions(n libnetwork.Network) ([]libnetwork.EndpointOption, error) {
func (container *Container) buildCreateEndpointOptions(n libnetwork.Network, controller libnetwork.NetworkController) ([]libnetwork.EndpointOption, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we avoid passing the libnetwork.controller and simply check for n.Name() == "bridge"
The bridge string has been anyway hard-coded all over this file.
Another PR should take care of replacing those with a daemon constant.

@tiborvass
Copy link
Contributor

needs rebase

@sanimej
Copy link

sanimej commented Nov 10, 2015

@aboch @coolljt0725 PTAL at the updated changes for #17858
It should address this problem also.

@mavenugo
Copy link
Contributor

we can close this PR once #17858 is merged

@tiborvass tiborvass closed this Nov 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants