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.

Fix polarity of a test in NodePort allocation
cb95c479 · Tim Hockin · b2525d43 · cb95c479
Commit cb95c479 authored Mar 16, 2017 by Tim Hockin
Hide whitespace changes
Inline Side-by-side

Showing with 4 additions and 4 deletions

rest.go pkg/registry/core/service/rest.go +4 -4

No files found.
--- 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)