Merge branch 'master' into oci-pull

Author: mac641

.github/workflows/docker.yaml

@@ -29,7 +29,7 @@ jobs:
         password: ${{ secrets.DOCKER_REGISTRY_TOKEN }}
 
     - name: Checkout
-      uses: actions/checkout@v4
+      uses: actions/checkout@v5
 
     - name: Setup Go
       uses: actions/setup-go@v5
@@ -38,7 +38,7 @@ jobs:
         cache: false
 
     - name: Lint
-      uses: golangci/golangci-lint-action@v7
+      uses: golangci/golangci-lint-action@v8
       with:
         args: --build-tags integration -D protogetter --timeout=5m
 
@@ -52,18 +52,22 @@ jobs:
       run: |
         make release
 
+    - name: Set up Docker Buildx
+      uses: docker/setup-buildx-action@v3
+
     - name: Push image
-      uses: docker/build-push-action@v5
+      uses: docker/build-push-action@v6
       with:
         context: .
         push: true
+        sbom: true
         tags: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ env.tag }}
 
   integration:
     runs-on: ubuntu-latest
     steps:
     - name: Checkout
-      uses: actions/checkout@v4
+      uses: actions/checkout@v5
 
     - name: Setup Go
       uses: actions/setup-go@v5

CONTRIBUTING.md

