From 2f9cd08831c802b7755741d1b48cc7b6c0494a2c Mon Sep 17 00:00:00 2001 From: "Khaled (Kal) Henidak" Date: Thu, 19 Aug 2021 23:06:17 +0000 Subject: [PATCH 1/2] fix 104329: check for headless before trying to release the ClusterIPs --- pkg/registry/core/service/storage/rest.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/registry/core/service/storage/rest.go b/pkg/registry/core/service/storage/rest.go index 313a41417eb..d2cd8cc9a17 100644 --- a/pkg/registry/core/service/storage/rest.go +++ b/pkg/registry/core/service/storage/rest.go @@ -759,6 +759,12 @@ func (rs *REST) handleClusterIPsForUpdatedService(oldService *api.Service, servi } // CASE B: + + // if headless service then we bail out early (no clusterIPs management needed) + if len(oldService.Spec.ClusterIPs) > 0 && oldService.Spec.ClusterIPs[0] == api.ClusterIPNone { + return nil, nil, nil + } + // Update service from non-ExternalName to ExternalName, should release ClusterIP if exists. if oldService.Spec.Type != api.ServiceTypeExternalName && service.Spec.Type == api.ServiceTypeExternalName { toRelease = make(map[api.IPFamily]string) @@ -775,11 +781,6 @@ func (rs *REST) handleClusterIPsForUpdatedService(oldService *api.Service, servi return nil, toRelease, nil } - // if headless service then we bail out early (no clusterIPs management needed) - if len(oldService.Spec.ClusterIPs) > 0 && oldService.Spec.ClusterIPs[0] == api.ClusterIPNone { - return nil, nil, nil - } - // upgrade and downgrade are specific to dualstack if !utilfeature.DefaultFeatureGate.Enabled(features.IPv6DualStack) { return nil, nil, nil From 89ae53d510e2d329591ea56144549fedc5c362f3 Mon Sep 17 00:00:00 2001 From: "Khaled (Kal) Henidak" Date: Fri, 20 Aug 2021 21:13:03 +0000 Subject: [PATCH 2/2] integration test --- test/integration/dualstack/dualstack_test.go | 56 ++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/test/integration/dualstack/dualstack_test.go b/test/integration/dualstack/dualstack_test.go index a5738c916c2..56de16a9e2a 100644 --- a/test/integration/dualstack/dualstack_test.go +++ b/test/integration/dualstack/dualstack_test.go @@ -1666,3 +1666,59 @@ func TestDowngradeServicePreferToDualStack(t *testing.T) { t.Fatalf("Unexpected error validating the service %s %v", svc.Name, err) } } + +type serviceMergePatch struct { + Spec specMergePatch `json:"spec,omitempty"` +} +type specMergePatch struct { + Type v1.ServiceType `json:"type,omitempty"` + ExternalName string `json:"externalName,omitempty"` +} + +// tests success when converting ClusterIP:Headless service to ExternalName +func Test_ServiceChangeTypeHeadlessToExternalNameWithPatch(t *testing.T) { + controlPlaneConfig := framework.NewIntegrationTestControlPlaneConfig() + _, server, closeFn := framework.RunAnAPIServer(controlPlaneConfig) + defer closeFn() + + config := restclient.Config{Host: server.URL} + client, err := clientset.NewForConfig(&config) + if err != nil { + t.Fatalf("Error creating clientset: %v", err) + } + + ns := framework.CreateTestingNamespace("test-service-allocate-node-ports", server, t) + defer framework.DeleteTestingNamespace(ns, server, t) + + service := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-123", + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeClusterIP, + ClusterIP: "None", + Selector: map[string]string{"foo": "bar"}, + }, + } + + service, err = client.CoreV1().Services(ns.Name).Create(context.TODO(), service, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Error creating test service: %v", err) + } + + serviceMergePatch := serviceMergePatch{ + Spec: specMergePatch{ + Type: v1.ServiceTypeExternalName, + ExternalName: "foo.bar", + }, + } + patchBytes, err := json.Marshal(&serviceMergePatch) + if err != nil { + t.Fatalf("failed to json.Marshal ports: %v", err) + } + + _, err = client.CoreV1().Services(ns.Name).Patch(context.TODO(), service.Name, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{}) + if err != nil { + t.Fatalf("unexpected error patching service using strategic merge patch. %v", err) + } +}