Upgrade Public Gateway when type is updated and it's possible to (#15)

Author: Tomy2e

docs/scalewaycluster.md

@@ -259,7 +259,9 @@ If not set, a new IP will be created.
 > 🚫📶 Updating existing Public Gateways can lead to a loss of network on the nodes, be
 > very careful when updating this field.
 >
-> 🚮 Updating a Public Gateway will lead to its recreation, which will make its private IP change.
+> 🚮 Updating a Public Gateway will lead to its re-creation, which will make its private IP change.
+> The only change that won't lead to a re-creation of the Public Gateway is a type upgrade
+> (e.g. VPC-GW-S to VPC-GW-M). Downgrading a Public Gateway is only possible through a re-creation.
 >
 > ⏳ Because the default routes are advertised via DHCP, the DHCP leases of the nodes must
 > be renewed for changes to be propagated (~24 hours). You can reboot the nodes or

internal/service/scaleway/client/vpcgw.go

@@ -111,3 +111,41 @@ func (c *Client) CreateGatewayNetwork(ctx context.Context, zone scw.Zone, gatewa
 
 	return nil
 }
+
+func (c *Client) ListGatewayTypes(ctx context.Context, zone scw.Zone) ([]string, error) {
+	if err := c.validateZone(c.vpcgw, zone); err != nil {
+		return nil, err
+	}
+
+	resp, err := c.vpcgw.ListGatewayTypes(&vpcgw.ListGatewayTypesRequest{
+		Zone: zone,
+	}, scw.WithContext(ctx))
+	if err != nil {
+		return nil, newCallError("ListGatewayTypes", err)
+	}
+
+	// We assume the API returns the gateway types in the correct order (S -> M -> L, etc.).
+	types := make([]string, 0, len(resp.Types))
+	for _, t := range resp.Types {
+		types = append(types, t.Name)
+	}
+
+	return types, nil
+}
+
+func (c *Client) UpgradeGateway(ctx context.Context, zone scw.Zone, gatewayID, newType string) (*vpcgw.Gateway, error) {
+	if err := c.validateZone(c.vpcgw, zone); err != nil {
+		return nil, err
+	}
+
+	gateway, err := c.vpcgw.UpgradeGateway(&vpcgw.UpgradeGatewayRequest{
+		Zone:      zone,
+		GatewayID: gatewayID,
+		Type:      &newType,
+	}, scw.WithContext(ctx))
+	if err != nil {
+		return nil, newCallError("UpgradeGateway", err)
+	}
+
+	return gateway, nil
+}

internal/service/scaleway/common/resource_ensurer.go

@@ -31,7 +31,7 @@ type ResourceReconciler[D, R any] interface {
 
 	// ShouldKeepResource returns true if a resource should be kept because it
 	// matches the desired specs.
-	ShouldKeepResource(resource R, desired D) bool
+	ShouldKeepResource(ctx context.Context, resource R, desired D) (bool, error)
 }
 
 // ResourceEnsurer is a utility that ensures a list of desired resources
@@ -96,11 +96,16 @@ func (e *ResourceEnsurer[D, R]) ensureExistingResources(
 				continue
 			}
 
