Skip to content

Fix Multiple Network Support #23 #40

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.

Sign up for GitHub

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

Merged
stffabi merged 5 commits into from
Jan 24, 2017
Merged

Fix Multiple Network Support #23 #40

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fe0e34e
Reuse the network config for the relaunch
dolanor Dec 17, 2016
f8a2f80
Make an updated container connects to all the previously connected net
dolanor Dec 17, 2016
4909b90
go fmt done!
dolanor Dec 17, 2016
b45fc5a
Fix comment from HoundCI
dolanor Jan 2, 2017
db47382
Merge branch 'master' into net
stffabi Jan 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions container/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

log "github.com/Sirupsen/logrus"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/network"
dockerclient "github.com/docker/docker/client"
"golang.org/x/net/context"
)
Expand Down Expand Up @@ -118,17 +119,49 @@ func (client dockerClient) StartContainer(c Container) error {
bg := context.Background()
config := c.runtimeConfig()
hostConfig := c.hostConfig()
networkConfig := &network.NetworkingConfig{EndpointsConfig: c.containerInfo.NetworkSettings.Networks}
// simpleNetworkConfig is a networkConfig with only 1 network.
// see: https://github.com/docker/docker/issues/29265
simpleNetworkConfig := func() *network.NetworkingConfig {
oneEndpoint := make(map[string]*network.EndpointSettings)
for k, v := range networkConfig.EndpointsConfig {
oneEndpoint[k] = v
// we only need 1
break
}
return &network.NetworkingConfig{EndpointsConfig: oneEndpoint}
}()

name := c.Name()

log.Infof("Starting %s", name)
creation, err := client.api.ContainerCreate(bg, config, hostConfig, nil, name)
creation, err := client.api.ContainerCreate(bg, config, hostConfig, simpleNetworkConfig, name)
if err != nil {
return err
}

log.Debugf("Starting container %s (%s)", name, creation.ID)

return client.api.ContainerStart(bg, creation.ID, types.ContainerStartOptions{})
err = client.api.ContainerStart(bg, creation.ID, types.ContainerStartOptions{})
Copy link
Contributor

@stffabi stffabi Jan 24, 2017

Choose a reason for hiding this comment

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

@dolanor I've just seen that you reattach the networks after the container has already been started. I think it would be better if we first reattach the networks and afterwards start the container again. Or did you have some problems and made it in this order?

Copy link
Contributor Author

@dolanor dolanor Jan 25, 2017

Choose a reason for hiding this comment

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

The explanation of this behaviour is detailed in the git message 😉

f8a2f80

Copy link
Contributor

@stffabi stffabi Jan 25, 2017

Choose a reason for hiding this comment

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

Yes, I'm aware of the behaviour that we can't directly create the container with all attached networks. But my question was more if we could do it this way.

  1. ContainerCreate(simpleNetworkConfig)
  2. NetworkDisconnect(simpleNetworkConfig)
  3. NetworkConnect(networkConfig.EndpointsConfig)
  4. ContainerStart()

I think starting the container with only one attached network and afterwards disconnect it and reconnecting the correct ones, may result in strange behaviors for the containers.
It seems like docker-compose also first creates the container, then connects/disconnect the networks and as last step starts the container.

Copy link
Contributor Author

@dolanor dolanor Jan 25, 2017

Choose a reason for hiding this comment

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

Oh I see.
Good point. In fact I didn't understand the answer from thaJetzah like that. But you're right.
So I guess my current implementation doesn't work like docker-compose then.
I will fix it tomorrow.

Copy link
Contributor

@stffabi stffabi Jan 25, 2017

Choose a reason for hiding this comment

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

Thanks, that would be great.

if err != nil {
return err
}

for k := range simpleNetworkConfig.EndpointsConfig {
err = client.api.NetworkDisconnect(bg, k, creation.ID, true)
if err != nil {
return err
}
}

for k, v := range networkConfig.EndpointsConfig {
err = client.api.NetworkConnect(bg, k, creation.ID, v)
if err != nil {
return err
}
}
return nil

}

func (client dockerClient) RenameContainer(c Container, newName string) error {
Expand Down