[feat] add support for deleting reserved IPv4 addresses in NodeBalancer deletion

cloud/annotations/annotations.go

@@ -29,6 +29,10 @@ const (
 	// for the NodeBalancer. If not specified, Linode will automatically assign an IPv4 address.
 	AnnLinodeLoadBalancerReservedIPv4 = "service.beta.kubernetes.io/linode-loadbalancer-reserved-ipv4"
 
+	// AnnLinodeLoadBalancerRetainReservedIPv4 is the annotation used to specify whether a reserved IPv4 address
+	// should be retained when the NodeBalancer is deleted. Defaults to "true".
+	AnnLinodeLoadBalancerRetainReservedIPv4 = "service.beta.kubernetes.io/linode-loadbalancer-retain-reserved-ipv4"
+
 	AnnLinodeLoadBalancerPreserve = "service.beta.kubernetes.io/linode-loadbalancer-preserve"
 	AnnLinodeNodeBalancerID       = "service.beta.kubernetes.io/linode-loadbalancer-nodebalancer-id"
 	AnnLinodeNodeBalancerType     = "service.beta.kubernetes.io/linode-loadbalancer-nodebalancer-type"

cloud/linode/fake_linode_test.go

@@ -29,6 +29,8 @@ type fakeAPI struct {
 	vpc    map[int]*linodego.VPC
 	subnet map[int]*linodego.VPCSubnet
 
+	failDeleteReservedIP bool
+
 	requests map[fakeRequest]struct{}
 	mux      *http.ServeMux
 }
@@ -333,6 +335,16 @@ func (f *fakeAPI) setupRoutes() {
 		_, _ = w.Write(resp)
 	})
 
