From 08b5c809d2fd7d5761b41d1e12b6bd0eeb1695b3 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Fri, 15 Nov 2019 15:30:42 -0500
Subject: [PATCH] Fix label mutation in endpoints controller

---
 pkg/controller/endpoint/BUILD                      |  2 ++
 pkg/controller/endpoint/endpoints_controller.go    |  5 +++--
 .../endpoint/endpoints_controller_test.go          | 14 +++++++++++---
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/pkg/controller/endpoint/BUILD b/pkg/controller/endpoint/BUILD
index b0fec6504f5..f4fc32b463f 100644
--- a/pkg/controller/endpoint/BUILD
+++ b/pkg/controller/endpoint/BUILD
@@ -16,6 +16,7 @@ go_library(
         "//pkg/controller:go_default_library",
         "//pkg/controller/util/endpoint:go_default_library",
         "//pkg/features:go_default_library",
+        "//pkg/util/labels:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
@@ -54,6 +55,7 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
diff --git a/pkg/controller/endpoint/endpoints_controller.go b/pkg/controller/endpoint/endpoints_controller.go
index 0a7b592932e..4c3186d2c5c 100644
--- a/pkg/controller/endpoint/endpoints_controller.go
+++ b/pkg/controller/endpoint/endpoints_controller.go
@@ -48,6 +48,7 @@ import (
 	"k8s.io/kubernetes/pkg/controller"
 	endpointutil "k8s.io/kubernetes/pkg/controller/util/endpoint"
 	"k8s.io/kubernetes/pkg/features"
+	utillabels "k8s.io/kubernetes/pkg/util/labels"
 	utilnet "k8s.io/utils/net"
 )
 
@@ -503,9 +504,9 @@ func (e *EndpointController) syncService(key string) error {
 	}
 
 	if !helper.IsServiceIPSet(service) {
-		newEndpoints.Labels[v1.IsHeadlessService] = ""
+		newEndpoints.Labels = utillabels.CloneAndAddLabel(newEndpoints.Labels, v1.IsHeadlessService, "")
 	} else {
-		delete(newEndpoints.Labels, v1.IsHeadlessService)
+		newEndpoints.Labels = utillabels.CloneAndRemoveLabel(newEndpoints.Labels, v1.IsHeadlessService)
 	}
 
 	klog.V(4).Infof("Update endpoints for %v/%v, ready: %d not ready: %d", service.Namespace, service.Name, totalReadyEps, totalNotReadyEps)
diff --git a/pkg/controller/endpoint/endpoints_controller_test.go b/pkg/controller/endpoint/endpoints_controller_test.go
index f1cd5e01ac4..cd6fcc95387 100644
--- a/pkg/controller/endpoint/endpoints_controller_test.go
+++ b/pkg/controller/endpoint/endpoints_controller_test.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"reflect"
 	"strconv"
 	"testing"
 	"time"
@@ -28,6 +29,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/wait"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
@@ -825,14 +827,16 @@ func TestSyncEndpointsHeadlessService(t *testing.T) {
 		}},
 	})
 	addPods(endpoints.podStore, ns, 1, 1, 0, false)
-	endpoints.serviceStore.Add(&v1.Service{
-		ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: ns},
+	service := &v1.Service{
+		ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: ns, Labels: map[string]string{"a": "b"}},
 		Spec: v1.ServiceSpec{
 			Selector:  map[string]string{},
 			ClusterIP: api.ClusterIPNone,
 			Ports:     []v1.ServicePort{},
 		},
-	})
+	}
+	originalService := service.DeepCopy()
+	endpoints.serviceStore.Add(service)
 	endpoints.syncService(ns + "/foo")
 	data := runtime.EncodeOrDie(testapi.Default.Codec(), &v1.Endpoints{
 		ObjectMeta: metav1.ObjectMeta{
@@ -840,6 +844,7 @@ func TestSyncEndpointsHeadlessService(t *testing.T) {
 			Namespace:       ns,
 			ResourceVersion: "1",
 			Labels: map[string]string{
+				"a":                  "b",
 				v1.IsHeadlessService: "",
 			},
 		},
@@ -848,6 +853,9 @@ func TestSyncEndpointsHeadlessService(t *testing.T) {
 			Ports:     []v1.EndpointPort{},
 		}},
 	})
+	if !reflect.DeepEqual(originalService, service) {
+		t.Fatalf("syncing endpoints changed service: %s", diff.ObjectReflectDiff(service, originalService))
+	}
 	endpointsHandler.ValidateRequestCount(t, 1)
 	endpointsHandler.ValidateRequest(t, testapi.Default.ResourcePath("endpoints", ns, "foo"), "PUT", &data)
 }