-			if !e.ShouldKeepResource(resource, desiredResource) {
-				continue
+			// Writes to the keep variable outside the scope of this for-loop.
+			keep, err = e.ShouldKeepResource(ctx, resource, desiredResource)
+			if err != nil {
+				return nil, err
 			}
 
-			keep = true
+			// Continue looping to the next resource until we find one to keep.
+			if !keep {
+				continue
+			}
 
 			resource, err = e.UpdateResource(ctx, resource, desiredResource)
 			if err != nil {

internal/service/scaleway/lb/lb.go

@@ -289,23 +289,24 @@ func (d *desiredResourceListManager) GetDesiredZone(desired infrav1.LoadBalancer
 }
 
 func (d *desiredResourceListManager) ShouldKeepResource(
+	_ context.Context,
 	resource *lb.LB,
 	desired infrav1.LoadBalancerSpec,
-) bool {
+) (bool, error) {
 	// If LB does not have an IP, remove it and recreate it.
 	if len(resource.IP) == 0 {
-		return false
+		return false, nil
 	}
 
 	if desired.IP == nil && !slices.Contains(resource.Tags, capsManagedIPTag) {
-		return false
+		return false, nil
 	}
 
 	if desired.IP != nil && resource.IP[0].IPAddress != *desired.IP {
-		return false
+		return false, nil
 	}
 
-	return true
+	return true, nil
 }
 
 func (d *desiredResourceListManager) GetDesiredResourceName(i int) string {

internal/service/scaleway/vpcgw/vpcgw.go

@@ -41,7 +41,7 @@ func (s *Service) ensureGateways(ctx context.Context, delete bool) ([]*vpcgw.Gat
 	}
 
 	drle := &common.ResourceEnsurer[infrav1.PublicGatewaySpec, *vpcgw.Gateway]{
-		ResourceReconciler: &desiredResourceListManager{s.Cluster},
+		ResourceReconciler: &desiredResourceListManager{s.Cluster, make(map[scw.Zone][]string)},
 	}
 	return drle.Do(ctx, desired)
 }
@@ -106,6 +106,8 @@ func (s *Service) Delete(ctx context.Context) error {
 
 type desiredResourceListManager struct {
 	*scope.Cluster
+
+	gatewayTypesCache map[scw.Zone][]string
 }
 
 func (d *desiredResourceListManager) ListResources(ctx context.Context) ([]*vpcgw.Gateway, error) {
@@ -132,6 +134,18 @@ func (d *desiredResourceListManager) UpdateResource(
 	resource *vpcgw.Gateway,
 	desired infrav1.PublicGatewaySpec,
 ) (*vpcgw.Gateway, error) {
+	if desired.Type != nil && *desired.Type != resource.Type {
+		canUpgradeType, err := d.canUpgradeType(ctx, resource.Zone, resource.Type, *desired.Type)
+		if err != nil {
+			return nil, err
+		}
+
+		if canUpgradeType {
+			logf.FromContext(ctx).Info("Upgrading Gateway", "gatewayName", resource.Name, "zone", resource.Zone)
+			return d.ScalewayClient.UpgradeGateway(ctx, resource.Zone, resource.ID, *desired.Type)
+		}
+	}
+
 	return resource, nil
 }
 
@@ -148,27 +162,35 @@ func (d *desiredResourceListManager) GetDesiredZone(desired infrav1.PublicGatewa
 }
 
 func (d *desiredResourceListManager) ShouldKeepResource(
+	ctx context.Context,
 	resource *vpcgw.Gateway,
 	desired infrav1.PublicGatewaySpec,
-) bool {
+) (bool, error) {
 	// Gateway has no IPv4, remove it and recreate it.
 	if resource.IPv4 == nil {
-		return false
+		return false, nil
 	}
 
 	if desired.Type != nil && *desired.Type != resource.Type {
-		return false
+		canUpgradeType, err := d.canUpgradeType(ctx, resource.Zone, resource.Type, *desired.Type)
+		if err != nil {
+			return false, err
+		}
+
+		if !canUpgradeType {
+			return false, nil
+		}
 	}
 
 	if desired.IP == nil && !slices.Contains(resource.Tags, capsManagedIPTag) {
-		return false
+		return false, nil
 	}
 
 	if desired.IP != nil && resource.IPv4.Address.String() != *desired.IP {
-		return false
+		return false, nil
 	}
 
-	return true
+	return true, nil
 }
 
 func (d *desiredResourceListManager) GetDesiredResourceName(i int) string {
@@ -213,3 +235,25 @@ func (d *desiredResourceListManager) CreateResource(
 
 	return gateway, nil
 }
+
+func (d *desiredResourceListManager) canUpgradeType(ctx context.Context, zone scw.Zone, current, desired string) (bool, error) {
+	types, ok := d.gatewayTypesCache[zone]
+	if !ok {
+		var err error
+		types, err = d.ScalewayClient.ListGatewayTypes(ctx, zone)
+		if err != nil {
+			return false, err
+		}
+
+		d.gatewayTypesCache[zone] = types
+	}
+
+	return canUpgradeTypes(types, current, desired), nil
+}
+
+func canUpgradeTypes(types []string, current, desired string) bool {
+	desiredIndex := slices.Index(types, desired)
+	currentIndex := slices.Index(types, current)
+
+	return currentIndex != -1 && desiredIndex > currentIndex
+}

internal/service/scaleway/vpcgw/vpcgw_test.go

@@ -0,0 +1,82 @@
+package vpcgw
+
+import "testing"
+
+func Test_canUpgradeTypes(t *testing.T) {
+	t.Parallel()
+
+	type args struct {
+		types   []string
+		current string
+		desired string
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{
+			name: "upgrade from VPC-GW-S to VPC-GW-XL",
+			args: args{
+				types:   []string{"VPC-GW-S", "VPC-GW-M", "VPC-GW-L", "VPC-GW-XL"},
+				current: "VPC-GW-S",
+				desired: "VPC-GW-XL",
+			},
+			want: true,
+		},
+		{
+			name: "upgrade from VPC-GW-S to VPC-GW-M",
+			args: args{
+				types:   []string{"VPC-GW-S", "VPC-GW-M", "VPC-GW-L", "VPC-GW-XL"},
+				current: "VPC-GW-S",
+				desired: "VPC-GW-XL",
+			},
+			want: true,
+		},
+		{
+			name: "current equals desired, not upgradable",
+			args: args{
+				types:   []string{"VPC-GW-S", "VPC-GW-M", "VPC-GW-L", "VPC-GW-XL"},
+				current: "VPC-GW-S",
+				desired: "VPC-GW-S",
+			},
+			want: false,
+		},
+		{
+			name: "unknown current type, not upgradable",
+			args: args{
+				types:   []string{"VPC-GW-S", "VPC-GW-M", "VPC-GW-L", "VPC-GW-XL"},
+				current: "UNKNOWN-S",
+				desired: "VPC-GW-L",
+			},
+			want: false,
+		},
+		{
+			name: "unknown current and desired type, not upgradable",
+			args: args{
+				types:   []string{"VPC-GW-S", "VPC-GW-M", "VPC-GW-L", "VPC-GW-XL"},
+				current: "UNKNOWN-S",
+				desired: "UNKNOWN-M",
+			},
+			want: false,
+		},
+		{
+			name: "unknown desired type, not upgradable",
+			args: args{
+				types:   []string{"VPC-GW-S", "VPC-GW-M", "VPC-GW-L", "VPC-GW-XL"},
+				current: "VPC-GW-S",
+				desired: "UNKNOWN-M",
+			},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			if got := canUpgradeTypes(tt.args.types, tt.args.current, tt.args.desired); got != tt.want {
+				t.Errorf("canUpgradeTypes() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}