From 0f7c9436ed324bf1bf44dd92fe0eb32e3c621ddb Mon Sep 17 00:00:00 2001 From: igooch Date: Tue, 25 Jun 2024 11:20:30 -0700 Subject: [PATCH] Drop Counts and Lists feature-gated data from the fleet and game server set Status if the flag has been turned off (#3881) --- pkg/fleets/controller.go | 7 +++ pkg/fleets/controller_test.go | 90 +++++++++++++++++++++++++++ pkg/gameserversets/controller.go | 7 +++ pkg/gameserversets/controller_test.go | 83 +++++++++++++++++++++++- 4 files changed, 186 insertions(+), 1 deletion(-) diff --git a/pkg/fleets/controller.go b/pkg/fleets/controller.go index 3caf1c69cf..297cc9352f 100644 --- a/pkg/fleets/controller.go +++ b/pkg/fleets/controller.go @@ -678,6 +678,13 @@ func (c *Controller) updateFleetStatus(ctx context.Context, fleet *agonesv1.Flee fCopy.Status.Counters = make(map[string]agonesv1.AggregatedCounterStatus) fCopy.Status.Lists = make(map[string]agonesv1.AggregatedListStatus) } + // Drop Counters and Lists status if the feature flag has been set to false + if !runtime.FeatureEnabled(runtime.FeatureCountsAndLists) { + if len(fCopy.Status.Counters) != 0 || len(fCopy.Status.Lists) != 0 { + fCopy.Status.Counters = map[string]agonesv1.AggregatedCounterStatus{} + fCopy.Status.Lists = map[string]agonesv1.AggregatedListStatus{} + } + } for _, gsSet := range list { fCopy.Status.Replicas += gsSet.Status.Replicas diff --git a/pkg/fleets/controller_test.go b/pkg/fleets/controller_test.go index 0aa19c79bd..7db2b75cf6 100644 --- a/pkg/fleets/controller_test.go +++ b/pkg/fleets/controller_test.go @@ -868,6 +868,96 @@ func TestControllerUpdateFleetListStatus(t *testing.T) { assert.True(t, updated) } +// Test that the aggregated Counters and Lists are removed from the Fleet status if the +// FeatureCountsAndLists flag is set to false. +func TestFleetDropCountsAndListsStatus(t *testing.T) { + t.Parallel() + + utilruntime.FeatureTestMutex.Lock() + defer utilruntime.FeatureTestMutex.Unlock() + + f := defaultFixture() + defaultFleetName := "default/fleet-1" + c, m := newFakeController() + + gss := f.GameServerSet() + gss.ObjectMeta.Name = "gsSet1" + gss.ObjectMeta.UID = "4321" + gss.Spec.Replicas = f.Spec.Replicas + gss.Status.Counters = map[string]agonesv1.AggregatedCounterStatus{ + "aCounter": { + AllocatedCount: 1, + AllocatedCapacity: 10, + Capacity: 1000, + Count: 100, + }, + } + gss.Status.Lists = map[string]agonesv1.AggregatedListStatus{ + "aList": { + AllocatedCount: 10, + AllocatedCapacity: 100, + Capacity: 10000, + Count: 1000, + }, + } + + flag := "" + updated := false + + m.AgonesClient.AddReactor("list", "gameserversets", + func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, &agonesv1.GameServerSetList{Items: []agonesv1.GameServerSet{*gss}}, nil + }) + + m.AgonesClient.AddReactor("list", "fleets", + func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, &agonesv1.FleetList{Items: []agonesv1.Fleet{*f}}, nil + }) + + m.AgonesClient.AddReactor("update", "fleets", + func(action k8stesting.Action) (bool, runtime.Object, error) { + ua := action.(k8stesting.UpdateAction) + fleet := ua.GetObject().(*agonesv1.Fleet) + updated = true + + switch flag { + case string(utilruntime.FeatureCountsAndLists) + "=true": + assert.Equal(t, gss.Status.Counters, fleet.Status.Counters) + assert.Equal(t, gss.Status.Lists, fleet.Status.Lists) + case string(utilruntime.FeatureCountsAndLists) + "=false": + assert.Nil(t, fleet.Status.Counters) + assert.Nil(t, fleet.Status.Lists) + default: + return false, fleet, errors.Errorf("Flag string(utilruntime.FeatureCountsAndLists) should be set") + } + return true, fleet, nil + }) + + ctx, cancel := agtesting.StartInformers(m, c.fleetSynced, c.gameServerSetSynced) + defer cancel() + + // Expect starting fleet to have Aggregated Counter and List Statuses + flag = string(utilruntime.FeatureCountsAndLists) + "=true" + require.NoError(t, utilruntime.ParseFeatures(flag)) + err := c.syncFleet(ctx, defaultFleetName) + assert.NoError(t, err) + assert.True(t, updated) + + updated = false + flag = string(utilruntime.FeatureCountsAndLists) + "=false" + require.NoError(t, utilruntime.ParseFeatures(flag)) + err = c.syncFleet(ctx, defaultFleetName) + assert.NoError(t, err) + assert.True(t, updated) + + updated = false + flag = string(utilruntime.FeatureCountsAndLists) + "=true" + require.NoError(t, utilruntime.ParseFeatures(flag)) + err = c.syncFleet(ctx, defaultFleetName) + assert.NoError(t, err) + assert.True(t, updated) +} + func TestControllerFilterGameServerSetByActive(t *testing.T) { t.Parallel() diff --git a/pkg/gameserversets/controller.go b/pkg/gameserversets/controller.go index 68f749be31..20a0a5f71f 100644 --- a/pkg/gameserversets/controller.go +++ b/pkg/gameserversets/controller.go @@ -635,6 +635,13 @@ func computeStatus(list []*agonesv1.GameServer) agonesv1.GameServerSetStatus { status.ReservedReplicas++ } + // Drop Counters and Lists status if the feature flag has been set to false + if !runtime.FeatureEnabled(runtime.FeatureCountsAndLists) { + if len(status.Counters) != 0 || len(status.Lists) != 0 { + status.Counters = map[string]agonesv1.AggregatedCounterStatus{} + status.Lists = map[string]agonesv1.AggregatedListStatus{} + } + } // Aggregates all Counters and Lists only for GameServer all states (except IsBeingDeleted) if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) { status.Counters = aggregateCounters(status.Counters, gs.Status.Counters, gs.Status.State) diff --git a/pkg/gameserversets/controller_test.go b/pkg/gameserversets/controller_test.go index c5ccb04b18..770eac36af 100644 --- a/pkg/gameserversets/controller_test.go +++ b/pkg/gameserversets/controller_test.go @@ -17,7 +17,6 @@ package gameserversets import ( "context" "encoding/json" - "errors" "fmt" "math/rand" "net/http" @@ -33,6 +32,7 @@ import ( utilruntime "agones.dev/agones/pkg/util/runtime" "agones.dev/agones/pkg/util/webhooks" "github.com/heptiolabs/healthcheck" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -464,6 +464,87 @@ func TestComputeStatus(t *testing.T) { }) } +// Test that the aggregated Counters and Lists are removed from the Game Server Set status if the +// FeatureCountsAndLists flag is set to false. +func TestGameServerSetDropCountsAndListsStatus(t *testing.T) { + t.Parallel() + utilruntime.FeatureTestMutex.Lock() + defer utilruntime.FeatureTestMutex.Unlock() + + gss := defaultFixture() + c, m := newFakeController() + + list := createGameServers(gss, 2) + list[0].Status.Counters = map[string]agonesv1.CounterStatus{ + "firstCounter": {Count: 5, Capacity: 10}, + } + list[1].Status.Lists = map[string]agonesv1.ListStatus{ + "firstList": {Capacity: 100, Values: []string{"4", "5", "6"}}, + } + gsList := []*agonesv1.GameServer{&list[0], &list[1]} + + expectedCounterStatus := map[string]agonesv1.AggregatedCounterStatus{ + "firstCounter": { + AllocatedCount: 0, + AllocatedCapacity: 0, + Capacity: 10, + Count: 5, + }, + } + expectedListStatus := map[string]agonesv1.AggregatedListStatus{ + "firstList": { + AllocatedCount: 0, + AllocatedCapacity: 0, + Capacity: 100, + Count: 3, + }, + } + + flag := "" + updated := false + + m.AgonesClient.AddReactor("update", "gameserversets", + func(action k8stesting.Action) (bool, runtime.Object, error) { + updated = true + ua := action.(k8stesting.UpdateAction) + gsSet := ua.GetObject().(*agonesv1.GameServerSet) + + switch flag { + case string(utilruntime.FeatureCountsAndLists) + "=true": + assert.Equal(t, expectedCounterStatus, gsSet.Status.Counters) + assert.Equal(t, expectedListStatus, gsSet.Status.Lists) + case string(utilruntime.FeatureCountsAndLists) + "=false": + assert.Nil(t, gsSet.Status.Counters) + assert.Nil(t, gsSet.Status.Lists) + default: + return false, nil, errors.Errorf("Flag string(utilruntime.FeatureCountsAndLists) should be set") + } + + return true, gsSet, nil + }) + + // Expect starting fleet to have Aggregated Counter and List Statuses + flag = string(utilruntime.FeatureCountsAndLists) + "=true" + require.NoError(t, utilruntime.ParseFeatures(flag)) + err := c.syncGameServerSetStatus(context.Background(), gss, gsList) + assert.Nil(t, err) + assert.True(t, updated) + + updated = false + flag = string(utilruntime.FeatureCountsAndLists) + "=false" + require.NoError(t, utilruntime.ParseFeatures(flag)) + err = c.syncGameServerSetStatus(context.Background(), gss, gsList) + assert.Nil(t, err) + assert.True(t, updated) + + updated = false + flag = string(utilruntime.FeatureCountsAndLists) + "=true" + require.NoError(t, utilruntime.ParseFeatures(flag)) + err = c.syncGameServerSetStatus(context.Background(), gss, gsList) + assert.Nil(t, err) + assert.True(t, updated) +} + func TestControllerWatchGameServers(t *testing.T) { t.Parallel() utilruntime.FeatureTestMutex.Lock()