Merge branch 'master' of https://github.com/metal-stack/metal-api into dualstack-support

Author: majst01

cmd/metal-api/internal/datastore/migrations/06_childprefixlength.go

@@ -1,6 +1,8 @@
 package migrations
 
 import (
+	"net/netip"
+
 	r "gopkg.in/rethinkdb/rethinkdb-go.v6"
 
 	"github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore"
@@ -9,6 +11,7 @@ import (
 
 func init() {
 	type tmpPartition struct {
+		// In theory this might be set in a partition, but in reality its not set anywhere
 		PrivateNetworkPrefixLength uint8 `rethinkdb:"privatenetworkprefixlength"`
 	}
 	datastore.MustRegisterMigration(datastore.Migration{
@@ -31,24 +34,40 @@ func init() {
 					return err
 				}
 
-				// TODO: does not work somehow
 				new := old
 
-				af, err := metal.GetAddressFamily(new.Prefixes)
+				var (
+					af                       metal.AddressFamily
+					defaultChildPrefixLength = metal.ChildPrefixLength{}
+				)
+				parsed, err := netip.ParsePrefix(new.Prefixes[0].String())
 				if err != nil {
 					return err
 				}
-				if af != nil {
-					if new.AddressFamilies == nil {
-						new.AddressFamilies = make(map[metal.AddressFamily]bool)
-					}
-					new.AddressFamilies[*af] = true
+				if parsed.Addr().Is4() {
+					af = metal.IPv4AddressFamily
+					defaultChildPrefixLength[af] = 22
+				}
+				if parsed.Addr().Is6() {
+					af = metal.IPv6AddressFamily
+					defaultChildPrefixLength[af] = 64
+				}
+
+				if new.AddressFamilies == nil {
+					new.AddressFamilies = make(map[metal.AddressFamily]bool)
 				}
+				new.AddressFamilies[af] = true
+
 				if new.PrivateSuper {
 					if new.DefaultChildPrefixLength == nil {
 						new.DefaultChildPrefixLength = make(map[metal.AddressFamily]uint8)
 					}
-					new.DefaultChildPrefixLength[*af] = partition.PrivateNetworkPrefixLength
+					if partition.PrivateNetworkPrefixLength > 0 {
+						new.DefaultChildPrefixLength[af] = partition.PrivateNetworkPrefixLength
+					} else {
+						new.DefaultChildPrefixLength = defaultChildPrefixLength
+					}
+
 				}
 				err = rs.UpdateNetwork(&old, &new)
 				if err != nil {

cmd/metal-api/internal/datastore/migrations_integration/migrate_integration_test.go

@@ -163,6 +163,9 @@ func Test_MigrationChildPrefixLength(t *testing.T) {
 			ID:                         "p2",
 			PrivateNetworkPrefixLength: 24,
 		}
+		p3 = &tmpPartition{
+			ID: "p3",
+		}
 		n1 = &metal.Network{
 			Base: metal.Base{
 				ID: "n1",
@@ -193,18 +196,32 @@ func Test_MigrationChildPrefixLength(t *testing.T) {
 			PartitionID:  "p2",
 			PrivateSuper: false,
 		}
+		n4 = &metal.Network{
+			Base: metal.Base{
+				ID: "n4",
+			},
+			Prefixes: metal.Prefixes{
+				{IP: "100.1.0.0", Length: "22"},
+			},
+			PartitionID:  "p3",
+			PrivateSuper: true,
+		}
 	)
 	_, err = r.DB("metal").Table("partition").Insert(p1).RunWrite(rs.Session())
 	require.NoError(t, err)
 	_, err = r.DB("metal").Table("partition").Insert(p2).RunWrite(rs.Session())
 	require.NoError(t, err)
+	_, err = r.DB("metal").Table("partition").Insert(p3).RunWrite(rs.Session())
+	require.NoError(t, err)
 
 	err = rs.CreateNetwork(n1)
 	require.NoError(t, err)
 	err = rs.CreateNetwork(n2)
 	require.NoError(t, err)
 	err = rs.CreateNetwork(n3)
 	require.NoError(t, err)
+	err = rs.CreateNetwork(n4)
+	require.NoError(t, err)
 
 	err = rs.Migrate(nil, false)
 	require.NoError(t, err)
@@ -233,4 +250,11 @@ func Test_MigrationChildPrefixLength(t *testing.T) {
 	require.NotNil(t, n3fetched)
 	require.Nil(t, n3fetched.DefaultChildPrefixLength)
 	require.True(t, n3fetched.AddressFamilies[metal.IPv4AddressFamily])
+
+	n4fetched, err := rs.FindNetworkByID(n4.ID)
+	require.NoError(t, err)
+	require.NotNil(t, n4fetched)
+	require.NotNil(t, n4fetched.DefaultChildPrefixLength)
+	require.True(t, n4fetched.AddressFamilies[metal.IPv4AddressFamily])
+	require.Equal(t, uint8(22), n4fetched.DefaultChildPrefixLength[metal.IPv4AddressFamily])
 }

cmd/metal-api/internal/ipam/ipam.go

@@ -52,7 +52,7 @@ func (i *ipam) AllocateChildPrefix(ctx context.Context, parentPrefix metal.Prefi
 		Length: uint32(childLength),
 	}))
 	if err != nil {
-		return nil, fmt.Errorf("error creating new prefix in ipam: %w", err)
+		return nil, fmt.Errorf("error creating new prefix from:%s in ipam: %w", parentPrefix.String(), err)
 	}
 
 	prefix, _, err := metal.NewPrefixFromCIDR(ipamPrefix.Msg.Prefix.Cidr)

cmd/metal-api/internal/metal/network.go

@@ -1,12 +1,10 @@
 package metal
 
 import (
-	"fmt"
 	"net"
 	"net/netip"
 	"strconv"
 
-	"github.com/metal-stack/metal-lib/pkg/pointer"
 	"github.com/samber/lo"
 )
 
@@ -351,21 +349,3 @@ func (nics Nics) ByIdentifier() map[string]*Nic {
 
 	return res
 }
-
-func GetAddressFamily(prefixes Prefixes) (*AddressFamily, error) {
-	if len(prefixes) == 0 {
-		return nil, nil
-	}
-
-	parsed, err := netip.ParsePrefix(prefixes[0].String())
-	if err != nil {
-		return nil, err
-	}
-	if parsed.Addr().Is4() {
-		return pointer.Pointer(IPv4AddressFamily), nil
-	}
-	if parsed.Addr().Is6() {
-		return pointer.Pointer(IPv6AddressFamily), nil
-	}
-	return nil, fmt.Errorf("unable to detect addressfamily from prefixes:%v", prefixes)
-}

cmd/metal-api/internal/metal/network_test.go

@@ -4,8 +4,6 @@ import (
 	"fmt"
 	"reflect"
 	"testing"
-
-	"github.com/metal-stack/metal-lib/pkg/pointer"
 )
 
 func TestNics_ByIdentifier(t *testing.T) {
@@ -337,47 +335,3 @@ func TestNicState_SetState(t *testing.T) {
 		})
 	}
 }
-
-func Test_getAddressFamily(t *testing.T) {
-	tests := []struct {
-		name     string
-		prefixes Prefixes
-		want     *AddressFamily
-		wantErr  bool
-	}{
-		{
-			name:     "ipv4",
-			prefixes: Prefixes{{IP: "10.0.0.0", Length: "8"}},
-			want:     pointer.Pointer(IPv4AddressFamily),
-		},
-		{
-			name:     "ipv6",
-			prefixes: Prefixes{{IP: "2001::", Length: "64"}},
-			want:     pointer.Pointer(IPv6AddressFamily),
-		},
-		{
-			name:     "empty prefixes",
-			prefixes: Prefixes{},
-			want:     nil,
-			wantErr:  false,
-		},
-		{
-			name:     "malformed ipv4",
-			prefixes: Prefixes{{IP: "10.0.0.0.0", Length: "6"}},
-			want:     nil,
-			wantErr:  true,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got, err := GetAddressFamily(tt.prefixes)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("getAddressFamily() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("getAddressFamily() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}

cmd/metal-api/internal/service/async-actor.go

@@ -6,9 +6,9 @@ import (
 	"fmt"
 	"log/slog"
 
+	"connectrpc.com/connect"
 	"github.com/metal-stack/metal-api/cmd/metal-api/internal/headscale"
 
-	ipamer "github.com/metal-stack/go-ipam"
 	"github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore"
 	"github.com/metal-stack/metal-api/cmd/metal-api/internal/ipam"
 	"github.com/metal-stack/metal-api/cmd/metal-api/internal/metal"
@@ -216,12 +216,15 @@ func (a *asyncActor) releaseIP(ip metal.IP) error {
 
 	// now the IP should not exist any more in our datastore
 	// so cleanup the ipam
-	
+
 	ctx := context.Background()
 	err = a.ReleaseIP(ctx, ip)
 	if err != nil {
-		if errors.Is(err, ipamer.ErrNotFound) {
-			return nil
+		var connectErr *connect.Error
+		if errors.As(err, &connectErr) {
+			if connectErr.Code() == connect.CodeNotFound {
+				return nil
+			}
 		}
 		return fmt.Errorf("cannot release IP %q: %w", ip, err)
 	}

cmd/metal-api/internal/service/ip-service.go

@@ -23,8 +23,6 @@ import (
 
 	v1 "github.com/metal-stack/metal-api/cmd/metal-api/internal/service/v1"
 
-	goipam "github.com/metal-stack/go-ipam"
-
 	restfulspec "github.com/emicklei/go-restful-openapi/v2"
 	restful "github.com/emicklei/go-restful/v3"
 	"github.com/metal-stack/metal-lib/httperrors"
@@ -289,7 +287,7 @@ func (r *ipResource) allocateIP(request *restful.Request, response *restful.Resp
 		ok := nw.AddressFamilies[metal.ToAddressFamily(string(*requestPayload.AddressFamily))]
 		if !ok {
 			r.sendError(request, response, httperrors.BadRequest(
-				fmt.Errorf("there is no prefix for the given addressfamily:%s present in this network:%s", string(*requestPayload.AddressFamily), requestPayload.NetworkID)),
+				fmt.Errorf("there is no prefix for the given addressfamily:%s present in network:%s", string(*requestPayload.AddressFamily), requestPayload.NetworkID)),
 			)
 			return
 		}
@@ -471,10 +469,13 @@ func allocateRandomIP(ctx context.Context, parent *metal.Network, ipamer ipam.IP
 		}
 
 		ipAddress, err = ipamer.AllocateIP(ctx, prefix)
-		if err != nil && errors.Is(err, goipam.ErrNoIPAvailable) {
-			continue
-		}
 		if err != nil {
+			var connectErr *connect.Error
+			if errors.As(err, &connectErr) {
+				if connectErr.Code() == connect.CodeNotFound {
+					continue
+				}
+			}
 			return "", "", err
 		}
 

cmd/metal-api/internal/service/ip-service_test.go

@@ -313,7 +313,7 @@ func TestAllocateIP(t *testing.T) {
 				AddressFamily: pointer.Pointer(metal.IPv6AddressFamily),
 			},
 			wantedStatus: http.StatusBadRequest,
-			wantErr:      errors.New("there is no prefix for the given addressfamily:IPv6 present in this network:4"),
+			wantErr:      errors.New("there is no prefix for the given addressfamily:IPv6 present in network:4"),
 		},
 	}
 	for i := range tests {

cmd/metal-api/internal/service/machine-service.go

@@ -1396,6 +1396,14 @@ func findWaitingMachine(ctx context.Context, ds *datastore.RethinkStore, allocat
 // is enabled to clean up networks that were already created.
 func makeNetworks(ctx context.Context, ds *datastore.RethinkStore, ipamer ipam.IPAMer, allocationSpec *machineAllocationSpec, networks allocationNetworkMap, alloc *metal.MachineAllocation) error {
 	for _, n := range networks {
+		if n == nil || n.network == nil {
+			continue
+		}
+		if len(n.network.AddressFamilies) == 0 {
+			n.network.AddressFamilies = metal.AddressFamilies{
+				metal.IPv4AddressFamily: true,
+			}
+		}
 		machineNetwork, err := makeMachineNetwork(ctx, ds, ipamer, allocationSpec, n)
 		if err != nil {
 			return err
@@ -1472,10 +1480,12 @@ func gatherNetworksFromSpec(ds *datastore.RethinkStore, allocationSpec *machineA
 	// - user specifies administrative networks, i.e. underlay or privatesuper networks
 	// - user's private network is specified with noauto but no specific IPs are given: this would yield a machine with no ip address
 
-	specNetworks := make(map[string]*allocationNetwork)
-	var primaryPrivateNetwork *allocationNetwork
-	var privateNetworks []*allocationNetwork
-	var privateSharedNetworks []*allocationNetwork
+	var (
+		specNetworks          = make(map[string]*allocationNetwork)
+		primaryPrivateNetwork *allocationNetwork
+		privateNetworks       []*allocationNetwork
+		privateSharedNetworks []*allocationNetwork
+	)
 
 	for _, networkSpec := range allocationSpec.Networks {
 		auto := true