Skip to content

Commit

Permalink
Move ResetMetricsOnDelete to Stable (#3518)
Browse files Browse the repository at this point in the history 
* Move ResetMetricsOnDelete to Stable

* Modify controller_test.go

* changes

* Delete test functions to pass when condition was false
  • Loading branch information
@Kalaiselvi84
Kalaiselvi84 authored Nov 22, 2023
1 parent ddadfe7 commit 018dc20
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 184 deletions.
4 changes: 2 additions & 2 deletions cloudbuild.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,10 +296,10 @@ steps:
region=${versionsAndRegions[$version]}
if [ $cloudProduct = generic ]
then
featureWithGate="PlayerAllocationFilter=true&PlayerTracking=true&ResetMetricsOnDelete=false&SplitControllerAndExtensions=false&FleetAllocationOverflow=false&CountsAndLists=true&DisableResyncOnSDKServer=true&Example=true"
featureWithGate="PlayerAllocationFilter=true&PlayerTracking=true&SplitControllerAndExtensions=false&FleetAllocationOverflow=false&CountsAndLists=true&DisableResyncOnSDKServer=true&Example=true"
testCluster="standard-e2e-test-cluster-${version//./-}"
else
featureWithGate="PlayerAllocationFilter=true&PlayerTracking=true&ResetMetricsOnDelete=false&SplitControllerAndExtensions=true&FleetAllocationOverflow=false&CountsAndLists=true&DisableResyncOnSDKServer=true&Example=true"
featureWithGate="PlayerAllocationFilter=true&PlayerTracking=true&SplitControllerAndExtensions=true&FleetAllocationOverflow=false&CountsAndLists=true&DisableResyncOnSDKServer=true&Example=true"
testCluster="gke-autopilot-e2e-test-cluster-${version//./-}"
fi
featureWithoutGate=""
Expand Down
1 change: 0 additions & 1 deletion install/helm/agones/defaultfeaturegates.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

# Beta features
FleetAllocationOverflow: true
ResetMetricsOnDelete: true
SplitControllerAndExtensions: true

# Alpha features
Expand Down
34 changes: 9 additions & 25 deletions pkg/metrics/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,25 +217,13 @@ func (c *Controller) recordFleetAutoScalerChanges(old, next interface{}) {
}

func (c *Controller) recordFleetAutoScalerDeletion(obj interface{}) {
fas, ok := obj.(*autoscalingv1.FleetAutoscaler)
_, ok := obj.(*autoscalingv1.FleetAutoscaler)
if !ok {
return
}

if runtime.FeatureEnabled(runtime.FeatureResetMetricsOnDelete) {
if err := c.resyncFleetAutoScaler(); err != nil {
c.logger.WithError(err).Warn("Could not resync Fleet Autoscaler metrics")
}
} else {
ctx, _ := tag.New(context.Background(), tag.Upsert(keyName, fas.Name),
tag.Upsert(keyFleetName, fas.Spec.FleetName), tag.Upsert(keyNamespace, fas.Namespace))

// recording status
stats.Record(ctx,
fasCurrentReplicasStats.M(int64(0)),
fasDesiredReplicasStats.M(int64(0)),
fasAbleToScaleStats.M(int64(0)),
fasLimitedStats.M(int64(0)))
if err := c.resyncFleetAutoScaler(); err != nil {
c.logger.WithError(err).Warn("Could not resync Fleet Autoscaler metrics")
}
}

Expand All @@ -256,20 +244,16 @@ func (c *Controller) recordFleetChanges(obj interface{}) {
}

func (c *Controller) recordFleetDeletion(obj interface{}) {
f, ok := obj.(*agonesv1.Fleet)
_, ok := obj.(*agonesv1.Fleet)
if !ok {
return
}

if runtime.FeatureEnabled(runtime.FeatureResetMetricsOnDelete) {
if err := c.resyncFleets(); err != nil {
// If for some reason resync fails, the entire metric state for fleets
// will be reset whenever the next Fleet gets deleted, in which case
// we end up back in a healthy state - so we aren't going to actively retry.
c.logger.WithError(err).Warn("Could not resync Fleet Metrics")
}
} else {
c.recordFleetReplicas(f.Name, f.Namespace, 0, 0, 0, 0, 0)
if err := c.resyncFleets(); err != nil {
// If for some reason resync fails, the entire metric state for fleets
// will be reset whenever the next Fleet gets deleted, in which case
// we end up back in a healthy state - so we aren't going to actively retry.
c.logger.WithError(err).Warn("Could not resync Fleet Metrics")
}
}

Expand Down
149 changes: 0 additions & 149 deletions pkg/metrics/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package metrics

import (
"context"
"fmt"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -289,66 +288,9 @@ func TestControllerGameServersTotal(t *testing.T) {
})
}

func TestControllerFleetReplicasCount(t *testing.T) {
runtime.FeatureTestMutex.Lock()
defer runtime.FeatureTestMutex.Unlock()
require.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=false", runtime.FeatureResetMetricsOnDelete)))

resetMetrics()
exporter := &metricExporter{}
reader := metricexport.NewReader()
c := newFakeController()
defer c.close()
c.run(t)

f := fleet("fleet-test", 8, 2, 5, 1, 1)
fd := fleet("fleet-deleted", 100, 100, 100, 100, 100)
c.fleetWatch.Add(f)
f = f.DeepCopy()
f.Status.ReadyReplicas = 1
f.Spec.Replicas = 5
c.fleetWatch.Modify(f)
c.fleetWatch.Add(fd)
c.fleetWatch.Delete(fd)

// wait until we have a fleet deleted and it's allocation count is 0
// since that is our last operation
require.Eventually(t, func() bool {
ex := &metricExporter{}
reader.ReadAndExport(ex)

for _, m := range ex.metrics {
if m.Descriptor.Name == fleetReplicaCountName {
for _, d := range m.TimeSeries {
if d.LabelValues[0].Value == "fleet-deleted" && d.LabelValues[1].Value == defaultNs && d.LabelValues[2].Value == "total" {
return d.Points[0].Value == int64(0)
}
}
}
}

return false
}, 5*time.Second, time.Second)

reader.ReadAndExport(exporter)
assertMetricData(t, exporter, fleetReplicaCountName, []expectedMetricData{
{labels: []string{"fleet-deleted", defaultNs, "reserved"}, val: int64(0)},
{labels: []string{"fleet-deleted", defaultNs, "allocated"}, val: int64(0)},
{labels: []string{"fleet-deleted", defaultNs, "desired"}, val: int64(0)},
{labels: []string{"fleet-deleted", defaultNs, "ready"}, val: int64(0)},
{labels: []string{"fleet-deleted", defaultNs, "total"}, val: int64(0)},
{labels: []string{"fleet-test", defaultNs, "reserved"}, val: int64(1)},
{labels: []string{"fleet-test", defaultNs, "allocated"}, val: int64(2)},
{labels: []string{"fleet-test", defaultNs, "desired"}, val: int64(5)},
{labels: []string{"fleet-test", defaultNs, "ready"}, val: int64(1)},
{labels: []string{"fleet-test", defaultNs, "total"}, val: int64(8)},
})
}

func TestControllerFleetReplicasCount_ResetMetricsOnDelete(t *testing.T) {
runtime.FeatureTestMutex.Lock()
defer runtime.FeatureTestMutex.Unlock()
require.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true", runtime.FeatureResetMetricsOnDelete)))

resetMetrics()
exporter := &metricExporter{}
Expand Down Expand Up @@ -396,100 +338,9 @@ func TestControllerFleetReplicasCount_ResetMetricsOnDelete(t *testing.T) {
})
}

func TestControllerFleetAutoScalerState(t *testing.T) {
runtime.FeatureTestMutex.Lock()
defer runtime.FeatureTestMutex.Unlock()
require.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=false", runtime.FeatureResetMetricsOnDelete)))

resetMetrics()
exporter := &metricExporter{}
reader := metricexport.NewReader()
c := newFakeController()
defer c.close()
c.run(t)

// testing fleet name change
fasFleetNameChange := fleetAutoScaler("first-fleet", "name-switch")
c.fasWatch.Add(fasFleetNameChange)
fasFleetNameChange = fasFleetNameChange.DeepCopy()
fasFleetNameChange.Spec.Policy.Buffer.BufferSize = intstr.FromInt(10)
fasFleetNameChange.Spec.Policy.Buffer.MaxReplicas = 50
fasFleetNameChange.Spec.Policy.Buffer.MinReplicas = 10
fasFleetNameChange.Status.CurrentReplicas = 20
fasFleetNameChange.Status.DesiredReplicas = 10
fasFleetNameChange.Status.ScalingLimited = true
c.fasWatch.Modify(fasFleetNameChange)
fasFleetNameChange = fasFleetNameChange.DeepCopy()
fasFleetNameChange.Spec.FleetName = "second-fleet"
c.fasWatch.Modify(fasFleetNameChange)
// testing deletion
fasDeleted := fleetAutoScaler("deleted-fleet", "deleted")
fasDeleted.Spec.Policy.Buffer.BufferSize = intstr.FromString("50%")
fasDeleted.Spec.Policy.Buffer.MaxReplicas = 150
fasDeleted.Spec.Policy.Buffer.MinReplicas = 15
c.fasWatch.Add(fasDeleted)
c.fasWatch.Delete(fasDeleted)

c.sync()
// wait until we have a fleet deleted and it's allocation count is 0
// since that is our last operation
require.Eventually(t, func() bool {
ex := &metricExporter{}
reader.ReadAndExport(ex)

for _, m := range ex.metrics {
if m.Descriptor.Name == fleetAutoscalersLimitedName {
for _, d := range m.TimeSeries {
if d.LabelValues[0].Value == "deleted-fleet" && d.LabelValues[1].Value == "deleted" && d.LabelValues[2].Value == defaultNs {
return d.Points[0].Value == int64(0)
}
}
}
}

return false
}, 5*time.Second, time.Second)

reader.ReadAndExport(exporter)
assertMetricData(t, exporter, fleetAutoscalersAbleToScaleName, []expectedMetricData{
{labels: []string{"first-fleet", "name-switch", defaultNs}, val: int64(0)},
{labels: []string{"second-fleet", "name-switch", defaultNs}, val: int64(1)},
{labels: []string{"deleted-fleet", "deleted", defaultNs}, val: int64(0)},
})
assertMetricData(t, exporter, fleetAutoscalerBufferLimitName, []expectedMetricData{
{labels: []string{"first-fleet", "name-switch", defaultNs, "max"}, val: int64(50)},
{labels: []string{"first-fleet", "name-switch", defaultNs, "min"}, val: int64(10)},
{labels: []string{"second-fleet", "name-switch", defaultNs, "max"}, val: int64(50)},
{labels: []string{"second-fleet", "name-switch", defaultNs, "min"}, val: int64(10)},
{labels: []string{"deleted-fleet", "deleted", defaultNs, "max"}, val: int64(150)},
{labels: []string{"deleted-fleet", "deleted", defaultNs, "min"}, val: int64(15)},
})
assertMetricData(t, exporter, fleetAutoscalterBufferSizeName, []expectedMetricData{
{labels: []string{"first-fleet", "name-switch", defaultNs, "count"}, val: int64(10)},
{labels: []string{"second-fleet", "name-switch", defaultNs, "count"}, val: int64(10)},
{labels: []string{"deleted-fleet", "deleted", defaultNs, "percentage"}, val: int64(50)},
})
assertMetricData(t, exporter, fleetAutoscalerCurrentReplicaCountName, []expectedMetricData{
{labels: []string{"first-fleet", "name-switch", defaultNs}, val: int64(0)},
{labels: []string{"second-fleet", "name-switch", defaultNs}, val: int64(20)},
{labels: []string{"deleted-fleet", "deleted", defaultNs}, val: int64(0)},
})
assertMetricData(t, exporter, fleetAutoscalersDesiredReplicaCountName, []expectedMetricData{
{labels: []string{"first-fleet", "name-switch", defaultNs}, val: int64(0)},
{labels: []string{"second-fleet", "name-switch", defaultNs}, val: int64(10)},
{labels: []string{"deleted-fleet", "deleted", defaultNs}, val: int64(0)},
})
assertMetricData(t, exporter, fleetAutoscalersLimitedName, []expectedMetricData{
{labels: []string{"first-fleet", "name-switch", defaultNs}, val: int64(0)},
{labels: []string{"second-fleet", "name-switch", defaultNs}, val: int64(1)},
{labels: []string{"deleted-fleet", "deleted", defaultNs}, val: int64(0)},
})
}

func TestControllerFleetAutoScalerState_ResetMetricsOnDelete(t *testing.T) {
runtime.FeatureTestMutex.Lock()
defer runtime.FeatureTestMutex.Unlock()
require.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true", runtime.FeatureResetMetricsOnDelete)))

resetMetrics()
exporter := &metricExporter{}
Expand Down
5 changes: 0 additions & 5 deletions pkg/util/runtime/features.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,6 @@ const (
// FeaturePlayerTracking is a feature flag to enable/disable player tracking features.
FeaturePlayerTracking Feature = "PlayerTracking"

// FeatureResetMetricsOnDelete is a feature flag that tells the metrics service to unregister and register
// relevant metric views to reset their state immediately when an Agones resource is deleted.
FeatureResetMetricsOnDelete Feature = "ResetMetricsOnDelete"

// FeatureFleetAllocateOverflow enables setting labels and/or annotations on Allocated GameServers
// if the desired number of the underlying GameServerSet drops below the number of Allocated GameServers.
FeatureFleetAllocateOverflow = "FleetAllocationOverflow"
Expand Down Expand Up @@ -99,7 +95,6 @@ var (
featureDefaults = map[Feature]bool{
// Beta features
FeatureFleetAllocateOverflow: true,
FeatureResetMetricsOnDelete: true,
FeatureSplitControllerAndExtensions: true,

// Alpha features
Expand Down
2 changes: 0 additions & 2 deletions site/content/en/docs/Guides/feature-stages.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ that can be found in the [Helm configuration]({{< ref "/docs/Installation/Instal

The current set of `alpha` and `beta` feature gates:


{{% feature expiryVersion="1.37.0" %}}
| Feature Name | Gate | Default | Stage | Since |
|-----------------------------------------------------------------------------------------------------------------------|--------------------------------|----------|---------|--------|
Expand All @@ -41,7 +40,6 @@ The current set of `alpha` and `beta` feature gates:
| Feature Name | Gate | Default | Stage | Since |
|-----------------------------------------------------------------------------------------------------------------------|--------------------------------|----------|---------|--------|
| [Allocated GameServers are notified on relevant Fleet Updates][fleet-updates] | `FleetAllocationOverflow` | Enabled | `Beta` | 1.37.0 |
| [Reset Metric Export on Fleet / Autoscaler deletion]({{% relref "./metrics.md#dropping-metric-labels" %}}) | `ResetMetricsOnDelete` | Enabled | `Beta` | 1.32.0 |
| [Split `agones-controller` ](https://github.com/googleforgames/agones/issues/2797) | `SplitControllerAndExtensions` | Enabled | `Beta` | 1.32.0 |
| [GameServer player capacity filtering on GameServerAllocations](https://github.com/googleforgames/agones/issues/1239) | `PlayerAllocationFilter` | Disabled | `Alpha` | 1.14.0 |
| [Player Tracking]({{< ref "/docs/Guides/player-tracking.md" >}}) | `PlayerTracking` | Disabled | `Alpha` | 1.6.0 |
Expand Down
7 changes: 7 additions & 0 deletions site/content/en/docs/Guides/metrics.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,19 @@ Follow the [Google Cloud Monitoring installation steps](#google-cloud-monitoring

### Dropping Metric Labels

{{% feature expiryVersion="1.37.0" %}}
{{% beta title="Reset Metric Export on Fleet / Autoscaler deletion" gate="ResetMetricsOnDelete" %}}

When a Fleet or FleetAutoscaler is deleted from the system, Agones will automatically clear metrics that utilise
their name as a label from the exported metrics, so the metrics exported do not continuously grow in size over the
lifecycle of the Agones installation.
{{% /feature %}}

{{% feature publishVersion="1.37.0" %}}
When a Fleet or FleetAutoscaler is deleted from the system, Agones will automatically clear metrics that utilise
their name as a label from the exported metrics, so the metrics exported do not continuously grow in size over the
lifecycle of the Agones installation.
{{% /feature %}}

## Dashboard

Expand Down

0 comments on commit 018dc20

Please sign in to comment.