From cb95c479cbe1d215cf963d6f58caf3b7d2fbd874 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Thu, 16 Mar 2017 17:19:26 -0700 Subject: [PATCH] Fix polarity of a test in NodePort allocation The result of this was that an update to a Service would release the NodePort temporarily (the repair loop would fix it in a minute). During that window, another Service could get allocated that Port. --- pkg/registry/core/service/rest.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/registry/core/service/rest.go b/pkg/registry/core/service/rest.go index b3bb390d30..88f4891e45 100644 --- a/pkg/registry/core/service/rest.go +++ b/pkg/registry/core/service/rest.go @@ -93,9 +93,6 @@ func (rs *REST) Create(ctx genericapirequest.Context, obj runtime.Object) (runti } }() - nodePortOp := portallocator.StartOperation(rs.serviceNodePorts) - defer nodePortOp.Finish() - if api.IsServiceIPRequested(service) { // Allocate next available. ip, err := rs.serviceIPs.AllocateNext() @@ -117,6 +114,9 @@ func (rs *REST) Create(ctx genericapirequest.Context, obj runtime.Object) (runti releaseServiceIP = true } + nodePortOp := portallocator.StartOperation(rs.serviceNodePorts) + defer nodePortOp.Finish() + assignNodePorts := shouldAssignNodePorts(service) svcPortToNodePort := map[int]int{} for i := range service.Spec.Ports { @@ -436,7 +436,7 @@ func (rs *REST) Update(ctx genericapirequest.Context, name string, objInfo rest. // The comparison loops are O(N^2), but we don't expect N to be huge // (there's a hard-limit at 2^16, because they're ports; and even 4 ports would be a lot) for _, oldNodePort := range oldNodePorts { - if !contains(newNodePorts, oldNodePort) { + if contains(newNodePorts, oldNodePort) { continue } nodePortOp.ReleaseDeferred(oldNodePort)