From 15c0dbff523c14b1ac355b5af5badcf0656e796d Mon Sep 17 00:00:00 2001 From: Ivy Gooch Date: Mon, 2 Dec 2024 13:35:12 -0800 Subject: [PATCH 1/7] Adds DeleteValues as a ListAction during allocation --- pkg/allocation/converters/converter.go | 11 +++++++ pkg/apis/agones/v1/gameserver.go | 31 +++++++++++++++++++ .../allocation/v1/gameserverallocation.go | 9 ++++++ proto/allocation/allocation.proto | 2 ++ 4 files changed, 53 insertions(+) diff --git a/pkg/allocation/converters/converter.go b/pkg/allocation/converters/converter.go index 3673ebcd6d..b4b80cfc68 100644 --- a/pkg/allocation/converters/converter.go +++ b/pkg/allocation/converters/converter.go @@ -615,6 +615,12 @@ func convertAllocationListsToGSAListActions(in map[string]*pb.ListAction) map[st copy(copyValues, addValues) la.AddValues = copyValues } + if v.DeleteValues != nil { + deleteValues := v.GetDeleteValues() + copyValues := make([]string, len(deleteValues)) + copy(copyValues, deleteValues) + la.DeleteValues = copyValues + } if v.Capacity != nil { capacity := v.Capacity.GetValue() la.Capacity = &capacity @@ -639,6 +645,11 @@ func convertGSAListActionsToAllocationLists(in map[string]allocationv1.ListActio copy(copyValues, v.AddValues) la.AddValues = copyValues } + if v.DeleteValues != nil { + copyValues := make([]string, len(v.DeleteValues)) + copy(copyValues, v.DeleteValues) + la.DeleteValues = copyValues + } if v.Capacity != nil { la.Capacity = wrapperspb.Int64(*v.Capacity) } diff --git a/pkg/apis/agones/v1/gameserver.go b/pkg/apis/agones/v1/gameserver.go index b47c1492de..0f34a814d6 100644 --- a/pkg/apis/agones/v1/gameserver.go +++ b/pkg/apis/agones/v1/gameserver.go @@ -1026,6 +1026,37 @@ func (gs *GameServer) AppendListValues(name string, values []string) error { return errors.Errorf("unable to AppendListValues: Name %s, Values %s. List not found in GameServer %s", name, values, gs.ObjectMeta.GetName()) } +// DeleteListValues removes values from the ListStatus Values list. Values in the DeleteListValues +// list that are not in the ListStatus Values list are ignored. +func (gs *GameServer) DeleteListValues(name string, values []string) error { + if values == nil { + return errors.Errorf("unable to DeleteListValues: Name %s, Values %s. Values must not be nil", name, values) + } + if list, ok := gs.Status.Lists[name]; ok { + deleteValuesMap := make(map[string]bool) + for _, value := range values { + deleteValuesMap[value] = true + } + newList := deleteValues(list.Values, deleteValuesMap) + list.Values = newList + gs.Status.Lists[name] = list + return nil + } + return errors.Errorf("unable to DeleteListValues: Name %s, Values %s. List not found in GameServer %s", name, values, gs.ObjectMeta.GetName()) +} + +// deleteValues returns a new list with all the values in valuesList that are not keys in deleteValuesMap. +func deleteValues(valuesList []string, deleteValuesMap map[string]bool) []string { + newValuesList := []string{} + for _, value := range valuesList { + if _, ok := deleteValuesMap[value]; ok { + continue + } + newValuesList = append(newValuesList, value) + } + return newValuesList +} + // truncateList truncates the list to the given capacity func truncateList(capacity int64, list []string) []string { if list == nil || len(list) <= int(capacity) { diff --git a/pkg/apis/allocation/v1/gameserverallocation.go b/pkg/apis/allocation/v1/gameserverallocation.go index 2e92775811..5f12b38e5e 100644 --- a/pkg/apis/allocation/v1/gameserverallocation.go +++ b/pkg/apis/allocation/v1/gameserverallocation.go @@ -209,6 +209,9 @@ type ListAction struct { // AddValues appends values to a List's Values array. Any duplicate values will be ignored. // +optional AddValues []string `json:"addValues,omitempty"` + // DeleteValues removes values from a List's Values array. Any nonexistant values will be ignored. + // +optional + DeleteValues []string `json:"deleteValues,omitempty"` // Capacity updates the maximum capacity of the Counter to this number. Min 0, Max 1000. // +optional Capacity *int64 `json:"capacity,omitempty"` @@ -349,6 +352,12 @@ func (la *ListAction) ListActions(list string, gs *agonesv1.GameServer) error { errs = errors.Join(errs, cntErr) } } + if len(la.DeleteValues) > 0 { + cntErr := gs.DeleteListValues(list, la.DeleteValues) + if cntErr != nil { + errs = errors.Join(errs, cntErr) + } + } return errs } diff --git a/proto/allocation/allocation.proto b/proto/allocation/allocation.proto index 58d249eb11..af4354bcb4 100644 --- a/proto/allocation/allocation.proto +++ b/proto/allocation/allocation.proto @@ -239,7 +239,9 @@ message CounterAction { // ListAction is an optional action that can be performed on a List at allocation. // AddValues: Append values to a List's Values array (optional). Any duplicate values will be ignored. // Capacity: Update the maximum capacity of the Counter to this number (optional). Min 0, Max 1000. +// DeleteValues: Remove values from a List's Values array (optional). Any nonexistant values will be ignored. message ListAction { repeated string addValues = 1; google.protobuf.Int64Value capacity = 2; + repeated string deleteValues = 3; } From c531b5d56b2c51e91adb6c87860d3e3bf0e84b56 Mon Sep 17 00:00:00 2001 From: Ivy Gooch Date: Mon, 2 Dec 2024 14:48:07 -0800 Subject: [PATCH 2/7] Generated Files --- pkg/allocation/go/allocation.pb.go | 61 +++++++++++-------- pkg/allocation/go/allocation.swagger.json | 8 ++- .../allocation/v1/zz_generated.deepcopy.go | 5 ++ sdks/rust/proto/allocation/allocation.proto | 2 + 4 files changed, 50 insertions(+), 26 deletions(-) diff --git a/pkg/allocation/go/allocation.pb.go b/pkg/allocation/go/allocation.pb.go index ed77cb3e9f..56264493df 100644 --- a/pkg/allocation/go/allocation.pb.go +++ b/pkg/allocation/go/allocation.pb.go @@ -1096,13 +1096,15 @@ func (x *CounterAction) GetCapacity() *wrapperspb.Int64Value { // ListAction is an optional action that can be performed on a List at allocation. // AddValues: Append values to a List's Values array (optional). Any duplicate values will be ignored. // Capacity: Update the maximum capacity of the Counter to this number (optional). Min 0, Max 1000. +// DeleteValues: Remove values from a List's Values array (optional). Any nonexistant values will be ignored. type ListAction struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - AddValues []string `protobuf:"bytes,1,rep,name=addValues,proto3" json:"addValues,omitempty"` - Capacity *wrapperspb.Int64Value `protobuf:"bytes,2,opt,name=capacity,proto3" json:"capacity,omitempty"` + AddValues []string `protobuf:"bytes,1,rep,name=addValues,proto3" json:"addValues,omitempty"` + Capacity *wrapperspb.Int64Value `protobuf:"bytes,2,opt,name=capacity,proto3" json:"capacity,omitempty"` + DeleteValues []string `protobuf:"bytes,3,rep,name=deleteValues,proto3" json:"deleteValues,omitempty"` } func (x *ListAction) Reset() { @@ -1151,6 +1153,13 @@ func (x *ListAction) GetCapacity() *wrapperspb.Int64Value { return nil } +func (x *ListAction) GetDeleteValues() []string { + if x != nil { + return x.DeleteValues + } + return nil +} + // The gameserver port info that is allocated. type AllocationResponse_GameServerStatusPort struct { state protoimpl.MessageState @@ -1718,29 +1727,31 @@ var file_proto_allocation_allocation_proto_rawDesc = []byte{ 0x63, 0x69, 0x74, 0x79, 0x18, 0x03, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1b, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x49, 0x6e, 0x74, 0x36, 0x34, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x52, 0x08, 0x63, 0x61, 0x70, 0x61, 0x63, 0x69, 0x74, - 0x79, 0x22, 0x63, 0x0a, 0x0a, 0x4c, 0x69, 0x73, 0x74, 0x41, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x12, - 0x1c, 0x0a, 0x09, 0x61, 0x64, 0x64, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x73, 0x18, 0x01, 0x20, 0x03, - 0x28, 0x09, 0x52, 0x09, 0x61, 0x64, 0x64, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x73, 0x12, 0x37, 0x0a, - 0x08, 0x63, 0x61, 0x70, 0x61, 0x63, 0x69, 0x74, 0x79, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, - 0x1b, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, - 0x66, 0x2e, 0x49, 0x6e, 0x74, 0x36, 0x34, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x52, 0x08, 0x63, 0x61, - 0x70, 0x61, 0x63, 0x69, 0x74, 0x79, 0x32, 0x80, 0x01, 0x0a, 0x11, 0x41, 0x6c, 0x6c, 0x6f, 0x63, - 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x12, 0x6b, 0x0a, 0x08, - 0x41, 0x6c, 0x6c, 0x6f, 0x63, 0x61, 0x74, 0x65, 0x12, 0x1d, 0x2e, 0x61, 0x6c, 0x6c, 0x6f, 0x63, - 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x2e, 0x41, 0x6c, 0x6c, 0x6f, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, - 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x1e, 0x2e, 0x61, 0x6c, 0x6c, 0x6f, 0x63, 0x61, - 0x74, 0x69, 0x6f, 0x6e, 0x2e, 0x41, 0x6c, 0x6c, 0x6f, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x52, - 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x20, 0x82, 0xd3, 0xe4, 0x93, 0x02, 0x1a, 0x22, - 0x15, 0x2f, 0x67, 0x61, 0x6d, 0x65, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x61, 0x6c, 0x6c, 0x6f, - 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x3a, 0x01, 0x2a, 0x42, 0x6e, 0x5a, 0x0c, 0x2e, 0x2f, 0x61, - 0x6c, 0x6c, 0x6f, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x92, 0x41, 0x5d, 0x12, 0x34, 0x0a, 0x21, - 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x61, 0x6c, 0x6c, 0x6f, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, - 0x2f, 0x61, 0x6c, 0x6c, 0x6f, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x2e, 0x70, 0x72, 0x6f, 0x74, - 0x6f, 0x32, 0x0f, 0x76, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x20, 0x6e, 0x6f, 0x74, 0x20, 0x73, - 0x65, 0x74, 0x2a, 0x01, 0x02, 0x32, 0x10, 0x61, 0x70, 0x70, 0x6c, 0x69, 0x63, 0x61, 0x74, 0x69, - 0x6f, 0x6e, 0x2f, 0x6a, 0x73, 0x6f, 0x6e, 0x3a, 0x10, 0x61, 0x70, 0x70, 0x6c, 0x69, 0x63, 0x61, - 0x74, 0x69, 0x6f, 0x6e, 0x2f, 0x6a, 0x73, 0x6f, 0x6e, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, - 0x33, + 0x79, 0x22, 0x87, 0x01, 0x0a, 0x0a, 0x4c, 0x69, 0x73, 0x74, 0x41, 0x63, 0x74, 0x69, 0x6f, 0x6e, + 0x12, 0x1c, 0x0a, 0x09, 0x61, 0x64, 0x64, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x73, 0x18, 0x01, 0x20, + 0x03, 0x28, 0x09, 0x52, 0x09, 0x61, 0x64, 0x64, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x73, 0x12, 0x37, + 0x0a, 0x08, 0x63, 0x61, 0x70, 0x61, 0x63, 0x69, 0x74, 0x79, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, + 0x32, 0x1b, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, + 0x75, 0x66, 0x2e, 0x49, 0x6e, 0x74, 0x36, 0x34, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x52, 0x08, 0x63, + 0x61, 0x70, 0x61, 0x63, 0x69, 0x74, 0x79, 0x12, 0x22, 0x0a, 0x0c, 0x64, 0x65, 0x6c, 0x65, 0x74, + 0x65, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x73, 0x18, 0x03, 0x20, 0x03, 0x28, 0x09, 0x52, 0x0c, 0x64, + 0x65, 0x6c, 0x65, 0x74, 0x65, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x73, 0x32, 0x80, 0x01, 0x0a, 0x11, + 0x41, 0x6c, 0x6c, 0x6f, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, + 0x65, 0x12, 0x6b, 0x0a, 0x08, 0x41, 0x6c, 0x6c, 0x6f, 0x63, 0x61, 0x74, 0x65, 0x12, 0x1d, 0x2e, + 0x61, 0x6c, 0x6c, 0x6f, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x2e, 0x41, 0x6c, 0x6c, 0x6f, 0x63, + 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x1e, 0x2e, 0x61, + 0x6c, 0x6c, 0x6f, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x2e, 0x41, 0x6c, 0x6c, 0x6f, 0x63, 0x61, + 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x20, 0x82, 0xd3, + 0xe4, 0x93, 0x02, 0x1a, 0x22, 0x15, 0x2f, 0x67, 0x61, 0x6d, 0x65, 0x73, 0x65, 0x72, 0x76, 0x65, + 0x72, 0x61, 0x6c, 0x6c, 0x6f, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x3a, 0x01, 0x2a, 0x42, 0x6e, + 0x5a, 0x0c, 0x2e, 0x2f, 0x61, 0x6c, 0x6c, 0x6f, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x92, 0x41, + 0x5d, 0x12, 0x34, 0x0a, 0x21, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x61, 0x6c, 0x6c, 0x6f, 0x63, + 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x2f, 0x61, 0x6c, 0x6c, 0x6f, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, + 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x32, 0x0f, 0x76, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x20, + 0x6e, 0x6f, 0x74, 0x20, 0x73, 0x65, 0x74, 0x2a, 0x01, 0x02, 0x32, 0x10, 0x61, 0x70, 0x70, 0x6c, + 0x69, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x2f, 0x6a, 0x73, 0x6f, 0x6e, 0x3a, 0x10, 0x61, 0x70, + 0x70, 0x6c, 0x69, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x2f, 0x6a, 0x73, 0x6f, 0x6e, 0x62, 0x06, + 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/pkg/allocation/go/allocation.swagger.json b/pkg/allocation/go/allocation.swagger.json index 48188ce2af..44bd8010cb 100644 --- a/pkg/allocation/go/allocation.swagger.json +++ b/pkg/allocation/go/allocation.swagger.json @@ -351,9 +351,15 @@ "capacity": { "type": "string", "format": "int64" + }, + "deleteValues": { + "type": "array", + "items": { + "type": "string" + } } }, - "description": "ListAction is an optional action that can be performed on a List at allocation.\nAddValues: Append values to a List's Values array (optional). Any duplicate values will be ignored.\nCapacity: Update the maximum capacity of the Counter to this number (optional). Min 0, Max 1000." + "description": "ListAction is an optional action that can be performed on a List at allocation.\nAddValues: Append values to a List's Values array (optional). Any duplicate values will be ignored.\nCapacity: Update the maximum capacity of the Counter to this number (optional). Min 0, Max 1000.\nDeleteValues: Remove values from a List's Values array (optional). Any nonexistant values will be ignored." }, "allocationListSelector": { "type": "object", diff --git a/pkg/apis/allocation/v1/zz_generated.deepcopy.go b/pkg/apis/allocation/v1/zz_generated.deepcopy.go index 5063ad1506..e789611647 100644 --- a/pkg/apis/allocation/v1/zz_generated.deepcopy.go +++ b/pkg/apis/allocation/v1/zz_generated.deepcopy.go @@ -311,6 +311,11 @@ func (in *ListAction) DeepCopyInto(out *ListAction) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.DeleteValues != nil { + in, out := &in.DeleteValues, &out.DeleteValues + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.Capacity != nil { in, out := &in.Capacity, &out.Capacity *out = new(int64) diff --git a/sdks/rust/proto/allocation/allocation.proto b/sdks/rust/proto/allocation/allocation.proto index 58d249eb11..af4354bcb4 100644 --- a/sdks/rust/proto/allocation/allocation.proto +++ b/sdks/rust/proto/allocation/allocation.proto @@ -239,7 +239,9 @@ message CounterAction { // ListAction is an optional action that can be performed on a List at allocation. // AddValues: Append values to a List's Values array (optional). Any duplicate values will be ignored. // Capacity: Update the maximum capacity of the Counter to this number (optional). Min 0, Max 1000. +// DeleteValues: Remove values from a List's Values array (optional). Any nonexistant values will be ignored. message ListAction { repeated string addValues = 1; google.protobuf.Int64Value capacity = 2; + repeated string deleteValues = 3; } From 71e382c2c61f69c9f5efa69136d6143cd74aec89 Mon Sep 17 00:00:00 2001 From: Ivy Gooch Date: Mon, 2 Dec 2024 14:49:04 -0800 Subject: [PATCH 3/7] Adds tests --- pkg/allocation/converters/converter_test.go | 16 +++++++++--- .../v1/gameserverallocation_test.go | 12 +++++---- pkg/gameserverallocations/allocator_test.go | 11 ++++---- test/e2e/allocator_test.go | 12 ++++++--- test/e2e/fleetautoscaler_test.go | 7 +++--- test/e2e/gameserverallocation_test.go | 25 +++++++++++++++++-- 6 files changed, 61 insertions(+), 22 deletions(-) diff --git a/pkg/allocation/converters/converter_test.go b/pkg/allocation/converters/converter_test.go index 0de964807a..4a26e011fb 100644 --- a/pkg/allocation/converters/converter_test.go +++ b/pkg/allocation/converters/converter_test.go @@ -135,8 +135,9 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) { }, Lists: map[string]*pb.ListAction{ "p": { - AddValues: []string{"foo", "bar", "baz"}, - Capacity: wrapperspb.Int64(10), + AddValues: []string{"foo", "bar", "baz"}, + Capacity: wrapperspb.Int64(10), + DeleteValues: []string{"alice", "bob", "cat"}, }, }, Scheduling: pb.AllocationRequest_Packed, @@ -217,8 +218,9 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) { }, Lists: map[string]allocationv1.ListAction{ "p": { - AddValues: []string{"foo", "bar", "baz"}, - Capacity: &ten, + AddValues: []string{"foo", "bar", "baz"}, + Capacity: &ten, + DeleteValues: []string{"alice", "bob", "cat"}, }, }, Selectors: []allocationv1.GameServerSelector{ @@ -742,6 +744,9 @@ func TestConvertGSAToAllocationRequest(t *testing.T) { "d": { Capacity: &two, }, + "c": { + DeleteValues: []string{"good", "bye"}, + }, }, Scheduling: apis.Distributed, }, @@ -821,6 +826,9 @@ func TestConvertGSAToAllocationRequest(t *testing.T) { "d": { Capacity: wrapperspb.Int64(two), }, + "c": { + DeleteValues: []string{"good", "bye"}, + }, }, }, }, diff --git a/pkg/apis/allocation/v1/gameserverallocation_test.go b/pkg/apis/allocation/v1/gameserverallocation_test.go index c7304db2a5..f1f709a812 100644 --- a/pkg/apis/allocation/v1/gameserverallocation_test.go +++ b/pkg/apis/allocation/v1/gameserverallocation_test.go @@ -1019,20 +1019,21 @@ func TestGameServerListActions(t *testing.T) { }, "update list values and capacity": { la: ListAction{ - AddValues: []string{"magician1", "magician3"}, - Capacity: int64Pointer(42), + AddValues: []string{"magician1", "magician3"}, + Capacity: int64Pointer(42), + DeleteValues: []string{"magician2", "magician5", "magician6"}, }, list: "magicians", gs: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ Lists: map[string]agonesv1.ListStatus{ "magicians": { - Values: []string{"magician1", "magician2"}, + Values: []string{"magician1", "magician2", "magician4", "magician5"}, Capacity: 100, }}}}, want: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ Lists: map[string]agonesv1.ListStatus{ "magicians": { - Values: []string{"magician1", "magician2", "magician3"}, + Values: []string{"magician1", "magician4", "magician3"}, Capacity: 42, }}}}, wantErr: false, @@ -1263,7 +1264,8 @@ func TestValidateListActions(t *testing.T) { Capacity: int64Pointer(0), }, "baz": { - AddValues: []string{}, + AddValues: []string{}, + DeleteValues: []string{"good", "bye"}, }, }, wantErr: false, diff --git a/pkg/gameserverallocations/allocator_test.go b/pkg/gameserverallocations/allocator_test.go index 457baddedc..af0434de11 100644 --- a/pkg/gameserverallocations/allocator_test.go +++ b/pkg/gameserverallocations/allocator_test.go @@ -309,7 +309,7 @@ func TestAllocatorApplyAllocationToGameServerCountsListsActions(t *testing.T) { Status: agonesv1.GameServerStatus{NodeName: "node1", State: agonesv1.GameServerStateReady, Lists: map[string]agonesv1.ListStatus{ "players": { - Values: []string{}, + Values: []string{"alice", "bob", "cat"}, Capacity: 100, }, }, @@ -339,7 +339,7 @@ func TestAllocatorApplyAllocationToGameServerCountsListsActions(t *testing.T) { wantCounters map[string]agonesv1.CounterStatus wantLists map[string]agonesv1.ListStatus }{ - "CounterActions increment and ListActions append and update capacity": { + "CounterActions increment and ListActions add, delete, and update capacity": { features: fmt.Sprintf("%s=true", runtime.FeatureCountsAndLists), gs: &gs1, gsa: &allocationv1.GameServerAllocation{ @@ -356,8 +356,9 @@ func TestAllocatorApplyAllocationToGameServerCountsListsActions(t *testing.T) { }}, Lists: map[string]allocationv1.ListAction{ "players": { - AddValues: []string{"x7un", "8inz"}, - Capacity: &FORTY, + AddValues: []string{"x7un", "8inz"}, + Capacity: &FORTY, + DeleteValues: []string{"bob"}, }}}}, wantCounters: map[string]agonesv1.CounterStatus{ "rooms": { @@ -366,7 +367,7 @@ func TestAllocatorApplyAllocationToGameServerCountsListsActions(t *testing.T) { }}, wantLists: map[string]agonesv1.ListStatus{ "players": { - Values: []string{"x7un", "8inz"}, + Values: []string{"alice", "cat", "x7un", "8inz"}, Capacity: 40, }}, }, diff --git a/test/e2e/allocator_test.go b/test/e2e/allocator_test.go index 89071dd74f..26b954ac01 100644 --- a/test/e2e/allocator_test.go +++ b/test/e2e/allocator_test.go @@ -352,7 +352,8 @@ func TestAllocatorWithCountersAndLists(t *testing.T) { }, Lists: map[string]*pb.ListAction{ "rooms": { - AddValues: []string{"1"}, + AddValues: []string{"1"}, + DeleteValues: []string{"2"}, // This action is ignored. (Value does not exist.) }, }, } @@ -379,6 +380,7 @@ func TestAllocatorWithCountersAndLists(t *testing.T) { assert.Contains(t, response.GetLists(), "rooms") assert.Equal(t, int64(10), response.GetLists()["rooms"].Capacity.GetValue()) assert.EqualValues(t, request.Lists["rooms"].AddValues, response.GetLists()["rooms"].Values) + assert.NotEqualValues(t, request.Lists["rooms"].DeleteValues, response.GetLists()["rooms"].Values) return true, nil }) require.NoError(t, err) @@ -405,6 +407,7 @@ func TestRestAllocatorWithCountersAndLists(t *testing.T) { } f.Spec.Template.Spec.Lists = map[string]agonesv1.ListStatus{ "rooms": { + Values: []string{"one", "two", "three"}, Capacity: 10, }, } @@ -434,7 +437,8 @@ func TestRestAllocatorWithCountersAndLists(t *testing.T) { }, Lists: map[string]*pb.ListAction{ "rooms": { - AddValues: []string{"1"}, + AddValues: []string{"1"}, + DeleteValues: []string{"three", "one"}, }, }, } @@ -478,7 +482,9 @@ func TestRestAllocatorWithCountersAndLists(t *testing.T) { assert.Equal(t, int64(1), response.GetCounters()["players"].Count.GetValue()) assert.Contains(t, response.GetLists(), "rooms") assert.Equal(t, int64(10), response.GetLists()["rooms"].Capacity.GetValue()) - assert.EqualValues(t, request.Lists["rooms"].AddValues, response.GetLists()["rooms"].Values) + assert.Contains(t, response.GetLists()["rooms"].Values, request.Lists["rooms"].AddValues[0]) + assert.NotContains(t, response.GetLists()["rooms"].Values, request.Lists["rooms"].DeleteValues[0]) + assert.NotContains(t, response.GetLists()["rooms"].Values, request.Lists["rooms"].DeleteValues[1]) return true, nil }) require.NoError(t, err) diff --git a/test/e2e/fleetautoscaler_test.go b/test/e2e/fleetautoscaler_test.go index 2ea0adc25f..62291896ed 100644 --- a/test/e2e/fleetautoscaler_test.go +++ b/test/e2e/fleetautoscaler_test.go @@ -1386,7 +1386,7 @@ func TestListAutoscalerAllocated(t *testing.T) { defaultFlt := defaultFleet(framework.Namespace) defaultFlt.Spec.Template.Spec.Lists = map[string]agonesv1.ListStatus{ "gamers": { - Values: []string{}, + Values: []string{"gamer5", "gamer6"}, Capacity: 6, // AggregateCapacity 18 }, } @@ -1441,7 +1441,7 @@ func TestListAutoscalerAllocated(t *testing.T) { defer client.Fleets(framework.Namespace).Delete(ctx, flt.ObjectMeta.Name, metav1.DeleteOptions{}) // nolint:errcheck framework.AssertFleetCondition(t, flt, e2e.FleetReadyCount(flt.Spec.Replicas)) - // Adds 4 gamers to each allocated gameserver. + // Adds 4 gamers to each allocated gameserver, and removes 2 existing gamers. gsa := allocationv1.GameServerAllocation{ Spec: allocationv1.GameServerAllocationSpec{ Selectors: []allocationv1.GameServerSelector{ @@ -1450,7 +1450,8 @@ func TestListAutoscalerAllocated(t *testing.T) { }, Lists: map[string]allocationv1.ListAction{ "gamers": { - AddValues: []string{"gamer1", "gamer2", "gamer3", "gamer4"}, + AddValues: []string{"gamer1", "gamer2", "gamer3", "gamer4"}, + DeleteValues: []string{"gamer5", "gamer6"}, }}}} // Allocate game servers, as Buffer Percent scales up (or down) based on allocated aggregate capacity diff --git a/test/e2e/gameserverallocation_test.go b/test/e2e/gameserverallocation_test.go index 6fa162d529..9a09a3ce59 100644 --- a/test/e2e/gameserverallocation_test.go +++ b/test/e2e/gameserverallocation_test.go @@ -877,6 +877,10 @@ func TestListGameServerAllocationActions(t *testing.T) { Values: []string{"player0", "player1", "player2"}, Capacity: 8, } + lists["rooms"] = agonesv1.ListStatus{ + Values: []string{"room0", "room1", "room2"}, + Capacity: 20, + } flt := defaultFleet(framework.Namespace) flt.Spec.Template.Spec.Lists = lists @@ -893,6 +897,7 @@ func TestListGameServerAllocationActions(t *testing.T) { testCases := map[string]struct { gsa allocationv1.GameServerAllocation + listName string wantGsaErr bool wantCapacity *int64 wantValues []string @@ -907,6 +912,7 @@ func TestListGameServerAllocationActions(t *testing.T) { "players": { AddValues: []string{"player3", "player4", "player5"}, }}}}, + listName: "players", wantGsaErr: false, wantValues: []string{"player0", "player1", "player2", "player3", "player4", "player5"}, }, @@ -920,10 +926,25 @@ func TestListGameServerAllocationActions(t *testing.T) { "players": { Capacity: &one, }}}}, + listName: "players", wantGsaErr: false, wantValues: []string{"player0"}, wantCapacity: &one, }, + "delete List values": { + gsa: allocationv1.GameServerAllocation{ + Spec: allocationv1.GameServerAllocationSpec{ + Selectors: []allocationv1.GameServerSelector{ + {LabelSelector: fleetSelector}, + }, + Lists: map[string]allocationv1.ListAction{ + "rooms": { + DeleteValues: []string{"room1", "room0"}, + }}}}, + listName: "rooms", + wantGsaErr: false, + wantValues: []string{"room2"}, + }, } for name, testCase := range testCases { @@ -936,7 +957,7 @@ func TestListGameServerAllocationActions(t *testing.T) { } require.NoError(t, err) assert.Equal(t, string(allocated), string(gsa.Status.State)) - listStatus, ok := gsa.Status.Lists["players"] + listStatus, ok := gsa.Status.Lists[testCase.listName] assert.True(t, ok) if testCase.wantCapacity != nil { assert.Equal(t, *testCase.wantCapacity, listStatus.Capacity) @@ -948,7 +969,7 @@ func TestListGameServerAllocationActions(t *testing.T) { assert.Equal(t, allocated, gs1.Status.State) assert.NotNil(t, gs1.ObjectMeta.Annotations["agones.dev/last-allocated"]) - list, ok := gs1.Status.Lists["players"] + list, ok := gs1.Status.Lists[testCase.listName] assert.True(t, ok) if testCase.wantCapacity != nil { assert.Equal(t, *testCase.wantCapacity, list.Capacity) From 9786b90eadc1ae8391678d4e1aa75c7a003b79a3 Mon Sep 17 00:00:00 2001 From: Ivy Gooch Date: Mon, 2 Dec 2024 14:50:22 -0800 Subject: [PATCH 4/7] Adds DeleteValues documentation --- examples/gameserverallocation.yaml | 3 +++ site/content/en/docs/Guides/access-api.md | 16 ++++++++++++++-- .../en/docs/Reference/gameserverallocation.md | 7 +++++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/examples/gameserverallocation.yaml b/examples/gameserverallocation.yaml index 61594591b9..36cb4a58de 100644 --- a/examples/gameserverallocation.yaml +++ b/examples/gameserverallocation.yaml @@ -116,3 +116,6 @@ spec: - x7un - 8inz capacity: 40 # Updates the maximum capacity of the Counter to this number. Min 0, Max 1000. + deleteValues: # removes values from a List's Valules array. Any nonexistant values are ignored. + - alice + - bob diff --git a/site/content/en/docs/Guides/access-api.md b/site/content/en/docs/Guides/access-api.md index 1c3843ec02..272cf865ca 100644 --- a/site/content/en/docs/Guides/access-api.md +++ b/site/content/en/docs/Guides/access-api.md @@ -10,7 +10,7 @@ description: > --- Installing Agones creates several [Custom Resource Definitions (CRD)](https://kubernetes.io/docs/concepts/api-extension/custom-resources), -which can be accessed and manipulated through the Kubernetes API. +which can be accessed and manipulated through the Kubernetes API. The detailed list of Agones CRDs with their parameters could be found here - [Agones CRD API Reference](../../reference/agones_crd_api_reference/). @@ -402,7 +402,19 @@ curl http://localhost:8001/apis/agones.dev/v1/namespaces/default/gameservers ### Allocate a gameserver from a fleet named 'simple-game-server', with GameServerAllocation ```bash -curl -d '{"apiVersion":"allocation.agones.dev/v1","kind":"GameServerAllocation","spec":{"required":{"matchLabels":{"agones.dev/fleet":"simple-game-server"}}}}' -H "Content-Type: application/json" -X POST http://localhost:8001/apis/allocation.agones.dev/v1/namespaces/default/gameserverallocations +curl -d '{ + "apiVersion": "allocation.agones.dev/v1", + "kind": "GameServerAllocation", + "spec": { + "selectors": [ + { + "matchLabels": { + "agones.dev/fleet": "simple-game-server" + } + } + ] + } +}' -H "Content-Type: application/json" -X POST http://localhost:8001/apis/allocation.agones.dev/v1/namespaces/default/gameserverallocations ``` ``` { diff --git a/site/content/en/docs/Reference/gameserverallocation.md b/site/content/en/docs/Reference/gameserverallocation.md index 41c21720ff..a9e9a7a676 100644 --- a/site/content/en/docs/Reference/gameserverallocation.md +++ b/site/content/en/docs/Reference/gameserverallocation.md @@ -59,7 +59,7 @@ spec: containsValue: "x6k8z" # only match GameServers who has this value in the list. Defaults to "", which is all. minAvailable: 1 # minimum available (current capacity - current count). Defaults to 0. maxAvailable: 10 # maximum available (current capacity - current count) Defaults to 0, which translates to max(int64) - # [Stage:Alpha] + # [Stage:Alpha] # [FeatureFlag:PlayerAllocationFilter] # Provides a filter on minimum and maximum values for player capacity when retrieving a GameServer # through Allocation. Defaults to no limits. @@ -111,6 +111,9 @@ spec: - x7un - 8inz capacity: 40 # Updates the maximum capacity of the Counter to this number. Min 0, Max 1000. + deleteValues: # removes values from a List's Values array. Any nonexistant values are ignored. + - alice + - bob {{< /tab >}} {{< tab header="required & preferred (deprecated)" lang="yaml" >}} apiVersion: "allocation.agones.dev/v1" @@ -170,7 +173,7 @@ spec: annotations: map: garden22 {{< /tab >}} -{{< /tabpane >}} +{{< /tabpane >}} The `spec` field is the actual `GameServerAllocation` specification, and it is composed as follows: From 916ef02c9228bac3d56771e75c6cd3a2aaab02b7 Mon Sep 17 00:00:00 2001 From: Ivy Gooch Date: Thu, 12 Dec 2024 10:53:53 -0800 Subject: [PATCH 5/7] Updates tabs to spaces per protobuf style guide --- proto/allocation/allocation.proto | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/proto/allocation/allocation.proto b/proto/allocation/allocation.proto index af4354bcb4..8ef8111fe4 100644 --- a/proto/allocation/allocation.proto +++ b/proto/allocation/allocation.proto @@ -90,12 +90,12 @@ message AllocationRequest { // // For `Distributed` strategy sorting, the entire selection of `GameServers` will be sorted by this priority list to provide the // order that `GameServers` will be allocated by. - repeated Priority priorities = 9; + repeated Priority priorities = 9; // [Stage: Beta] // [FeatureFlag:CountsAndLists] // Counters and Lists provide a set of actions to perform - // on Counters and Lists during allocation. + // on Counters and Lists during allocation. map counters = 10; map lists = 11; } @@ -191,10 +191,10 @@ message PlayerSelector { // CounterSelector is the filter options for a GameServer based on the count and/or available capacity. // 0 for MaxCount or MaxAvailable means unlimited maximum. Default for all fields: 0 message CounterSelector { - int64 minCount = 1; - int64 maxCount = 2; - int64 minAvailable = 3; - int64 maxAvailable = 4; + int64 minCount = 1; + int64 maxCount = 2; + int64 minAvailable = 3; + int64 maxAvailable = 4; } // ListSelector is the filter options for a GameServer based on List available capacity and/or the @@ -202,9 +202,9 @@ message CounterSelector { // 0 for MaxAvailable means unlimited maximum. Default for integer fields: 0 // "" for ContainsValue means ignore field. Default for string field: "" message ListSelector { - string containsValue = 1; - int64 minAvailable = 2; - int64 maxAvailable = 3; + string containsValue = 1; + int64 minAvailable = 2; + int64 maxAvailable = 3; } // Priority is a sorting option for GameServers with Counters or Lists based on the Capacity. @@ -232,8 +232,8 @@ message Priority { // Capacity: Update the maximum capacity of the Counter to this number (optional). Min 0, Max int64. message CounterAction { google.protobuf.StringValue action = 1; - google.protobuf.Int64Value amount = 2; - google.protobuf.Int64Value capacity = 3; + google.protobuf.Int64Value amount = 2; + google.protobuf.Int64Value capacity = 3; } // ListAction is an optional action that can be performed on a List at allocation. @@ -241,7 +241,7 @@ message CounterAction { // Capacity: Update the maximum capacity of the Counter to this number (optional). Min 0, Max 1000. // DeleteValues: Remove values from a List's Values array (optional). Any nonexistant values will be ignored. message ListAction { - repeated string addValues = 1; - google.protobuf.Int64Value capacity = 2; + repeated string addValues = 1; + google.protobuf.Int64Value capacity = 2; repeated string deleteValues = 3; } From e56425759ec4b80083714665adbf8a6bd299e80d Mon Sep 17 00:00:00 2001 From: Ivy Gooch Date: Thu, 12 Dec 2024 11:00:22 -0800 Subject: [PATCH 6/7] Generate Rust file --- sdks/rust/proto/allocation/allocation.proto | 26 ++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/sdks/rust/proto/allocation/allocation.proto b/sdks/rust/proto/allocation/allocation.proto index af4354bcb4..8ef8111fe4 100644 --- a/sdks/rust/proto/allocation/allocation.proto +++ b/sdks/rust/proto/allocation/allocation.proto @@ -90,12 +90,12 @@ message AllocationRequest { // // For `Distributed` strategy sorting, the entire selection of `GameServers` will be sorted by this priority list to provide the // order that `GameServers` will be allocated by. - repeated Priority priorities = 9; + repeated Priority priorities = 9; // [Stage: Beta] // [FeatureFlag:CountsAndLists] // Counters and Lists provide a set of actions to perform - // on Counters and Lists during allocation. + // on Counters and Lists during allocation. map counters = 10; map lists = 11; } @@ -191,10 +191,10 @@ message PlayerSelector { // CounterSelector is the filter options for a GameServer based on the count and/or available capacity. // 0 for MaxCount or MaxAvailable means unlimited maximum. Default for all fields: 0 message CounterSelector { - int64 minCount = 1; - int64 maxCount = 2; - int64 minAvailable = 3; - int64 maxAvailable = 4; + int64 minCount = 1; + int64 maxCount = 2; + int64 minAvailable = 3; + int64 maxAvailable = 4; } // ListSelector is the filter options for a GameServer based on List available capacity and/or the @@ -202,9 +202,9 @@ message CounterSelector { // 0 for MaxAvailable means unlimited maximum. Default for integer fields: 0 // "" for ContainsValue means ignore field. Default for string field: "" message ListSelector { - string containsValue = 1; - int64 minAvailable = 2; - int64 maxAvailable = 3; + string containsValue = 1; + int64 minAvailable = 2; + int64 maxAvailable = 3; } // Priority is a sorting option for GameServers with Counters or Lists based on the Capacity. @@ -232,8 +232,8 @@ message Priority { // Capacity: Update the maximum capacity of the Counter to this number (optional). Min 0, Max int64. message CounterAction { google.protobuf.StringValue action = 1; - google.protobuf.Int64Value amount = 2; - google.protobuf.Int64Value capacity = 3; + google.protobuf.Int64Value amount = 2; + google.protobuf.Int64Value capacity = 3; } // ListAction is an optional action that can be performed on a List at allocation. @@ -241,7 +241,7 @@ message CounterAction { // Capacity: Update the maximum capacity of the Counter to this number (optional). Min 0, Max 1000. // DeleteValues: Remove values from a List's Values array (optional). Any nonexistant values will be ignored. message ListAction { - repeated string addValues = 1; - google.protobuf.Int64Value capacity = 2; + repeated string addValues = 1; + google.protobuf.Int64Value capacity = 2; repeated string deleteValues = 3; } From 87ec578e639dc8b5c490b78e5d4351a630dcefba Mon Sep 17 00:00:00 2001 From: Ivy Gooch Date: Thu, 12 Dec 2024 15:23:56 -0800 Subject: [PATCH 7/7] Adds unit test for DeleteListValues --- pkg/apis/agones/v1/gameserver_test.go | 57 +++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/pkg/apis/agones/v1/gameserver_test.go b/pkg/apis/agones/v1/gameserver_test.go index 4bedddef29..ac125cd14b 100644 --- a/pkg/apis/agones/v1/gameserver_test.go +++ b/pkg/apis/agones/v1/gameserver_test.go @@ -2223,6 +2223,63 @@ func TestGameServerAppendListValues(t *testing.T) { } } +func TestGameServerDeleteListValues(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + gs GameServer + name string + want ListStatus + values []string + wantErr bool + }{ + "list not in game server no-op and error": { + gs: GameServer{Status: GameServerStatus{ + Lists: map[string]ListStatus{ + "foos": { + Values: []string{"foo", "bar", "bax"}, + Capacity: 100, + }, + }, + }}, + name: "foo", + values: []string{"bar", "baz"}, + wantErr: true, + }, + "delete list value - one value not present": { + gs: GameServer{Status: GameServerStatus{ + Lists: map[string]ListStatus{ + "foo": { + Values: []string{"foo", "bar", "bax"}, + Capacity: 100, + }, + }, + }}, + name: "foo", + values: []string{"bar", "baz"}, + wantErr: false, + want: ListStatus{ + Values: []string{"foo", "bax"}, + Capacity: 100, + }, + }, + } + + for test, testCase := range testCases { + t.Run(test, func(t *testing.T) { + err := testCase.gs.DeleteListValues(testCase.name, testCase.values) + if err != nil { + assert.True(t, testCase.wantErr) + } else { + assert.False(t, testCase.wantErr) + } + if list, ok := testCase.gs.Status.Lists[testCase.name]; ok { + assert.Equal(t, testCase.want, list) + } + }) + } +} + func TestMergeRemoveDuplicates(t *testing.T) { t.Parallel()