fix panic in portallocator and deallocate endpoints when update with none EndpointSpec

manager/allocator/allocator_test.go

@@ -50,7 +50,17 @@ func TestAllocator(t *testing.T) {
 						},
 					},
 				},
-				Endpoint: &api.EndpointSpec{},
+				Endpoint: &api.EndpointSpec{
+					Mode: api.ResolutionModeVirtualIP,
+					Ports: []*api.PortConfig{
+						{
+							Name:          "portName",
+							Protocol:      api.ProtocolTCP,
+							TargetPort:    8000,
+							PublishedPort: 8001,
+						},
+					},
+				},
 			},
 		}
 		assert.NoError(t, store.CreateService(tx, s1))
@@ -242,6 +252,16 @@ func TestAllocator(t *testing.T) {
 	watchNetwork(t, netWatch, false, isValidNetwork)
 	watchNetwork(t, netWatch, true, nil)
 
+	// Try updating service which is already allocated with no endpointSpec
+	assert.NoError(t, s.Update(func(tx store.Tx) error {
+		s := store.GetService(tx, "testServiceID1")
+		s.Spec.Endpoint = nil
+
+		assert.NoError(t, store.UpdateService(tx, s))
+		return nil
+	}))
+	watchService(t, serviceWatch, false, nil)
+
 	// Try updating task which is already allocated
 	assert.NoError(t, s.Update(func(tx store.Tx) error {
 		t2 := store.GetTask(tx, "testTaskID2")

manager/allocator/network.go

@@ -564,7 +564,9 @@ func (a *Allocator) allocateNode(ctx context.Context, nc *networkContext, node *
 
 func (a *Allocator) allocateService(ctx context.Context, nc *networkContext, s *api.Service) error {
 	if s.Spec.Endpoint != nil {
+		// service has user-defined endpoint
 		if s.Endpoint == nil {
+			// service currently has no allocated endpoint, need allocated.
 			s.Endpoint = &api.Endpoint{
 				Spec: s.Spec.Endpoint.Copy(),
 			}
@@ -587,6 +589,12 @@ func (a *Allocator) allocateService(ctx context.Context, nc *networkContext, s *
 					&api.Endpoint_VirtualIP{NetworkID: nc.ingressNetwork.ID})
 			}
 		}
+	} else if s.Endpoint != nil {
+		// service has no user-defined endpoints while has already allocated network resources,
+		// need deallocated.
+		if err := nc.nwkAllocator.ServiceDeallocate(s); err != nil {
+			return err
+		}
 	}
 
 	if err := nc.nwkAllocator.ServiceAllocate(s); err != nil {

manager/allocator/networkallocator/portallocator.go

@@ -155,7 +155,18 @@ func (pa *portAllocator) serviceDeallocatePorts(s *api.Service) {
 }
 
 func (pa *portAllocator) isPortsAllocated(s *api.Service) bool {
-	if s.Endpoint == nil {
+	// If service has no user-defined endpoint and allocated endpoint,
+	// we assume it is allocated and return true.
+	if s.Endpoint == nil && s.Spec.Endpoint == nil {
+		return true
+	}
+
+	// If service has allocated endpoint while has no user-defined endpoint,
+	// we assume allocated endpoints are redudant, and they need deallocated.
+	// If service has no allocated endpoint while has user-defined endpoint,
+	// we assume it is not allocated.
+	if (s.Endpoint != nil && s.Spec.Endpoint == nil) ||
+		(s.Endpoint == nil && s.Spec.Endpoint != nil) {
 		return false
 	}