diff --git a/docs/loadbalancer-annotations.md b/docs/loadbalancer-annotations.md index 378e048..023e6f1 100644 --- a/docs/loadbalancer-annotations.md +++ b/docs/loadbalancer-annotations.md @@ -185,3 +185,14 @@ When set to `true`, this annotation makes the following changes in behavior: This annotation requires `service.beta.kubernetes.io/scw-loadbalancer-id` to be set to a valid existing LB. > Please note that this annotation is experimental and may not be supported. + +### `service.beta.kubernetes.io/scw-loadbalancer-ip-ids` + +This is the annotation to statically set the IPs of the loadbalancer. +It is possible to provide a single IP ID, or a comma delimited list of IP IDs. +You can provide at most one IPv4 and one IPv6. You must set at least one IPv4. +This annotation takes priority over the deprecated spec.loadBalancerIP field. +Changing the IPs will result in the re-creation of the LB. +The possible formats are: +- ``: will attach a single IP to the LB. +- `,`: will attach the two IPs to the LB. diff --git a/docs/loadbalancer-examples.md b/docs/loadbalancer-examples.md index 56a8cc5..a0b301e 100644 --- a/docs/loadbalancer-examples.md +++ b/docs/loadbalancer-examples.md @@ -87,16 +87,18 @@ _Warning: LoadBalancer ips are different from Instance ips. You cannot use an in To reserve a loadbalancer IP through API : ```bash curl -X POST "https://api.scaleway.com/lb/v1/regions/$SCW_DEFAULT_REGION/ips" -H "X-Auth-Token: $SCW_SECRET_KEY" -H "Content-Type: application/json" \ --d "{\"organization_id\":\"$SCW_DEFAULT_ORGANIZATION_ID\"} | jq -r .ip_address" +-d "{\"organization_id\":\"$SCW_DEFAULT_ORGANIZATION_ID\"} | jq -r .id" ``` -Now specify this IP address to spec.loadBalancerIP field of the service +Now specify the ID of this IP address in the `service.beta.kubernetes.io/scw-loadbalancer-ip-ids` annotation. ```yaml kind: Service apiVersion: v1 metadata: name: demo-nginx + annotations: + service.beta.kubernetes.io/scw-loadbalancer-ip-ids: IP_ID spec: selector: app: demo-nginx @@ -105,7 +107,6 @@ spec: - name: http port: 80 targetPort: 80 - loadBalancerIP: SCW.LB.IP.ADDR ``` ## Convert an ephemeral IP into reserved IP @@ -114,7 +115,8 @@ It's possible to keep an ephemeral address (converting into a reserved address) ```bash export CURRENT_EXTERNAL_IP=$(kubectl get svc $SERVICE_NAME -o json | jq -r .status.loadBalancer.ingress[0].ip) -kubectl patch svc $SERVICE_NAME --type merge --patch "{\"spec\":{\"loadBalancerIP\": \"$CURRENT_EXTERNAL_IP\"}}" +export CURRENT_EXTERNAL_IP_ID=$(curl -s "https://api.scaleway.com/lb/v1/regions/$SCW_DEFAULT_REGION/ips?ip_address=${CURRENT_EXTERNAL_IP}" -H "X-Auth-Token: $SCW_SECRET_KEY" | jq -r .ips[0].id) +kubectl patch svc $SERVICE_NAME --type merge --patch "{\"metadata\":{\"annotations\": {\"service.beta.kubernetes.io/scw-loadbalancer-ip-ids\": \"$CURRENT_EXTERNAL_IP_ID\"}}}" ``` This way, the ephemeral ip would not be deleted when the service will be deleted and could be reused in another service. diff --git a/scaleway/loadbalancers.go b/scaleway/loadbalancers.go index 43f38b7..28d5656 100644 --- a/scaleway/loadbalancers.go +++ b/scaleway/loadbalancers.go @@ -20,7 +20,6 @@ import ( "context" "errors" "fmt" - "net" "os" "reflect" "strconv" @@ -155,8 +154,9 @@ func (l *loadbalancers) EnsureLoadBalancer(ctx context.Context, clusterName stri if lbPrivate && l.pnID == "" { return nil, fmt.Errorf("scaleway-cloud-controller-manager cannot create private load balancers without a private network") } - if lbPrivate && service.Spec.LoadBalancerIP != "" { - return nil, fmt.Errorf("scaleway-cloud-controller-manager can only handle .spec.LoadBalancerIP for public load balancers. Unsetting the .spec.LoadBalancerIP can result in the loss of the IP") + + if lbPrivate && hasLoadBalancerStaticIPs(service) { + return nil, fmt.Errorf("scaleway-cloud-controller-manager can only handle static IPs for public load balancers. Unsetting the static IP can result in the loss of the IP") } lb, err := l.fetchLoadBalancer(ctx, clusterName, service) @@ -177,7 +177,7 @@ func (l *loadbalancers) EnsureLoadBalancer(ctx context.Context, clusterName stri if !lbExternallyManaged { privateModeMismatch := lbPrivate != (len(lb.IP) == 0) - reservedIPMismatch := service.Spec.LoadBalancerIP != "" && service.Spec.LoadBalancerIP != lb.IP[0].IPAddress + reservedIPMismatch := hasLoadBalancerStaticIPs(service) && !hasEqualLoadBalancerStaticIPs(service, lb) if privateModeMismatch || reservedIPMismatch { err = l.deleteLoadBalancer(ctx, lb, clusterName, service) if err != nil { @@ -327,13 +327,10 @@ func (l *loadbalancers) deleteLoadBalancer(ctx context.Context, lb *scwlb.LB, cl return nil } - // if loadBalancerIP is not set, it implies an ephemeral IP - releaseIP := service.Spec.LoadBalancerIP == "" - request := &scwlb.ZonedAPIDeleteLBRequest{ Zone: lb.Zone, LBID: lb.ID, - ReleaseIP: releaseIP, + ReleaseIP: !hasLoadBalancerStaticIPs(service), // if no static IP is set, it implies an ephemeral IP } err := l.api.DeleteLB(request) @@ -471,27 +468,12 @@ func (l *loadbalancers) createLoadBalancer(ctx context.Context, clusterName stri } // Attach specific IP if set - var ipID *string - if !lbPrivate && service.Spec.LoadBalancerIP != "" { - request := scwlb.ZonedAPIListIPsRequest{ - IPAddress: &service.Spec.LoadBalancerIP, - Zone: getLoadBalancerZone(service), - } - ipsResp, err := l.api.ListIPs(&request) + var ipIDs []string + if !lbPrivate { + ipIDs, err = l.getLoadBalancerStaticIPIDs(service) if err != nil { - klog.Errorf("error getting ip for service %s/%s: %v", service.Namespace, service.Name, err) - return nil, fmt.Errorf("createLoadBalancer: error getting ip for service %s: %s", service.Name, err.Error()) - } - - if len(ipsResp.IPs) == 0 { - return nil, IPAddressNotFound - } - - if ipsResp.IPs[0].LBID != nil && *ipsResp.IPs[0].LBID != "" { - return nil, IPAddressInUse + return nil, err } - - ipID = &ipsResp.IPs[0].ID } lbName := l.GetLoadBalancerName(ctx, clusterName, service) @@ -511,13 +493,15 @@ func (l *loadbalancers) createLoadBalancer(ctx context.Context, clusterName stri tags = append(tags, "managed-by-scaleway-cloud-controller-manager") request := scwlb.ZonedAPICreateLBRequest{ - Zone: getLoadBalancerZone(service), - Name: lbName, - Description: "kubernetes service " + service.Name, - Tags: tags, - IPID: ipID, - Type: lbType, - AssignFlexibleIP: scw.BoolPtr(!lbPrivate), + Zone: getLoadBalancerZone(service), + Name: lbName, + Description: "kubernetes service " + service.Name, + Tags: tags, + IPIDs: ipIDs, + Type: lbType, + // We must only assign a flexible IP if LB is public AND no IP ID is provided. + // If IP IDs are provided, there must be at least one IPv4. + AssignFlexibleIP: scw.BoolPtr(!lbPrivate && len(ipIDs) == 0), } lb, err := l.api.CreateLB(&request) if err != nil { @@ -533,6 +517,38 @@ func (l *loadbalancers) createLoadBalancer(ctx context.Context, clusterName stri return lb, nil } +// getLoadBalancerStaticIPIDs returns user-provided static IPs for the LB from annotations. +// If no annotation is found, it uses the LoadBalancerIP field from service spec. +// It returns nil if user provided no static IP. In this case, the CCM must manage a dynamic IP. +func (l *loadbalancers) getLoadBalancerStaticIPIDs(service *v1.Service) ([]string, error) { + if ipIDs := getIPIDs(service); len(ipIDs) > 0 { + return ipIDs, nil + } + + if service.Spec.LoadBalancerIP != "" { + ipsResp, err := l.api.ListIPs(&scwlb.ZonedAPIListIPsRequest{ + IPAddress: &service.Spec.LoadBalancerIP, + Zone: getLoadBalancerZone(service), + }) + if err != nil { + klog.Errorf("error getting ip for service %s/%s: %v", service.Namespace, service.Name, err) + return nil, fmt.Errorf("createLoadBalancer: error getting ip for service %s: %s", service.Name, err.Error()) + } + + if len(ipsResp.IPs) == 0 { + return nil, IPAddressNotFound + } + + if ipsResp.IPs[0].LBID != nil && *ipsResp.IPs[0].LBID != "" { + return nil, IPAddressInUse + } + + return []string{ipsResp.IPs[0].ID}, nil + } + + return nil, nil +} + // annotateAndPatch adds the loadbalancer id to the service's annotations func (l *loadbalancers) annotateAndPatch(service *v1.Service, loadbalancer *scwlb.LB) error { service = service.DeepCopy() @@ -854,11 +870,6 @@ func (l *loadbalancers) createPublicServiceStatus(service *v1.Service, lb *scwlb status := &v1.LoadBalancerStatus{} status.Ingress = make([]v1.LoadBalancerIngress, 0) for _, ip := range lb.IP { - // Skip ipv6 entries - if i := net.ParseIP(ip.IPAddress); i.To4() == nil { - continue - } - if getUseHostname(service) { status.Ingress = append(status.Ingress, v1.LoadBalancerIngress{Hostname: ip.Reverse}) } else { @@ -867,7 +878,7 @@ func (l *loadbalancers) createPublicServiceStatus(service *v1.Service, lb *scwlb } if len(status.Ingress) == 0 { - return nil, fmt.Errorf("no ipv4 found for lb %s", lb.Name) + return nil, fmt.Errorf("no ip found for lb %s", lb.Name) } return status, nil @@ -1673,3 +1684,46 @@ func ptrBoolToString(b *bool) string { } return fmt.Sprintf("%t", *b) } + +// hasLoadBalancerStaticIPs returns true if static IPs are specified for the loadbalancer. +func hasLoadBalancerStaticIPs(service *v1.Service) bool { + if ipIDs := getIPIDs(service); len(ipIDs) > 0 { + return true + } + + if service.Spec.LoadBalancerIP != "" { + return true + } + + return false +} + +// hasEqualLoadBalancerStaticIPs returns true if the LB has the expected static IPs. +// This function returns true if no static IP is configured. +func hasEqualLoadBalancerStaticIPs(service *v1.Service, lb *scwlb.LB) bool { + if ipIDs := getIPIDs(service); len(ipIDs) > 0 { + if len(ipIDs) != len(lb.IP) { + return false + } + + // Sort IP IDs. + sortedIPIDs := slices.Clone(ipIDs) + slices.Sort(sortedIPIDs) + + // Sort LB IP IDs. + sortedLBIPIDs := make([]string, 0, len(lb.IP)) + for _, ip := range lb.IP { + sortedLBIPIDs = append(sortedLBIPIDs, ip.ID) + } + slices.Sort(sortedLBIPIDs) + + // Compare the sorted list. + return reflect.DeepEqual(sortedIPIDs, sortedLBIPIDs) + } + + if lbIP := service.Spec.LoadBalancerIP; lbIP != "" { + return len(lb.IP) == 1 && lbIP == lb.IP[0].IPAddress + } + + return true +} diff --git a/scaleway/loadbalancers_annotations.go b/scaleway/loadbalancers_annotations.go index 8a1c55f..abe4560 100644 --- a/scaleway/loadbalancers_annotations.go +++ b/scaleway/loadbalancers_annotations.go @@ -183,6 +183,16 @@ const ( // * Will refuse to manage a LB with a name starting with the cluster id. // This annotation requires `service.beta.kubernetes.io/scw-loadbalancer-id` to be set to a valid existing LB. serviceAnnotationLoadBalancerExternallyManaged = "service.beta.kubernetes.io/scw-loadbalancer-externally-managed" + + // serviceAnnotationLoadBalancerIPIDs is the annotation to statically set the IPs of the loadbalancer. + // It is possible to provide a single IP ID, or a comma delimited list of IP IDs. + // You can provide at most one IPv4 and one IPv6. You must set at least one IPv4. + // This annotation takes priority over the deprecated spec.loadBalancerIP field. + // Changing the IPs will result in the re-creation of the LB. + // The possible formats are: + // "": will attach a single IP to the LB. + // ",": will attach the two IPs to the LB. + serviceAnnotationLoadBalancerIPIDs = "service.beta.kubernetes.io/scw-loadbalancer-ip-ids" ) func getLoadBalancerID(service *v1.Service) (scw.Zone, string, error) { @@ -245,6 +255,15 @@ func getStickySessionsCookieName(service *v1.Service) (string, error) { return stickySessionsCookieName, nil } +func getIPIDs(service *v1.Service) []string { + ipIDs := service.Annotations[serviceAnnotationLoadBalancerIPIDs] + if ipIDs == "" { + return nil + } + + return strings.Split(ipIDs, ",") +} + func getSendProxyV2(service *v1.Service, nodePort int32) (scwlb.ProxyProtocol, error) { sendProxyV2, ok := service.Annotations[serviceAnnotationLoadBalancerSendProxyV2] if !ok { diff --git a/scaleway/loadbalancers_test.go b/scaleway/loadbalancers_test.go index 6cf480f..26d5a2e 100644 --- a/scaleway/loadbalancers_test.go +++ b/scaleway/loadbalancers_test.go @@ -1255,3 +1255,149 @@ func deepCloneBackend(original *scwlb.Backend) *scwlb.Backend { return clone } + +func Test_hasEqualLoadBalancerStaticIPs(t *testing.T) { + type args struct { + service *v1.Service + lb *scwlb.LB + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "no static ip set", + args: args{ + service: &v1.Service{}, + lb: &scwlb.LB{}, + }, + want: true, + }, + { + name: "static ip set with field but mismatch", + args: args{ + service: &v1.Service{ + Spec: v1.ServiceSpec{ + LoadBalancerIP: "127.0.0.1", + }, + }, + lb: &scwlb.LB{ + IP: []*scwlb.IP{ + { + IPAddress: "127.0.0.2", + }, + }, + }, + }, + want: false, + }, + { + name: "static ip set with field match", + args: args{ + service: &v1.Service{ + Spec: v1.ServiceSpec{ + LoadBalancerIP: "127.0.0.1", + }, + }, + lb: &scwlb.LB{ + IP: []*scwlb.IP{ + { + IPAddress: "127.0.0.1", + }, + }, + }, + }, + want: true, + }, + { + name: "static ip set with annotation mismatch", + args: args{ + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{serviceAnnotationLoadBalancerIPIDs: "fakeid"}, + }, + }, + lb: &scwlb.LB{ + IP: []*scwlb.IP{ + { + ID: "fakeid", + }, + }, + }, + }, + want: true, + }, + { + name: "static ip set with annotation, count mismatch", + args: args{ + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{serviceAnnotationLoadBalancerIPIDs: "fakeid1,fakeid2"}, + }, + }, + lb: &scwlb.LB{ + IP: []*scwlb.IP{ + { + ID: "fakeid1", + }, + }, + }, + }, + want: false, + }, + { + name: "static ip set with annotation, match many", + args: args{ + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{serviceAnnotationLoadBalancerIPIDs: "fakeid1,fakeid2"}, + }, + }, + lb: &scwlb.LB{ + IP: []*scwlb.IP{ + { + ID: "fakeid1", + }, + { + ID: "fakeid2", + }, + }, + }, + }, + want: true, + }, + { + name: "static ip set with annotation, ignore field", + args: args{ + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{serviceAnnotationLoadBalancerIPIDs: "fakeid1,fakeid2"}, + }, + Spec: v1.ServiceSpec{ + LoadBalancerIP: "127.0.0.1", + }, + }, + lb: &scwlb.LB{ + IP: []*scwlb.IP{ + { + ID: "fakeid1", + IPAddress: "127.0.0.2", + }, + { + ID: "fakeid2", + }, + }, + }, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := hasEqualLoadBalancerStaticIPs(tt.args.service, tt.args.lb); got != tt.want { + t.Errorf("hasEqualLoadBalancerStaticIPs() = %v, want %v", got, tt.want) + } + }) + } +}