Merge pull request #780 from thaJeztah/fix-network-add

Fix --network-add adding duplicate networks
This commit is contained in:
Vincent Demeester 2018-01-22 14:45:58 -08:00 committed by GitHub
commit fbb9de2cfc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 136 additions and 16 deletions

View File

@ -16,6 +16,7 @@ type fakeClient struct {
serviceListFunc func(context.Context, types.ServiceListOptions) ([]swarm.Service, error)
taskListFunc func(context.Context, types.TaskListOptions) ([]swarm.Task, error)
infoFunc func(ctx context.Context) (types.Info, error)
networkInspectFunc func(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, error)
}
func (f *fakeClient) NodeList(ctx context.Context, options types.NodeListOptions) ([]swarm.Node, error) {
@ -60,6 +61,13 @@ func (f *fakeClient) Info(ctx context.Context) (types.Info, error) {
return f.infoFunc(ctx)
}
func (f *fakeClient) NetworkInspect(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, error) {
if f.networkInspectFunc != nil {
return f.networkInspectFunc(ctx, networkID, options)
}
return types.NetworkResource{}, nil
}
func newService(id string, name string) swarm.Service {
return swarm.Service{
ID: id,

View File

@ -353,22 +353,21 @@ func (c *credentialSpecOpt) Value() *swarm.CredentialSpec {
return c.value
}
func convertNetworks(ctx context.Context, apiClient client.NetworkAPIClient, networks opts.NetworkOpt) ([]swarm.NetworkAttachmentConfig, error) {
func resolveNetworkID(ctx context.Context, apiClient client.NetworkAPIClient, networkIDOrName string) (string, error) {
nw, err := apiClient.NetworkInspect(ctx, networkIDOrName, types.NetworkInspectOptions{Scope: "swarm"})
return nw.ID, err
}
func convertNetworks(networks opts.NetworkOpt) []swarm.NetworkAttachmentConfig {
var netAttach []swarm.NetworkAttachmentConfig
for _, net := range networks.Value() {
networkIDOrName := net.Target
_, err := apiClient.NetworkInspect(ctx, networkIDOrName, types.NetworkInspectOptions{Scope: "swarm"})
if err != nil {
return nil, err
}
netAttach = append(netAttach, swarm.NetworkAttachmentConfig{
Target: net.Target,
Aliases: net.Aliases,
DriverOpts: net.DriverOpts,
})
}
sort.Sort(byNetworkTarget(netAttach))
return netAttach, nil
return netAttach
}
type endpointOptions struct {
@ -590,10 +589,15 @@ func (options *serviceOptions) ToService(ctx context.Context, apiClient client.N
return service, err
}
networks, err := convertNetworks(ctx, apiClient, options.networks)
networks := convertNetworks(options.networks)
for i, net := range networks {
nwID, err := resolveNetworkID(ctx, apiClient, net.Target)
if err != nil {
return service, err
}
networks[i].Target = nwID
}
sort.Sort(byNetworkTarget(networks))
resources, err := options.resources.ToResourceRequirements()
if err != nil {

View File

@ -1,12 +1,17 @@
package service
import (
"context"
"fmt"
"testing"
"time"
"github.com/docker/cli/opts"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/swarm"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestMemBytesString(t *testing.T) {
@ -123,3 +128,37 @@ func TestResourceOptionsToResourceRequirements(t *testing.T) {
}
}
func TestToServiceNetwork(t *testing.T) {
nws := []types.NetworkResource{
{Name: "aaa-network", ID: "id555"},
{Name: "mmm-network", ID: "id999"},
{Name: "zzz-network", ID: "id111"},
}
client := &fakeClient{
networkInspectFunc: func(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, error) {
for _, network := range nws {
if network.ID == networkID || network.Name == networkID {
return network, nil
}
}
return types.NetworkResource{}, fmt.Errorf("network not found: %s", networkID)
},
}
nwo := opts.NetworkOpt{}
nwo.Set("zzz-network")
nwo.Set("mmm-network")
nwo.Set("aaa-network")
o := newServiceOptions()
o.mode = "replicated"
o.networks = nwo
ctx := context.Background()
flags := newCreateCommand(nil).Flags()
service, err := o.ToService(ctx, client, flags)
require.NoError(t, err)
assert.Equal(t, []swarm.NetworkAttachmentConfig{{Target: "id111"}, {Target: "id555"}, {Target: "id999"}}, service.TaskTemplate.Networks)
}

View File

@ -1119,14 +1119,16 @@ func updateNetworks(ctx context.Context, apiClient client.NetworkAPIClient, flag
if flags.Changed(flagNetworkAdd) {
values := flags.Lookup(flagNetworkAdd).Value.(*opts.NetworkOpt)
networks, err := convertNetworks(ctx, apiClient, *values)
networks := convertNetworks(*values)
for _, network := range networks {
nwID, err := resolveNetworkID(ctx, apiClient, network.Target)
if err != nil {
return err
}
for _, network := range networks {
if _, exists := existingNetworks[network.Target]; exists {
if _, exists := existingNetworks[nwID]; exists {
return errors.Errorf("service is already attached to network %s", network.Target)
}
network.Target = nwID
newNetworks = append(newNetworks, network)
existingNetworks[network.Target] = struct{}{}
}

View File

@ -1,6 +1,7 @@
package service
import (
"fmt"
"reflect"
"sort"
"testing"
@ -586,3 +587,69 @@ func TestRemoveGenericResources(t *testing.T) {
assert.NoError(t, removeGenericResources(flags, task))
assert.Len(t, task.Resources.Reservations.GenericResources, 1)
}
func TestUpdateNetworks(t *testing.T) {
ctx := context.Background()
nws := []types.NetworkResource{
{Name: "aaa-network", ID: "id555"},
{Name: "mmm-network", ID: "id999"},
{Name: "zzz-network", ID: "id111"},
}
client := &fakeClient{
networkInspectFunc: func(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, error) {
for _, network := range nws {
if network.ID == networkID || network.Name == networkID {
return network, nil
}
}
return types.NetworkResource{}, fmt.Errorf("network not found: %s", networkID)
},
}
svc := swarm.ServiceSpec{
TaskTemplate: swarm.TaskSpec{
ContainerSpec: &swarm.ContainerSpec{},
Networks: []swarm.NetworkAttachmentConfig{
{Target: "id999"},
},
},
}
flags := newUpdateCommand(nil).Flags()
err := flags.Set(flagNetworkAdd, "aaa-network")
require.NoError(t, err)
err = updateService(ctx, client, flags, &svc)
require.NoError(t, err)
assert.Equal(t, []swarm.NetworkAttachmentConfig{{Target: "id555"}, {Target: "id999"}}, svc.TaskTemplate.Networks)
flags = newUpdateCommand(nil).Flags()
err = flags.Set(flagNetworkAdd, "aaa-network")
require.NoError(t, err)
err = updateService(ctx, client, flags, &svc)
assert.EqualError(t, err, "service is already attached to network aaa-network")
assert.Equal(t, []swarm.NetworkAttachmentConfig{{Target: "id555"}, {Target: "id999"}}, svc.TaskTemplate.Networks)
flags = newUpdateCommand(nil).Flags()
err = flags.Set(flagNetworkAdd, "id555")
require.NoError(t, err)
err = updateService(ctx, client, flags, &svc)
assert.EqualError(t, err, "service is already attached to network id555")
assert.Equal(t, []swarm.NetworkAttachmentConfig{{Target: "id555"}, {Target: "id999"}}, svc.TaskTemplate.Networks)
flags = newUpdateCommand(nil).Flags()
err = flags.Set(flagNetworkRemove, "id999")
require.NoError(t, err)
err = updateService(ctx, client, flags, &svc)
assert.NoError(t, err)
assert.Equal(t, []swarm.NetworkAttachmentConfig{{Target: "id555"}}, svc.TaskTemplate.Networks)
flags = newUpdateCommand(nil).Flags()
err = flags.Set(flagNetworkAdd, "mmm-network")
require.NoError(t, err)
err = flags.Set(flagNetworkRemove, "aaa-network")
require.NoError(t, err)
err = updateService(ctx, client, flags, &svc)
assert.NoError(t, err)
assert.Equal(t, []swarm.NetworkAttachmentConfig{{Target: "id999"}}, svc.TaskTemplate.Networks)
}