From c8c1577ac159a42303bea99473741b4786675151 Mon Sep 17 00:00:00 2001 From: Kalaiselvi Murugesan Date: Tue, 23 Jan 2024 00:11:53 +0000 Subject: [PATCH 1/8] Prevent Int64 Overflow --- pkg/fleets/controller.go | 30 ++++++++++++++++++++++++++++-- pkg/gameserversets/controller.go | 29 +++++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/pkg/fleets/controller.go b/pkg/fleets/controller.go index a0cba7091e..a805d4d427 100644 --- a/pkg/fleets/controller.go +++ b/pkg/fleets/controller.go @@ -693,8 +693,18 @@ func (c *Controller) updateFleetStatus(ctx context.Context, fleet *agonesv1.Flee // TODO: integrate this extra loop into the above for loop when PlayerTracking moves to GA for _, gsSet := range list { if gsSet.Status.Players != nil { - fCopy.Status.Players.Count += gsSet.Status.Players.Count - fCopy.Status.Players.Capacity += gsSet.Status.Players.Capacity + fCopy.Status.Players.Count, err = SafeAddInt64(fCopy.Status.Players.Count, gsSet.Status.Players.Count) + if err != nil { + log.Printf("Error adding player counts: %v", err) + return err + } + + fCopy.Status.Players.Capacity, err = SafeAddInt64(fCopy.Status.Players.Capacity, gsSet.Status.Players.Capacity) + if err != nil { + log.Printf("Error adding player capacities: %v", err) + return err + } + } } } @@ -703,6 +713,22 @@ func (c *Controller) updateFleetStatus(ctx context.Context, fleet *agonesv1.Flee return errors.Wrapf(err, "error updating status of fleet %s", fCopy.ObjectMeta.Name) } +//int64 oveflow check +func SafeAddInt64(left, right int64) (int64, error) { + ErrOverflow := errors.New("integer overflow") + if right > 0 { + if left > math.MaxInt64-right { + return 0, ErrOverflow + } + } else { + if left < math.MinInt64-right { + return 0, ErrOverflow + } + } + return left + right, nil +} + + // filterGameServerSetByActive returns the active GameServerSet (or nil if it // doesn't exist) and then the rest of the GameServerSets that are controlled // by this Fleet diff --git a/pkg/gameserversets/controller.go b/pkg/gameserversets/controller.go index e5c098f41c..0b3496dbc3 100644 --- a/pkg/gameserversets/controller.go +++ b/pkg/gameserversets/controller.go @@ -33,6 +33,7 @@ import ( "agones.dev/agones/pkg/util/runtime" "agones.dev/agones/pkg/util/webhooks" "agones.dev/agones/pkg/util/workerqueue" + "agones.dev/agones/pkg/fleets" "github.com/google/go-cmp/cmp" "github.com/heptiolabs/healthcheck" "github.com/pkg/errors" @@ -679,12 +680,32 @@ func aggregateCounters(aggCounterStatus map[string]agonesv1.AggregatedCounterSta // If the Counter exists in both maps, aggregate the values. if counter, ok := aggCounterStatus[key]; ok { // Aggregate for all game server statuses (expected IsBeingDeleted) - counter.Count += val.Count - counter.Capacity += val.Capacity + counter.Count, err = fleets.SafeAddInt64(counter.Count + val.Count) + if err != nil { + log.Printf("Error adding counter count: %v", err) + return err + } + + counter.Capacity, err = fleets.SafeAddInt64(counter.Capacity + val.Capacity) + if err != nil { + log.Printf("Error adding counter capacity: %v", err) + return err + } + // Aggregate for Allocated game servers only if gsState == agonesv1.GameServerStateAllocated { - counter.AllocatedCount += val.Count - counter.AllocatedCapacity += val.Capacity + counter.AllocatedCount, err = fleets.SafeAddInt64(counter.AllocatedCount + val.Count) + if err != nil { + log.Printf("Error adding allocated count: %v", err) + return err + } + + counter.AllocatedCapacity, err = fleets.SafeAddInt64(counter.AllocatedCapacity + val.Capacity) + if err != nil { + log.Printf("Error adding allocated capacity: %v", err) + return err + } + } aggCounterStatus[key] = counter } else { From 9cfc3750a12f33b19e7fcc5f31037f8bef84b258 Mon Sep 17 00:00:00 2001 From: Kalaiselvi Murugesan Date: Tue, 23 Jan 2024 18:58:51 +0000 Subject: [PATCH 2/8] Necessary changes --- pkg/fleets/controller.go | 43 ++++++++++---------------------- pkg/gameserversets/controller.go | 29 ++++----------------- 2 files changed, 18 insertions(+), 54 deletions(-) diff --git a/pkg/fleets/controller.go b/pkg/fleets/controller.go index a805d4d427..02072cb640 100644 --- a/pkg/fleets/controller.go +++ b/pkg/fleets/controller.go @@ -18,6 +18,7 @@ import ( "context" "encoding/json" "fmt" + "math" "time" "agones.dev/agones/pkg/apis/agones" @@ -693,18 +694,8 @@ func (c *Controller) updateFleetStatus(ctx context.Context, fleet *agonesv1.Flee // TODO: integrate this extra loop into the above for loop when PlayerTracking moves to GA for _, gsSet := range list { if gsSet.Status.Players != nil { - fCopy.Status.Players.Count, err = SafeAddInt64(fCopy.Status.Players.Count, gsSet.Status.Players.Count) - if err != nil { - log.Printf("Error adding player counts: %v", err) - return err - } - - fCopy.Status.Players.Capacity, err = SafeAddInt64(fCopy.Status.Players.Capacity, gsSet.Status.Players.Capacity) - if err != nil { - log.Printf("Error adding player capacities: %v", err) - return err - } - + fCopy.Status.Players.Count += gsSet.Status.Players.Count + fCopy.Status.Players.Capacity += gsSet.Status.Players.Capacity } } } @@ -713,22 +704,6 @@ func (c *Controller) updateFleetStatus(ctx context.Context, fleet *agonesv1.Flee return errors.Wrapf(err, "error updating status of fleet %s", fCopy.ObjectMeta.Name) } -//int64 oveflow check -func SafeAddInt64(left, right int64) (int64, error) { - ErrOverflow := errors.New("integer overflow") - if right > 0 { - if left > math.MaxInt64-right { - return 0, ErrOverflow - } - } else { - if left < math.MinInt64-right { - return 0, ErrOverflow - } - } - return left + right, nil -} - - // filterGameServerSetByActive returns the active GameServerSet (or nil if it // doesn't exist) and then the rest of the GameServerSets that are controlled // by this Fleet @@ -747,6 +722,14 @@ func (c *Controller) filterGameServerSetByActive(fleet *agonesv1.Fleet, list []* return active, rest } +// SafeAdd prevents overflow by limiting the sum to math.MaxInt64. +func SafeAdd(x, y int64) int64 { + if x > math.MaxInt64-y { + return math.MaxInt64 + } + return x + y +} + // mergeCounters adds the contents of AggregatedCounterStatus c2 into c1. func mergeCounters(c1, c2 map[string]agonesv1.AggregatedCounterStatus) map[string]agonesv1.AggregatedCounterStatus { if c1 == nil { @@ -756,9 +739,9 @@ func mergeCounters(c1, c2 map[string]agonesv1.AggregatedCounterStatus) map[strin for key, val := range c2 { // If the Counter exists in both maps, aggregate the values. if counter, ok := c1[key]; ok { - counter.AllocatedCapacity += val.AllocatedCapacity + counter.AllocatedCapacity = SafeAdd(counter.AllocatedCapacity, val.AllocatedCapacity) counter.AllocatedCount += val.AllocatedCount - counter.Capacity += val.Capacity + counter.Capacity = SafeAdd(counter.Capacity, val.Capacity) counter.Count += val.Count c1[key] = counter } else { diff --git a/pkg/gameserversets/controller.go b/pkg/gameserversets/controller.go index 0b3496dbc3..96b79603fe 100644 --- a/pkg/gameserversets/controller.go +++ b/pkg/gameserversets/controller.go @@ -27,13 +27,13 @@ import ( getterv1 "agones.dev/agones/pkg/client/clientset/versioned/typed/agones/v1" "agones.dev/agones/pkg/client/informers/externalversions" listerv1 "agones.dev/agones/pkg/client/listers/agones/v1" + "agones.dev/agones/pkg/fleets" "agones.dev/agones/pkg/gameservers" "agones.dev/agones/pkg/util/crd" "agones.dev/agones/pkg/util/logfields" "agones.dev/agones/pkg/util/runtime" "agones.dev/agones/pkg/util/webhooks" "agones.dev/agones/pkg/util/workerqueue" - "agones.dev/agones/pkg/fleets" "github.com/google/go-cmp/cmp" "github.com/heptiolabs/healthcheck" "github.com/pkg/errors" @@ -680,32 +680,13 @@ func aggregateCounters(aggCounterStatus map[string]agonesv1.AggregatedCounterSta // If the Counter exists in both maps, aggregate the values. if counter, ok := aggCounterStatus[key]; ok { // Aggregate for all game server statuses (expected IsBeingDeleted) - counter.Count, err = fleets.SafeAddInt64(counter.Count + val.Count) - if err != nil { - log.Printf("Error adding counter count: %v", err) - return err - } - - counter.Capacity, err = fleets.SafeAddInt64(counter.Capacity + val.Capacity) - if err != nil { - log.Printf("Error adding counter capacity: %v", err) - return err - } + counter.Count += val.Count + counter.Capacity = fleets.SafeAdd(counter.Capacity, val.Capacity) // Aggregate for Allocated game servers only if gsState == agonesv1.GameServerStateAllocated { - counter.AllocatedCount, err = fleets.SafeAddInt64(counter.AllocatedCount + val.Count) - if err != nil { - log.Printf("Error adding allocated count: %v", err) - return err - } - - counter.AllocatedCapacity, err = fleets.SafeAddInt64(counter.AllocatedCapacity + val.Capacity) - if err != nil { - log.Printf("Error adding allocated capacity: %v", err) - return err - } - + counter.AllocatedCount += val.Count + counter.AllocatedCapacity = fleets.SafeAdd(counter.AllocatedCapacity, val.Capacity) } aggCounterStatus[key] = counter } else { From 8996c3382076efe42def45be80e8fc43acb4d012 Mon Sep 17 00:00:00 2001 From: Kalaiselvi Murugesan Date: Tue, 23 Jan 2024 19:11:33 +0000 Subject: [PATCH 3/8] Modify Counter count --- pkg/fleets/controller.go | 4 ++-- pkg/gameserversets/controller.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/fleets/controller.go b/pkg/fleets/controller.go index 02072cb640..d01310cd1b 100644 --- a/pkg/fleets/controller.go +++ b/pkg/fleets/controller.go @@ -740,9 +740,9 @@ func mergeCounters(c1, c2 map[string]agonesv1.AggregatedCounterStatus) map[strin // If the Counter exists in both maps, aggregate the values. if counter, ok := c1[key]; ok { counter.AllocatedCapacity = SafeAdd(counter.AllocatedCapacity, val.AllocatedCapacity) - counter.AllocatedCount += val.AllocatedCount + counter.AllocatedCount = SafeAdd(counter.AllocatedCount, val.AllocatedCount) counter.Capacity = SafeAdd(counter.Capacity, val.Capacity) - counter.Count += val.Count + counter.Count = SafeAdd(counter.Count, val.Count) c1[key] = counter } else { c1[key] = *val.DeepCopy() diff --git a/pkg/gameserversets/controller.go b/pkg/gameserversets/controller.go index 96b79603fe..3ccb6aa160 100644 --- a/pkg/gameserversets/controller.go +++ b/pkg/gameserversets/controller.go @@ -680,12 +680,12 @@ func aggregateCounters(aggCounterStatus map[string]agonesv1.AggregatedCounterSta // If the Counter exists in both maps, aggregate the values. if counter, ok := aggCounterStatus[key]; ok { // Aggregate for all game server statuses (expected IsBeingDeleted) - counter.Count += val.Count + counter.Count = fleets.SafeAdd(counter.Count, val.Count) counter.Capacity = fleets.SafeAdd(counter.Capacity, val.Capacity) // Aggregate for Allocated game servers only if gsState == agonesv1.GameServerStateAllocated { - counter.AllocatedCount += val.Count + counter.AllocatedCount = fleets.SafeAdd(counter.AllocatedCount, val.Count) counter.AllocatedCapacity = fleets.SafeAdd(counter.AllocatedCapacity, val.Capacity) } aggCounterStatus[key] = counter From a18ece47cddbace2d0ed6f133a863b00e2ada1e5 Mon Sep 17 00:00:00 2001 From: Kalaiselvi Murugesan Date: Tue, 23 Jan 2024 21:50:32 +0000 Subject: [PATCH 4/8] Review changes --- pkg/apis/agones/v1/common.go | 10 ++++++++++ pkg/fleets/controller.go | 17 ++++------------- pkg/gameserversets/controller.go | 9 ++++----- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/pkg/apis/agones/v1/common.go b/pkg/apis/agones/v1/common.go index 4d40f6622f..9e709e0b0b 100644 --- a/pkg/apis/agones/v1/common.go +++ b/pkg/apis/agones/v1/common.go @@ -15,6 +15,8 @@ package v1 import ( + "math" + apivalidation "k8s.io/apimachinery/pkg/api/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" @@ -183,3 +185,11 @@ type Priority struct { // "Ascending" would be smaller Capacity is preferred. Order string `json:"order"` } + +// SafeAdd prevents overflow by limiting the sum to math.MaxInt64. +func SafeAdd(x, y int64) int64 { + if x > math.MaxInt64-y { + return math.MaxInt64 + } + return x + y +} diff --git a/pkg/fleets/controller.go b/pkg/fleets/controller.go index d01310cd1b..37f7f227a3 100644 --- a/pkg/fleets/controller.go +++ b/pkg/fleets/controller.go @@ -18,7 +18,6 @@ import ( "context" "encoding/json" "fmt" - "math" "time" "agones.dev/agones/pkg/apis/agones" @@ -722,14 +721,6 @@ func (c *Controller) filterGameServerSetByActive(fleet *agonesv1.Fleet, list []* return active, rest } -// SafeAdd prevents overflow by limiting the sum to math.MaxInt64. -func SafeAdd(x, y int64) int64 { - if x > math.MaxInt64-y { - return math.MaxInt64 - } - return x + y -} - // mergeCounters adds the contents of AggregatedCounterStatus c2 into c1. func mergeCounters(c1, c2 map[string]agonesv1.AggregatedCounterStatus) map[string]agonesv1.AggregatedCounterStatus { if c1 == nil { @@ -739,10 +730,10 @@ func mergeCounters(c1, c2 map[string]agonesv1.AggregatedCounterStatus) map[strin for key, val := range c2 { // If the Counter exists in both maps, aggregate the values. if counter, ok := c1[key]; ok { - counter.AllocatedCapacity = SafeAdd(counter.AllocatedCapacity, val.AllocatedCapacity) - counter.AllocatedCount = SafeAdd(counter.AllocatedCount, val.AllocatedCount) - counter.Capacity = SafeAdd(counter.Capacity, val.Capacity) - counter.Count = SafeAdd(counter.Count, val.Count) + counter.AllocatedCapacity = agonesv1.SafeAdd(counter.AllocatedCapacity, val.AllocatedCapacity) + counter.AllocatedCount = agonesv1.SafeAdd(counter.AllocatedCount, val.AllocatedCount) + counter.Capacity = agonesv1.SafeAdd(counter.Capacity, val.Capacity) + counter.Count = agonesv1.SafeAdd(counter.Count, val.Count) c1[key] = counter } else { c1[key] = *val.DeepCopy() diff --git a/pkg/gameserversets/controller.go b/pkg/gameserversets/controller.go index 3ccb6aa160..8bf4c22afa 100644 --- a/pkg/gameserversets/controller.go +++ b/pkg/gameserversets/controller.go @@ -27,7 +27,6 @@ import ( getterv1 "agones.dev/agones/pkg/client/clientset/versioned/typed/agones/v1" "agones.dev/agones/pkg/client/informers/externalversions" listerv1 "agones.dev/agones/pkg/client/listers/agones/v1" - "agones.dev/agones/pkg/fleets" "agones.dev/agones/pkg/gameservers" "agones.dev/agones/pkg/util/crd" "agones.dev/agones/pkg/util/logfields" @@ -680,13 +679,13 @@ func aggregateCounters(aggCounterStatus map[string]agonesv1.AggregatedCounterSta // If the Counter exists in both maps, aggregate the values. if counter, ok := aggCounterStatus[key]; ok { // Aggregate for all game server statuses (expected IsBeingDeleted) - counter.Count = fleets.SafeAdd(counter.Count, val.Count) - counter.Capacity = fleets.SafeAdd(counter.Capacity, val.Capacity) + counter.Count = agonesv1.SafeAdd(counter.Count, val.Count) + counter.Capacity = agonesv1.SafeAdd(counter.Capacity, val.Capacity) // Aggregate for Allocated game servers only if gsState == agonesv1.GameServerStateAllocated { - counter.AllocatedCount = fleets.SafeAdd(counter.AllocatedCount, val.Count) - counter.AllocatedCapacity = fleets.SafeAdd(counter.AllocatedCapacity, val.Capacity) + counter.AllocatedCount = agonesv1.SafeAdd(counter.AllocatedCount, val.Count) + counter.AllocatedCapacity = agonesv1.SafeAdd(counter.AllocatedCapacity, val.Capacity) } aggCounterStatus[key] = counter } else { From cf2a0fb114b189f212b4006eb86dbdc1745e0fda Mon Sep 17 00:00:00 2001 From: Kalaiselvi Murugesan Date: Tue, 23 Jan 2024 22:48:45 +0000 Subject: [PATCH 5/8] update unit test --- pkg/fleets/controller_test.go | 28 +++++++++++++-------------- pkg/gameserversets/controller_test.go | 16 +++++++-------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/pkg/fleets/controller_test.go b/pkg/fleets/controller_test.go index 6ee68b1ea5..54a8b76ff0 100644 --- a/pkg/fleets/controller_test.go +++ b/pkg/fleets/controller_test.go @@ -752,20 +752,20 @@ func TestControllerUpdateFleetCounterStatus(t *testing.T) { ua := action.(k8stesting.UpdateAction) fleet := ua.GetObject().(*agonesv1.Fleet) - assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].AllocatedCount) - assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].AllocatedCapacity) - assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].Capacity) - assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].Count) - - assert.Equal(t, int64(11), fleet.Status.Counters["anotherCounter"].AllocatedCount) - assert.Equal(t, int64(20), fleet.Status.Counters["anotherCounter"].AllocatedCapacity) - assert.Equal(t, int64(200), fleet.Status.Counters["anotherCounter"].Capacity) - assert.Equal(t, int64(42), fleet.Status.Counters["anotherCounter"].Count) - - assert.Equal(t, int64(21), fleet.Status.Counters["thirdCounter"].AllocatedCount) - assert.Equal(t, int64(30), fleet.Status.Counters["thirdCounter"].AllocatedCapacity) - assert.Equal(t, int64(400), fleet.Status.Counters["thirdCounter"].Capacity) - assert.Equal(t, int64(21), fleet.Status.Counters["thirdCounter"].Count) + assert.Equal(t, agonesv1.SafeAdd(1000, 100), fleet.Status.Counters["fullCounter"].AllocatedCount) + assert.Equal(t, agonesv1.SafeAdd(1000, 100), fleet.Status.Counters["fullCounter"].AllocatedCapacity) + assert.Equal(t, agonesv1.SafeAdd(1000, 100), fleet.Status.Counters["fullCounter"].Capacity) + assert.Equal(t, agonesv1.SafeAdd(1000, 100), fleet.Status.Counters["fullCounter"].Count) + + assert.Equal(t, agonesv1.SafeAdd(10, 1), fleet.Status.Counters["anotherCounter"].AllocatedCount) + assert.Equal(t, agonesv1.SafeAdd(15, 5), fleet.Status.Counters["anotherCounter"].AllocatedCapacity) + assert.Equal(t, agonesv1.SafeAdd(150, 50), fleet.Status.Counters["anotherCounter"].Capacity) + assert.Equal(t, agonesv1.SafeAdd(40, 2), fleet.Status.Counters["anotherCounter"].Count) + + assert.Equal(t, agonesv1.SafeAdd(21, 0), fleet.Status.Counters["thirdCounter"].AllocatedCount) + assert.Equal(t, agonesv1.SafeAdd(25, 5), fleet.Status.Counters["thirdCounter"].AllocatedCapacity) + assert.Equal(t, agonesv1.SafeAdd(200, 200), fleet.Status.Counters["thirdCounter"].Capacity) + assert.Equal(t, agonesv1.SafeAdd(20, 1), fleet.Status.Counters["thirdCounter"].Count) return true, fleet, nil }) diff --git a/pkg/gameserversets/controller_test.go b/pkg/gameserversets/controller_test.go index b986086d7c..adda37d74b 100644 --- a/pkg/gameserversets/controller_test.go +++ b/pkg/gameserversets/controller_test.go @@ -380,16 +380,16 @@ func TestComputeStatus(t *testing.T) { AllocatedReplicas: 1, Counters: map[string]agonesv1.AggregatedCounterStatus{ "firstCounter": { - AllocatedCount: 5, - AllocatedCapacity: 10, - Count: 50, - Capacity: 85, + AllocatedCount: agonesv1.SafeAdd(5, 0), + AllocatedCapacity: agonesv1.SafeAdd(5, 5), + Count: agonesv1.SafeAdd(40, 10), + Capacity: agonesv1.SafeAdd(55, 30), }, "secondCounter": { - AllocatedCount: 100, - AllocatedCapacity: 1000, - Count: 220, - Capacity: 2200, + AllocatedCount: agonesv1.SafeAdd(50, 50), + AllocatedCapacity: agonesv1.SafeAdd(500, 500), + Count: agonesv1.SafeAdd(200, 20), + Capacity: agonesv1.SafeAdd(1000, 1200), }, }, Lists: map[string]agonesv1.AggregatedListStatus{}, From 0c6a587c0a2d304852318a5304ce52c92db815f7 Mon Sep 17 00:00:00 2001 From: Kalaiselvi Murugesan Date: Wed, 24 Jan 2024 04:39:32 +0000 Subject: [PATCH 6/8] use big.Int --- pkg/apis/agones/v1/common.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/pkg/apis/agones/v1/common.go b/pkg/apis/agones/v1/common.go index 9e709e0b0b..4926cb8136 100644 --- a/pkg/apis/agones/v1/common.go +++ b/pkg/apis/agones/v1/common.go @@ -16,6 +16,7 @@ package v1 import ( "math" + "math/big" apivalidation "k8s.io/apimachinery/pkg/api/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -187,9 +188,22 @@ type Priority struct { } // SafeAdd prevents overflow by limiting the sum to math.MaxInt64. +// func SafeAdd(x, y int64) int64 { +// if x > math.MaxInt64-y { +// return math.MaxInt64 +// } +// return x + y +// } + +// SafeAdd adds two int64 values using big.Int to prevent overflow. +// Returns the result as int64. If the result exceeds int64 range, returns math.MaxInt64. func SafeAdd(x, y int64) int64 { - if x > math.MaxInt64-y { - return math.MaxInt64 + num1 := big.NewInt(x) + num2 := big.NewInt(y) + sum := new(big.Int).Add(num1, num2) + + if sum.IsInt64() { + return sum.Int64() } - return x + y + return math.MaxInt64 } From 5675fd8514f7f0b56f08d43371a2cd0feed1be5e Mon Sep 17 00:00:00 2001 From: Kalaiselvi Murugesan Date: Wed, 24 Jan 2024 17:43:10 +0000 Subject: [PATCH 7/8] Review changes --- pkg/apis/agones/v1/common.go | 20 +++---------------- pkg/fleets/controller_test.go | 28 +++++++++++++-------------- pkg/gameserversets/controller_test.go | 26 +++++++++++++++++-------- 3 files changed, 35 insertions(+), 39 deletions(-) diff --git a/pkg/apis/agones/v1/common.go b/pkg/apis/agones/v1/common.go index 4926cb8136..9e709e0b0b 100644 --- a/pkg/apis/agones/v1/common.go +++ b/pkg/apis/agones/v1/common.go @@ -16,7 +16,6 @@ package v1 import ( "math" - "math/big" apivalidation "k8s.io/apimachinery/pkg/api/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -188,22 +187,9 @@ type Priority struct { } // SafeAdd prevents overflow by limiting the sum to math.MaxInt64. -// func SafeAdd(x, y int64) int64 { -// if x > math.MaxInt64-y { -// return math.MaxInt64 -// } -// return x + y -// } - -// SafeAdd adds two int64 values using big.Int to prevent overflow. -// Returns the result as int64. If the result exceeds int64 range, returns math.MaxInt64. func SafeAdd(x, y int64) int64 { - num1 := big.NewInt(x) - num2 := big.NewInt(y) - sum := new(big.Int).Add(num1, num2) - - if sum.IsInt64() { - return sum.Int64() + if x > math.MaxInt64-y { + return math.MaxInt64 } - return math.MaxInt64 + return x + y } diff --git a/pkg/fleets/controller_test.go b/pkg/fleets/controller_test.go index 54a8b76ff0..6ee68b1ea5 100644 --- a/pkg/fleets/controller_test.go +++ b/pkg/fleets/controller_test.go @@ -752,20 +752,20 @@ func TestControllerUpdateFleetCounterStatus(t *testing.T) { ua := action.(k8stesting.UpdateAction) fleet := ua.GetObject().(*agonesv1.Fleet) - assert.Equal(t, agonesv1.SafeAdd(1000, 100), fleet.Status.Counters["fullCounter"].AllocatedCount) - assert.Equal(t, agonesv1.SafeAdd(1000, 100), fleet.Status.Counters["fullCounter"].AllocatedCapacity) - assert.Equal(t, agonesv1.SafeAdd(1000, 100), fleet.Status.Counters["fullCounter"].Capacity) - assert.Equal(t, agonesv1.SafeAdd(1000, 100), fleet.Status.Counters["fullCounter"].Count) - - assert.Equal(t, agonesv1.SafeAdd(10, 1), fleet.Status.Counters["anotherCounter"].AllocatedCount) - assert.Equal(t, agonesv1.SafeAdd(15, 5), fleet.Status.Counters["anotherCounter"].AllocatedCapacity) - assert.Equal(t, agonesv1.SafeAdd(150, 50), fleet.Status.Counters["anotherCounter"].Capacity) - assert.Equal(t, agonesv1.SafeAdd(40, 2), fleet.Status.Counters["anotherCounter"].Count) - - assert.Equal(t, agonesv1.SafeAdd(21, 0), fleet.Status.Counters["thirdCounter"].AllocatedCount) - assert.Equal(t, agonesv1.SafeAdd(25, 5), fleet.Status.Counters["thirdCounter"].AllocatedCapacity) - assert.Equal(t, agonesv1.SafeAdd(200, 200), fleet.Status.Counters["thirdCounter"].Capacity) - assert.Equal(t, agonesv1.SafeAdd(20, 1), fleet.Status.Counters["thirdCounter"].Count) + assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].AllocatedCount) + assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].AllocatedCapacity) + assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].Capacity) + assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].Count) + + assert.Equal(t, int64(11), fleet.Status.Counters["anotherCounter"].AllocatedCount) + assert.Equal(t, int64(20), fleet.Status.Counters["anotherCounter"].AllocatedCapacity) + assert.Equal(t, int64(200), fleet.Status.Counters["anotherCounter"].Capacity) + assert.Equal(t, int64(42), fleet.Status.Counters["anotherCounter"].Count) + + assert.Equal(t, int64(21), fleet.Status.Counters["thirdCounter"].AllocatedCount) + assert.Equal(t, int64(30), fleet.Status.Counters["thirdCounter"].AllocatedCapacity) + assert.Equal(t, int64(400), fleet.Status.Counters["thirdCounter"].Capacity) + assert.Equal(t, int64(21), fleet.Status.Counters["thirdCounter"].Count) return true, fleet, nil }) diff --git a/pkg/gameserversets/controller_test.go b/pkg/gameserversets/controller_test.go index adda37d74b..bb1b09ff2c 100644 --- a/pkg/gameserversets/controller_test.go +++ b/pkg/gameserversets/controller_test.go @@ -356,20 +356,24 @@ func TestComputeStatus(t *testing.T) { gs1.Status.Counters = map[string]agonesv1.CounterStatus{ "firstCounter": {Count: 5, Capacity: 10}, "secondCounter": {Count: 100, Capacity: 1000}, + "fullCounter": {Count: 9223372036854775807, Capacity: 9223372036854775807}, } gs2 := gsWithState(agonesv1.GameServerStateReserved) gs2.Status.Counters = map[string]agonesv1.CounterStatus{ "firstCounter": {Count: 10, Capacity: 15}, + "fullCounter": {Count: 9223372036854775807, Capacity: 9223372036854775807}, } gs3 := gsWithState(agonesv1.GameServerStateCreating) gs3.Status.Counters = map[string]agonesv1.CounterStatus{ "firstCounter": {Count: 20, Capacity: 30}, "secondCounter": {Count: 100, Capacity: 1000}, + "fullCounter": {Count: 9223372036854775807, Capacity: 9223372036854775807}, } gs4 := gsWithState(agonesv1.GameServerStateReady) gs4.Status.Counters = map[string]agonesv1.CounterStatus{ "firstCounter": {Count: 15, Capacity: 30}, "secondCounter": {Count: 20, Capacity: 200}, + "fullCounter": {Count: 9223372036854775807, Capacity: 9223372036854775807}, } list = append(list, gs1, gs2, gs3, gs4) @@ -380,16 +384,22 @@ func TestComputeStatus(t *testing.T) { AllocatedReplicas: 1, Counters: map[string]agonesv1.AggregatedCounterStatus{ "firstCounter": { - AllocatedCount: agonesv1.SafeAdd(5, 0), - AllocatedCapacity: agonesv1.SafeAdd(5, 5), - Count: agonesv1.SafeAdd(40, 10), - Capacity: agonesv1.SafeAdd(55, 30), + AllocatedCount: 5, + AllocatedCapacity: 10, + Count: 50, + Capacity: 85, }, "secondCounter": { - AllocatedCount: agonesv1.SafeAdd(50, 50), - AllocatedCapacity: agonesv1.SafeAdd(500, 500), - Count: agonesv1.SafeAdd(200, 20), - Capacity: agonesv1.SafeAdd(1000, 1200), + AllocatedCount: 100, + AllocatedCapacity: 1000, + Count: 220, + Capacity: 2200, + }, + "fullCounter": { + AllocatedCount: 9223372036854775807, + AllocatedCapacity: 9223372036854775807, + Count: 9223372036854775807, + Capacity: 9223372036854775807, }, }, Lists: map[string]agonesv1.AggregatedListStatus{}, From 62ffce63ba908a7ba4c98fbc9e769b3eaf87d234 Mon Sep 17 00:00:00 2001 From: Kalaiselvi Murugesan Date: Wed, 24 Jan 2024 17:55:01 +0000 Subject: [PATCH 8/8] changes in controller_test.go --- pkg/fleets/controller_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/fleets/controller_test.go b/pkg/fleets/controller_test.go index 6ee68b1ea5..5b4f4909d4 100644 --- a/pkg/fleets/controller_test.go +++ b/pkg/fleets/controller_test.go @@ -704,10 +704,10 @@ func TestControllerUpdateFleetCounterStatus(t *testing.T) { gsSet1.ObjectMeta.Name = "gsSet1" gsSet1.Status.Counters = map[string]agonesv1.AggregatedCounterStatus{ "fullCounter": { - AllocatedCount: 1000, - AllocatedCapacity: 1000, - Capacity: 1000, - Count: 1000, + AllocatedCount: 9223372036854775807, + AllocatedCapacity: 9223372036854775807, + Capacity: 9223372036854775807, + Count: 9223372036854775807, }, "anotherCounter": { AllocatedCount: 11, @@ -752,10 +752,10 @@ func TestControllerUpdateFleetCounterStatus(t *testing.T) { ua := action.(k8stesting.UpdateAction) fleet := ua.GetObject().(*agonesv1.Fleet) - assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].AllocatedCount) - assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].AllocatedCapacity) - assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].Capacity) - assert.Equal(t, int64(1100), fleet.Status.Counters["fullCounter"].Count) + assert.Equal(t, int64(9223372036854775807), fleet.Status.Counters["fullCounter"].AllocatedCount) + assert.Equal(t, int64(9223372036854775807), fleet.Status.Counters["fullCounter"].AllocatedCapacity) + assert.Equal(t, int64(9223372036854775807), fleet.Status.Counters["fullCounter"].Capacity) + assert.Equal(t, int64(9223372036854775807), fleet.Status.Counters["fullCounter"].Count) assert.Equal(t, int64(11), fleet.Status.Counters["anotherCounter"].AllocatedCount) assert.Equal(t, int64(20), fleet.Status.Counters["anotherCounter"].AllocatedCapacity)