Skip to content

Commit 78ae345

Browse files
author
Aaron Lehmann
authored
Merge pull request moby#1875 from yongtang/30199-update-publish-mode-host
Fix issue in service update of published ports in host mode
2 parents 0e4c281 + b270785 commit 78ae345

5 files changed

Lines changed: 216 additions & 6 deletions

File tree

‎manager/allocator/network.go‎

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -371,12 +371,15 @@ func (a *Allocator) doNetworkAlloc(ctx context.Context, ev events.Event) {
371371
s := v.Service.Copy()
372372

373373
if nc.nwkAllocator.IsServiceAllocated(s) {
374-
break
375-
}
376-
377-
if err := a.allocateService(ctx, s); err != nil {
378-
log.G(ctx).WithError(err).Errorf("Failed allocation during update of service %s", s.ID)
379-
break
374+
if nc.nwkAllocator.PortsAllocatedInHostPublishMode(s) {
375+
break
376+
}
377+
updatePortsInHostPublishMode(s)
378+
} else {
379+
if err := a.allocateService(ctx, s); err != nil {
380+
log.G(ctx).WithError(err).Errorf("Failed allocation during update of service %s", s.ID)
381+
break
382+
}
380383
}
381384

382385
if _, err := a.store.Batch(func(batch *store.Batch) error {
@@ -641,6 +644,36 @@ func (a *Allocator) commitAllocatedNode(ctx context.Context, batch *store.Batch,
641644
return nil
642645
}
643646

647+
// This function prepares the service object for being updated when the change regards
648+
// the published ports in host mode: It resets the runtime state ports (s.Endpoint.Ports)
649+
// to the current ingress mode runtime state ports plus the newly configured publish mode ports,
650+
// so that the service allocation invoked on this new service object will trigger the deallocation
651+
// of any old publish mode port and allocation of any new one.
652+
func updatePortsInHostPublishMode(s *api.Service) {
653+
if s.Endpoint != nil {
654+
var portConfigs []*api.PortConfig
655+
for _, portConfig := range s.Endpoint.Ports {
656+
if portConfig.PublishMode == api.PublishModeIngress {
657+
portConfigs = append(portConfigs, portConfig)
658+
}
659+
}
660+
s.Endpoint.Ports = portConfigs
661+
}
662+
663+
if s.Spec.Endpoint != nil {
664+
if s.Endpoint == nil {
665+
s.Endpoint = &api.Endpoint{}
666+
}
667+
for _, portConfig := range s.Spec.Endpoint.Ports {
668+
if portConfig.PublishMode == api.PublishModeIngress {
669+
continue
670+
}
671+
s.Endpoint.Ports = append(s.Endpoint.Ports, portConfig.Copy())
672+
}
673+
s.Endpoint.Spec = s.Spec.Endpoint.Copy()
674+
}
675+
}
676+
644677
func (a *Allocator) allocateService(ctx context.Context, s *api.Service) error {
645678
nc := a.netCtx
646679

‎manager/allocator/network_test.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package allocator
2+
3+
import (
4+
"testing"
5+
6+
"github.com/docker/swarmkit/api"
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func TestUpdatePortsInHostPublishMode(t *testing.T) {
11+
service := api.Service{
12+
Spec: api.ServiceSpec{
13+
Endpoint: &api.EndpointSpec{
14+
Ports: []*api.PortConfig{
15+
{
16+
Protocol: api.ProtocolTCP,
17+
TargetPort: 80,
18+
PublishedPort: 10000,
19+
PublishMode: api.PublishModeHost,
20+
},
21+
},
22+
},
23+
},
24+
Endpoint: &api.Endpoint{
25+
Ports: []*api.PortConfig{
26+
{
27+
Protocol: api.ProtocolTCP,
28+
TargetPort: 80,
29+
PublishedPort: 15000,
30+
PublishMode: api.PublishModeHost,
31+
},
32+
},
33+
},
34+
}
35+
updatePortsInHostPublishMode(&service)
36+
37+
assert.Equal(t, len(service.Endpoint.Ports), 1)
38+
assert.Equal(t, service.Endpoint.Ports[0].PublishedPort, uint32(10000))
39+
assert.Equal(t, service.Endpoint.Spec.Ports[0].PublishedPort, uint32(10000))
40+
}

‎manager/allocator/networkallocator/networkallocator.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,12 @@ func (na *NetworkAllocator) IsTaskAllocated(t *api.Task) bool {
283283
return true
284284
}
285285

286+
// PortsAllocatedInHostPublishMode returns if the passed service has its published ports in
287+
// host (non ingress) mode allocated
288+
func (na *NetworkAllocator) PortsAllocatedInHostPublishMode(s *api.Service) bool {
289+
return na.portAllocator.portsAllocatedInHostPublishMode(s)
290+
}
291+
286292
// IsServiceAllocated returns if the passed service has its network resources allocated or not.
287293
func (na *NetworkAllocator) IsServiceAllocated(s *api.Service) bool {
288294
// If endpoint mode is VIP and allocator does not have the

‎manager/allocator/networkallocator/portallocator.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,33 @@ func (pa *portAllocator) serviceDeallocatePorts(s *api.Service) {
269269
s.Endpoint.Ports = nil
270270
}
271271

272+
func (pa *portAllocator) portsAllocatedInHostPublishMode(s *api.Service) bool {
273+
if s.Endpoint == nil && s.Spec.Endpoint == nil {
274+
return true
275+
}
276+
277+
portStates := allocatedPorts{}
278+
if s.Endpoint != nil {
279+
for _, portState := range s.Endpoint.Ports {
280+
if portState.PublishMode == api.PublishModeHost {
281+
portStates.addState(portState)
282+
}
283+
}
284+
}
285+
286+
if s.Spec.Endpoint != nil {
287+
for _, portConfig := range s.Spec.Endpoint.Ports {
288+
if portConfig.PublishMode == api.PublishModeHost {
289+
if portStates.delState(portConfig) == nil {
290+
return false
291+
}
292+
}
293+
}
294+
}
295+
296+
return true
297+
}
298+
272299
func (pa *portAllocator) isPortsAllocated(s *api.Service) bool {
273300
// If service has no user-defined endpoint and allocated endpoint,
274301
// we assume it is allocated and return true.

‎manager/allocator/networkallocator/portallocator_test.go‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,110 @@ func TestServiceAllocatePorts(t *testing.T) {
264264
assert.Error(t, err)
265265
}
266266

267+
func TestPortsAllocatedInHostPublishMode(t *testing.T) {
268+
pa, err := newPortAllocator()
269+
assert.NoError(t, err)
270+
271+
type Data struct {
272+
input *api.Service
273+
expect bool
274+
}
275+
276+
testCases := []Data{
277+
{
278+
// both Endpoint and Spec.Endpoint are nil
279+
input: &api.Service{
280+
Spec: api.ServiceSpec{
281+
Endpoint: nil,
282+
},
283+
Endpoint: nil,
284+
},
285+
expect: true,
286+
},
287+
{
288+
// non host mode does not impact
289+
input: &api.Service{
290+
Spec: api.ServiceSpec{
291+
Endpoint: &api.EndpointSpec{
292+
Ports: []*api.PortConfig{
293+
{
294+
Name: "test1",
295+
Protocol: api.ProtocolTCP,
296+
TargetPort: 10000,
297+
PublishedPort: 10000,
298+
},
299+
},
300+
},
301+
},
302+
Endpoint: nil,
303+
},
304+
expect: true,
305+
},
306+
{
307+
// publish mode is different
308+
input: &api.Service{
309+
Spec: api.ServiceSpec{
310+
Endpoint: &api.EndpointSpec{
311+
Ports: []*api.PortConfig{
312+
{
313+
Name: "test1",
314+
Protocol: api.ProtocolTCP,
315+
TargetPort: 10000,
316+
PublishedPort: 10000,
317+
PublishMode: api.PublishModeHost,
318+
},
319+
},
320+
},
321+
},
322+
Endpoint: &api.Endpoint{
323+
Ports: []*api.PortConfig{
324+
{
325+
Name: "test1",
326+
Protocol: api.ProtocolTCP,
327+
TargetPort: 10000,
328+
PublishedPort: 10000,
329+
},
330+
},
331+
},
332+
},
333+
expect: false,
334+
},
335+
{
336+
input: &api.Service{
337+
Spec: api.ServiceSpec{
338+
Endpoint: &api.EndpointSpec{
339+
Ports: []*api.PortConfig{
340+
{
341+
Name: "test1",
342+
Protocol: api.ProtocolTCP,
343+
TargetPort: 10000,
344+
PublishedPort: 10000,
345+
PublishMode: api.PublishModeHost,
346+
},
347+
},
348+
},
349+
},
350+
Endpoint: &api.Endpoint{
351+
Ports: []*api.PortConfig{
352+
{
353+
Name: "test1",
354+
Protocol: api.ProtocolTCP,
355+
TargetPort: 10000,
356+
PublishedPort: 10000,
357+
PublishMode: api.PublishModeHost,
358+
},
359+
},
360+
},
361+
},
362+
expect: true,
363+
},
364+
}
365+
for _, singleTest := range testCases {
366+
expect := pa.portsAllocatedInHostPublishMode(singleTest.input)
367+
assert.Equal(t, expect, singleTest.expect)
368+
}
369+
}
370+
267371
func TestIsPortsAllocated(t *testing.T) {
268372
pa, err := newPortAllocator()
269373
assert.NoError(t, err)

0 commit comments

Comments
 (0)