@@ -422,7 +422,7 @@ func (rs *RethinkStore) UpdateMachine(oldMachine *metal.Machine, newMachine *met
 // FindWaitingMachine returns an available, not allocated, waiting and alive machine of given size within the given partition.
 // TODO: the algorithm can be optimized / shortened by using a rethinkdb join command and then using .Sample(1)
 // but current implementation should have a slightly better readability.
-func (rs *RethinkStore) FindWaitingMachine(ctx context.Context, projectid, partitionid string, size metal.Size, placementTags []string) (*metal.Machine, error) {
+func (rs *RethinkStore) FindWaitingMachine(ctx context.Context, projectid, partitionid string, size metal.Size, placementTags []string, role metal.Role) (*metal.Machine, error) {
 	q := *rs.machineTable()
 	q = q.Filter(map[string]interface{}{
 		"allocation":  nil,
@@ -473,6 +473,7 @@ func (rs *RethinkStore) FindWaitingMachine(ctx context.Context, projectid, parti
 	var partitionMachines metal.Machines
 	err = rs.SearchMachines(&MachineSearchQuery{
 		PartitionID: &partitionid,
+		SizeID:      &size.ID,
 	}, &partitionMachines)
 	if err != nil {
 		return nil, err
@@ -487,12 +488,12 @@ func (rs *RethinkStore) FindWaitingMachine(ctx context.Context, projectid, parti
 		return nil, err
 	}
 
-	ok := checkSizeReservations(available, projectid, partitionMachines.WithSize(size.ID).ByProjectID(), reservations)
+	ok := checkSizeReservations(available, projectid, partitionMachines.ByProjectID(), reservations)
 	if !ok {
 		return nil, errors.New("no machine available")
 	}
 
-	projectMachines := partitionMachines.ByProjectID()[projectid]
+	projectMachines := partitionMachines.WithRole(role).ByProjectID()[projectid]
 
 	spreadCandidates := spreadAcrossRacks(available, projectMachines, placementTags)
 	if len(spreadCandidates) == 0 {
@@ -523,8 +524,6 @@ func checkSizeReservations(available metal.Machines, projectid string, machinesB
 	)
 
 	for _, r := range reservations {
-		r := r
-
 		// sum up the amount of reservations
 		amount += r.Amount
 

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

@@ -5,6 +5,7 @@ package datastore
 
 import (
 	"context"
+	"fmt"
 	"log/slog"
 	"os"
 	"strconv"
@@ -993,8 +994,6 @@ func Test_FindWaitingMachine_NoConcurrentModificationErrors(t *testing.T) {
 			},
 		},
 	} {
-		initEntity := initEntity
-
 		err := sharedDS.createEntity(initEntity.table, initEntity.entity)
 		require.NoError(t, err)
 
@@ -1004,8 +1003,7 @@ func Test_FindWaitingMachine_NoConcurrentModificationErrors(t *testing.T) {
 		}()
 	}
 
-	for i := 0; i < 100; i++ {
-		i := i
+	for i := range 100 {
 		wg.Add(1)
 
 		log := root.With("worker", i)
@@ -1014,7 +1012,7 @@ func Test_FindWaitingMachine_NoConcurrentModificationErrors(t *testing.T) {
 			defer wg.Done()
 
 			for {
-				machine, err := sharedDS.FindWaitingMachine(context.Background(), "project", "partition", size, nil)
+				machine, err := sharedDS.FindWaitingMachine(context.Background(), "project", "partition", size, nil, metal.RoleMachine)
 				if err != nil {
 					if metal.IsConflict(err) {
 						t.Errorf("concurrent modification occurred, shared mutex is not working")
@@ -1061,3 +1059,121 @@ func Test_FindWaitingMachine_NoConcurrentModificationErrors(t *testing.T) {
 
 	assert.Equal(t, 100, count)
 }
+
+func Test_FindWaitingMachine_RackSpreadingDistribution(t *testing.T) {
+	var (
+		partitionID = "partition"
+		projectID   = "project"
+		size1       = metal.Size{Base: metal.Base{ID: "1"}}
+		fiveRacks   = func(i int) string {
+			return "rack-" + strconv.FormatInt(int64((i%5)+1), 10)
+		}
+	)
+
+	defer func() {
+		_, err := sharedDS.machineTable().Delete().RunWrite(sharedDS.session)
+		require.NoError(t, err)
+		_, err = sharedDS.eventTable().Delete().RunWrite(sharedDS.session)
+		require.NoError(t, err)
+	}()
+
+	for i := range 200 {
+		err := sharedDS.createEntity(sharedDS.machineTable(), &metal.Machine{
+			Base: metal.Base{
+				ID: strconv.Itoa(i),
+			},
+			PartitionID: partitionID,
+			SizeID:      size1.ID,
+			State: metal.MachineState{
+				Value: metal.AvailableState,
+			},
+			Waiting:      true,
+			PreAllocated: false,
+			RackID:       fiveRacks(i),
+		})
+		require.NoError(t, err)
+
+		err = sharedDS.createEntity(sharedDS.eventTable(), &metal.ProvisioningEventContainer{
+			Base: metal.Base{
+				ID: strconv.Itoa(i),
+			},
+			Liveliness: metal.MachineLivelinessAlive,
+		})
+		require.NoError(t, err)
+	}
+
+	// just allocate some machines with different specs that should not influence later allocs
+	for i, spec := range []struct {
+		role metal.Role
+		size string
+	}{
+		{role: metal.RoleFirewall, size: "firewall"},
+		{role: metal.RoleFirewall, size: "firewall"},
+		{role: metal.RoleMachine, size: "machine"},
+		{role: metal.RoleMachine, size: "machine"},
+		// just to prove that it affects the algorithm:
+		// {role: metal.RoleMachine, size: size1.ID},
+	} {
+		err := sharedDS.createEntity(sharedDS.machineTable(), &metal.Machine{
+			Base: metal.Base{
+				ID: "allocated-" + strconv.Itoa(i),
+			},
+			PartitionID: partitionID,
+			SizeID:      spec.size,
+			State: metal.MachineState{
+				Value: metal.AvailableState,
+			},
+			Allocation: &metal.MachineAllocation{
+				Project: projectID,
+				Role:    spec.role,
+			},
+			RackID: fiveRacks(i),
+		})
+		require.NoError(t, err)
+
+		err = sharedDS.createEntity(sharedDS.eventTable(), &metal.ProvisioningEventContainer{
+			Base: metal.Base{
+				ID: "allocated-" + strconv.Itoa(i),
+			},
+			Liveliness: metal.MachineLivelinessAlive,
+		})
+		require.NoError(t, err)
+	}
+
+	for range 100 {
+		machine, err := sharedDS.FindWaitingMachine(context.Background(), projectID, partitionID, size1, nil, metal.RoleMachine)
+		require.NoError(t, err)
+
+		newMachine := *machine
+		newMachine.PreAllocated = false
+		newMachine.Allocation = &metal.MachineAllocation{
+			Project: projectID,
+		}
+		newMachine.Allocation.Role = metal.RoleMachine
+		newMachine.SizeID = size1.ID
+
+		err = sharedDS.updateEntity(sharedDS.machineTable(), &newMachine, machine)
+		if err != nil {
+			t.Errorf("unable to update machine: %s", err)
+		}
+
+		t.Logf("machine %s allocated in %s", newMachine.ID, newMachine.RackID)
+	}
+
+	var ms metal.Machines
+	err := sharedDS.SearchMachines(&MachineSearchQuery{AllocationProject: &projectID, SizeID: &size1.ID, PartitionID: &partitionID}, &ms)
+	require.NoError(t, err)
+
+	require.Len(t, ms, 100)
+
+	machinesByRack := map[string]int{}
+	for _, m := range ms {
+		machinesByRack[m.RackID]++
+	}
+
+	for id, count := range machinesByRack {
+		assert.Equal(t, 100/5, count, "uneven machine distribution in %s", id)
+	}
+
+	fmt.Println(machinesByRack)
+}

cmd/metal-api/internal/datastore/machine_integration_test.go

@@ -0,0 +1,90 @@
+package migrations
+
+import (
+	"slices"
+
+	"github.com/metal-stack/metal-lib/pkg/pointer"
+
+	"github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore"
+	"github.com/metal-stack/metal-api/cmd/metal-api/internal/metal"
+
+	r "gopkg.in/rethinkdb/rethinkdb-go.v6"
+)
+
+func init() {
+	datastore.MustRegisterMigration(datastore.Migration{
+		Name:    "migrate networks to have proper networkType set",
+		Version: 9,
+		Up: func(db *r.Term, session r.QueryExecutor, ds *datastore.RethinkStore) error {
+			nws, err := ds.ListNetworks()
+			if err != nil {
+				return err
+			}
+
+			// first detect all shared vrf ids
+			// maps vrfid > count
+			var (
+				vrfCount   = make(map[uint]int)
+				sharedVrfs []uint
+			)
+
+			for _, nw := range nws {
+				if nw.Vrf == 0 {
+					continue
+				}
+				count, ok := vrfCount[nw.Vrf]
+				if !ok {
+					vrfCount[nw.Vrf] = 1
+				} else {
+					vrfCount[nw.Vrf] = count + 1
+				}
+			}
+
+			for vrf, count := range vrfCount {
+				if count > 1 {
+					sharedVrfs = append(sharedVrfs, vrf)
+				}
+			}
+
+			// now convert all networks
+			for _, old := range nws {
+				new := old
+
+				// assume external network by default
+				new.NetworkType = pointer.Pointer(metal.ExternalNetworkType)
+
+				if old.Shared && old.ParentNetworkID != "" {
+					new.NetworkType = pointer.Pointer(metal.ChildSharedNetworkType)
+				}
+				if old.Shared && old.ParentNetworkID == "" {
+					new.NetworkType = pointer.Pointer(metal.ExternalNetworkType)
+				}
+				if !old.Shared && old.ParentNetworkID != "" && !slices.Contains(sharedVrfs, old.Vrf) {
+					new.NetworkType = pointer.Pointer(metal.ChildNetworkType)
+				}
+				if old.ProjectID == "" && old.ParentNetworkID == "" && !slices.Contains(sharedVrfs, old.Vrf) {
+					new.NetworkType = pointer.Pointer(metal.ExternalNetworkType)
+				}
+				if old.PrivateSuper {
+					new.NetworkType = pointer.Pointer(metal.SuperNetworkType)
+				}
+				if old.Underlay {
+					new.NetworkType = pointer.Pointer(metal.UnderlayNetworkType)
+				}
+
+				if old.Nat {
+					new.NATType = pointer.Pointer(metal.IPv4MasqueradeNATType)
+				} else {
+					new.NATType = pointer.Pointer(metal.NoneNATType)
+				}
+
+				err := ds.UpdateNetwork(&old, &new)
+				if err != nil {
+					return err
+				}
+			}
+
+			return nil
+		},
+	})
+}