From 1c0a78a4fdb881cb6c14a0a29c2890334974d984 Mon Sep 17 00:00:00 2001 From: Manohar Reddy Date: Tue, 25 Feb 2020 16:27:36 +0530 Subject: [PATCH 1/3] Use servicePatch methods from cloud-provider repo in service-controller --- pkg/controller/service/controller.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/controller/service/controller.go b/pkg/controller/service/controller.go index aa82c6ed999..2cfc08363b1 100644 --- a/pkg/controller/service/controller.go +++ b/pkg/controller/service/controller.go @@ -804,8 +804,7 @@ func (s *Controller) addFinalizer(service *v1.Service) error { updated.ObjectMeta.Finalizers = append(updated.ObjectMeta.Finalizers, servicehelper.LoadBalancerCleanupFinalizer) klog.V(2).Infof("Adding finalizer to service %s/%s", updated.Namespace, updated.Name) - // TODO(87447) use PatchService from k8s.io/cloud-provider/service/helpers - _, err := patch(s.kubeClient.CoreV1(), service, updated) + _, err := servicehelper.PatchService(s.kubeClient.CoreV1(), service, updated) return err } @@ -820,7 +819,7 @@ func (s *Controller) removeFinalizer(service *v1.Service) error { updated.ObjectMeta.Finalizers = removeString(updated.ObjectMeta.Finalizers, servicehelper.LoadBalancerCleanupFinalizer) klog.V(2).Infof("Removing finalizer from service %s/%s", updated.Namespace, updated.Name) - _, err := patch(s.kubeClient.CoreV1(), service, updated) + _, err := servicehelper.PatchService(s.kubeClient.CoreV1(), service, updated) return err } @@ -846,7 +845,7 @@ func (s *Controller) patchStatus(service *v1.Service, previousStatus, newStatus updated.Status.LoadBalancer = *newStatus klog.V(2).Infof("Patching status for service %s/%s", updated.Namespace, updated.Name) - _, err := patch(s.kubeClient.CoreV1(), service, updated) + _, err := servicehelper.PatchService(s.kubeClient.CoreV1(), service, updated) return err } From 08473a49497d1b3fffce8f19ee12ac603512a10c Mon Sep 17 00:00:00 2001 From: Manohar Reddy Date: Tue, 25 Feb 2020 20:13:43 +0530 Subject: [PATCH 2/3] remote patch.go and patch_test.go files --- pkg/controller/service/BUILD | 2 - pkg/controller/service/patch.go | 61 ---------------- pkg/controller/service/patch_test.go | 102 --------------------------- 3 files changed, 165 deletions(-) delete mode 100644 pkg/controller/service/patch.go delete mode 100644 pkg/controller/service/patch_test.go diff --git a/pkg/controller/service/BUILD b/pkg/controller/service/BUILD index ddceb9e498c..398b8d295f2 100644 --- a/pkg/controller/service/BUILD +++ b/pkg/controller/service/BUILD @@ -5,7 +5,6 @@ go_library( srcs = [ "controller.go", "doc.go", - "patch.go", ], importpath = "k8s.io/kubernetes/pkg/controller/service", visibility = ["//visibility:public"], @@ -39,7 +38,6 @@ go_test( name = "go_default_test", srcs = [ "controller_test.go", - "patch_test.go", ], embed = [":go_default_library"], deps = [ diff --git a/pkg/controller/service/patch.go b/pkg/controller/service/patch.go deleted file mode 100644 index ceb6221fdfb..00000000000 --- a/pkg/controller/service/patch.go +++ /dev/null @@ -1,61 +0,0 @@ -/* -Copyright 2019 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package service - -import ( - "context" - "encoding/json" - "fmt" - - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/strategicpatch" - v1core "k8s.io/client-go/kubernetes/typed/core/v1" -) - -// patch patches service's Status or ObjectMeta given the origin and -// updated ones. Change to spec will be ignored. -func patch(c v1core.CoreV1Interface, oldSvc *v1.Service, newSvc *v1.Service) (*v1.Service, error) { - // Reset spec to make sure only patch for Status or ObjectMeta. - newSvc.Spec = oldSvc.Spec - - patchBytes, err := getPatchBytes(oldSvc, newSvc) - if err != nil { - return nil, err - } - - return c.Services(oldSvc.Namespace).Patch(context.TODO(), oldSvc.Name, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{}, "status") -} - -func getPatchBytes(oldSvc *v1.Service, newSvc *v1.Service) ([]byte, error) { - oldData, err := json.Marshal(oldSvc) - if err != nil { - return nil, fmt.Errorf("failed to Marshal oldData for svc %s/%s: %v", oldSvc.Namespace, oldSvc.Name, err) - } - - newData, err := json.Marshal(newSvc) - if err != nil { - return nil, fmt.Errorf("failed to Marshal newData for svc %s/%s: %v", newSvc.Namespace, newSvc.Name, err) - } - - patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, v1.Service{}) - if err != nil { - return nil, fmt.Errorf("failed to CreateTwoWayMergePatch for svc %s/%s: %v", oldSvc.Namespace, oldSvc.Name, err) - } - return patchBytes, nil -} diff --git a/pkg/controller/service/patch_test.go b/pkg/controller/service/patch_test.go deleted file mode 100644 index a1f8e08c3a9..00000000000 --- a/pkg/controller/service/patch_test.go +++ /dev/null @@ -1,102 +0,0 @@ -/* -Copyright 2019 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package service - -import ( - "context" - "reflect" - "testing" - - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/fake" -) - -func addAnnotations(svc *v1.Service) { - svc.Annotations["foo"] = "bar" -} - -func TestPatch(t *testing.T) { - svcOrigin := &v1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-patch", - Annotations: map[string]string{}, - }, - Spec: v1.ServiceSpec{ - ClusterIP: "10.0.0.1", - }, - } - fakeCs := fake.NewSimpleClientset(svcOrigin) - - // Issue a separate update and verify patch doesn't fail after this. - svcToUpdate := svcOrigin.DeepCopy() - addAnnotations(svcToUpdate) - if _, err := fakeCs.CoreV1().Services(svcOrigin.Namespace).Update(context.TODO(), svcToUpdate, metav1.UpdateOptions{}); err != nil { - t.Fatalf("Failed to update service: %v", err) - } - - // Attempt to patch based the original service. - svcToPatch := svcOrigin.DeepCopy() - svcToPatch.Finalizers = []string{"foo"} - svcToPatch.Spec.ClusterIP = "10.0.0.2" - svcToPatch.Status = v1.ServiceStatus{ - LoadBalancer: v1.LoadBalancerStatus{ - Ingress: []v1.LoadBalancerIngress{ - {IP: "8.8.8.8"}, - }, - }, - } - svcPatched, err := patch(fakeCs.CoreV1(), svcOrigin, svcToPatch) - if err != nil { - t.Fatalf("Failed to patch service: %v", err) - } - - // Service returned by patch will contain latest content (e.g from - // the separate update). - addAnnotations(svcToPatch) - if !reflect.DeepEqual(svcPatched, svcToPatch) { - t.Errorf("PatchStatus() = %+v, want %+v", svcPatched, svcToPatch) - } - // Explicitly validate if spec is unchanged from origin. - if !reflect.DeepEqual(svcPatched.Spec, svcOrigin.Spec) { - t.Errorf("Got spec = %+v, want %+v", svcPatched.Spec, svcOrigin.Spec) - } -} - -func TestGetPatchBytes(t *testing.T) { - origin := &v1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-patch-bytes", - Finalizers: []string{"foo"}, - }, - } - updated := &v1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-patch-bytes", - Finalizers: []string{"foo", "bar"}, - }, - } - - b, err := getPatchBytes(origin, updated) - if err != nil { - t.Fatal(err) - } - expected := `{"metadata":{"$setElementOrder/finalizers":["foo","bar"],"finalizers":["bar"]}}` - if string(b) != expected { - t.Errorf("getPatchBytes(%+v, %+v) = %s ; want %s", origin, updated, string(b), expected) - } -} From 7b2a0a426010495d43378e879ca27a358a528ca2 Mon Sep 17 00:00:00 2001 From: Manohar Reddy Date: Wed, 26 Feb 2020 06:14:11 +0530 Subject: [PATCH 3/3] update bazel --- pkg/controller/service/BUILD | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/controller/service/BUILD b/pkg/controller/service/BUILD index 398b8d295f2..b21021e7824 100644 --- a/pkg/controller/service/BUILD +++ b/pkg/controller/service/BUILD @@ -11,12 +11,9 @@ go_library( deps = [ "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/util/strategicpatch: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", "//staging/src/k8s.io/client-go/informers/core/v1:go_default_library", @@ -36,9 +33,7 @@ go_library( go_test( name = "go_default_test", - srcs = [ - "controller_test.go", - ], + srcs = ["controller_test.go"], embed = [":go_default_library"], deps = [ "//pkg/controller:go_default_library",