diff --git a/pkg/registry/core/service/storage/storage.go b/pkg/registry/core/service/storage/storage.go index 880b0127bf7..b659aaad630 100644 --- a/pkg/registry/core/service/storage/storage.go +++ b/pkg/registry/core/service/storage/storage.go @@ -127,6 +127,11 @@ func NewREST( store.BeginCreate = genericStore.beginCreate store.BeginUpdate = genericStore.beginUpdate + // users can patch the status to remove the finalizer, + // hence statusStore must participate on the AfterDelete + // hook to release the allocated resources + statusStore.AfterDelete = genericStore.afterDelete + return genericStore, &StatusREST{store: &statusStore}, &svcreg.ProxyREST{Redirector: genericStore, ProxyTransport: proxyTransport}, nil } diff --git a/test/integration/network/services_test.go b/test/integration/network/services_test.go index f3dc63ddc6a..39ce1a08374 100644 --- a/test/integration/network/services_test.go +++ b/test/integration/network/services_test.go @@ -18,6 +18,8 @@ package network import ( "context" + "encoding/json" + "fmt" "testing" "time" @@ -25,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/kubernetes/cmd/kube-apiserver/app/options" "k8s.io/kubernetes/pkg/controlplane" @@ -134,6 +137,113 @@ func TestServicesFinalizersRepairLoop(t *testing.T) { t.Logf("Created service: %s", svcNodePort.Name) } +func TestServicesFinalizersPatchStatus(t *testing.T) { + serviceCIDR := "10.0.0.0/16" + clusterIP := "10.0.0.21" + nodePort := 30443 + _, ctx := ktesting.NewTestContext(t) + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + client, _, tearDownFn := framework.StartTestServer(ctx, t, framework.TestServerSetup{ + ModifyServerRunOptions: func(opts *options.ServerRunOptions) { + opts.ServiceClusterIPRanges = serviceCIDR + }, + }) + defer tearDownFn() + + for _, testcase := range []string{"spec", "status"} { + t.Run(testcase, func(t *testing.T) { + // Create a NodePort service with one finalizer + svcNodePort := v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc" + testcase, + Finalizers: []string{"foo.bar/some-finalizer"}, + }, + Spec: v1.ServiceSpec{ + ClusterIP: clusterIP, + Ports: []v1.ServicePort{{ + Port: 8443, + NodePort: int32(nodePort), + TargetPort: intstr.FromInt32(8443), + Protocol: v1.ProtocolTCP, + }}, + Type: v1.ServiceTypeNodePort, + }, + } + + ns := framework.CreateNamespaceOrDie(client, "test-service-finalizers-"+testcase, t) + defer framework.DeleteNamespaceOrDie(client, ns, t) + + // Create service + if _, err := client.CoreV1().Services(ns.Name).Create(ctx, &svcNodePort, metav1.CreateOptions{}); err != nil { + t.Fatalf("unexpected error creating service: %v", err) + } + t.Logf("Created service: %s", svcNodePort.Name) + + // Check the service has been created correctly + svc, err := client.CoreV1().Services(ns.Name).Get(ctx, svcNodePort.Name, metav1.GetOptions{}) + if err != nil || svc.Spec.ClusterIP != clusterIP { + t.Fatalf("created service is not correct: %v", err) + } + t.Logf("Service created successfully: %+v", svc) + + // Delete service + if err := client.CoreV1().Services(ns.Name).Delete(ctx, svcNodePort.Name, metav1.DeleteOptions{}); err != nil { + t.Fatalf("unexpected error deleting service: %v", err) + } + t.Logf("Deleted service: %s", svcNodePort.Name) + + // Check that the service was not deleted and the IP is already allocated + svc, err = client.CoreV1().Services(ns.Name).Get(ctx, svcNodePort.Name, metav1.GetOptions{}) + if err != nil || + svc.Spec.ClusterIP != clusterIP || + int(svc.Spec.Ports[0].NodePort) != nodePort || + svc.DeletionTimestamp == nil || + len(svc.ObjectMeta.Finalizers) != 1 { + t.Fatalf("Service expected to be deleting and with the same values: %v", err) + } + t.Logf("Service after Delete: %+v", svc) + + // Remove the finalizer + updated := svc.DeepCopy() + updated.ObjectMeta.Finalizers = []string{} + patchBytes, err := getPatchBytes(svc, updated) + if err != nil { + t.Fatalf("unexpected error getting patch bytes: %v", err) + } + + if testcase == "spec" { + if _, err = client.CoreV1().Services(ns.Name).Patch(ctx, svcNodePort.Name, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{}); err != nil { + t.Fatalf("unexpected error removing finalizer: %v", err) + } + } else { + if _, err = client.CoreV1().Services(ns.Name).Patch(ctx, svcNodePort.Name, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{}, "status"); err != nil { + t.Fatalf("unexpected error removing finalizer: %v", err) + } + } + t.Logf("Removed service finalizer: %s", svcNodePort.Name) + + // Check that the service was deleted + _, err = client.CoreV1().Services(ns.Name).Get(ctx, svcNodePort.Name, metav1.GetOptions{}) + if err == nil { + t.Fatalf("service was not delete: %v", err) + } + + // Try to create service again without the finalizer to check the ClusterIP and NodePort are deallocated + svc = svcNodePort.DeepCopy() + svc.Finalizers = []string{} + if _, err := client.CoreV1().Services(ns.Name).Create(ctx, svc, metav1.CreateOptions{}); err != nil { + t.Fatalf("unexpected error creating service: %v", err) + } + // Delete service + if err := client.CoreV1().Services(ns.Name).Delete(ctx, svc.Name, metav1.DeleteOptions{}); err != nil { + t.Fatalf("unexpected error deleting service: %v", err) + } + }) + } +} + // Regresion test for https://issues.k8s.io/115316 func TestServiceCIDR28bits(t *testing.T) { serviceCIDR := "10.0.0.0/28" @@ -183,3 +293,22 @@ func TestServiceCIDR28bits(t *testing.T) { t.Fatalf("Error creating test service: %v", err) } } + +func getPatchBytes(oldSvc, 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 + +}