Merge pull request #46258 from MrHohn/esipp-fix-needsUpdate

Automatic merge from submit-queue (batch tested with PRs 42042, 46139, 46126, 46258, 46312) Detect ExternalTrafficPolicy and HealthCheckNodePort changes in needsUpdate() Fix a bug that editing ExternalTrafficPolicy doesn't trigger LoadBalancer update. I'm surprise that ESIPP e2e tests didn't catch this. /assign @freehan @thockin **Release note**: ```release-note NONE ```

Merge pull request #46258 from MrHohn/esipp-fix-needsUpdate
447ee4a1 · Kubernetes Submit Queue · GitHub · 2b1b7f92 · 5f814d95 · 447ee4a1
Commit 447ee4a1 authored May 23, 2017 by Kubernetes Submit Queue Committed by GitHub May 23, 2017
Hide whitespace changes
Inline Side-by-side

Showing with 28 additions and 0 deletions

servicecontroller.go pkg/controller/service/servicecontroller.go +10 -0

servicecontroller_test.go pkg/controller/service/servicecontroller_test.go +18 -0

No files found.
--- a/pkg/controller/service/servicecontroller.go
+++ b/pkg/controller/service/servicecontroller.go
@@ -474,6 +474,16 @@ func (s *ServiceController) needsUpdate(oldService *v1.Service, newService *v1.S
 			oldService.UID, newService.UID)
 		return true
 	}
+	if oldService.Spec.ExternalTrafficPolicy != newService.Spec.ExternalTrafficPolicy {
+		s.eventRecorder.Eventf(newService, v1.EventTypeNormal, "ExternalTrafficPolicy", "%v -> %v",
+			oldService.Spec.ExternalTrafficPolicy, newService.Spec.ExternalTrafficPolicy)
+		return true
+	}
+	if oldService.Spec.HealthCheckNodePort != newService.Spec.HealthCheckNodePort {
+		s.eventRecorder.Eventf(newService, v1.EventTypeNormal, "HealthCheckNodePort", "%v -> %v",
+			oldService.Spec.HealthCheckNodePort, newService.Spec.HealthCheckNodePort)
+		return true
+	}

 	return false
 }

--- a/pkg/controller/service/servicecontroller_test.go
+++ b/pkg/controller/service/servicecontroller_test.go
@@ -673,6 +673,24 @@ func TestDoesExternalLoadBalancerNeedsUpdate(t *testing.T) {
 			},
 			expectedNeedsUpdate: true,
 		},
+		{
+			testName: "If ExternalTrafficPolicy is different",
+			updateFn: func() {
+				oldSvc = defaultExternalService()
+				newSvc = defaultExternalService()
+				newSvc.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeLocal
+			},
+			expectedNeedsUpdate: true,
+		},
+		{
+			testName: "If HealthCheckNodePort is different",
+			updateFn: func() {
+				oldSvc = defaultExternalService()
+				newSvc = defaultExternalService()
+				newSvc.Spec.HealthCheckNodePort = 30123
+			},
+			expectedNeedsUpdate: true,
+		},
 	}

 	controller, _, _ := newController()