+	f.mux.HandleFunc("DELETE /v4/networking/reserved/ips/{ipAddress}", func(w http.ResponseWriter, r *http.Request) {
+		if f.failDeleteReservedIP {
+			w.WriteHeader(http.StatusInternalServerError)
+			_, _ = w.Write([]byte(`{"errors":[{"reason":"forced reserved IP delete failure"}]}`))
+			return
+		}
+
+		w.WriteHeader(http.StatusOK)
+	})
+
 	f.mux.HandleFunc("POST /v4/nodebalancers", func(w http.ResponseWriter, r *http.Request) {
 		nbco := linodego.NodeBalancerCreateOptions{}
 		if err := json.NewDecoder(r.Body).Decode(&nbco); err != nil {

cloud/linode/loadbalancers.go

@@ -606,6 +606,13 @@ func (l *loadbalancers) shouldPreserveNodeBalancer(service *v1.Service) bool {
 	return shouldPreserve != nil && *shouldPreserve
 }
 
+// shouldRetainReservedIP determines whether a reserved IP should be retained based on the
+// service's retain reserved IP annotation. If the annotation is not present, the default behavior is to retain the reserved IP.
+func (l *loadbalancers) shouldRetainReservedIP(service *v1.Service) bool {
+	shouldRetain := getServiceBoolAnnotation(service, annotations.AnnLinodeLoadBalancerRetainReservedIPv4)
+	return shouldRetain == nil || *shouldRetain
+}
+
 // EnsureLoadBalancerDeleted deletes the specified loadbalancer if it exists.
 // nil is returned if the load balancer for service does not exist or is
 // successfully deleted.
@@ -668,6 +675,15 @@ func (l *loadbalancers) EnsureLoadBalancerDeleted(ctx context.Context, clusterNa
 		return err
 	}
 
+	if !l.shouldRetainReservedIP(service) {
+		klog.Infof("deleting reserved IP (%s) for NodeBalancer (%d) for service (%s)", *nb.IPv4, nb.ID, serviceNn)
+		if err = l.client.DeleteReservedIPAddress(ctx, *nb.IPv4); err != nil {
+			klog.Errorf("failed to delete reserved IP (%s) for NodeBalancer (%d) for service (%s): %s", *nb.IPv4, nb.ID, serviceNn, err)
+			sentry.CaptureError(ctx, err)
+			return err
+		}
+	}
+
 	if err = l.client.DeleteNodeBalancer(ctx, nb.ID); err != nil {
 		klog.Errorf("failed to delete NodeBalancer (%d) for service (%s): %s", nb.ID, serviceNn, err)
 		sentry.CaptureError(ctx, err)

cloud/linode/loadbalancers_test.go

@@ -4667,84 +4667,127 @@ func testEnsureLoadBalancerPreserveAnnotation(t *testing.T, client *linodego.Cli
 func testEnsureLoadBalancerDeleted(t *testing.T, client *linodego.Client, fake *fakeAPI) {
 	t.Helper()
 
-	svc := &v1.Service{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "test",
-			UID:  "foobar123",
-			Annotations: map[string]string{
-				annotations.AnnLinodeDefaultProtocol: "tcp",
+	newService := func(name string, annotationsMap map[string]string) *v1.Service {
+		return &v1.Service{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:        name,
+				UID:         types.UID(name + "-uid"),
+				Annotations: annotationsMap,
 			},
-		},
-		Spec: v1.ServiceSpec{
-			Ports: []v1.ServicePort{
-				{
-					Name:     "test",
-					Protocol: "TCP",
-					Port:     int32(80),
-					NodePort: int32(30000),
+			Spec: v1.ServiceSpec{
+				Ports: []v1.ServicePort{
+					{
+						Name:     "test",
+						Protocol: "TCP",
+						Port:     int32(80),
+						NodePort: int32(30000),
+					},
 				},
 			},
-		},
+		}
 	}
+
 	testcases := []struct {
-		name        string
-		clusterName string
-		service     *v1.Service
-		err         error
+		name                   string
+		clusterName            string
+		service                *v1.Service
+		createNodeBalancer     bool
+		expectReservedIPDelete bool
+		err                    error
 	}{
 		{
-			"load balancer delete",
-			"linodelb",
-			svc,
-			nil,
+			name:                   "load balancer delete retains reserved ip by default",
+			clusterName:            "linodelb",
+			service:                newService("test-retain-default", map[string]string{annotations.AnnLinodeDefaultProtocol: "tcp"}),
+			createNodeBalancer:     true,
+			expectReservedIPDelete: false,
+			err:                    nil,
 		},
 		{
-			"load balancer not exists",
-			"linodelb",
-			&v1.Service{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "notexists",
-					UID:  "notexists123",
-					Annotations: map[string]string{
-						annotations.AnnLinodeDefaultProtocol: "tcp",
-					},
-				},
-				Spec: v1.ServiceSpec{
-					Ports: []v1.ServicePort{
-						{
-							Name:     "test",
-							Protocol: "TCP",
-							Port:     int32(80),
-							NodePort: int32(30000),
-						},
-					},
-				},
-			},
-			nil,
+			name:        "load balancer delete removes reserved ip when retain annotation is false",
+			clusterName: "linodelb",
+			service: newService("test-delete-reserved-ip", map[string]string{
+				annotations.AnnLinodeDefaultProtocol:                "tcp",
+				annotations.AnnLinodeLoadBalancerRetainReservedIPv4: "false",
+			}),
+			createNodeBalancer:     true,
+			expectReservedIPDelete: true,
+			err:                    nil,
+		},
+		{
+			name:                   "load balancer not exists",
+			clusterName:            "linodelb",
+			service:                newService("notexists", map[string]string{annotations.AnnLinodeDefaultProtocol: "tcp"}),
+			createNodeBalancer:     false,
+			expectReservedIPDelete: false,
+			err:                    nil,
 		},
 	}
 
 	lb, assertion := newLoadbalancers(client, "us-west").(*loadbalancers)
 	if !assertion {
 		t.Error("type assertion failed")
 	}
-	configs := []linodego.NodeBalancerConfigCreateOptions{}
-	_, err := lb.createNodeBalancer(t.Context(), "linodelb", svc, configs)
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer func() { _ = lb.EnsureLoadBalancerDeleted(t.Context(), "linodelb", svc) }()
 
 	for _, test := range testcases {
 		t.Run(test.name, func(t *testing.T) {
+			fake.ResetRequests()
+
+			if test.createNodeBalancer {
+				nb, err := lb.createNodeBalancer(t.Context(), test.clusterName, test.service, []linodego.NodeBalancerConfigCreateOptions{})
+				if err != nil {
+					t.Fatal(err)
+				}
+				test.service.Status.LoadBalancer = *makeLoadBalancerStatus(test.service, nb)
+			}
+
 			err := lb.EnsureLoadBalancerDeleted(t.Context(), test.clusterName, test.service)
 			if !reflect.DeepEqual(err, test.err) {
 				t.Error("unexpected error")
 				t.Logf("expected: %v", test.err)
 				t.Logf("actual: %v", err)
 			}
+
+			if test.createNodeBalancer {
+				ingressIP := test.service.Status.LoadBalancer.Ingress[0].IP
+				didDeleteReservedIP := fake.didRequestOccur(http.MethodDelete, fmt.Sprintf("/networking/reserved/ips/%s", ingressIP), "")
+				if didDeleteReservedIP != test.expectReservedIPDelete {
+					t.Fatalf("reserved IP delete request mismatch: got %v, want %v", didDeleteReservedIP, test.expectReservedIPDelete)
+				}
+			}
 		})
 	}
+
+	t.Run("load balancer delete returns error when reserved ip deletion fails", func(t *testing.T) {
+		fake.ResetRequests()
+		fake.failDeleteReservedIP = true
+		defer func() { fake.failDeleteReservedIP = false }()
+
+		service := newService("test-delete-reserved-ip-fails", map[string]string{
+			annotations.AnnLinodeDefaultProtocol:                "tcp",
+			annotations.AnnLinodeLoadBalancerRetainReservedIPv4: "false",
+		})
+
+		nb, err := lb.createNodeBalancer(t.Context(), "linodelb", service, []linodego.NodeBalancerConfigCreateOptions{})
+		if err != nil {
+			t.Fatal(err)
+		}
+		service.Status.LoadBalancer = *makeLoadBalancerStatus(service, nb)
+
+		err = lb.EnsureLoadBalancerDeleted(t.Context(), "linodelb", service)
+		if err == nil {
+			t.Fatal("expected reserved IP deletion error")
+		}
+
+		ingressIP := service.Status.LoadBalancer.Ingress[0].IP
+		if !fake.didRequestOccur(http.MethodDelete, fmt.Sprintf("/networking/reserved/ips/%s", ingressIP), "") {
+			t.Fatal("expected reserved IP delete request")
+		}
+
+		if fake.didRequestOccur(http.MethodDelete, fmt.Sprintf("/nodebalancers/%d", nb.ID), "") {
+			t.Fatal("unexpected node balancer delete request after reserved IP deletion failure")
+		}
+	})
 }
 
 func testEnsureExistingLoadBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI) {

docs/configuration/annotations.md

@@ -5,6 +5,7 @@
 Service annotations allow you to customize the behavior of your LoadBalancer services. All Service annotations must be prefixed with: `service.beta.kubernetes.io/linode-loadbalancer-`
 
 For implementation details, see:
+
 - [LoadBalancer Configuration](loadbalancer.md)
 - [Basic Service Examples](../examples/basic.md)
 - [Advanced Configuration Examples](../examples/advanced.md)
@@ -50,7 +51,8 @@ The keys and the values in [annotations must be strings](https://kubernetes.io/d
 | `frontend-subnet-name` | string | | Frontend subnet name for the NodeBalancer frontend VPC configuration. See [Nodebalancer VPC Configuration](#nodebalancer-vpc-configuration) |
 | `frontend-ipv4-range` | string | | Optional IPv4 CIDR range from the frontend subnet. See [Nodebalancer VPC Configuration](#nodebalancer-vpc-configuration) |
 | `frontend-ipv6-range` | string | | Optional IPv6 CIDR range from the frontend subnet. See [Nodebalancer VPC Configuration](#nodebalancer-vpc-configuration) |
-| `reserved-ipv4` | string | | An existing Reserved IPv4 address that wil be used to initialize the NodeBalancer instance. See [LoadBalancer Configuration](loadbalancer.md#reserved-ipv4-addresses)) |
+| `reserved-ipv4` | string | | An existing Reserved IPv4 address that will be used to initialize the NodeBalancer instance. See [LoadBalancer Configuration](loadbalancer.md#reserved-ipv4-addresses) |
+| `retain-reserved-ipv4` | bool | `true` | When `false`, deleting a `LoadBalancer` service also releases the reserved IPv4 address attached to the NodeBalancer |
 
 ### Port Specific Configuration
 
@@ -70,6 +72,7 @@ metadata:
 ```
 
 Available port options:
+
 - `protocol`: Protocol for this port (tcp, http, https)
 - `tls-secret-name`: Name of TLS secret for HTTPS. The secret type should be `kubernetes.io/tls`
 - `proxy-protocol`: Proxy protocol version for this port
@@ -84,7 +87,9 @@ Available port options:
 | `proxy-protocol` | `none`, `v1`, `v2` | `none` | Specifies whether to use a version of Proxy Protocol on the underlying NodeBalancer | Q4 2021 |
 
 ### Annotation Boolean Values
+
 For annotations with bool value types, the following string values are interpreted as `true`:
+
 - `"1"`
 - `"t"`
 - `"T"`
@@ -97,6 +102,7 @@ Any other values will be interpreted as `false`. For more details, see [strconv.
 ## Examples
 
 ### Basic HTTP Service
+
 ```yaml
 metadata:
   annotations:
@@ -106,6 +112,7 @@ metadata:
 ```
 
 ### HTTPS Service with TLS
+
 ```yaml
 metadata:
   annotations:
@@ -117,6 +124,7 @@ metadata:
 ```
 
 ### Firewall Configuration
+
 ```yaml
 metadata:
   annotations:
@@ -130,16 +138,19 @@ metadata:
 ```
 
 ### NodeBalancer Type
+
 Linode supports nodebalancers of different types: common, premium, and premium_40gb. By default, nodebalancers of type common are provisioned. If an account is allowed to provision premium nodebalancers and one wants to use them, it can be achieved by specifying the annotation:
 
 **Note:** NodeBalancer types should always be specified in lowercase. The controller will automatically convert uppercase values to lowercase for safety.
+
 ```yaml
 metadata:
   annotations:
     service.beta.kubernetes.io/linode-loadbalancer-nodebalancer-type: premium
 ```
 
 ### Nodebalancer VPC Configuration
+
 ```yaml
 metadata:
   annotations:
@@ -149,6 +160,7 @@ metadata:
 ```
 
 Frontend VPC configuration:
+
 ```yaml
 metadata:
   annotations:
@@ -159,20 +171,23 @@ metadata:
 ```
 
 ### Service with IPv6 Address
+
 ```yaml
 metadata:
   annotations:
     service.beta.kubernetes.io/linode-loadbalancer-enable-ipv6-ingress: "true"
 ```
 
 For more examples and detailed configuration options, see:
+
 - [LoadBalancer Configuration](loadbalancer.md)
 - [Firewall Configuration](firewall.md)
 - [Basic Service Examples](../examples/basic.md)
 - [Advanced Configuration Examples](../examples/advanced.md)
 - [Examples](../examples/README.md)
 
 See also:
+
 - [Environment Variables](environment.md)
 - [Route Configuration](routes.md)
 - [Session Affinity](session-affinity.md)