Merge pull request kubernetes#104973 from dprotaso/automated-cherry-p…

…ick-of-#104467-upstream-release-1.21 Automated cherry pick of kubernetes#104467: fix 104329: check for headless before trying to release
openshift · Oct 6, 2021 · 620e9cf · 620e9cf
2 parents 70867d5 + 98c2f13
commit 620e9cf
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 5 deletions.
diff --git a/pkg/registry/core/service/storage/rest.go b/pkg/registry/core/service/storage/rest.go
@@ -744,6 +744,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)
@@ -760,11 +766,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

diff --git a/test/integration/dualstack/dualstack_test.go b/test/integration/dualstack/dualstack_test.go
@@ -1479,3 +1479,59 @@ func validateServiceAndClusterIPFamily(svc *v1.Service, expectedIPFamilies []v1.
 
 	return nil
 }
+
+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.NewIntegrationTestMasterConfig()
+	_, server, closeFn := framework.RunAMaster(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)
+	}
+}