From e089c3644711f8760663232a29cbc09890c2f90a Mon Sep 17 00:00:00 2001
From: Zihong Zheng <zihongz@google.com>
Date: Thu, 3 Aug 2017 16:45:42 -0700
Subject: [PATCH] Use nodePortOp for allocating healthCheck nodePort

---
 pkg/registry/core/service/rest.go | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/pkg/registry/core/service/rest.go b/pkg/registry/core/service/rest.go
index 46ac0100e37..3a353c34495 100644
--- a/pkg/registry/core/service/rest.go
+++ b/pkg/registry/core/service/rest.go
@@ -124,7 +124,7 @@ func (rs *REST) Create(ctx genericapirequest.Context, obj runtime.Object, includ
 	// Handle ExternalTraiffc related fields during service creation.
 	if utilfeature.DefaultFeatureGate.Enabled(features.ExternalTrafficLocalOnly) {
 		if apiservice.NeedsHealthCheck(service) {
-			if err := rs.allocateHealthCheckNodePort(service); err != nil {
+			if err := rs.allocateHealthCheckNodePort(service, nodePortOp); err != nil {
 				return nil, errors.NewInternalError(err)
 			}
 		}
@@ -244,7 +244,7 @@ func externalTrafficPolicyUpdate(oldService, service *api.Service) {
 
 // healthCheckNodePortUpdate handles HealthCheckNodePort allocation/release
 // and adjusts HealthCheckNodePort during service update if needed.
-func (rs *REST) healthCheckNodePortUpdate(oldService, service *api.Service) (bool, error) {
+func (rs *REST) healthCheckNodePortUpdate(oldService, service *api.Service, nodePortOp *portallocator.PortAllocationOperation) (bool, error) {
 	neededHealthCheckNodePort := apiservice.NeedsHealthCheck(oldService)
 	oldHealthCheckNodePort := oldService.Spec.HealthCheckNodePort
 
@@ -257,7 +257,7 @@ func (rs *REST) healthCheckNodePortUpdate(oldService, service *api.Service) (boo
 	// Insert health check node port into the service's HealthCheckNodePort field if needed.
 	case !neededHealthCheckNodePort && needsHealthCheckNodePort:
 		glog.Infof("Transition to LoadBalancer type service with ExternalTrafficPolicy=Local")
-		if err := rs.allocateHealthCheckNodePort(service); err != nil {
+		if err := rs.allocateHealthCheckNodePort(service, nodePortOp); err != nil {
 			return false, errors.NewInternalError(err)
 		}
 
@@ -265,12 +265,8 @@ func (rs *REST) healthCheckNodePortUpdate(oldService, service *api.Service) (boo
 	// Free the existing healthCheckNodePort and clear the HealthCheckNodePort field.
 	case neededHealthCheckNodePort && !needsHealthCheckNodePort:
 		glog.Infof("Transition to non LoadBalancer type service or LoadBalancer type service with ExternalTrafficPolicy=Global")
-		err := rs.serviceNodePorts.Release(int(oldHealthCheckNodePort))
-		if err != nil {
-			glog.Warningf("error releasing service health check %s node port %d: %v", service.Name, oldHealthCheckNodePort, err)
-			return false, errors.NewInternalError(fmt.Errorf("failed to free health check nodePort: %v", err))
-		}
-		glog.Infof("Freed health check nodePort: %d", oldHealthCheckNodePort)
+		glog.V(4).Infof("Releasing healthCheckNodePort: %d", oldHealthCheckNodePort)
+		nodePortOp.ReleaseDeferred(int(oldHealthCheckNodePort))
 		// Clear the HealthCheckNodePort field.
 		service.Spec.HealthCheckNodePort = 0
 
@@ -354,7 +350,7 @@ func (rs *REST) Update(ctx genericapirequest.Context, name string, objInfo rest.
 
 	// Handle ExternalTraiffc related updates.
 	if utilfeature.DefaultFeatureGate.Enabled(features.ExternalTrafficLocalOnly) {
-		success, err := rs.healthCheckNodePortUpdate(oldService, service)
+		success, err := rs.healthCheckNodePortUpdate(oldService, service, nodePortOp)
 		if !success || err != nil {
 			return nil, false, err
 		}
@@ -474,24 +470,24 @@ func findRequestedNodePort(port int, servicePorts []api.ServicePort) int {
 }
 
 // allocateHealthCheckNodePort allocates health check node port to service.
-func (rs *REST) allocateHealthCheckNodePort(service *api.Service) error {
+func (rs *REST) allocateHealthCheckNodePort(service *api.Service, nodePortOp *portallocator.PortAllocationOperation) error {
 	healthCheckNodePort := service.Spec.HealthCheckNodePort
 	if healthCheckNodePort != 0 {
 		// If the request has a health check nodePort in mind, attempt to reserve it.
-		err := rs.serviceNodePorts.Allocate(int(healthCheckNodePort))
+		err := nodePortOp.Allocate(int(healthCheckNodePort))
 		if err != nil {
 			return fmt.Errorf("failed to allocate requested HealthCheck NodePort %v: %v",
 				healthCheckNodePort, err)
 		}
-		glog.Infof("Reserved user requested nodePort: %d", healthCheckNodePort)
+		glog.V(4).Infof("Reserved user requested healthCheckNodePort: %d", healthCheckNodePort)
 	} else {
 		// If the request has no health check nodePort specified, allocate any.
-		healthCheckNodePort, err := rs.serviceNodePorts.AllocateNext()
+		healthCheckNodePort, err := nodePortOp.AllocateNext()
 		if err != nil {
 			return fmt.Errorf("failed to allocate a HealthCheck NodePort %v: %v", healthCheckNodePort, err)
 		}
 		service.Spec.HealthCheckNodePort = int32(healthCheckNodePort)
-		glog.Infof("Reserved allocated nodePort: %d", healthCheckNodePort)
+		glog.V(4).Infof("Reserved allocated healthCheckNodePort: %d", healthCheckNodePort)
 	}
 	return nil
 }