Support different disk types for size matching (#528)

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

@@ -39,8 +39,7 @@ type MachineSearchQuery struct {
 	NetworkASNs                []int64  `json:"network_asns" optional:"true"`
 
 	// hardware
-	HardwareMemory   *int64 `json:"hardware_memory" optional:"true"`
-	HardwareCPUCores *int64 `json:"hardware_cpu_cores" optional:"true"`
+	HardwareMemory *int64 `json:"hardware_memory" optional:"true"`
 
 	// nics
 	NicsMacAddresses         []string `json:"nics_mac_addresses" optional:"true"`
@@ -211,12 +210,6 @@ func (p *MachineSearchQuery) generateTerm(rs *RethinkStore) *r.Term {
 		})
 	}
 
-	if p.HardwareCPUCores != nil {
-		q = q.Filter(func(row r.Term) r.Term {
-			return row.Field("hardware").Field("cpu_cores").Eq(*p.HardwareCPUCores)
-		})
-	}
-
 	for _, mac := range p.NicsMacAddresses {
 		mac := mac
 		q = q.Filter(func(row r.Term) r.Term {

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

@@ -479,21 +479,6 @@ func TestRethinkStore_SearchMachines(t *testing.T) {
 			},
 			wantErr: nil,
 		},
-		{
-			name: "search by hardware cpus",
-			q: &MachineSearchQuery{
-				HardwareCPUCores: pointer.Pointer(int64(8)),
-			},
-			mock: []*metal.Machine{
-				{Base: metal.Base{ID: "1"}, Hardware: metal.MachineHardware{CPUCores: 1}},
-				{Base: metal.Base{ID: "2"}, Hardware: metal.MachineHardware{CPUCores: 2}},
-				{Base: metal.Base{ID: "3"}, Hardware: metal.MachineHardware{CPUCores: 8}},
-			},
-			want: []*metal.Machine{
-				tt.defaultBody(&metal.Machine{Base: metal.Base{ID: "3"}, Hardware: metal.MachineHardware{CPUCores: 8}}),
-			},
-			wantErr: nil,
-		},
 		{
 			name: "search by nic mac address",
 			q: &MachineSearchQuery{

cmd/metal-api/internal/grpc/boot-service.go

@@ -144,7 +144,6 @@ func (b *BootService) Register(ctx context.Context, req *v1.BootServiceRegisterR
 
 	machineHardware := metal.MachineHardware{
 		Memory:    req.Hardware.Memory,
-		CPUCores:  int(req.Hardware.CpuCores),
 		Disks:     disks,
 		Nics:      nics,
 		MetalCPUs: cpus,

cmd/metal-api/internal/grpc/boot-service_test.go

@@ -118,8 +118,15 @@ func TestBootService_Register(t *testing.T) {
 			req := &v1.BootServiceRegisterRequest{
 				Uuid: tt.uuid,
 				Hardware: &v1.MachineHardware{
-					Memory:   uint64(tt.memory),
-					CpuCores: uint32(tt.numcores),
+					Memory: uint64(tt.memory),
+
+					Cpus: []*v1.MachineCPU{
+						{
+							Model:   "Intel Xeon Silver",
+							Cores:   uint32(tt.numcores),
+							Threads: uint32(tt.numcores),
+						},
+					},
 					Nics: []*v1.MachineNic{
 						{
 							Mac: "aa", Neighbors: []*v1.MachineNic{{Mac: string(tt.neighbormac1)}},

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

@@ -5,12 +5,14 @@ import (
 	"log/slog"
 	"net/netip"
 	"os"
+	"path/filepath"
 	"slices"
 	"strings"
 	"time"
 
 	"github.com/dustin/go-humanize"
 	mn "github.com/metal-stack/metal-lib/pkg/net"
+	"github.com/samber/lo"
 )
 
 // A MState is an enum which indicates the state of a machine
@@ -458,7 +460,6 @@ func (n NetworkType) String() string {
 // MachineHardware stores the data which is collected by our system on the hardware when it registers itself.
 type MachineHardware struct {
 	Memory    uint64        `rethinkdb:"memory" json:"memory"`
-	CPUCores  int           `rethinkdb:"cpu_cores" json:"cpu_cores"`
 	Nics      Nics          `rethinkdb:"network_interfaces" json:"network_interfaces"`
 	Disks     []BlockDevice `rethinkdb:"block_devices" json:"block_devices"`
 	MetalCPUs []MetalCPU    `rethinkdb:"cpus" json:"cpus"`
@@ -489,13 +490,51 @@ const (
 	MachineResurrectAfter    time.Duration     = time.Hour
 )
 
-// DiskCapacity calculates the capacity of all disks.
-func (hw *MachineHardware) DiskCapacity() uint64 {
-	var c uint64
-	for _, d := range hw.Disks {
-		c += d.Size
+func capacityOf[V any](identifier string, vs []V, countFn func(v V) (model string, count uint64)) (uint64, []V) {
+	var (
+		sum     uint64
+		matched []V
+	)
+
+	if identifier == "" {
+		identifier = "*"
+	}
+
+	for _, v := range vs {
+		model, count := countFn(v)
+
+		matches, err := filepath.Match(identifier, model)
+		if err != nil {
+			// illegal identifiers are already prevented by size validation
+			continue
+		}
+
+		if !matches {
+			continue
+		}
+
+		sum += count
+		matched = append(matched, v)
 	}
-	return c
+
+	return sum, matched
+}
+
+func exhaustiveMatch[V comparable](cs []Constraint, vs []V, countFn func(v V) (model string, count uint64)) bool {
+	unmatched := slices.Clone(vs)
+
+	for _, c := range cs {
+		capacity, matched := capacityOf(c.Identifier, vs, countFn)
+
+		match := c.inRange(capacity)
+		if !match {
+			continue
+		}
+
+		unmatched, _ = lo.Difference(unmatched, matched)
+	}
+
+	return len(unmatched) == 0
 }
 
 func (hw *MachineHardware) GPUModels() map[string]uint64 {
@@ -513,7 +552,10 @@ func (hw *MachineHardware) GPUModels() map[string]uint64 {
 
 // ReadableSpec returns a human readable string for the hardware.
 func (hw *MachineHardware) ReadableSpec() string {
-	return fmt.Sprintf("Cores: %d, Memory: %s, Storage: %s GPUs:%s", hw.CPUCores, humanize.Bytes(hw.Memory), humanize.Bytes(hw.DiskCapacity()), hw.MetalGPUs)
+	diskCapacity, _ := capacityOf("*", hw.Disks, countDisk)
+	cpus, _ := capacityOf("*", hw.MetalCPUs, countCPU)
+	gpus, _ := capacityOf("*", hw.MetalGPUs, countGPU)
+	return fmt.Sprintf("CPUs: %d, Memory: %s, Storage: %s, GPUs: %d", cpus, humanize.Bytes(hw.Memory), humanize.Bytes(diskCapacity), gpus)
 }
 
 // BlockDevice information.

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

@@ -24,8 +24,14 @@ func TestMachine_HasMAC(t *testing.T) {
 				SizeID:      "1",
 				Allocation:  nil,
 				Hardware: MachineHardware{
-					Memory:   100,
-					CPUCores: 1,
+					Memory: 100,
+					MetalCPUs: []MetalCPU{
+						{
+							Model:   "Intel Xeon Silver",
+							Cores:   1,
+							Threads: 1,
+						},
+					},
 					Nics: Nics{
 						Nic{
 							MacAddress: "11:11:11:11:11:11",

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

@@ -38,15 +38,28 @@ const (
 	GPUConstraint     ConstraintType = "gpu"
 )
 
-// A Constraint describes the hardware constraints for a given size. At the moment we only
-// consider the cpu cores and the memory.
+var allConstraintTypes = []ConstraintType{CoreConstraint, MemoryConstraint, StorageConstraint, GPUConstraint}
+
+// A Constraint describes the hardware constraints for a given size.
 type Constraint struct {
 	Type       ConstraintType `rethinkdb:"type" json:"type"`
 	Min        uint64         `rethinkdb:"min" json:"min"`
 	Max        uint64         `rethinkdb:"max" json:"max"`
 	Identifier string         `rethinkdb:"identifier" json:"identifier" description:"glob of the identifier of this type"`
 }
 
+func countCPU(cpu MetalCPU) (model string, count uint64) {
+	return cpu.Model, uint64(cpu.Cores)
+}
+
+func countGPU(gpu MetalGPU) (model string, count uint64) {
+	return gpu.Model, 1
+}
+
+func countDisk(disk BlockDevice) (model string, count uint64) {
+	return disk.Name, disk.Size
+}
+
 // Sizes is a list of sizes.
 type Sizes []Size
 
@@ -72,55 +85,85 @@ func UnknownSize() *Size {
 	}
 }
 
-// Matches returns true if the given machine hardware is inside the min/max values of the
+func (c *Constraint) inRange(value uint64) bool {
+	return value >= c.Min && value <= c.Max
+}
+
+// matches returns true if the given machine hardware is inside the min/max values of the
 // constraint.
-func (c *Constraint) Matches(hw MachineHardware) bool {
+func (c *Constraint) matches(hw MachineHardware) bool {
 	res := false
 	switch c.Type {
 	case CoreConstraint:
-		res = uint64(hw.CPUCores) >= c.Min && uint64(hw.CPUCores) <= c.Max
+		cores, _ := capacityOf(c.Identifier, hw.MetalCPUs, countCPU)
+		res = c.inRange(cores)
 	case MemoryConstraint:
-		res = hw.Memory >= c.Min && hw.Memory <= c.Max
+		res = c.inRange(hw.Memory)
 	case StorageConstraint:
-		res = hw.DiskCapacity() >= c.Min && hw.DiskCapacity() <= c.Max
+		capacity, _ := capacityOf(c.Identifier, hw.Disks, countDisk)
+		res = c.inRange(capacity)
 	case GPUConstraint:
-		for model, count := range hw.GPUModels() {
-			idMatches, err := filepath.Match(c.Identifier, model)
-			if err != nil {
-				return false
-			}
-			res = count >= c.Min && count <= c.Max && idMatches
-			if res {
-				break
-			}
-		}
-
+		count, _ := capacityOf(c.Identifier, hw.MetalGPUs, countGPU)
+		res = c.inRange(count)
 	}
 	return res
 }
 
+// matches returns true if all provided disks and later GPUs are covered with at least one constraint.
+// With this we ensure that hardware matches exhaustive against the constraints.
+func (hw *MachineHardware) matches(constraints []Constraint, constraintType ConstraintType) bool {
+	filtered := lo.Filter(constraints, func(c Constraint, _ int) bool { return c.Type == constraintType })
+	if len(filtered) == 0 {
+		return true
+	}
+
+	switch constraintType {
+	case StorageConstraint:
+		return exhaustiveMatch(filtered, hw.Disks, countDisk)
+	case GPUConstraint:
+		return exhaustiveMatch(filtered, hw.MetalGPUs, countGPU)
+	case CoreConstraint:
+		return exhaustiveMatch(filtered, hw.MetalCPUs, countCPU)
+	case MemoryConstraint:
+		// Noop because we do not have different Memory types
+		return true
+	default:
+		return true
+	}
+}
+
 // FromHardware searches a Size for given hardware specs. It will search
 // for a size where the constraints matches the given hardware.
 func (sz Sizes) FromHardware(hardware MachineHardware) (*Size, error) {
-	var found []Size
+	var (
+		matchedSizes []Size
+	)
+
 nextsize:
 	for _, s := range sz {
 		for _, c := range s.Constraints {
-			match := c.Matches(hardware)
+			match := c.matches(hardware)
+			if !match {
+				continue nextsize
+			}
+		}
+
+		for _, ct := range allConstraintTypes {
+			match := hardware.matches(s.Constraints, ct)
 			if !match {
 				continue nextsize
 			}
 		}
-		found = append(found, s)
+		matchedSizes = append(matchedSizes, s)
 	}
 
-	if len(found) == 0 {
+	if len(matchedSizes) == 0 {
 		return nil, NotFound("no size found for hardware (%s)", hardware.ReadableSpec())
 	}
-	if len(found) > 1 {
-		return nil, fmt.Errorf("%d sizes found for hardware (%s)", len(found), hardware.ReadableSpec())
+	if len(matchedSizes) > 1 {
+		return nil, fmt.Errorf("%d sizes found for hardware (%s)", len(matchedSizes), hardware.ReadableSpec())
 	}
-	return &found[0], nil
+	return &matchedSizes[0], nil
 }
 
 func (s *Size) overlaps(so *Size) bool {
@@ -172,23 +215,35 @@ func (c *Constraint) overlaps(other Constraint) bool {
 
 // Validate a size, returns error if a invalid size is passed
 func (s *Size) Validate(partitions PartitionMap, projects map[string]*mdmv1.Project) error {
-	constraintTypes := map[ConstraintType]bool{}
+	constraintTypes := map[ConstraintType]uint{}
 	for _, c := range s.Constraints {
 		if c.Max < c.Min {
 			return fmt.Errorf("size:%q type:%q max:%d is smaller than min:%d", s.ID, c.Type, c.Max, c.Min)
 		}
 
-		_, ok := constraintTypes[c.Type]
-		if ok {
-			return fmt.Errorf("size:%q type:%q min:%d max:%d has duplicate constraint type", s.ID, c.Type, c.Min, c.Max)
+		// CPU and Memory Constraints are not allowed more than once
+		constraintTypes[c.Type]++
+		count := constraintTypes[c.Type]
+		if c.Type == CoreConstraint || c.Type == MemoryConstraint {
+			if count > 1 {
+				return fmt.Errorf("size:%q type:%q min:%d max:%d has duplicate constraint type", s.ID, c.Type, c.Min, c.Max)
+			}
 		}
 
 		// Ensure GPU Constraints always have identifier specified
 		if c.Type == GPUConstraint && c.Identifier == "" {
 			return fmt.Errorf("size:%q type:%q min:%d max:%d is a gpu size but has no identifier specified", s.ID, c.Type, c.Min, c.Max)
 		}
 
-		constraintTypes[c.Type] = true
+		// Ensure Memory Constraints do not have a identifier specified
+		if c.Type == MemoryConstraint && c.Identifier != "" {
+			return fmt.Errorf("size:%q type:%q min:%d max:%d is a memory size but has a identifier specified", s.ID, c.Type, c.Min, c.Max)
+		}
+
+		if _, err := filepath.Match(c.Identifier, ""); err != nil {
+			return fmt.Errorf("size:%q type:%q min:%d max:%d identifier:%q identifier is malformed:%w", s.ID, c.Type, c.Min, c.Max, c.Identifier, err)
+		}
+
 	}
 
 	if err := s.Reservations.Validate(partitions, projects); err != nil {