From e30d6e98579293a7bc854ce97e280405b1b77018 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 27 Dec 2023 23:13:07 +0000 Subject: [PATCH 01/23] Introduce size reservations for projects. This allows reserving machines of a specific size for projects in the the allocation process. This is feature can be important for the operators to have some spare machines available, e.g. for updating Gardener Seed clusters while preventing regular users to allocate the last machines available in a partition. There are also use-cases where certain customers want to use a certain machine size exclusively, which can also be realized with this approach. --- cmd/metal-api/internal/datastore/machine.go | 61 +++++-- .../internal/datastore/machine_test.go | 149 ++++++++++++++++++ cmd/metal-api/internal/metal/machine.go | 16 ++ cmd/metal-api/internal/metal/size.go | 48 +++++- .../internal/service/machine-service.go | 2 +- .../internal/service/size-service.go | 24 ++- cmd/metal-api/internal/service/v1/size.go | 30 +++- spec/metal-api.json | 50 ++++++ 8 files changed, 363 insertions(+), 17 deletions(-) diff --git a/cmd/metal-api/internal/datastore/machine.go b/cmd/metal-api/internal/datastore/machine.go index dcc32a2d2..c5dcfc5cf 100644 --- a/cmd/metal-api/internal/datastore/machine.go +++ b/cmd/metal-api/internal/datastore/machine.go @@ -428,12 +428,12 @@ 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(projectid, partitionid, sizeid string, placementTags []string) (*metal.Machine, error) { +func (rs *RethinkStore) FindWaitingMachine(projectid, partitionid string, size metal.Size, placementTags []string) (*metal.Machine, error) { q := *rs.machineTable() q = q.Filter(map[string]interface{}{ "allocation": nil, "partitionid": partitionid, - "sizeid": sizeid, + "sizeid": size.ID, "state": map[string]string{ "value": string(metal.AvailableState), }, @@ -467,21 +467,25 @@ func (rs *RethinkStore) FindWaitingMachine(projectid, partitionid, sizeid string available = append(available, m) } - if available == nil || len(available) < 1 { + if len(available) == 0 { return nil, errors.New("no machine available") } - query := MachineSearchQuery{ - AllocationProject: &projectid, - PartitionID: &partitionid, - } - - var projectMachines metal.Machines - err = rs.SearchMachines(&query, &projectMachines) + var partitionMachines metal.Machines + err = rs.SearchMachines(&MachineSearchQuery{ + PartitionID: &partitionid, + }, &partitionMachines) if err != nil { return nil, err } + ok := checkSizeReservations(available, projectid, partitionid, partitionMachines.WithSize(size.ID).ByProjectID(), size) + if !ok { + return nil, errors.New("no machine available") + } + + projectMachines := partitionMachines.ByProjectID()[projectid] + spreadCandidates := spreadAcrossRacks(available, projectMachines, placementTags) if len(spreadCandidates) == 0 { return nil, errors.New("no machine available") @@ -499,6 +503,43 @@ func (rs *RethinkStore) FindWaitingMachine(projectid, partitionid, sizeid string return &newMachine, nil } +// checkSizeReservations returns true when an allocation is possible and +// false when size reservations prevent the allocation for the given project in the given partition +func checkSizeReservations(available metal.Machines, projectid, partitionid string, machinesByProject map[string]metal.Machines, size metal.Size) bool { + if size.Reservations == nil { + return true + } + + var ( + reservations = 0 + max = func(x, y int) int { + if x > y { + return x + } + return y + } + ) + + for _, r := range size.Reservations.ForPartition(partitionid) { + r := r + + // sum up the amount of reservations + reservations += r.Amount + + alreadyAllocated := len(machinesByProject[r.ProjectID]) + + if projectid == r.ProjectID && alreadyAllocated < r.Amount { + // allow allocation for the project when it has a reservation and there are still allocations left + return true + } + + // substract aleady used up reservations of the project + reservations = max(reservations-alreadyAllocated, 0) + } + + return reservations < len(available) +} + func spreadAcrossRacks(allMachines, projectMachines metal.Machines, tags []string) metal.Machines { var ( allRacks = groupByRack(allMachines) diff --git a/cmd/metal-api/internal/datastore/machine_test.go b/cmd/metal-api/internal/datastore/machine_test.go index f030cb684..09bdd31be 100644 --- a/cmd/metal-api/internal/datastore/machine_test.go +++ b/cmd/metal-api/internal/datastore/machine_test.go @@ -9,6 +9,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/metal-stack/metal-api/cmd/metal-api/internal/metal" "github.com/metal-stack/metal-api/cmd/metal-api/internal/testdata" + "github.com/stretchr/testify/require" "golang.org/x/exp/slices" ) @@ -718,3 +719,151 @@ func getTestMachines(numPerRack int, rackids []string, tags []string) metal.Mach return machines } + +func Test_checkSizeReservations(t *testing.T) { + var ( + available = metal.Machines{ + {Base: metal.Base{ID: "1"}}, + {Base: metal.Base{ID: "2"}}, + {Base: metal.Base{ID: "3"}}, + {Base: metal.Base{ID: "4"}}, + {Base: metal.Base{ID: "5"}}, + } + + partitionA = "a" + p0 = "0" + p1 = "1" + p2 = "2" + + size = metal.Size{ + Base: metal.Base{ + ID: "c1-xlarge-x86", + }, + Reservations: metal.Reservations{ + { + Amount: 1, + ProjectID: p1, + PartitionIDs: []string{partitionA}, + }, + { + Amount: 2, + ProjectID: p2, + PartitionIDs: []string{partitionA}, + }, + }, + } + + projectMachines = map[string]metal.Machines{} + + allocate = func(id, project string) { + available = slices.DeleteFunc(available, func(m metal.Machine) bool { + return m.ID == id + }) + projectMachines[project] = append(projectMachines[project], metal.Machine{Base: metal.Base{ID: id}}) + } + ) + + // 5 available, 3 reserved, project 0 can allocate + ok := checkSizeReservations(available, p0, partitionA, projectMachines, size) + require.True(t, ok) + allocate(available[0].ID, p0) + + require.Equal(t, metal.Machines{ + {Base: metal.Base{ID: "2"}}, + {Base: metal.Base{ID: "3"}}, + {Base: metal.Base{ID: "4"}}, + {Base: metal.Base{ID: "5"}}, + }, available) + require.Equal(t, map[string]metal.Machines{ + p0: { + {Base: metal.Base{ID: "1"}}, + }, + }, projectMachines) + + // 4 available, 3 reserved, project 2 can allocate + ok = checkSizeReservations(available, p2, partitionA, projectMachines, size) + require.True(t, ok) + allocate(available[0].ID, p2) + + require.Equal(t, metal.Machines{ + {Base: metal.Base{ID: "3"}}, + {Base: metal.Base{ID: "4"}}, + {Base: metal.Base{ID: "5"}}, + }, available) + require.Equal(t, map[string]metal.Machines{ + p0: { + {Base: metal.Base{ID: "1"}}, + }, + p2: { + {Base: metal.Base{ID: "2"}}, + }, + }, projectMachines) + + // 3 available, 3 reserved (1 used), project 0 can allocate + ok = checkSizeReservations(available, p0, partitionA, projectMachines, size) + require.True(t, ok) + allocate(available[0].ID, p0) + + require.Equal(t, metal.Machines{ + {Base: metal.Base{ID: "4"}}, + {Base: metal.Base{ID: "5"}}, + }, available) + require.Equal(t, map[string]metal.Machines{ + p0: { + {Base: metal.Base{ID: "1"}}, + {Base: metal.Base{ID: "3"}}, + }, + p2: { + {Base: metal.Base{ID: "2"}}, + }, + }, projectMachines) + + // 2 available, 3 reserved (1 used), project 0 cannot allocate anymore + ok = checkSizeReservations(available, p0, partitionA, projectMachines, size) + require.False(t, ok) + + // 2 available, 3 reserved (1 used), project 2 can allocate + ok = checkSizeReservations(available, p2, partitionA, projectMachines, size) + require.True(t, ok) + allocate(available[0].ID, p2) + + require.Equal(t, metal.Machines{ + {Base: metal.Base{ID: "5"}}, + }, available) + require.Equal(t, map[string]metal.Machines{ + p0: { + {Base: metal.Base{ID: "1"}}, + {Base: metal.Base{ID: "3"}}, + }, + p2: { + {Base: metal.Base{ID: "2"}}, + {Base: metal.Base{ID: "4"}}, + }, + }, projectMachines) + + // 1 available, 3 reserved (2 used), project 0 and 2 cannot allocate anymore + ok = checkSizeReservations(available, p0, partitionA, projectMachines, size) + require.False(t, ok) + ok = checkSizeReservations(available, p2, partitionA, projectMachines, size) + require.False(t, ok) + + // 1 available, 3 reserved (2 used), project 1 can allocate + ok = checkSizeReservations(available, p1, partitionA, projectMachines, size) + require.True(t, ok) + allocate(available[0].ID, p1) + + require.Equal(t, metal.Machines{}, available) + require.Equal(t, map[string]metal.Machines{ + p0: { + {Base: metal.Base{ID: "1"}}, + {Base: metal.Base{ID: "3"}}, + }, + p1: { + {Base: metal.Base{ID: "5"}}, + }, + p2: { + {Base: metal.Base{ID: "2"}}, + {Base: metal.Base{ID: "4"}}, + }, + }, projectMachines) +} diff --git a/cmd/metal-api/internal/metal/machine.go b/cmd/metal-api/internal/metal/machine.go index 0195ec646..ecd246604 100644 --- a/cmd/metal-api/internal/metal/machine.go +++ b/cmd/metal-api/internal/metal/machine.go @@ -171,6 +171,22 @@ func (ms Machines) ByProjectID() map[string]Machines { return res } +func (ms Machines) WithSize(id string) Machines { + var res Machines + + for _, m := range ms { + m := m + + if m.SizeID != id { + continue + } + + res = append(res, m) + } + + return res +} + // MachineNetwork stores the Network details of the machine type MachineNetwork struct { NetworkID string `rethinkdb:"networkid" json:"networkid"` diff --git a/cmd/metal-api/internal/metal/size.go b/cmd/metal-api/internal/metal/size.go index 67d996ed9..8f9548e59 100644 --- a/cmd/metal-api/internal/metal/size.go +++ b/cmd/metal-api/internal/metal/size.go @@ -2,6 +2,7 @@ package metal import ( "fmt" + "slices" ) // UnknownSize is the size to use, when someone requires a size we do not know. @@ -15,9 +16,20 @@ var UnknownSize = &Size{ // A Size represents a supported machine size. type Size struct { Base - Constraints []Constraint `rethinkdb:"constraints" json:"constraints"` + Constraints []Constraint `rethinkdb:"constraints" json:"constraints"` + Reservations Reservations `rethinkdb:"reservations" json:"reservations"` } +// Reservation defines a reservation of a size for machine allocations +type Reservation struct { + Amount int `rethinkdb:"amount" json:"amount"` + Description string `rethinkdb:"description" json:"description"` + ProjectID string `rethinkdb:"projectid" json:"projectid"` + PartitionIDs []string `rethinkdb:"partitionids" json:"partitionids"` +} + +type Reservations []Reservation + // ConstraintType ... type ConstraintType string @@ -124,6 +136,13 @@ func (s *Size) Validate() error { return fmt.Errorf("size:%q type:%q max:%d is smaller than min:%d", s.ID, c.Type, c.Max, c.Min) } } + + if s.Reservations != nil { + if err := s.Reservations.Validate(); err != nil { + return fmt.Errorf("invalid size reservation: %w", err) + } + } + return nil } @@ -138,6 +157,33 @@ func (s *Size) Overlaps(ss *Sizes) *Size { return nil } +func (rs Reservations) ForPartition(partitionID string) Reservations { + var result Reservations + for _, r := range rs { + r := r + if slices.Contains(r.PartitionIDs, partitionID) { + result = append(result, r) + } + } + return result +} + +func (rs Reservations) Validate() error { + for _, r := range rs { + if r.Amount <= 0 { + return fmt.Errorf("amount must be a positive integer") + } + if len(r.PartitionIDs) == 0 { + return fmt.Errorf("at least one partition id must be specified") + } + if r.ProjectID == "" { + return fmt.Errorf("project id must be specified") + } + } + + return nil +} + // A ConstraintMatchingLog is used do return a log message to the caller // beside the contraint itself. type ConstraintMatchingLog struct { diff --git a/cmd/metal-api/internal/service/machine-service.go b/cmd/metal-api/internal/service/machine-service.go index 54e9b4821..842fb0287 100644 --- a/cmd/metal-api/internal/service/machine-service.go +++ b/cmd/metal-api/internal/service/machine-service.go @@ -1317,7 +1317,7 @@ func findWaitingMachine(ds *datastore.RethinkStore, allocationSpec *machineAlloc return nil, fmt.Errorf("partition cannot be found: %w", err) } - machine, err := ds.FindWaitingMachine(allocationSpec.ProjectID, partition.ID, size.ID, allocationSpec.PlacementTags) + machine, err := ds.FindWaitingMachine(allocationSpec.ProjectID, partition.ID, *size, allocationSpec.PlacementTags) if err != nil { return nil, err } diff --git a/cmd/metal-api/internal/service/size-service.go b/cmd/metal-api/internal/service/size-service.go index af76f8591..10ae69e5e 100644 --- a/cmd/metal-api/internal/service/size-service.go +++ b/cmd/metal-api/internal/service/size-service.go @@ -213,6 +213,15 @@ func (r *sizeResource) createSize(request *restful.Request, response *restful.Re } constraints = append(constraints, constraint) } + var reservations metal.Reservations + for _, r := range requestPayload.SizeReservations { + reservations = append(reservations, metal.Reservation{ + Amount: r.Amount, + Description: r.Description, + ProjectID: r.ProjectID, + PartitionIDs: r.PartitionIDs, + }) + } s := &metal.Size{ Base: metal.Base{ @@ -220,7 +229,8 @@ func (r *sizeResource) createSize(request *restful.Request, response *restful.Re Name: name, Description: description, }, - Constraints: constraints, + Constraints: constraints, + Reservations: reservations, } ss, err := r.ds.ListSizes() @@ -302,6 +312,18 @@ func (r *sizeResource) updateSize(request *restful.Request, response *restful.Re } newSize.Constraints = constraints } + var reservations metal.Reservations + if requestPayload.SizeReservations != nil { + for _, r := range requestPayload.SizeReservations { + reservations = append(reservations, metal.Reservation{ + Amount: r.Amount, + Description: r.Description, + ProjectID: r.ProjectID, + PartitionIDs: r.PartitionIDs, + }) + } + newSize.Reservations = reservations + } ss, err := r.ds.ListSizes() if err != nil { diff --git a/cmd/metal-api/internal/service/v1/size.go b/cmd/metal-api/internal/service/v1/size.go index 9c875d778..73d074bd3 100644 --- a/cmd/metal-api/internal/service/v1/size.go +++ b/cmd/metal-api/internal/service/v1/size.go @@ -10,19 +10,29 @@ type SizeConstraint struct { Max uint64 `json:"max" description:"the maximum value of the constraint"` } +type SizeReservation struct { + Amount int `json:"amount" description:"the amount of reserved machine allocations for this size"` + Description string `json:"description,omitempty" description:"a description for this reservation"` + ProjectID string `json:"projectid" description:"the project for which this size reservation is considered"` + PartitionIDs []string `json:"partitionids" description:"the partitions in which this size reservation is considered, the amount is valid for every partition"` +} + type SizeCreateRequest struct { Common - SizeConstraints []SizeConstraint `json:"constraints" description:"a list of constraints that defines this size"` + SizeConstraints []SizeConstraint `json:"constraints" description:"a list of constraints that defines this size"` + SizeReservations []SizeReservation `json:"reservations,omitempty" description:"reservations for this size, which are considered during machine allocation" optional:"true"` } type SizeUpdateRequest struct { Common - SizeConstraints *[]SizeConstraint `json:"constraints" description:"a list of constraints that defines this size" optional:"true"` + SizeConstraints *[]SizeConstraint `json:"constraints" description:"a list of constraints that defines this size" optional:"true"` + SizeReservations []SizeReservation `json:"reservations,omitempty" description:"reservations for this size, which are considered during machine allocation" optional:"true"` } type SizeResponse struct { Common - SizeConstraints []SizeConstraint `json:"constraints" description:"a list of constraints that defines this size"` + SizeConstraints []SizeConstraint `json:"constraints" description:"a list of constraints that defines this size"` + SizeReservations []SizeReservation `json:"reservations,omitempty" description:"reservations for this size, which are considered during machine allocation" optional:"true"` Timestamps } @@ -80,6 +90,17 @@ func NewSizeResponse(s *metal.Size) *SizeResponse { constraints = append(constraints, constraint) } + reservations := []SizeReservation{} + for _, r := range s.Reservations { + reservation := SizeReservation{ + Amount: r.Amount, + Description: r.Description, + ProjectID: r.ProjectID, + PartitionIDs: r.PartitionIDs, + } + reservations = append(reservations, reservation) + } + return &SizeResponse{ Common: Common{ Identifiable: Identifiable{ @@ -90,7 +111,8 @@ func NewSizeResponse(s *metal.Size) *SizeResponse { Description: &s.Description, }, }, - SizeConstraints: constraints, + SizeReservations: reservations, + SizeConstraints: constraints, Timestamps: Timestamps{ Created: s.Created, Changed: s.Changed, diff --git a/spec/metal-api.json b/spec/metal-api.json index c4320ad98..c392a13e3 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -4340,6 +4340,13 @@ "name": { "description": "a readable name for this entity", "type": "string" + }, + "reservations": { + "description": "reservations for this size, which are considered during machine allocation", + "items": { + "$ref": "#/definitions/v1.SizeReservation" + }, + "type": "array" } }, "required": [ @@ -4473,6 +4480,35 @@ "name" ] }, + "v1.SizeReservation": { + "properties": { + "amount": { + "description": "the amount of reserved machine allocations for this size", + "format": "int32", + "type": "integer" + }, + "description": { + "description": "a description for this reservation", + "type": "string" + }, + "partitionids": { + "description": "the partitions in which this size reservation is considered, the amount is valid for every partition", + "items": { + "type": "string" + }, + "type": "array" + }, + "projectid": { + "description": "the project for which this size reservation is considered", + "type": "string" + } + }, + "required": [ + "amount", + "partitionids", + "projectid" + ] + }, "v1.SizeResponse": { "properties": { "changed": { @@ -4506,6 +4542,13 @@ "name": { "description": "a readable name for this entity", "type": "string" + }, + "reservations": { + "description": "reservations for this size, which are considered during machine allocation", + "items": { + "$ref": "#/definitions/v1.SizeReservation" + }, + "type": "array" } }, "required": [ @@ -4545,6 +4588,13 @@ "name": { "description": "a readable name for this entity", "type": "string" + }, + "reservations": { + "description": "reservations for this size, which are considered during machine allocation", + "items": { + "$ref": "#/definitions/v1.SizeReservation" + }, + "type": "array" } }, "required": [ From 469c821641e0aee131d4b563585640a35523cc21 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Mon, 8 Jan 2024 10:50:29 +0100 Subject: [PATCH 02/23] Review comments. --- cmd/metal-api/internal/datastore/machine.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/cmd/metal-api/internal/datastore/machine.go b/cmd/metal-api/internal/datastore/machine.go index c5dcfc5cf..4975ad037 100644 --- a/cmd/metal-api/internal/datastore/machine.go +++ b/cmd/metal-api/internal/datastore/machine.go @@ -512,12 +512,6 @@ func checkSizeReservations(available metal.Machines, projectid, partitionid stri var ( reservations = 0 - max = func(x, y int) int { - if x > y { - return x - } - return y - } ) for _, r := range size.Reservations.ForPartition(partitionid) { @@ -533,7 +527,7 @@ func checkSizeReservations(available metal.Machines, projectid, partitionid stri return true } - // substract aleady used up reservations of the project + // substract already used up reservations of the project reservations = max(reservations-alreadyAllocated, 0) } From 977e18c0095194cf39332709a32858d45360b23a Mon Sep 17 00:00:00 2001 From: Gerrit Date: Mon, 8 Jan 2024 10:51:48 +0100 Subject: [PATCH 03/23] Make `UnknownSize` a func to prevent modification. --- cmd/metal-api/internal/grpc/boot-service.go | 2 +- .../internal/grpc/boot-service_test.go | 4 ++-- cmd/metal-api/internal/metal/size.go | 18 ++++++++++-------- .../internal/service/machine-service.go | 4 ++-- cmd/metal-api/internal/service/size-service.go | 4 ++-- 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/cmd/metal-api/internal/grpc/boot-service.go b/cmd/metal-api/internal/grpc/boot-service.go index 4f35c4aed..b8d99025c 100644 --- a/cmd/metal-api/internal/grpc/boot-service.go +++ b/cmd/metal-api/internal/grpc/boot-service.go @@ -133,7 +133,7 @@ func (b *BootService) Register(ctx context.Context, req *v1.BootServiceRegisterR size, _, err := b.ds.FromHardware(machineHardware) if err != nil { - size = metal.UnknownSize + size = metal.UnknownSize() b.log.Errorw("no size found for hardware, defaulting to unknown size", "hardware", machineHardware, "error", err) } diff --git a/cmd/metal-api/internal/grpc/boot-service_test.go b/cmd/metal-api/internal/grpc/boot-service_test.go index c15954b10..a112e811f 100644 --- a/cmd/metal-api/internal/grpc/boot-service_test.go +++ b/cmd/metal-api/internal/grpc/boot-service_test.go @@ -90,7 +90,7 @@ func TestBootService_Register(t *testing.T) { neighbormac2: testdata.Switch2.Nics[0].MacAddress, numcores: 2, memory: 100, - expectedSizeId: metal.UnknownSize.ID, + expectedSizeId: metal.UnknownSize().ID, }, } @@ -109,7 +109,7 @@ func TestBootService_Register(t *testing.T) { Conflict: "replace", })).Return(testdata.EmptyResult, nil) } - mock.On(r.DB("mockdb").Table("size").Get(metal.UnknownSize.ID)).Return([]metal.Size{*metal.UnknownSize}, nil) + mock.On(r.DB("mockdb").Table("size").Get(metal.UnknownSize().ID)).Return([]metal.Size{*metal.UnknownSize()}, nil) mock.On(r.DB("mockdb").Table("switch").Filter(r.MockAnything(), r.FilterOpts{})).Return([]metal.Switch{testdata.Switch1, testdata.Switch2}, nil) mock.On(r.DB("mockdb").Table("event").Filter(r.MockAnything(), r.FilterOpts{})).Return([]metal.ProvisioningEventContainer{}, nil) mock.On(r.DB("mockdb").Table("event").Insert(r.MockAnything(), r.InsertOpts{})).Return(testdata.EmptyResult, nil) diff --git a/cmd/metal-api/internal/metal/size.go b/cmd/metal-api/internal/metal/size.go index 8f9548e59..dbb9eb90b 100644 --- a/cmd/metal-api/internal/metal/size.go +++ b/cmd/metal-api/internal/metal/size.go @@ -5,14 +5,6 @@ import ( "slices" ) -// UnknownSize is the size to use, when someone requires a size we do not know. -var UnknownSize = &Size{ - Base: Base{ - ID: "unknown", - Name: "unknown", - }, -} - // A Size represents a supported machine size. type Size struct { Base @@ -63,6 +55,16 @@ func (sz Sizes) ByID() SizeMap { return res } +// UnknownSize is the size to use, when someone requires a size we do not know. +func UnknownSize() *Size { + return &Size{ + Base: Base{ + ID: "unknown", + Name: "unknown", + }, + } +} + // Matches returns true if the given machine hardware is inside the min/max values of the // constraint. func (c *Constraint) Matches(hw MachineHardware) (ConstraintMatchingLog, bool) { diff --git a/cmd/metal-api/internal/service/machine-service.go b/cmd/metal-api/internal/service/machine-service.go index 842fb0287..041d52ee7 100644 --- a/cmd/metal-api/internal/service/machine-service.go +++ b/cmd/metal-api/internal/service/machine-service.go @@ -2319,8 +2319,8 @@ func findMachineReferencedEntities(m *metal.Machine, ds *datastore.RethinkStore) var s *metal.Size if m.SizeID != "" { - if m.SizeID == metal.UnknownSize.GetID() { - s = metal.UnknownSize + if m.SizeID == metal.UnknownSize().GetID() { + s = metal.UnknownSize() } else { s, err = ds.FindSize(m.SizeID) if err != nil { diff --git a/cmd/metal-api/internal/service/size-service.go b/cmd/metal-api/internal/service/size-service.go index 10ae69e5e..3d97f752d 100644 --- a/cmd/metal-api/internal/service/size-service.go +++ b/cmd/metal-api/internal/service/size-service.go @@ -191,8 +191,8 @@ func (r *sizeResource) createSize(request *restful.Request, response *restful.Re return } - if requestPayload.ID == metal.UnknownSize.GetID() { - r.sendError(request, response, httperrors.BadRequest(fmt.Errorf("id cannot be %q", metal.UnknownSize.GetID()))) + if requestPayload.ID == metal.UnknownSize().GetID() { + r.sendError(request, response, httperrors.BadRequest(fmt.Errorf("id cannot be %q", metal.UnknownSize().GetID()))) return } From 1a66797c9d229983a0beeb88a3856a11680cd2c5 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Mon, 8 Jan 2024 10:52:31 +0100 Subject: [PATCH 04/23] Return reservations in partition capacity. --- .../internal/service/partition-service.go | 30 ++++++++++++++----- .../service/partition-service_test.go | 2 ++ .../internal/service/v1/partition.go | 18 ++++++----- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/cmd/metal-api/internal/service/partition-service.go b/cmd/metal-api/internal/service/partition-service.go index ee1a90173..52c1676ad 100644 --- a/cmd/metal-api/internal/service/partition-service.go +++ b/cmd/metal-api/internal/service/partition-service.go @@ -362,6 +362,11 @@ func (r *partitionResource) calcPartitionCapacity(pcr *v1.PartitionCapacityReque return nil, fmt.Errorf("unable to fetch provisioning event containers: %w", err) } + sizes, err := r.ds.ListSizes() + if err != nil { + return nil, fmt.Errorf("unable to list sizes: %w", err) + } + machinesWithIssues, err := issues.Find(&issues.Config{ Machines: ms, EventContainers: ecs, @@ -371,8 +376,12 @@ func (r *partitionResource) calcPartitionCapacity(pcr *v1.PartitionCapacityReque return nil, fmt.Errorf("unable to calculate machine issues: %w", err) } - partitionsByID := ps.ByID() - ecsByID := ecs.ByID() + var ( + partitionsByID = ps.ByID() + ecsByID = ecs.ByID() + sizesByID = sizes.ByID() + machinesByProject = ms.ByProjectID() + ) for _, m := range ms { m := m @@ -404,21 +413,28 @@ func (r *partitionResource) calcPartitionCapacity(pcr *v1.PartitionCapacityReque } pcs[m.PartitionID] = pc - size := metal.UnknownSize.ID - if m.SizeID != "" { - size = m.SizeID + size, ok := sizesByID[m.SizeID] + if !ok { + size = *metal.UnknownSize() } - cap := pc.ServerCapacities.FindBySize(size) + cap := pc.ServerCapacities.FindBySize(size.ID) if cap == nil { cap = &v1.ServerCapacity{ - Size: size, + Size: size.ID, } pc.ServerCapacities = append(pc.ServerCapacities, cap) } cap.Total++ + for _, reservation := range size.Reservations { + reservation := reservation + + cap.Reservations += reservation.Amount + cap.UnusedReservations += max(reservation.Amount-len(machinesByProject[reservation.ProjectID]), 0) + } + if m.Allocation != nil { cap.Allocated++ continue diff --git a/cmd/metal-api/internal/service/partition-service_test.go b/cmd/metal-api/internal/service/partition-service_test.go index 0606015ab..5a370a120 100644 --- a/cmd/metal-api/internal/service/partition-service_test.go +++ b/cmd/metal-api/internal/service/partition-service_test.go @@ -288,4 +288,6 @@ func TestPartitionCapacity(t *testing.T) { require.Equal(t, "1", c.Size) require.Equal(t, 5, c.Total) require.Equal(t, 0, c.Free) + require.Equal(t, 0, c.Reservations) + require.Equal(t, 0, c.UnusedReservations) } diff --git a/cmd/metal-api/internal/service/v1/partition.go b/cmd/metal-api/internal/service/v1/partition.go index 522075f31..908b150f2 100644 --- a/cmd/metal-api/internal/service/v1/partition.go +++ b/cmd/metal-api/internal/service/v1/partition.go @@ -47,14 +47,16 @@ type PartitionCapacity struct { } type ServerCapacity struct { - Size string `json:"size" description:"the size of the server"` - Total int `json:"total" description:"total amount of servers with this size"` - Free int `json:"free" description:"free servers with this size"` - Allocated int `json:"allocated" description:"allocated servers with this size"` - Faulty int `json:"faulty" description:"servers with issues with this size"` - FaultyMachines []string `json:"faultymachines" description:"servers with issues with this size"` - Other int `json:"other" description:"servers neither free, allocated or faulty with this size"` - OtherMachines []string `json:"othermachines" description:"servers neither free, allocated or faulty with this size"` + Size string `json:"size" description:"the size of the server"` + Total int `json:"total" description:"total amount of servers with this size"` + Free int `json:"free" description:"free servers with this size"` + Allocated int `json:"allocated" description:"allocated servers with this size"` + Reservations int `json:"reservations" description:"the amount of reservations for this size"` + UnusedReservations int `json:"unusedreservations" description:"the amount of unused reservations for this size"` + Faulty int `json:"faulty" description:"servers with issues with this size"` + FaultyMachines []string `json:"faultymachines" description:"servers with issues with this size"` + Other int `json:"other" description:"servers neither free, allocated or faulty with this size"` + OtherMachines []string `json:"othermachines" description:"servers neither free, allocated or faulty with this size"` } func NewPartitionResponse(p *metal.Partition) *PartitionResponse { From fc2ea665ccdfb4754944d560f50012eb9d2db200 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Mon, 8 Jan 2024 10:52:40 +0100 Subject: [PATCH 05/23] Add labels to sizes. --- cmd/metal-api/internal/metal/size.go | 5 +++-- cmd/metal-api/internal/service/size-service.go | 8 ++++++++ cmd/metal-api/internal/service/v1/size.go | 4 ++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/cmd/metal-api/internal/metal/size.go b/cmd/metal-api/internal/metal/size.go index dbb9eb90b..eca53cbe3 100644 --- a/cmd/metal-api/internal/metal/size.go +++ b/cmd/metal-api/internal/metal/size.go @@ -8,8 +8,9 @@ import ( // A Size represents a supported machine size. type Size struct { Base - Constraints []Constraint `rethinkdb:"constraints" json:"constraints"` - Reservations Reservations `rethinkdb:"reservations" json:"reservations"` + Constraints []Constraint `rethinkdb:"constraints" json:"constraints"` + Reservations Reservations `rethinkdb:"reservations" json:"reservations"` + Labels map[string]string `rethinkdb:"labels" json:"labels"` } // Reservation defines a reservation of a size for machine allocations diff --git a/cmd/metal-api/internal/service/size-service.go b/cmd/metal-api/internal/service/size-service.go index 3d97f752d..46572b9ea 100644 --- a/cmd/metal-api/internal/service/size-service.go +++ b/cmd/metal-api/internal/service/size-service.go @@ -204,6 +204,10 @@ func (r *sizeResource) createSize(request *restful.Request, response *restful.Re if requestPayload.Description != nil { description = *requestPayload.Description } + labels := map[string]string{} + if requestPayload.Labels != nil { + labels = requestPayload.Labels + } var constraints []metal.Constraint for _, c := range requestPayload.SizeConstraints { constraint := metal.Constraint{ @@ -231,6 +235,7 @@ func (r *sizeResource) createSize(request *restful.Request, response *restful.Re }, Constraints: constraints, Reservations: reservations, + Labels: labels, } ss, err := r.ds.ListSizes() @@ -299,6 +304,9 @@ func (r *sizeResource) updateSize(request *restful.Request, response *restful.Re if requestPayload.Description != nil { newSize.Description = *requestPayload.Description } + if requestPayload.Labels != nil { + newSize.Labels = requestPayload.Labels + } var constraints []metal.Constraint if requestPayload.SizeConstraints != nil { sizeConstraints := *requestPayload.SizeConstraints diff --git a/cmd/metal-api/internal/service/v1/size.go b/cmd/metal-api/internal/service/v1/size.go index 73d074bd3..2b9ea953a 100644 --- a/cmd/metal-api/internal/service/v1/size.go +++ b/cmd/metal-api/internal/service/v1/size.go @@ -21,18 +21,21 @@ type SizeCreateRequest struct { Common SizeConstraints []SizeConstraint `json:"constraints" description:"a list of constraints that defines this size"` SizeReservations []SizeReservation `json:"reservations,omitempty" description:"reservations for this size, which are considered during machine allocation" optional:"true"` + Labels map[string]string `json:"labels" description:"free labels that you associate with this network." optional:"true"` } type SizeUpdateRequest struct { Common SizeConstraints *[]SizeConstraint `json:"constraints" description:"a list of constraints that defines this size" optional:"true"` SizeReservations []SizeReservation `json:"reservations,omitempty" description:"reservations for this size, which are considered during machine allocation" optional:"true"` + Labels map[string]string `json:"labels" description:"free labels that you associate with this network." optional:"true"` } type SizeResponse struct { Common SizeConstraints []SizeConstraint `json:"constraints" description:"a list of constraints that defines this size"` SizeReservations []SizeReservation `json:"reservations,omitempty" description:"reservations for this size, which are considered during machine allocation" optional:"true"` + Labels map[string]string `json:"labels" description:"free labels that you associate with this network."` Timestamps } @@ -117,5 +120,6 @@ func NewSizeResponse(s *metal.Size) *SizeResponse { Created: s.Created, Changed: s.Changed, }, + Labels: s.Labels, } } From 7de8ccbe7c650a90c08b9f4500248e1b7d8338eb Mon Sep 17 00:00:00 2001 From: Gerrit Date: Mon, 8 Jan 2024 10:52:45 +0100 Subject: [PATCH 06/23] Generate spec. --- spec/metal-api.json | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/spec/metal-api.json b/spec/metal-api.json index c392a13e3..02fe540d2 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -4248,6 +4248,11 @@ }, "type": "array" }, + "reservations": { + "description": "the amount of reservations for this size", + "format": "int32", + "type": "integer" + }, "size": { "description": "the size of the server", "type": "string" @@ -4256,6 +4261,11 @@ "description": "total amount of servers with this size", "format": "int32", "type": "integer" + }, + "unusedreservations": { + "description": "the amount of unused reservations for this size", + "format": "int32", + "type": "integer" } }, "required": [ @@ -4265,8 +4275,10 @@ "free", "other", "othermachines", + "reservations", "size", - "total" + "total", + "unusedreservations" ] }, "v1.SizeConstraint": { @@ -4337,6 +4349,13 @@ "type": "string", "uniqueItems": true }, + "labels": { + "additionalProperties": { + "type": "string" + }, + "description": "free labels that you associate with this network.", + "type": "object" + }, "name": { "description": "a readable name for this entity", "type": "string" @@ -4539,6 +4558,13 @@ "type": "string", "uniqueItems": true }, + "labels": { + "additionalProperties": { + "type": "string" + }, + "description": "free labels that you associate with this network.", + "type": "object" + }, "name": { "description": "a readable name for this entity", "type": "string" @@ -4553,7 +4579,8 @@ }, "required": [ "constraints", - "id" + "id", + "labels" ] }, "v1.SizeSuggestRequest": { @@ -4585,6 +4612,13 @@ "type": "string", "uniqueItems": true }, + "labels": { + "additionalProperties": { + "type": "string" + }, + "description": "free labels that you associate with this network.", + "type": "object" + }, "name": { "description": "a readable name for this entity", "type": "string" From 4c454aa75eefc09a689e0a936b067d7396f2e6d9 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Mon, 8 Jan 2024 10:55:30 +0100 Subject: [PATCH 07/23] Prevent partition id duplicates. --- cmd/metal-api/internal/metal/size.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/cmd/metal-api/internal/metal/size.go b/cmd/metal-api/internal/metal/size.go index eca53cbe3..5c0fb6aee 100644 --- a/cmd/metal-api/internal/metal/size.go +++ b/cmd/metal-api/internal/metal/size.go @@ -176,9 +176,18 @@ func (rs Reservations) Validate() error { if r.Amount <= 0 { return fmt.Errorf("amount must be a positive integer") } + if len(r.PartitionIDs) == 0 { return fmt.Errorf("at least one partition id must be specified") } + ids := map[string]bool{} + for _, partition := range r.PartitionIDs { + ids[partition] = true + } + if len(ids) != len(r.PartitionIDs) { + return fmt.Errorf("partitions must not contain duplicates") + } + if r.ProjectID == "" { return fmt.Errorf("project id must be specified") } From 88342b86b11919b7e8ec163fd5ba51bdaaa28c00 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Mon, 8 Jan 2024 10:58:47 +0100 Subject: [PATCH 08/23] Fix. --- cmd/metal-api/internal/service/partition-service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/metal-api/internal/service/partition-service.go b/cmd/metal-api/internal/service/partition-service.go index 52c1676ad..d3d844cd2 100644 --- a/cmd/metal-api/internal/service/partition-service.go +++ b/cmd/metal-api/internal/service/partition-service.go @@ -428,7 +428,7 @@ func (r *partitionResource) calcPartitionCapacity(pcr *v1.PartitionCapacityReque cap.Total++ - for _, reservation := range size.Reservations { + for _, reservation := range size.Reservations.ForPartition(p.ID) { reservation := reservation cap.Reservations += reservation.Amount From 4b02f83211364f802eb63399032f1e96969c62eb Mon Sep 17 00:00:00 2001 From: Gerrit Date: Mon, 8 Jan 2024 10:59:08 +0100 Subject: [PATCH 09/23] Fix. --- cmd/metal-api/internal/service/partition-service.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cmd/metal-api/internal/service/partition-service.go b/cmd/metal-api/internal/service/partition-service.go index d3d844cd2..09a1bc389 100644 --- a/cmd/metal-api/internal/service/partition-service.go +++ b/cmd/metal-api/internal/service/partition-service.go @@ -428,11 +428,13 @@ func (r *partitionResource) calcPartitionCapacity(pcr *v1.PartitionCapacityReque cap.Total++ - for _, reservation := range size.Reservations.ForPartition(p.ID) { - reservation := reservation + if size.Reservations != nil { + for _, reservation := range size.Reservations.ForPartition(p.ID) { + reservation := reservation - cap.Reservations += reservation.Amount - cap.UnusedReservations += max(reservation.Amount-len(machinesByProject[reservation.ProjectID]), 0) + cap.Reservations += reservation.Amount + cap.UnusedReservations += max(reservation.Amount-len(machinesByProject[reservation.ProjectID]), 0) + } } if m.Allocation != nil { From 3e9edce79b936173314366bc9bb7f9f56db36d67 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Mon, 8 Jan 2024 11:26:44 +0100 Subject: [PATCH 10/23] Return used reservations. --- .../internal/service/partition-service.go | 2 +- .../service/partition-service_test.go | 2 +- .../internal/service/v1/partition.go | 20 +++++++++---------- spec/metal-api.json | 6 +++--- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/cmd/metal-api/internal/service/partition-service.go b/cmd/metal-api/internal/service/partition-service.go index 09a1bc389..b5fcfbe64 100644 --- a/cmd/metal-api/internal/service/partition-service.go +++ b/cmd/metal-api/internal/service/partition-service.go @@ -433,7 +433,7 @@ func (r *partitionResource) calcPartitionCapacity(pcr *v1.PartitionCapacityReque reservation := reservation cap.Reservations += reservation.Amount - cap.UnusedReservations += max(reservation.Amount-len(machinesByProject[reservation.ProjectID]), 0) + cap.UsedReservations += len(machinesByProject[reservation.ProjectID]) } } diff --git a/cmd/metal-api/internal/service/partition-service_test.go b/cmd/metal-api/internal/service/partition-service_test.go index 5a370a120..2ac060d60 100644 --- a/cmd/metal-api/internal/service/partition-service_test.go +++ b/cmd/metal-api/internal/service/partition-service_test.go @@ -289,5 +289,5 @@ func TestPartitionCapacity(t *testing.T) { require.Equal(t, 5, c.Total) require.Equal(t, 0, c.Free) require.Equal(t, 0, c.Reservations) - require.Equal(t, 0, c.UnusedReservations) + require.Equal(t, 0, c.UsedReservations) } diff --git a/cmd/metal-api/internal/service/v1/partition.go b/cmd/metal-api/internal/service/v1/partition.go index 908b150f2..7bea44237 100644 --- a/cmd/metal-api/internal/service/v1/partition.go +++ b/cmd/metal-api/internal/service/v1/partition.go @@ -47,16 +47,16 @@ type PartitionCapacity struct { } type ServerCapacity struct { - Size string `json:"size" description:"the size of the server"` - Total int `json:"total" description:"total amount of servers with this size"` - Free int `json:"free" description:"free servers with this size"` - Allocated int `json:"allocated" description:"allocated servers with this size"` - Reservations int `json:"reservations" description:"the amount of reservations for this size"` - UnusedReservations int `json:"unusedreservations" description:"the amount of unused reservations for this size"` - Faulty int `json:"faulty" description:"servers with issues with this size"` - FaultyMachines []string `json:"faultymachines" description:"servers with issues with this size"` - Other int `json:"other" description:"servers neither free, allocated or faulty with this size"` - OtherMachines []string `json:"othermachines" description:"servers neither free, allocated or faulty with this size"` + Size string `json:"size" description:"the size of the server"` + Total int `json:"total" description:"total amount of servers with this size"` + Free int `json:"free" description:"free servers with this size"` + Allocated int `json:"allocated" description:"allocated servers with this size"` + Reservations int `json:"reservations" description:"the amount of reservations for this size"` + UsedReservations int `json:"usedreservations" description:"the amount of used reservations for this size"` + Faulty int `json:"faulty" description:"servers with issues with this size"` + FaultyMachines []string `json:"faultymachines" description:"servers with issues with this size"` + Other int `json:"other" description:"servers neither free, allocated or faulty with this size"` + OtherMachines []string `json:"othermachines" description:"servers neither free, allocated or faulty with this size"` } func NewPartitionResponse(p *metal.Partition) *PartitionResponse { diff --git a/spec/metal-api.json b/spec/metal-api.json index 02fe540d2..6310ee36b 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -4262,8 +4262,8 @@ "format": "int32", "type": "integer" }, - "unusedreservations": { - "description": "the amount of unused reservations for this size", + "usedreservations": { + "description": "the amount of used reservations for this size", "format": "int32", "type": "integer" } @@ -4278,7 +4278,7 @@ "reservations", "size", "total", - "unusedreservations" + "usedreservations" ] }, "v1.SizeConstraint": { From f007dab804cc4294a6288e43312c6239b7f0f951 Mon Sep 17 00:00:00 2001 From: Gerrit91 Date: Mon, 8 Jan 2024 12:49:51 +0100 Subject: [PATCH 11/23] Only count up reservations once. --- .../internal/service/partition-service.go | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/cmd/metal-api/internal/service/partition-service.go b/cmd/metal-api/internal/service/partition-service.go index b5fcfbe64..5c8f3b3e0 100644 --- a/cmd/metal-api/internal/service/partition-service.go +++ b/cmd/metal-api/internal/service/partition-service.go @@ -428,15 +428,6 @@ func (r *partitionResource) calcPartitionCapacity(pcr *v1.PartitionCapacityReque cap.Total++ - if size.Reservations != nil { - for _, reservation := range size.Reservations.ForPartition(p.ID) { - reservation := reservation - - cap.Reservations += reservation.Amount - cap.UsedReservations += len(machinesByProject[reservation.ProjectID]) - } - } - if m.Allocation != nil { cap.Allocated++ continue @@ -460,6 +451,20 @@ func (r *partitionResource) calcPartitionCapacity(pcr *v1.PartitionCapacityReque res := []v1.PartitionCapacity{} for _, pc := range pcs { pc := pc + + for _, cap := range pc.ServerCapacities { + size := sizesByID[cap.Size] + + if size.Reservations != nil { + for _, reservation := range size.Reservations.ForPartition(pc.ID) { + reservation := reservation + + cap.Reservations += reservation.Amount + cap.UsedReservations += len(machinesByProject[reservation.ProjectID]) + } + } + } + res = append(res, *pc) } From 06249b7876160beb911ad7eb8a048c3aaeac96d5 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Mon, 8 Jan 2024 13:00:15 +0100 Subject: [PATCH 12/23] Don't let reservation amount exceed. --- cmd/metal-api/internal/service/partition-service.go | 4 +++- cmd/metal-api/internal/service/partition-service_test.go | 2 +- cmd/metal-api/internal/testdata/testdata.go | 7 +++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/cmd/metal-api/internal/service/partition-service.go b/cmd/metal-api/internal/service/partition-service.go index 5c8f3b3e0..5fd1c7398 100644 --- a/cmd/metal-api/internal/service/partition-service.go +++ b/cmd/metal-api/internal/service/partition-service.go @@ -453,6 +453,8 @@ func (r *partitionResource) calcPartitionCapacity(pcr *v1.PartitionCapacityReque pc := pc for _, cap := range pc.ServerCapacities { + cap := cap + size := sizesByID[cap.Size] if size.Reservations != nil { @@ -460,7 +462,7 @@ func (r *partitionResource) calcPartitionCapacity(pcr *v1.PartitionCapacityReque reservation := reservation cap.Reservations += reservation.Amount - cap.UsedReservations += len(machinesByProject[reservation.ProjectID]) + cap.UsedReservations += min(len(machinesByProject[reservation.ProjectID]), reservation.Amount) } } } diff --git a/cmd/metal-api/internal/service/partition-service_test.go b/cmd/metal-api/internal/service/partition-service_test.go index 2ac060d60..17d40a29e 100644 --- a/cmd/metal-api/internal/service/partition-service_test.go +++ b/cmd/metal-api/internal/service/partition-service_test.go @@ -288,6 +288,6 @@ func TestPartitionCapacity(t *testing.T) { require.Equal(t, "1", c.Size) require.Equal(t, 5, c.Total) require.Equal(t, 0, c.Free) - require.Equal(t, 0, c.Reservations) + require.Equal(t, 3, c.Reservations) require.Equal(t, 0, c.UsedReservations) } diff --git a/cmd/metal-api/internal/testdata/testdata.go b/cmd/metal-api/internal/testdata/testdata.go index 3ca14f7d0..377bbf881 100644 --- a/cmd/metal-api/internal/testdata/testdata.go +++ b/cmd/metal-api/internal/testdata/testdata.go @@ -194,6 +194,13 @@ var ( Max: 100, }, }, + Reservations: metal.Reservations{ + { + Amount: 3, + PartitionIDs: []string{Partition1.ID}, + ProjectID: "a", + }, + }, } Sz2 = metal.Size{ Base: metal.Base{ From ad94a14de8e26b425241f2d7fd1fee5fec261c56 Mon Sep 17 00:00:00 2001 From: Gerrit91 Date: Mon, 8 Jan 2024 15:38:51 +0100 Subject: [PATCH 13/23] More validations. --- cmd/metal-api/internal/metal/size.go | 25 +++++++- cmd/metal-api/internal/metal/size_test.go | 4 +- .../internal/service/integration_test.go | 2 +- .../internal/service/project-service.go | 20 +++++++ .../internal/service/project-service_test.go | 1 + .../internal/service/size-service.go | 57 ++++++++++++++++++- .../internal/service/size-service_test.go | 38 ++++++++++--- cmd/metal-api/main.go | 2 +- 8 files changed, 132 insertions(+), 17 deletions(-) diff --git a/cmd/metal-api/internal/metal/size.go b/cmd/metal-api/internal/metal/size.go index 5c0fb6aee..62b4a194d 100644 --- a/cmd/metal-api/internal/metal/size.go +++ b/cmd/metal-api/internal/metal/size.go @@ -3,6 +3,8 @@ package metal import ( "fmt" "slices" + + mdmv1 "github.com/metal-stack/masterdata-api/api/v1" ) // A Size represents a supported machine size. @@ -133,7 +135,7 @@ func (s *Size) overlaps(so *Size) bool { } // Validate a size, returns error if a invalid size is passed -func (s *Size) Validate() error { +func (s *Size) Validate(partitions PartitionMap, projects map[string]*mdmv1.Project) error { 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) @@ -141,7 +143,7 @@ func (s *Size) Validate() error { } if s.Reservations != nil { - if err := s.Reservations.Validate(); err != nil { + if err := s.Reservations.Validate(partitions, projects); err != nil { return fmt.Errorf("invalid size reservation: %w", err) } } @@ -171,7 +173,18 @@ func (rs Reservations) ForPartition(partitionID string) Reservations { return result } -func (rs Reservations) Validate() error { +func (rs Reservations) ForProject(projectID string) Reservations { + var result Reservations + for _, r := range rs { + r := r + if r.ProjectID == projectID { + result = append(result, r) + } + } + return result +} + +func (rs Reservations) Validate(partitions PartitionMap, projects map[string]*mdmv1.Project) error { for _, r := range rs { if r.Amount <= 0 { return fmt.Errorf("amount must be a positive integer") @@ -183,6 +196,9 @@ func (rs Reservations) Validate() error { ids := map[string]bool{} for _, partition := range r.PartitionIDs { ids[partition] = true + if _, ok := partitions[partition]; !ok { + return fmt.Errorf("partition must exist before creating a size reservation") + } } if len(ids) != len(r.PartitionIDs) { return fmt.Errorf("partitions must not contain duplicates") @@ -191,6 +207,9 @@ func (rs Reservations) Validate() error { if r.ProjectID == "" { return fmt.Errorf("project id must be specified") } + if _, ok := projects[r.ProjectID]; !ok { + return fmt.Errorf("project must exist before creating a size reservation") + } } return nil diff --git a/cmd/metal-api/internal/metal/size_test.go b/cmd/metal-api/internal/metal/size_test.go index 72303d571..b11e621f8 100644 --- a/cmd/metal-api/internal/metal/size_test.go +++ b/cmd/metal-api/internal/metal/size_test.go @@ -541,7 +541,7 @@ func TestSizes_Overlaps(t *testing.T) { for i := range tests { tt := tests[i] t.Run(tt.name, func(t *testing.T) { - err := tt.sz.Validate() + err := tt.sz.Validate(nil, nil) require.NoError(t, err) got := tt.sz.Overlaps(&tt.args.sizes) if !reflect.DeepEqual(got, tt.want) { @@ -608,7 +608,7 @@ func TestSize_Validate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := tt.size.Validate() + err := tt.size.Validate(nil, nil) if err != nil { require.EqualError(t, err, *tt.wantErrMessage) } diff --git a/cmd/metal-api/internal/service/integration_test.go b/cmd/metal-api/internal/service/integration_test.go index 772c4c864..65bacdf43 100644 --- a/cmd/metal-api/internal/service/integration_test.go +++ b/cmd/metal-api/internal/service/integration_test.go @@ -104,7 +104,7 @@ func createTestEnvironment(t *testing.T) testEnv { require.NoError(t, err) imageService := NewImage(log, ds) switchService := NewSwitch(log, ds) - sizeService := NewSize(log, ds) + sizeService := NewSize(log, ds, mdc) sizeImageConstraintService := NewSizeImageConstraint(log, ds) networkService := NewNetwork(log, ds, ipamer, mdc) partitionService := NewPartition(log, ds, &emptyPublisher{}) diff --git a/cmd/metal-api/internal/service/project-service.go b/cmd/metal-api/internal/service/project-service.go index fa19a736b..74488844c 100644 --- a/cmd/metal-api/internal/service/project-service.go +++ b/cmd/metal-api/internal/service/project-service.go @@ -245,6 +245,26 @@ func (r *projectResource) deleteProject(request *restful.Request, response *rest return } + sizes, err := r.ds.ListSizes() + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } + + var sizeReservations metal.Reservations + for _, size := range sizes { + size := size + + if size.Reservations != nil { + sizeReservations = size.Reservations.ForProject(id) + } + } + + if len(sizeReservations) > 0 { + r.sendError(request, response, httperrors.BadRequest(errors.New("there are still size reservations made by this project"))) + return + } + pdr := &mdmv1.ProjectDeleteRequest{ Id: p.Project.Meta.Id, } diff --git a/cmd/metal-api/internal/service/project-service_test.go b/cmd/metal-api/internal/service/project-service_test.go index 7a4a7fe30..96d72cb9c 100644 --- a/cmd/metal-api/internal/service/project-service_test.go +++ b/cmd/metal-api/internal/service/project-service_test.go @@ -244,6 +244,7 @@ func Test_projectResource_deleteProject(t *testing.T) { mock.On(r.DB("mockdb").Table("machine").Filter(r.MockAnything(), r.FilterOpts{})).Return([]metal.Machines{}, nil) mock.On(r.DB("mockdb").Table("network").Filter(r.MockAnything(), r.FilterOpts{})).Return([]metal.Networks{}, nil) mock.On(r.DB("mockdb").Table("ip").Filter(r.MockAnything(), r.FilterOpts{})).Return([]metal.IPs{}, nil) + mock.On(r.DB("mockdb").Table("size")).Return([]metal.Size{}, nil) }, want: &v1.ProjectResponse{}, wantStatus: 200, diff --git a/cmd/metal-api/internal/service/size-service.go b/cmd/metal-api/internal/service/size-service.go index 46572b9ea..cfc461e35 100644 --- a/cmd/metal-api/internal/service/size-service.go +++ b/cmd/metal-api/internal/service/size-service.go @@ -1,10 +1,13 @@ package service import ( + "context" "errors" "fmt" "net/http" + mdmv1 "github.com/metal-stack/masterdata-api/api/v1" + mdm "github.com/metal-stack/masterdata-api/pkg/client" "github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore" "github.com/metal-stack/metal-api/cmd/metal-api/internal/metal" v1 "github.com/metal-stack/metal-api/cmd/metal-api/internal/service/v1" @@ -18,15 +21,17 @@ import ( type sizeResource struct { webResource + mdc mdm.Client } // NewSize returns a webservice for size specific endpoints. -func NewSize(log *zap.SugaredLogger, ds *datastore.RethinkStore) *restful.WebService { +func NewSize(log *zap.SugaredLogger, ds *datastore.RethinkStore, mdc mdm.Client) *restful.WebService { r := sizeResource{ webResource: webResource{ log: log, ds: ds, }, + mdc: mdc, } return r.webService() } @@ -244,7 +249,30 @@ func (r *sizeResource) createSize(request *restful.Request, response *restful.Re return } - err = s.Validate() + ps, err := r.ds.ListPartitions() + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } + + projects, err := r.mdc.Project().Find(context.Background(), &mdmv1.ProjectFindRequest{}) + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } + + projectMap := map[string]*mdmv1.Project{} + for _, p := range projects.GetProjects() { + p := p + + if p.Meta == nil { + continue + } + + projectMap[p.GetMeta().GetId()] = p + } + + err = s.Validate(ps.ByID(), projectMap) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -339,7 +367,30 @@ func (r *sizeResource) updateSize(request *restful.Request, response *restful.Re return } - err = newSize.Validate() + ps, err := r.ds.ListPartitions() + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } + + projects, err := r.mdc.Project().Find(context.Background(), &mdmv1.ProjectFindRequest{}) + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } + + projectMap := map[string]*mdmv1.Project{} + for _, p := range projects.GetProjects() { + p := p + + if p.Meta == nil { + continue + } + + projectMap[p.GetMeta().GetId()] = p + } + + err = newSize.Validate(ps.ByID(), projectMap) if err != nil { r.sendError(request, response, defaultError(err)) return diff --git a/cmd/metal-api/internal/service/size-service_test.go b/cmd/metal-api/internal/service/size-service_test.go index 9e0a02345..853cb94ac 100644 --- a/cmd/metal-api/internal/service/size-service_test.go +++ b/cmd/metal-api/internal/service/size-service_test.go @@ -2,11 +2,15 @@ package service import ( "bytes" + "context" "encoding/json" "net/http" "net/http/httptest" "testing" + mdmv1 "github.com/metal-stack/masterdata-api/api/v1" + mdmv1mock "github.com/metal-stack/masterdata-api/api/v1/mocks" + mdm "github.com/metal-stack/masterdata-api/pkg/client" "github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore" "github.com/metal-stack/metal-api/cmd/metal-api/internal/metal" v1 "github.com/metal-stack/metal-api/cmd/metal-api/internal/service/v1" @@ -23,7 +27,7 @@ func TestGetSizes(t *testing.T) { ds, mock := datastore.InitMockDB(t) testdata.InitMockDBData(mock) - sizeservice := NewSize(zaptest.NewLogger(t).Sugar(), ds) + sizeservice := NewSize(zaptest.NewLogger(t).Sugar(), ds, nil) container := restful.NewContainer().Add(sizeservice) req := httptest.NewRequest("GET", "/v1/size", nil) w := httptest.NewRecorder() @@ -52,7 +56,7 @@ func TestGetSize(t *testing.T) { ds, mock := datastore.InitMockDB(t) testdata.InitMockDBData(mock) - sizeservice := NewSize(zaptest.NewLogger(t).Sugar(), ds) + sizeservice := NewSize(zaptest.NewLogger(t).Sugar(), ds, nil) container := restful.NewContainer().Add(sizeservice) req := httptest.NewRequest("GET", "/v1/size/1", nil) w := httptest.NewRecorder() @@ -82,7 +86,7 @@ func TestSuggest(t *testing.T) { require.NoError(t, err) body := bytes.NewBuffer(js) - sizeservice := NewSize(zaptest.NewLogger(t).Sugar(), ds) + sizeservice := NewSize(zaptest.NewLogger(t).Sugar(), ds, nil) container := restful.NewContainer().Add(sizeservice) req := httptest.NewRequest("POST", "/v1/size/suggest", body) req.Header.Add("Content-Type", "application/json") @@ -122,7 +126,7 @@ func TestGetSizeNotFound(t *testing.T) { ds, mock := datastore.InitMockDB(t) testdata.InitMockDBData(mock) - sizeservice := NewSize(zaptest.NewLogger(t).Sugar(), ds) + sizeservice := NewSize(zaptest.NewLogger(t).Sugar(), ds, nil) container := restful.NewContainer().Add(sizeservice) req := httptest.NewRequest("GET", "/v1/size/999", nil) w := httptest.NewRecorder() @@ -144,7 +148,7 @@ func TestDeleteSize(t *testing.T) { testdata.InitMockDBData(mock) log := zaptest.NewLogger(t).Sugar() - sizeservice := NewSize(log, ds) + sizeservice := NewSize(log, ds, nil) container := restful.NewContainer().Add(sizeservice) req := httptest.NewRequest("DELETE", "/v1/size/1", nil) container = injectAdmin(log, container, req) @@ -168,7 +172,13 @@ func TestCreateSize(t *testing.T) { testdata.InitMockDBData(mock) log := zaptest.NewLogger(t).Sugar() - sizeservice := NewSize(log, ds) + psc := &mdmv1mock.ProjectServiceClient{} + psc.On("Find", context.Background(), &mdmv1.ProjectFindRequest{}).Return(&mdmv1.ProjectListResponse{Projects: []*mdmv1.Project{ + {Meta: &mdmv1.Meta{Id: "a"}}, + }}, nil) + mdc := mdm.NewMock(psc, &mdmv1mock.TenantServiceClient{}) + + sizeservice := NewSize(log, ds, mdc) container := restful.NewContainer().Add(sizeservice) createRequest := v1.SizeCreateRequest{ @@ -193,6 +203,14 @@ func TestCreateSize(t *testing.T) { Max: 100, }, }, + SizeReservations: []v1.SizeReservation{ + { + Amount: 3, + ProjectID: "a", + PartitionIDs: []string{testdata.Partition1.ID}, + Description: "test", + }, + }, } js, err := json.Marshal(createRequest) require.NoError(t, err) @@ -220,7 +238,13 @@ func TestUpdateSize(t *testing.T) { testdata.InitMockDBData(mock) log := zaptest.NewLogger(t).Sugar() - sizeservice := NewSize(log, ds) + psc := &mdmv1mock.ProjectServiceClient{} + psc.On("Find", context.Background(), &mdmv1.ProjectFindRequest{}).Return(&mdmv1.ProjectListResponse{Projects: []*mdmv1.Project{ + {Meta: &mdmv1.Meta{Id: "a"}}, + }}, nil) + mdc := mdm.NewMock(psc, &mdmv1mock.TenantServiceClient{}) + + sizeservice := NewSize(log, ds, mdc) container := restful.NewContainer().Add(sizeservice) minCores := uint64(8) diff --git a/cmd/metal-api/main.go b/cmd/metal-api/main.go index 92a9c455a..f3780c22b 100644 --- a/cmd/metal-api/main.go +++ b/cmd/metal-api/main.go @@ -752,7 +752,7 @@ func initRestServices(audit auditing.Auditing, withauth bool, ipmiSuperUser meta restful.DefaultContainer.Add(service.NewAudit(logger.Named("audit-service"), audit)) restful.DefaultContainer.Add(service.NewPartition(logger.Named("partition-service"), ds, nsqer)) restful.DefaultContainer.Add(service.NewImage(logger.Named("image-service"), ds)) - restful.DefaultContainer.Add(service.NewSize(logger.Named("size-service"), ds)) + restful.DefaultContainer.Add(service.NewSize(logger.Named("size-service"), ds, mdc)) restful.DefaultContainer.Add(service.NewSizeImageConstraint(logger.Named("size-image-constraint-service"), ds)) restful.DefaultContainer.Add(service.NewNetwork(logger.Named("network-service"), ds, ipamer, mdc)) restful.DefaultContainer.Add(ipService) From d4b2aa48b3211f1a2b223aaf6d71c8f6639888d9 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Mon, 8 Jan 2024 15:58:36 +0100 Subject: [PATCH 14/23] Return size reservation list server-side. --- .../internal/service/project-service.go | 16 ++++ .../internal/service/size-service.go | 88 ++++++++++++++----- cmd/metal-api/internal/service/v1/size.go | 11 +++ spec/metal-api.json | 81 +++++++++++++++++ 4 files changed, 172 insertions(+), 24 deletions(-) diff --git a/cmd/metal-api/internal/service/project-service.go b/cmd/metal-api/internal/service/project-service.go index 74488844c..36edec520 100644 --- a/cmd/metal-api/internal/service/project-service.go +++ b/cmd/metal-api/internal/service/project-service.go @@ -357,3 +357,19 @@ func (r *projectResource) setProjectQuota(project *mdmv1.Project) (*v1.Project, return p, nil } + +func projectsByID(projects []*mdmv1.Project) map[string]*mdmv1.Project { + result := map[string]*mdmv1.Project{} + + for _, p := range projects { + p := p + + if p.Meta == nil { + continue + } + + result[p.GetMeta().GetId()] = p + } + + return result +} diff --git a/cmd/metal-api/internal/service/size-service.go b/cmd/metal-api/internal/service/size-service.go index cfc461e35..0a53b6bc9 100644 --- a/cmd/metal-api/internal/service/size-service.go +++ b/cmd/metal-api/internal/service/size-service.go @@ -12,6 +12,7 @@ import ( "github.com/metal-stack/metal-api/cmd/metal-api/internal/metal" v1 "github.com/metal-stack/metal-api/cmd/metal-api/internal/service/v1" "github.com/metal-stack/metal-lib/auditing" + "github.com/metal-stack/metal-lib/pkg/pointer" "go.uber.org/zap" restfulspec "github.com/emicklei/go-restful-openapi/v2" @@ -64,6 +65,15 @@ func (r *sizeResource) webService() *restful.WebService { Returns(http.StatusOK, "OK", []v1.SizeResponse{}). DefaultReturns("Error", httperrors.HTTPErrorResponse{})) + ws.Route(ws.GET("/reservations"). + To(r.listSizeReservations). + Operation("listSizeReservations"). + Doc("get all size reservations"). + Metadata(restfulspec.KeyOpenAPITags, tags). + Writes([]v1.SizeReservationResponse{}). + Returns(http.StatusOK, "OK", []v1.SizeReservationResponse{}). + DefaultReturns("Error", httperrors.HTTPErrorResponse{})) + ws.Route(ws.POST("/suggest"). To(r.suggestSize). Operation("suggest"). @@ -261,18 +271,7 @@ func (r *sizeResource) createSize(request *restful.Request, response *restful.Re return } - projectMap := map[string]*mdmv1.Project{} - for _, p := range projects.GetProjects() { - p := p - - if p.Meta == nil { - continue - } - - projectMap[p.GetMeta().GetId()] = p - } - - err = s.Validate(ps.ByID(), projectMap) + err = s.Validate(ps.ByID(), projectsByID(projects.Projects)) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -379,18 +378,7 @@ func (r *sizeResource) updateSize(request *restful.Request, response *restful.Re return } - projectMap := map[string]*mdmv1.Project{} - for _, p := range projects.GetProjects() { - p := p - - if p.Meta == nil { - continue - } - - projectMap[p.GetMeta().GetId()] = p - } - - err = newSize.Validate(ps.ByID(), projectMap) + err = newSize.Validate(ps.ByID(), projectsByID(projects.Projects)) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -432,3 +420,55 @@ func (r *sizeResource) fromHardware(request *restful.Request, response *restful. r.send(request, response, http.StatusOK, v1.NewSizeMatchingLog(lg[0])) } + +func (r *sizeResource) listSizeReservations(request *restful.Request, response *restful.Response) { + ss, err := r.ds.ListSizes() + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } + + projects, err := r.mdc.Project().Find(context.Background(), &mdmv1.ProjectFindRequest{}) + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } + + ms, err := r.ds.ListMachines() + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } + + var ( + result []*v1.SizeReservationResponse + projectsByID = projectsByID(projects.Projects) + machinesByProjectID = ms.ByProjectID() + ) + + for _, size := range ss { + size := size + + for _, reservation := range size.Reservations { + reservation := reservation + + for _, partitionID := range reservation.PartitionIDs { + project := pointer.SafeDeref(projectsByID[reservation.ProjectID]) + allocations := len(machinesByProjectID[reservation.ProjectID].WithSize(size.ID)) + + result = append(result, &v1.SizeReservationResponse{ + SizeID: size.ID, + PartitionID: partitionID, + Tenant: project.TenantId, + ProjectID: reservation.ProjectID, + ProjectName: project.Name, + Reservations: reservation.Amount, + UsedReservations: min(reservation.Amount, allocations), + ProjectAllocations: allocations, + }) + } + } + } + + r.send(request, response, http.StatusOK, result) +} diff --git a/cmd/metal-api/internal/service/v1/size.go b/cmd/metal-api/internal/service/v1/size.go index 2b9ea953a..6aa6f484e 100644 --- a/cmd/metal-api/internal/service/v1/size.go +++ b/cmd/metal-api/internal/service/v1/size.go @@ -39,6 +39,17 @@ type SizeResponse struct { Timestamps } +type SizeReservationResponse struct { + SizeID string `json:"sizeid" description:"the size id of this size reservation"` + PartitionID string `json:"partitionid" description:"the partition id of this size reservation"` + Tenant string `json:"tenant" description:"the tenant of this size reservation"` + ProjectID string `json:"projectid" description:"the project id of this size reservation"` + ProjectName string `json:"projectname" description:"the project name of this size reservation"` + Reservations int `json:"reservations" description:"the amount of reservations of this size reservation"` + UsedReservations int `json:"usedreservations" description:"the used amount of reservations of this size reservation"` + ProjectAllocations int `json:"projectallocations" description:"the amount of allocations of this project referenced by this size reservation"` +} + type SizeSuggestRequest struct { MachineID string `json:"machineID" description:"machineID to retrieve size suggestion for"` } diff --git a/spec/metal-api.json b/spec/metal-api.json index 6310ee36b..bf7844f20 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -4528,6 +4528,55 @@ "projectid" ] }, + "v1.SizeReservationResponse": { + "properties": { + "partitionid": { + "description": "the partition id of this size reservation", + "type": "string" + }, + "projectallocations": { + "description": "the amount of allocations of this project referenced by this size reservation", + "format": "int32", + "type": "integer" + }, + "projectid": { + "description": "the project id of this size reservation", + "type": "string" + }, + "projectname": { + "description": "the project name of this size reservation", + "type": "string" + }, + "reservations": { + "description": "the amount of reservations of this size reservation", + "format": "int32", + "type": "integer" + }, + "sizeid": { + "description": "the size id of this size reservation", + "type": "string" + }, + "tenant": { + "description": "the tenant of this size reservation", + "type": "string" + }, + "usedreservations": { + "description": "the used amount of reservations of this size reservation", + "format": "int32", + "type": "integer" + } + }, + "required": [ + "partitionid", + "projectallocations", + "projectid", + "projectname", + "reservations", + "sizeid", + "tenant", + "usedreservations" + ] + }, "v1.SizeResponse": { "properties": { "changed": { @@ -8884,6 +8933,38 @@ ] } }, + "/v1/size/reservations": { + "get": { + "consumes": [ + "application/json" + ], + "operationId": "listSizeReservations", + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "items": { + "$ref": "#/definitions/v1.SizeReservationResponse" + }, + "type": "array" + } + }, + "default": { + "description": "Error", + "schema": { + "$ref": "#/definitions/httperrors.HTTPErrorResponse" + } + } + }, + "summary": "get all size reservations", + "tags": [ + "size" + ] + } + }, "/v1/size/suggest": { "post": { "consumes": [ From cad4b8c541a56c2ec9993e742ce2349f47b5dd0c Mon Sep 17 00:00:00 2001 From: Gerrit91 Date: Mon, 8 Jan 2024 16:07:14 +0100 Subject: [PATCH 15/23] Fix integration test. --- cmd/metal-api/internal/service/integration_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/metal-api/internal/service/integration_test.go b/cmd/metal-api/internal/service/integration_test.go index 65bacdf43..5889e4c0a 100644 --- a/cmd/metal-api/internal/service/integration_test.go +++ b/cmd/metal-api/internal/service/integration_test.go @@ -80,6 +80,9 @@ func createTestEnvironment(t *testing.T) testEnv { Id: "test-project-1", }, }}, nil) + psc.On("Find", context.Background(), &mdmv1.ProjectFindRequest{}).Return(&mdmv1.ProjectListResponse{Projects: []*mdmv1.Project{ + {Meta: &mdmv1.Meta{Id: "test-project-1"}}, + }}, nil) mdc := mdm.NewMock(psc, nil) log := zaptest.NewLogger(t).Sugar() From adc39d511ebf9bbfec9709e483c252603d3c464a Mon Sep 17 00:00:00 2001 From: Gerrit91 Date: Mon, 8 Jan 2024 16:29:22 +0100 Subject: [PATCH 16/23] Minor optimizations. --- cmd/metal-api/internal/metal/size.go | 32 +++++++++++++------ .../internal/service/partition-service.go | 10 +++--- .../internal/service/project-service.go | 4 +-- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/cmd/metal-api/internal/metal/size.go b/cmd/metal-api/internal/metal/size.go index 62b4a194d..8d0f3852a 100644 --- a/cmd/metal-api/internal/metal/size.go +++ b/cmd/metal-api/internal/metal/size.go @@ -142,10 +142,8 @@ func (s *Size) Validate(partitions PartitionMap, projects map[string]*mdmv1.Proj } } - if s.Reservations != nil { - if err := s.Reservations.Validate(partitions, projects); err != nil { - return fmt.Errorf("invalid size reservation: %w", err) - } + if err := s.Reservations.Validate(partitions, projects); err != nil { + return fmt.Errorf("invalid size reservation: %w", err) } return nil @@ -162,30 +160,44 @@ func (s *Size) Overlaps(ss *Sizes) *Size { return nil } -func (rs Reservations) ForPartition(partitionID string) Reservations { +func (rs *Reservations) ForPartition(partitionID string) Reservations { + if rs == nil { + return nil + } + var result Reservations - for _, r := range rs { + for _, r := range *rs { r := r if slices.Contains(r.PartitionIDs, partitionID) { result = append(result, r) } } + return result } -func (rs Reservations) ForProject(projectID string) Reservations { +func (rs *Reservations) ForProject(projectID string) Reservations { + if rs == nil { + return nil + } + var result Reservations - for _, r := range rs { + for _, r := range *rs { r := r if r.ProjectID == projectID { result = append(result, r) } } + return result } -func (rs Reservations) Validate(partitions PartitionMap, projects map[string]*mdmv1.Project) error { - for _, r := range rs { +func (rs *Reservations) Validate(partitions PartitionMap, projects map[string]*mdmv1.Project) error { + if rs == nil { + return nil + } + + for _, r := range *rs { if r.Amount <= 0 { return fmt.Errorf("amount must be a positive integer") } diff --git a/cmd/metal-api/internal/service/partition-service.go b/cmd/metal-api/internal/service/partition-service.go index 5fd1c7398..615dc95b8 100644 --- a/cmd/metal-api/internal/service/partition-service.go +++ b/cmd/metal-api/internal/service/partition-service.go @@ -457,13 +457,11 @@ func (r *partitionResource) calcPartitionCapacity(pcr *v1.PartitionCapacityReque size := sizesByID[cap.Size] - if size.Reservations != nil { - for _, reservation := range size.Reservations.ForPartition(pc.ID) { - reservation := reservation + for _, reservation := range size.Reservations.ForPartition(pc.ID) { + reservation := reservation - cap.Reservations += reservation.Amount - cap.UsedReservations += min(len(machinesByProject[reservation.ProjectID]), reservation.Amount) - } + cap.Reservations += reservation.Amount + cap.UsedReservations += min(len(machinesByProject[reservation.ProjectID]), reservation.Amount) } } diff --git a/cmd/metal-api/internal/service/project-service.go b/cmd/metal-api/internal/service/project-service.go index 36edec520..8116d7a88 100644 --- a/cmd/metal-api/internal/service/project-service.go +++ b/cmd/metal-api/internal/service/project-service.go @@ -255,9 +255,7 @@ func (r *projectResource) deleteProject(request *restful.Request, response *rest for _, size := range sizes { size := size - if size.Reservations != nil { - sizeReservations = size.Reservations.ForProject(id) - } + sizeReservations = size.Reservations.ForProject(id) } if len(sizeReservations) > 0 { From 279ff260a85bcf3bbda0708abdab20acb3731078 Mon Sep 17 00:00:00 2001 From: Gerrit91 Date: Tue, 9 Jan 2024 09:22:53 +0100 Subject: [PATCH 17/23] Some tests. --- cmd/metal-api/internal/metal/size_test.go | 273 ++++++++++++++++++++++ 1 file changed, 273 insertions(+) diff --git a/cmd/metal-api/internal/metal/size_test.go b/cmd/metal-api/internal/metal/size_test.go index b11e621f8..113799420 100644 --- a/cmd/metal-api/internal/metal/size_test.go +++ b/cmd/metal-api/internal/metal/size_test.go @@ -1,10 +1,14 @@ package metal import ( + "fmt" "reflect" "testing" + "github.com/google/go-cmp/cmp" + mdmv1 "github.com/metal-stack/masterdata-api/api/v1" "github.com/metal-stack/metal-lib/pkg/pointer" + "github.com/metal-stack/metal-lib/pkg/testcommon" "github.com/stretchr/testify/require" ) @@ -618,3 +622,272 @@ func TestSize_Validate(t *testing.T) { }) } } + +func TestReservations_ForPartition(t *testing.T) { + tests := []struct { + name string + rs *Reservations + partitionID string + want Reservations + }{ + { + name: "nil", + rs: nil, + partitionID: "a", + want: nil, + }, + { + name: "correctly filtered", + rs: &Reservations{ + { + PartitionIDs: []string{"a", "b"}, + }, + { + PartitionIDs: []string{"c"}, + }, + { + PartitionIDs: []string{"a"}, + }, + }, + partitionID: "a", + want: Reservations{ + { + PartitionIDs: []string{"a", "b"}, + }, + { + PartitionIDs: []string{"a"}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.rs.ForPartition(tt.partitionID); !reflect.DeepEqual(got, tt.want) { + t.Errorf("Reservations.ForPartition() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestReservations_ForProject(t *testing.T) { + tests := []struct { + name string + rs *Reservations + projectID string + want Reservations + }{ + { + name: "nil", + rs: nil, + projectID: "a", + want: nil, + }, + { + name: "correctly filtered", + rs: &Reservations{ + { + ProjectID: "a", + }, + { + ProjectID: "c", + }, + { + ProjectID: "a", + }, + }, + projectID: "a", + want: Reservations{ + { + ProjectID: "a", + }, + { + ProjectID: "a", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.rs.ForProject(tt.projectID); !reflect.DeepEqual(got, tt.want) { + t.Errorf("Reservations.ForProject() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestReservations_Validate(t *testing.T) { + tests := []struct { + name string + partitions PartitionMap + projects map[string]*mdmv1.Project + rs *Reservations + wantErr error + }{ + { + name: "empty reservations", + partitions: nil, + projects: nil, + rs: nil, + wantErr: nil, + }, + { + name: "invalid amount", + partitions: PartitionMap{ + "a": Partition{}, + "b": Partition{}, + "c": Partition{}, + }, + projects: map[string]*mdmv1.Project{ + "1": {}, + "2": {}, + "3": {}, + }, + rs: &Reservations{ + { + Amount: -3, + Description: "test", + ProjectID: "3", + PartitionIDs: []string{"b"}, + }, + }, + wantErr: fmt.Errorf("amount must be a positive integer"), + }, + { + name: "no partitions referenced", + partitions: PartitionMap{ + "a": Partition{}, + "b": Partition{}, + "c": Partition{}, + }, + projects: map[string]*mdmv1.Project{ + "1": {}, + "2": {}, + "3": {}, + }, + rs: &Reservations{ + { + Amount: 3, + Description: "test", + ProjectID: "3", + }, + }, + wantErr: fmt.Errorf("at least one partition id must be specified"), + }, + { + name: "partition does not exist", + partitions: PartitionMap{ + "a": Partition{}, + "b": Partition{}, + "c": Partition{}, + }, + projects: map[string]*mdmv1.Project{ + "1": {}, + "2": {}, + "3": {}, + }, + rs: &Reservations{ + { + Amount: 3, + Description: "test", + ProjectID: "3", + PartitionIDs: []string{"d"}, + }, + }, + wantErr: fmt.Errorf("partition must exist before creating a size reservation"), + }, + { + name: "partition duplicates", + partitions: PartitionMap{ + "a": Partition{}, + "b": Partition{}, + "c": Partition{}, + }, + projects: map[string]*mdmv1.Project{ + "1": {}, + "2": {}, + "3": {}, + }, + rs: &Reservations{ + { + Amount: 3, + Description: "test", + ProjectID: "3", + PartitionIDs: []string{"a", "b", "c", "b"}, + }, + }, + wantErr: fmt.Errorf("partitions must not contain duplicates"), + }, + { + name: "no project referenced", + partitions: PartitionMap{ + "a": Partition{}, + "b": Partition{}, + "c": Partition{}, + }, + projects: map[string]*mdmv1.Project{ + "1": {}, + "2": {}, + "3": {}, + }, + rs: &Reservations{ + { + Amount: 3, + Description: "test", + PartitionIDs: []string{"a"}, + }, + }, + wantErr: fmt.Errorf("project id must be specified"), + }, + { + name: "project does not exist", + partitions: PartitionMap{ + "a": Partition{}, + "b": Partition{}, + "c": Partition{}, + }, + projects: map[string]*mdmv1.Project{ + "1": {}, + "2": {}, + "3": {}, + }, + rs: &Reservations{ + { + Amount: 3, + Description: "test", + ProjectID: "4", + PartitionIDs: []string{"a"}, + }, + }, + wantErr: fmt.Errorf("project must exist before creating a size reservation"), + }, + { + name: "valid reservation", + partitions: PartitionMap{ + "a": Partition{}, + "b": Partition{}, + "c": Partition{}, + }, + projects: map[string]*mdmv1.Project{ + "1": {}, + "2": {}, + "3": {}, + }, + rs: &Reservations{ + { + Amount: 3, + Description: "test", + ProjectID: "2", + PartitionIDs: []string{"b", "c"}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.rs.Validate(tt.partitions, tt.projects) + if diff := cmp.Diff(tt.wantErr, err, testcommon.ErrorStringComparer()); diff != "" { + t.Errorf("error diff (-want +got):\n%s", diff) + } + }) + } +} From e9ed3f3294872dbadaec92b2765e96be710d52fc Mon Sep 17 00:00:00 2001 From: Gerrit91 Date: Tue, 9 Jan 2024 09:25:46 +0100 Subject: [PATCH 18/23] More tests. --- cmd/metal-api/internal/service/partition-service_test.go | 2 +- cmd/metal-api/internal/testdata/testdata.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/metal-api/internal/service/partition-service_test.go b/cmd/metal-api/internal/service/partition-service_test.go index 17d40a29e..30c722c59 100644 --- a/cmd/metal-api/internal/service/partition-service_test.go +++ b/cmd/metal-api/internal/service/partition-service_test.go @@ -289,5 +289,5 @@ func TestPartitionCapacity(t *testing.T) { require.Equal(t, 5, c.Total) require.Equal(t, 0, c.Free) require.Equal(t, 3, c.Reservations) - require.Equal(t, 0, c.UsedReservations) + require.Equal(t, 1, c.UsedReservations) } diff --git a/cmd/metal-api/internal/testdata/testdata.go b/cmd/metal-api/internal/testdata/testdata.go index 377bbf881..ae90776bc 100644 --- a/cmd/metal-api/internal/testdata/testdata.go +++ b/cmd/metal-api/internal/testdata/testdata.go @@ -198,7 +198,7 @@ var ( { Amount: 3, PartitionIDs: []string{Partition1.ID}, - ProjectID: "a", + ProjectID: "p1", }, }, } From 59575aa0d97b9afcfdb948389eb1948677e981da Mon Sep 17 00:00:00 2001 From: Gerrit Date: Tue, 9 Jan 2024 09:35:31 +0100 Subject: [PATCH 19/23] Use request context when talking to masterdata-api. --- cmd/metal-api/internal/metal/size.go | 3 +++ cmd/metal-api/internal/service/ip-service.go | 3 +-- .../internal/service/ip-service_test.go | 4 ++-- .../internal/service/network-service.go | 5 ++--- .../internal/service/project-service.go | 14 ++++++------- .../internal/service/project-service_test.go | 20 +++++++++---------- .../internal/service/size-service.go | 7 +++---- .../internal/service/size-service_test.go | 8 ++++---- 8 files changed, 32 insertions(+), 32 deletions(-) diff --git a/cmd/metal-api/internal/metal/size.go b/cmd/metal-api/internal/metal/size.go index 8d0f3852a..30d1aef7c 100644 --- a/cmd/metal-api/internal/metal/size.go +++ b/cmd/metal-api/internal/metal/size.go @@ -4,6 +4,7 @@ import ( "fmt" "slices" + "github.com/davecgh/go-spew/spew" mdmv1 "github.com/metal-stack/masterdata-api/api/v1" ) @@ -197,6 +198,8 @@ func (rs *Reservations) Validate(partitions PartitionMap, projects map[string]*m return nil } + spew.Dump(*rs) + for _, r := range *rs { if r.Amount <= 0 { return fmt.Errorf("amount must be a positive integer") diff --git a/cmd/metal-api/internal/service/ip-service.go b/cmd/metal-api/internal/service/ip-service.go index e74505ae7..cf7a504b1 100644 --- a/cmd/metal-api/internal/service/ip-service.go +++ b/cmd/metal-api/internal/service/ip-service.go @@ -1,7 +1,6 @@ package service import ( - "context" "errors" "fmt" "net/http" @@ -294,7 +293,7 @@ func (r *ipResource) allocateIP(request *restful.Request, response *restful.Resp return } - p, err := r.mdc.Project().Get(context.Background(), &mdmv1.ProjectGetRequest{Id: requestPayload.ProjectID}) + p, err := r.mdc.Project().Get(request.Request.Context(), &mdmv1.ProjectGetRequest{Id: requestPayload.ProjectID}) if err != nil { r.sendError(request, response, defaultError(err)) return diff --git a/cmd/metal-api/internal/service/ip-service_test.go b/cmd/metal-api/internal/service/ip-service_test.go index a1b2bb099..42f2d7ec7 100644 --- a/cmd/metal-api/internal/service/ip-service_test.go +++ b/cmd/metal-api/internal/service/ip-service_test.go @@ -2,7 +2,6 @@ package service import ( "bytes" - "context" "encoding/json" "errors" "net/http" @@ -27,6 +26,7 @@ import ( "github.com/google/go-cmp/cmp" goipam "github.com/metal-stack/go-ipam" + testifymock "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" restful "github.com/emicklei/go-restful/v3" @@ -174,7 +174,7 @@ func TestAllocateIP(t *testing.T) { logger := zaptest.NewLogger(t).Sugar() psc := mdmock.ProjectServiceClient{} - psc.On("Get", context.Background(), &mdmv1.ProjectGetRequest{Id: "123"}).Return(&mdmv1.ProjectResponse{ + psc.On("Get", testifymock.Anything, &mdmv1.ProjectGetRequest{Id: "123"}).Return(&mdmv1.ProjectResponse{ Project: &mdmv1.Project{ Meta: &mdmv1.Meta{Id: "project-1"}, }, diff --git a/cmd/metal-api/internal/service/network-service.go b/cmd/metal-api/internal/service/network-service.go index 727a83ee9..0872f05c7 100644 --- a/cmd/metal-api/internal/service/network-service.go +++ b/cmd/metal-api/internal/service/network-service.go @@ -1,7 +1,6 @@ package service import ( - "context" "errors" "fmt" "net/http" @@ -242,7 +241,7 @@ func (r *networkResource) createNetwork(request *restful.Request, response *rest nat := requestPayload.Nat if projectID != "" { - _, err = r.mdc.Project().Get(context.Background(), &mdmv1.ProjectGetRequest{Id: projectID}) + _, err = r.mdc.Project().Get(request.Request.Context(), &mdmv1.ProjectGetRequest{Id: projectID}) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -445,7 +444,7 @@ func (r *networkResource) allocateNetwork(request *restful.Request, response *re return } - project, err := r.mdc.Project().Get(context.Background(), &mdmv1.ProjectGetRequest{Id: projectID}) + project, err := r.mdc.Project().Get(request.Request.Context(), &mdmv1.ProjectGetRequest{Id: projectID}) if err != nil { r.sendError(request, response, defaultError(err)) return diff --git a/cmd/metal-api/internal/service/project-service.go b/cmd/metal-api/internal/service/project-service.go index 8116d7a88..f3725e064 100644 --- a/cmd/metal-api/internal/service/project-service.go +++ b/cmd/metal-api/internal/service/project-service.go @@ -112,7 +112,7 @@ func (r *projectResource) webService() *restful.WebService { func (r *projectResource) findProject(request *restful.Request, response *restful.Response) { id := request.PathParameter("id") - p, err := r.mdc.Project().Get(context.Background(), &mdmv1.ProjectGetRequest{Id: id}) + p, err := r.mdc.Project().Get(request.Request.Context(), &mdmv1.ProjectGetRequest{Id: id}) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -128,7 +128,7 @@ func (r *projectResource) findProject(request *restful.Request, response *restfu } func (r *projectResource) listProjects(request *restful.Request, response *restful.Response) { - res, err := r.mdc.Project().Find(context.Background(), &mdmv1.ProjectFindRequest{}) + res, err := r.mdc.Project().Find(request.Request.Context(), &mdmv1.ProjectFindRequest{}) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -151,7 +151,7 @@ func (r *projectResource) findProjects(request *restful.Request, response *restf return } - res, err := r.mdc.Project().Find(context.Background(), mapper.ToMdmV1ProjectFindRequest(&requestPayload)) + res, err := r.mdc.Project().Find(request.Request.Context(), mapper.ToMdmV1ProjectFindRequest(&requestPayload)) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -185,7 +185,7 @@ func (r *projectResource) createProject(request *restful.Request, response *rest Project: project, } - p, err := r.mdc.Project().Create(context.Background(), mdmv1pcr) + p, err := r.mdc.Project().Create(request.Request.Context(), mdmv1pcr) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -205,7 +205,7 @@ func (r *projectResource) deleteProject(request *restful.Request, response *rest pgr := &mdmv1.ProjectGetRequest{ Id: id, } - p, err := r.mdc.Project().Get(context.Background(), pgr) + p, err := r.mdc.Project().Get(request.Request.Context(), pgr) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -266,7 +266,7 @@ func (r *projectResource) deleteProject(request *restful.Request, response *rest pdr := &mdmv1.ProjectDeleteRequest{ Id: p.Project.Meta.Id, } - pdresponse, err := r.mdc.Project().Delete(context.Background(), pdr) + pdresponse, err := r.mdc.Project().Delete(request.Request.Context(), pdr) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -293,7 +293,7 @@ func (r *projectResource) updateProject(request *restful.Request, response *rest return } - existingProject, err := r.mdc.Project().Get(context.Background(), &mdmv1.ProjectGetRequest{Id: requestPayload.Project.Meta.Id}) + existingProject, err := r.mdc.Project().Get(request.Request.Context(), &mdmv1.ProjectGetRequest{Id: requestPayload.Project.Meta.Id}) if err != nil { r.sendError(request, response, defaultError(err)) return diff --git a/cmd/metal-api/internal/service/project-service_test.go b/cmd/metal-api/internal/service/project-service_test.go index 96d72cb9c..68b96344a 100644 --- a/cmd/metal-api/internal/service/project-service_test.go +++ b/cmd/metal-api/internal/service/project-service_test.go @@ -1,7 +1,6 @@ package service import ( - "context" "errors" "fmt" "testing" @@ -15,6 +14,7 @@ import ( "github.com/metal-stack/metal-api/cmd/metal-api/internal/metal" "github.com/metal-stack/metal-lib/httperrors" "github.com/metal-stack/security" + testifymock "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" r "gopkg.in/rethinkdb/rethinkdb-go.v6" @@ -100,7 +100,7 @@ func Test_projectResource_findProject(t *testing.T) { userScenarios: []security.User{*testViewUser}, id: "122", projectServiceMock: func(mock *mdmv1mock.ProjectServiceClient) { - mock.On("Get", context.Background(), &mdmv1.ProjectGetRequest{Id: "122"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) + mock.On("Get", testifymock.Anything, &mdmv1.ProjectGetRequest{Id: "122"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) }, wantStatus: 422, wantErr: httperrors.UnprocessableEntity(errors.New("project does not have a projectID")), @@ -109,7 +109,7 @@ func Test_projectResource_findProject(t *testing.T) { name: "entity allowed for user with admin privileges", userScenarios: []security.User{*testAdminUser}, projectServiceMock: func(mock *mdmv1mock.ProjectServiceClient) { - mock.On("Get", context.Background(), &mdmv1.ProjectGetRequest{Id: "123"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) + mock.On("Get", testifymock.Anything, &mdmv1.ProjectGetRequest{Id: "123"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) }, id: "123", wantStatus: 422, @@ -163,7 +163,7 @@ func Test_projectResource_createProject(t *testing.T) { userScenarios: []security.User{*testViewUser}, pcr: &mdmv1.ProjectCreateRequest{Project: &mdmv1.Project{}}, projectServiceMock: func(mock *mdmv1mock.ProjectServiceClient) { - mock.On("Create", context.Background(), &mdmv1.ProjectCreateRequest{Project: &mdmv1.Project{}}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) + mock.On("Create", testifymock.Anything, &mdmv1.ProjectCreateRequest{Project: &mdmv1.Project{}}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) }, wantStatus: 403, wantErr: httperrors.Forbidden(errors.New("you are not member in one of [k8s_kaas-admin maas-all-all-admin]")), @@ -173,7 +173,7 @@ func Test_projectResource_createProject(t *testing.T) { userScenarios: []security.User{*testAdminUser}, pcr: &mdmv1.ProjectCreateRequest{Project: &mdmv1.Project{}}, projectServiceMock: func(mock *mdmv1mock.ProjectServiceClient) { - mock.On("Create", context.Background(), &mdmv1.ProjectCreateRequest{Project: &mdmv1.Project{}}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) + mock.On("Create", testifymock.Anything, &mdmv1.ProjectCreateRequest{Project: &mdmv1.Project{}}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) }, wantStatus: 400, wantErr: httperrors.BadRequest(errors.New("no tenant given")), @@ -227,7 +227,7 @@ func Test_projectResource_deleteProject(t *testing.T) { userScenarios: []security.User{*testViewUser}, id: "122", projectServiceMock: func(mock *mdmv1mock.ProjectServiceClient) { - mock.On("Delete", context.Background(), &mdmv1.ProjectDeleteRequest{Id: "122"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) + mock.On("Delete", testifymock.Anything, &mdmv1.ProjectDeleteRequest{Id: "122"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) }, wantStatus: 403, wantErr: httperrors.Forbidden(errors.New("you are not member in one of [k8s_kaas-admin maas-all-all-admin]")), @@ -237,8 +237,8 @@ func Test_projectResource_deleteProject(t *testing.T) { userScenarios: []security.User{*testAdminUser}, id: "123", projectServiceMock: func(mock *mdmv1mock.ProjectServiceClient) { - mock.On("Get", context.Background(), &mdmv1.ProjectGetRequest{Id: "123"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{Meta: &mdmv1.Meta{Id: "123"}}}, nil) - mock.On("Delete", context.Background(), &mdmv1.ProjectDeleteRequest{Id: "123"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) + mock.On("Get", testifymock.Anything, &mdmv1.ProjectGetRequest{Id: "123"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{Meta: &mdmv1.Meta{Id: "123"}}}, nil) + mock.On("Delete", testifymock.Anything, &mdmv1.ProjectDeleteRequest{Id: "123"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) }, dsMock: func(mock *r.Mock) { mock.On(r.DB("mockdb").Table("machine").Filter(r.MockAnything(), r.FilterOpts{})).Return([]metal.Machines{}, nil) @@ -298,7 +298,7 @@ func Test_projectResource_updateProject(t *testing.T) { userScenarios: []security.User{*testViewUser}, pur: &mdmv1.ProjectUpdateRequest{Project: &mdmv1.Project{}}, projectServiceMock: func(mock *mdmv1mock.ProjectServiceClient) { - mock.On("Update", context.Background(), &mdmv1.ProjectUpdateRequest{Project: &mdmv1.Project{}}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) + mock.On("Update", testifymock.Anything, &mdmv1.ProjectUpdateRequest{Project: &mdmv1.Project{}}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) }, wantStatus: 403, wantErr: httperrors.Forbidden(errors.New("you are not member in one of [k8s_kaas-admin maas-all-all-admin]")), @@ -308,7 +308,7 @@ func Test_projectResource_updateProject(t *testing.T) { userScenarios: []security.User{*testAdminUser}, pur: &mdmv1.ProjectUpdateRequest{Project: &mdmv1.Project{}}, projectServiceMock: func(mock *mdmv1mock.ProjectServiceClient) { - mock.On("Update", context.Background(), &mdmv1.ProjectUpdateRequest{Project: &mdmv1.Project{}}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) + mock.On("Update", testifymock.Anything, &mdmv1.ProjectUpdateRequest{Project: &mdmv1.Project{}}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) }, wantStatus: 400, wantErr: httperrors.BadRequest(errors.New("project and project.meta must be specified")), diff --git a/cmd/metal-api/internal/service/size-service.go b/cmd/metal-api/internal/service/size-service.go index 0a53b6bc9..6fbe4c1fa 100644 --- a/cmd/metal-api/internal/service/size-service.go +++ b/cmd/metal-api/internal/service/size-service.go @@ -1,7 +1,6 @@ package service import ( - "context" "errors" "fmt" "net/http" @@ -265,7 +264,7 @@ func (r *sizeResource) createSize(request *restful.Request, response *restful.Re return } - projects, err := r.mdc.Project().Find(context.Background(), &mdmv1.ProjectFindRequest{}) + projects, err := r.mdc.Project().Find(request.Request.Context(), &mdmv1.ProjectFindRequest{}) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -372,7 +371,7 @@ func (r *sizeResource) updateSize(request *restful.Request, response *restful.Re return } - projects, err := r.mdc.Project().Find(context.Background(), &mdmv1.ProjectFindRequest{}) + projects, err := r.mdc.Project().Find(request.Request.Context(), &mdmv1.ProjectFindRequest{}) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -428,7 +427,7 @@ func (r *sizeResource) listSizeReservations(request *restful.Request, response * return } - projects, err := r.mdc.Project().Find(context.Background(), &mdmv1.ProjectFindRequest{}) + projects, err := r.mdc.Project().Find(request.Request.Context(), &mdmv1.ProjectFindRequest{}) if err != nil { r.sendError(request, response, defaultError(err)) return diff --git a/cmd/metal-api/internal/service/size-service_test.go b/cmd/metal-api/internal/service/size-service_test.go index 853cb94ac..1b414f704 100644 --- a/cmd/metal-api/internal/service/size-service_test.go +++ b/cmd/metal-api/internal/service/size-service_test.go @@ -2,7 +2,6 @@ package service import ( "bytes" - "context" "encoding/json" "net/http" "net/http/httptest" @@ -17,6 +16,7 @@ import ( "github.com/metal-stack/metal-api/cmd/metal-api/internal/testdata" "github.com/metal-stack/metal-lib/httperrors" "github.com/stretchr/testify/assert" + testifymock "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" @@ -173,7 +173,7 @@ func TestCreateSize(t *testing.T) { log := zaptest.NewLogger(t).Sugar() psc := &mdmv1mock.ProjectServiceClient{} - psc.On("Find", context.Background(), &mdmv1.ProjectFindRequest{}).Return(&mdmv1.ProjectListResponse{Projects: []*mdmv1.Project{ + psc.On("Find", testifymock.Anything, &mdmv1.ProjectFindRequest{}).Return(&mdmv1.ProjectListResponse{Projects: []*mdmv1.Project{ {Meta: &mdmv1.Meta{Id: "a"}}, }}, nil) mdc := mdm.NewMock(psc, &mdmv1mock.TenantServiceClient{}) @@ -239,8 +239,8 @@ func TestUpdateSize(t *testing.T) { log := zaptest.NewLogger(t).Sugar() psc := &mdmv1mock.ProjectServiceClient{} - psc.On("Find", context.Background(), &mdmv1.ProjectFindRequest{}).Return(&mdmv1.ProjectListResponse{Projects: []*mdmv1.Project{ - {Meta: &mdmv1.Meta{Id: "a"}}, + psc.On("Find", testifymock.Anything, &mdmv1.ProjectFindRequest{}).Return(&mdmv1.ProjectListResponse{Projects: []*mdmv1.Project{ + {Meta: &mdmv1.Meta{Id: "p1"}}, }}, nil) mdc := mdm.NewMock(psc, &mdmv1mock.TenantServiceClient{}) From 7a2a145c275a4bf8abf3197971485bdd079c8f7f Mon Sep 17 00:00:00 2001 From: Gerrit Date: Tue, 9 Jan 2024 09:43:44 +0100 Subject: [PATCH 20/23] Another test. --- .../internal/service/size-service.go | 3 +- .../internal/service/size-service_test.go | 44 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/cmd/metal-api/internal/service/size-service.go b/cmd/metal-api/internal/service/size-service.go index 6fbe4c1fa..b0a1736d0 100644 --- a/cmd/metal-api/internal/service/size-service.go +++ b/cmd/metal-api/internal/service/size-service.go @@ -64,11 +64,12 @@ func (r *sizeResource) webService() *restful.WebService { Returns(http.StatusOK, "OK", []v1.SizeResponse{}). DefaultReturns("Error", httperrors.HTTPErrorResponse{})) - ws.Route(ws.GET("/reservations"). + ws.Route(ws.POST("/reservations"). To(r.listSizeReservations). Operation("listSizeReservations"). Doc("get all size reservations"). Metadata(restfulspec.KeyOpenAPITags, tags). + Metadata(auditing.Exclude, true). Writes([]v1.SizeReservationResponse{}). Returns(http.StatusOK, "OK", []v1.SizeReservationResponse{}). DefaultReturns("Error", httperrors.HTTPErrorResponse{})) diff --git a/cmd/metal-api/internal/service/size-service_test.go b/cmd/metal-api/internal/service/size-service_test.go index 1b414f704..09ca22100 100644 --- a/cmd/metal-api/internal/service/size-service_test.go +++ b/cmd/metal-api/internal/service/size-service_test.go @@ -7,6 +7,7 @@ import ( "net/http/httptest" "testing" + "github.com/google/go-cmp/cmp" mdmv1 "github.com/metal-stack/masterdata-api/api/v1" mdmv1mock "github.com/metal-stack/masterdata-api/api/v1/mocks" mdm "github.com/metal-stack/masterdata-api/pkg/client" @@ -290,3 +291,46 @@ func TestUpdateSize(t *testing.T) { require.Equal(t, minCores, result.SizeConstraints[0].Min) require.Equal(t, maxCores, result.SizeConstraints[0].Max) } + +func TestListSizeReservations(t *testing.T) { + ds, mock := datastore.InitMockDB(t) + testdata.InitMockDBData(mock) + log := zaptest.NewLogger(t).Sugar() + + psc := &mdmv1mock.ProjectServiceClient{} + psc.On("Find", testifymock.Anything, &mdmv1.ProjectFindRequest{}).Return(&mdmv1.ProjectListResponse{Projects: []*mdmv1.Project{ + {Meta: &mdmv1.Meta{Id: "p1"}}, + }}, nil) + mdc := mdm.NewMock(psc, &mdmv1mock.TenantServiceClient{}) + + sizeservice := NewSize(log, ds, mdc) + container := restful.NewContainer().Add(sizeservice) + + req := httptest.NewRequest("POST", "/v1/size/reservations", nil) + req.Header.Add("Content-Type", "application/json") + container = injectAdmin(log, container, req) + w := httptest.NewRecorder() + container.ServeHTTP(w, req) + + resp := w.Result() + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode, w.Body.String()) + var result []*v1.SizeReservationResponse + err := json.NewDecoder(resp.Body).Decode(&result) + require.NoError(t, err) + + want := []*v1.SizeReservationResponse{ + { + SizeID: testdata.Sz1.ID, + PartitionID: "1", + ProjectID: "p1", + Reservations: 3, + UsedReservations: 1, + ProjectAllocations: 1, + }, + } + + if diff := cmp.Diff(result, want); diff != "" { + t.Errorf("diff (-want +got):\n%s", diff) + } +} From b3ecb249fe3b2a241b280d7fd0d24872cce5120f Mon Sep 17 00:00:00 2001 From: Gerrit Date: Tue, 9 Jan 2024 09:44:16 +0100 Subject: [PATCH 21/23] Generate spec. --- spec/metal-api.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/metal-api.json b/spec/metal-api.json index bf7844f20..eba6d34a2 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -8934,7 +8934,7 @@ } }, "/v1/size/reservations": { - "get": { + "post": { "consumes": [ "application/json" ], From 9d80382e2c84898124e329b139288723b66232f6 Mon Sep 17 00:00:00 2001 From: Gerrit91 Date: Tue, 9 Jan 2024 09:58:59 +0100 Subject: [PATCH 22/23] Fix integration test. --- cmd/metal-api/internal/service/integration_test.go | 5 +++-- .../internal/service/machine-service_allocation_test.go | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cmd/metal-api/internal/service/integration_test.go b/cmd/metal-api/internal/service/integration_test.go index 5889e4c0a..59fe11020 100644 --- a/cmd/metal-api/internal/service/integration_test.go +++ b/cmd/metal-api/internal/service/integration_test.go @@ -36,6 +36,7 @@ import ( v1 "github.com/metal-stack/metal-api/cmd/metal-api/internal/service/v1" grpcv1 "github.com/metal-stack/metal-api/pkg/api/v1" + testifymock "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -75,12 +76,12 @@ func createTestEnvironment(t *testing.T) testEnv { require.NoError(t, err) psc := &mdmv1mock.ProjectServiceClient{} - psc.On("Get", context.Background(), &mdmv1.ProjectGetRequest{Id: "test-project-1"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{ + psc.On("Get", testifymock.Anything, &mdmv1.ProjectGetRequest{Id: "test-project-1"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{ Meta: &mdmv1.Meta{ Id: "test-project-1", }, }}, nil) - psc.On("Find", context.Background(), &mdmv1.ProjectFindRequest{}).Return(&mdmv1.ProjectListResponse{Projects: []*mdmv1.Project{ + psc.On("Find", testifymock.Anything, &mdmv1.ProjectFindRequest{}).Return(&mdmv1.ProjectListResponse{Projects: []*mdmv1.Project{ {Meta: &mdmv1.Meta{Id: "test-project-1"}}, }}, nil) mdc := mdm.NewMock(psc, nil) diff --git a/cmd/metal-api/internal/service/machine-service_allocation_test.go b/cmd/metal-api/internal/service/machine-service_allocation_test.go index 92b7a8d7f..38d788600 100644 --- a/cmd/metal-api/internal/service/machine-service_allocation_test.go +++ b/cmd/metal-api/internal/service/machine-service_allocation_test.go @@ -21,6 +21,7 @@ import ( "github.com/avast/retry-go/v4" "github.com/emicklei/go-restful/v3" + testifymock "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" "golang.org/x/sync/errgroup" @@ -303,7 +304,7 @@ func setupTestEnvironment(machineCount int, t *testing.T) (*datastore.RethinkSto require.NoError(t, err) psc := &mdmv1mock.ProjectServiceClient{} - psc.On("Get", context.Background(), &mdmv1.ProjectGetRequest{Id: "pr1"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) + psc.On("Get", testifymock.Anything, &mdmv1.ProjectGetRequest{Id: "pr1"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{}}, nil) mdc := mdm.NewMock(psc, nil) _, pg, err := test.StartPostgres() From 44a1522169695a8cc3e6686bc7f7c7d59f614c41 Mon Sep 17 00:00:00 2001 From: Gerrit91 Date: Tue, 9 Jan 2024 10:34:53 +0100 Subject: [PATCH 23/23] Clean up. --- cmd/metal-api/internal/metal/size.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/cmd/metal-api/internal/metal/size.go b/cmd/metal-api/internal/metal/size.go index 30d1aef7c..8d0f3852a 100644 --- a/cmd/metal-api/internal/metal/size.go +++ b/cmd/metal-api/internal/metal/size.go @@ -4,7 +4,6 @@ import ( "fmt" "slices" - "github.com/davecgh/go-spew/spew" mdmv1 "github.com/metal-stack/masterdata-api/api/v1" ) @@ -198,8 +197,6 @@ func (rs *Reservations) Validate(partitions PartitionMap, projects map[string]*m return nil } - spew.Dump(*rs) - for _, r := range *rs { if r.Amount <= 0 { return fmt.Errorf("amount must be a positive integer")