diff --git a/manager/allocator/allocator_test.go b/manager/allocator/allocator_test.go index ac3f74bc30..6a509b9016 100644 --- a/manager/allocator/allocator_test.go +++ b/manager/allocator/allocator_test.go @@ -796,6 +796,149 @@ func TestAllocatorRestoreForDuplicateIPs(t *testing.T) { } } +// TestAllocatorRestartNoEndpointSpec covers the leader election case when the service Spec +// does not contain the EndpointSpec. +// The expected behavior is that the VIP(s) are still correctly populated inside +// the IPAM and that no configuration on the service is changed. +func TestAllocatorRestartNoEndpointSpec(t *testing.T) { + s := store.NewMemoryStore(nil) + assert.NotNil(t, s) + defer s.Close() + // Create 3 services with 1 task each + numsvcstsks := 3 + assert.NoError(t, s.Update(func(tx store.Tx) error { + // populate ingress network + in := &api.Network{ + ID: "overlay1", + Spec: api.NetworkSpec{ + Annotations: api.Annotations{ + Name: "net1", + }, + }, + } + assert.NoError(t, store.CreateNetwork(tx, in)) + + for i := 0; i != numsvcstsks; i++ { + svc := &api.Service{ + ID: "testServiceID" + strconv.Itoa(i), + Spec: api.ServiceSpec{ + Annotations: api.Annotations{ + Name: "service" + strconv.Itoa(i), + }, + // Endpoint: &api.EndpointSpec{ + // Mode: api.ResolutionModeVirtualIP, + // }, + Task: api.TaskSpec{ + Networks: []*api.NetworkAttachmentConfig{ + { + Target: "overlay1", + }, + }, + }, + }, + Endpoint: &api.Endpoint{ + Spec: &api.EndpointSpec{ + Mode: api.ResolutionModeVirtualIP, + }, + VirtualIPs: []*api.Endpoint_VirtualIP{ + { + NetworkID: "overlay1", + Addr: "10.0.0." + strconv.Itoa(2+2*i) + "/24", + }, + }, + }, + } + assert.NoError(t, store.CreateService(tx, svc)) + } + return nil + })) + + for i := 0; i != numsvcstsks; i++ { + assert.NoError(t, s.Update(func(tx store.Tx) error { + tsk := &api.Task{ + ID: "testTaskID" + strconv.Itoa(i), + Status: api.TaskStatus{ + State: api.TaskStateNew, + }, + ServiceID: "testServiceID" + strconv.Itoa(i), + DesiredState: api.TaskStateRunning, + Networks: []*api.NetworkAttachment{ + { + Network: &api.Network{ + ID: "overlay1", + }, + }, + }, + } + assert.NoError(t, store.CreateTask(tx, tsk)) + return nil + })) + } + + expectedIPs := map[string]string{ + "testServiceID0": "10.0.0.2/24", + "testServiceID1": "10.0.0.4/24", + "testServiceID2": "10.0.0.6/24", + "testTaskID0": "10.0.0.3/24", + "testTaskID1": "10.0.0.5/24", + "testTaskID2": "10.0.0.7/24", + } + assignedIPs := make(map[string]bool) + hasNoIPOverlapServices := func(fakeT assert.TestingT, service *api.Service) bool { + assert.NotEqual(fakeT, len(service.Endpoint.VirtualIPs), 0) + assert.NotEqual(fakeT, len(service.Endpoint.VirtualIPs[0].Addr), 0) + + assignedVIP := service.Endpoint.VirtualIPs[0].Addr + if assignedIPs[assignedVIP] { + t.Fatalf("service %s assigned duplicate IP %s", service.ID, assignedVIP) + } + assignedIPs[assignedVIP] = true + ip, ok := expectedIPs[service.ID] + assert.True(t, ok) + assert.Equal(t, ip, assignedVIP) + delete(expectedIPs, service.ID) + return true + } + + hasNoIPOverlapTasks := func(fakeT assert.TestingT, s *store.MemoryStore, task *api.Task) bool { + assert.NotEqual(fakeT, len(task.Networks), 0) + assert.NotEqual(fakeT, len(task.Networks[0].Addresses), 0) + + assignedIP := task.Networks[0].Addresses[0] + if assignedIPs[assignedIP] { + t.Fatalf("task %s assigned duplicate IP %s", task.ID, assignedIP) + } + assignedIPs[assignedIP] = true + ip, ok := expectedIPs[task.ID] + assert.True(t, ok) + assert.Equal(t, ip, assignedIP) + delete(expectedIPs, task.ID) + return true + } + + a, err := New(s, nil) + assert.NoError(t, err) + assert.NotNil(t, a) + // Start allocator + go func() { + assert.NoError(t, a.Run(context.Background())) + }() + defer a.Stop() + + taskWatch, cancel := state.Watch(s.WatchQueue(), api.EventUpdateTask{}, api.EventDeleteTask{}) + defer cancel() + + serviceWatch, cancel := state.Watch(s.WatchQueue(), api.EventUpdateService{}, api.EventDeleteService{}) + defer cancel() + + // Confirm tasks have no IPs that overlap with the services VIPs on restart + for i := 0; i != numsvcstsks; i++ { + watchTask(t, s, taskWatch, false, hasNoIPOverlapTasks) + watchService(t, serviceWatch, false, hasNoIPOverlapServices) + } + assert.Len(t, expectedIPs, 0) +} + func TestNodeAllocator(t *testing.T) { s := store.NewMemoryStore(nil) assert.NotNil(t, s) diff --git a/manager/allocator/cnmallocator/networkallocator.go b/manager/allocator/cnmallocator/networkallocator.go index b89e72ed6e..2d533a47cd 100644 --- a/manager/allocator/cnmallocator/networkallocator.go +++ b/manager/allocator/cnmallocator/networkallocator.go @@ -15,6 +15,7 @@ import ( "github.com/docker/swarmkit/log" "github.com/docker/swarmkit/manager/allocator/networkallocator" "github.com/pkg/errors" + "github.com/sirupsen/logrus" "golang.org/x/net/context" ) @@ -244,6 +245,7 @@ vipLoop: } for _, nAttach := range specNetworks { if nAttach.Target == eAttach.NetworkID { + log.L.WithFields(logrus.Fields{"service_id": s.ID, "vip": eAttach.Addr}).Debug("allocate vip") if err = na.allocateVIP(eAttach); err != nil { return err } diff --git a/manager/allocator/network.go b/manager/allocator/network.go index ac798c954f..aee03870b1 100644 --- a/manager/allocator/network.go +++ b/manager/allocator/network.go @@ -244,7 +244,7 @@ func (a *Allocator) doNetworkAlloc(ctx context.Context, ev events.Event) { break } - if err := a.allocateService(ctx, s); err != nil { + if err := a.allocateService(ctx, s, false); err != nil { log.G(ctx).WithError(err).Errorf("Failed allocation for service %s", s.ID) break } @@ -274,7 +274,7 @@ func (a *Allocator) doNetworkAlloc(ctx context.Context, ev events.Event) { } updatePortsInHostPublishMode(s) } else { - if err := a.allocateService(ctx, s); err != nil { + if err := a.allocateService(ctx, s, false); err != nil { log.G(ctx).WithError(err).Errorf("Failed allocation during update of service %s", s.ID) break } @@ -587,8 +587,8 @@ func (a *Allocator) allocateServices(ctx context.Context, existingAddressesOnly continue } - if err := a.allocateService(ctx, s); err != nil { - log.G(ctx).WithError(err).Errorf("failed allocating service %s during init", s.ID) + if err := a.allocateService(ctx, s, existingAddressesOnly); err != nil { + log.G(ctx).WithField("existingAddressesOnly", existingAddressesOnly).WithError(err).Errorf("failed allocating service %s during init", s.ID) continue } allocatedServices = append(allocatedServices, s) @@ -940,7 +940,10 @@ func updatePortsInHostPublishMode(s *api.Service) { s.Endpoint.Spec = s.Spec.Endpoint.Copy() } -func (a *Allocator) allocateService(ctx context.Context, s *api.Service) error { +// allocateService takes care to align the desired state with the spec passed +// the last parameter is true only during restart when the data is read from raft +// and used to build internal state +func (a *Allocator) allocateService(ctx context.Context, s *api.Service, existingAddressesOnly bool) error { nc := a.netCtx if s.Spec.Endpoint != nil { @@ -972,7 +975,9 @@ func (a *Allocator) allocateService(ctx context.Context, s *api.Service) error { &api.Endpoint_VirtualIP{NetworkID: nc.ingressNetwork.ID}) } } - } else if s.Endpoint != nil { + } else if s.Endpoint != nil && !existingAddressesOnly { + // if we are in the restart phase there is no reason to try to deallocate anything because the state + // is not there // service has no user-defined endpoints while has already allocated network resources, // need deallocated. if err := nc.nwkAllocator.DeallocateService(s); err != nil { @@ -1188,7 +1193,7 @@ func (a *Allocator) procUnallocatedServices(ctx context.Context) { var allocatedServices []*api.Service for _, s := range nc.unallocatedServices { if !nc.nwkAllocator.IsServiceAllocated(s) { - if err := a.allocateService(ctx, s); err != nil { + if err := a.allocateService(ctx, s, false); err != nil { log.G(ctx).WithError(err).Debugf("Failed allocation of unallocated service %s", s.ID) continue }