diff --git a/cloudbuild.yaml b/cloudbuild.yaml index a1f1e07528..709b2e99e3 100644 --- a/cloudbuild.yaml +++ b/cloudbuild.yaml @@ -232,7 +232,7 @@ steps: # - name: 'e2e-runner' - args: ['PlayerTracking=true&SDKWatchSendOnExecute=false&RollingUpdateOnReady=false&NodeExternalDNS=true&StateAllocationFilter=true&PlayerAllocationFilter=true&CustomFasSyncInterval=true', 'e2e-test-cluster'] + args: ['PlayerTracking=true&SDKWatchSendOnExecute=false&NodeExternalDNS=true&StateAllocationFilter=true&PlayerAllocationFilter=true&CustomFasSyncInterval=true', 'e2e-test-cluster'] id: e2e-feature-gates waitFor: - push-images diff --git a/pkg/fleets/controller.go b/pkg/fleets/controller.go index 12cecce3fd..3b72c87a54 100644 --- a/pkg/fleets/controller.go +++ b/pkg/fleets/controller.go @@ -603,57 +603,7 @@ func (c *Controller) rollingUpdateRestFixedOnReady(ctx context.Context, fleet *a // rollingUpdateRest applies the rolling update to the inactive GameServerSets func (c *Controller) rollingUpdateRest(ctx context.Context, fleet *agonesv1.Fleet, active *agonesv1.GameServerSet, rest []*agonesv1.GameServerSet) error { - if runtime.FeatureEnabled(runtime.FeatureRollingUpdateOnReady) { - return c.rollingUpdateRestFixedOnReady(ctx, fleet, active, rest) - } - return c.rollingUpdateRestBeforeFixOnReady(ctx, fleet, rest) -} - -func (c *Controller) rollingUpdateRestBeforeFixOnReady(ctx context.Context, fleet *agonesv1.Fleet, rest []*agonesv1.GameServerSet) error { - if len(rest) == 0 { - return nil - } - - r, err := intstr.GetValueFromIntOrPercent(fleet.Spec.Strategy.RollingUpdate.MaxUnavailable, int(fleet.Spec.Replicas), true) - - if err != nil { - return errors.Wrapf(err, "error parsing MaxUnavailable value: %s", fleet.ObjectMeta.Name) - } - unavailable := int32(r) - - for _, gsSet := range rest { - - // if the status.Replicas are less than or equal to 0, then that means we are done - // scaling this GameServerSet down, and can therefore exit/move to the next one. - if gsSet.Status.Replicas <= 0 { - continue - } - - // If the Spec.Replicas does not equal the Status.Replicas for this GameServerSet, this means - // that the rolling down process is currently ongoing, and we should therefore exit so we can wait for it to finish - if gsSet.Spec.Replicas != gsSet.Status.Replicas { - break - } - gsSetCopy := gsSet.DeepCopy() - newReplicasCount := fleet.LowerBoundReplicas(gsSetCopy.Spec.Replicas - unavailable) - if gsSet.Status.ShutdownReplicas == 0 { - gsSetCopy.Spec.Replicas = newReplicasCount - c.loggerForFleet(fleet).Debug(fmt.Sprintf("Replicas in a Shutdown state - %d", gsSet.Status.ShutdownReplicas)) - c.loggerForFleet(fleet).WithField("gameserverset", gsSet.ObjectMeta.Name).WithField("replicas", gsSetCopy.Spec.Replicas). - Debug("applying rolling update to inactive gameserverset") - - if _, err := c.gameServerSetGetter.GameServerSets(gsSetCopy.ObjectMeta.Namespace).Update(ctx, gsSetCopy, metav1.UpdateOptions{}); err != nil { - return errors.Wrapf(err, "error updating gameserverset %s", gsSetCopy.ObjectMeta.Name) - } - c.recorder.Eventf(fleet, corev1.EventTypeNormal, "ScalingGameServerSet", - "Scaling inactive GameServerSet %s from %d to %d", gsSetCopy.ObjectMeta.Name, gsSet.Spec.Replicas, gsSetCopy.Spec.Replicas) - - // let's update just one at a time, slightly slower, but a simpler solution that doesn't require us - // to make sure we don't overshoot the amount that is being shutdown at any given point and time - break - } - } - return nil + return c.rollingUpdateRestFixedOnReady(ctx, fleet, active, rest) } // updateFleetStatus gets the GameServerSets for this Fleet and then diff --git a/pkg/fleets/controller_test.go b/pkg/fleets/controller_test.go index 9450729bbb..37613c40d3 100644 --- a/pkg/fleets/controller_test.go +++ b/pkg/fleets/controller_test.go @@ -196,10 +196,6 @@ func TestControllerSyncFleet(t *testing.T) { }) t.Run("gameserverset with different image details", func(t *testing.T) { - utilruntime.FeatureTestMutex.Lock() - defer utilruntime.FeatureTestMutex.Unlock() - assert.NoError(t, utilruntime.ParseFeatures(string(utilruntime.FeatureRollingUpdateOnReady)+"=true")) - f := defaultFixture() f.Spec.Strategy.Type = appsv1.RollingUpdateDeploymentStrategyType f.Spec.Template.Spec.Ports = []agonesv1.GameServerPort{{HostPort: 5555}} @@ -1006,12 +1002,7 @@ func TestControllerRollingUpdateDeploymentGSSUpdateFailedErrExpected(t *testing. assert.EqualError(t, err, "error updating gameserverset inactive: random-err") } -func TestFeatureRollingUpdateOnReady(t *testing.T) { - utilruntime.FeatureTestMutex.Lock() - defer utilruntime.FeatureTestMutex.Unlock() - - assert.NoError(t, utilruntime.ParseFeatures(string(utilruntime.FeatureRollingUpdateOnReady)+"=true")) - +func TestRollingUpdateOnReady(t *testing.T) { type expected struct { inactiveSpecReplicas int32 replicas int32 @@ -1118,11 +1109,6 @@ func TestFeatureRollingUpdateOnReady(t *testing.T) { func TestControllerRollingUpdateDeployment(t *testing.T) { t.Parallel() - utilruntime.FeatureTestMutex.Lock() - defer utilruntime.FeatureTestMutex.Unlock() - - assert.NoError(t, utilruntime.ParseFeatures(string(utilruntime.FeatureRollingUpdateOnReady)+"=false")) - type expected struct { inactiveSpecReplicas int32 replicas int32 @@ -1197,23 +1183,11 @@ func TestControllerRollingUpdateDeployment(t *testing.T) { inactiveSpecReplicas: 95, inactiveStatusReplicas: 95, expected: expected{ - inactiveSpecReplicas: 65, + inactiveSpecReplicas: 45, replicas: 30, updated: true, }, }, - "statuses don't match the spec. nothing should happen": { - fleetSpecReplicas: 100, - activeSpecReplicas: 75, - activeStatusReplicas: 70, - inactiveSpecReplicas: 15, - inactiveStatusReplicas: 10, - expected: expected{ - inactiveSpecReplicas: 15, - replicas: 75, - updated: false, - }, - }, "test smalled numbers of active and allocated": { fleetSpecReplicas: 5, activeSpecReplicas: 0, @@ -1223,7 +1197,7 @@ func TestControllerRollingUpdateDeployment(t *testing.T) { inactiveStatusAllocationReplicas: 2, expected: expected{ - inactiveSpecReplicas: 3, + inactiveSpecReplicas: 4, replicas: 2, updated: true, }, diff --git a/pkg/util/runtime/features.go b/pkg/util/runtime/features.go index 4e3d737afb..855b76037d 100644 --- a/pkg/util/runtime/features.go +++ b/pkg/util/runtime/features.go @@ -37,10 +37,6 @@ const ( // FeatureSDKWatchSendOnExecute is a feature flag to enable/disable immediate game server return after SDK.WatchGameServer is called FeatureSDKWatchSendOnExecute Feature = "SDKWatchSendOnExecute" - // FeatureRollingUpdateOnReady is a feature flag to enable/disable rolling update fix of scale down, when ReadyReplicas - // count is taken into account - FeatureRollingUpdateOnReady Feature = "RollingUpdateOnReady" - // NodeExternalDNS is a feature flag to enable/disable node ExternalDNS and InternalDNS use as GameServer address NodeExternalDNS Feature = "NodeExternalDNS" @@ -66,7 +62,6 @@ var ( FeatureExample: true, FeaturePlayerTracking: false, FeatureSDKWatchSendOnExecute: true, - FeatureRollingUpdateOnReady: true, NodeExternalDNS: false, FeatureStateAllocationFilter: false, FeaturePlayerAllocationFilter: false, diff --git a/site/content/en/docs/Guides/feature-stages.md b/site/content/en/docs/Guides/feature-stages.md index 764b9183d9..249260a513 100644 --- a/site/content/en/docs/Guides/feature-stages.md +++ b/site/content/en/docs/Guides/feature-stages.md @@ -42,7 +42,6 @@ The current set of `alpha` and `beta` feature gates are: | Example Gate (not in use) | `Example` | Disabled | None | 0.13.0 | | [Player Tracking]({{< ref "/docs/Guides/player-tracking.md" >}}) | `PlayerTracking` | Disabled | `Alpha` | 1.6.0 | | [SDK Send GameServer on Watch execution]({{< ref "/docs/Guides/Client SDKs/_index.md#watchgameserverfunctiongameserver" >}}) | `SDKWatchSendOnExecute` | Enabled | `Beta` | 1.12.0 | -| Fix for RollingUpdate [Scale down](https://github.com/googleforgames/agones/issues/1625) | `RollingUpdateOnReady` | Enabled | `Beta` | 1.14.0 | | [Utilize Node ExternalDNS](https://github.com/googleforgames/agones/issues/1921) and additional [details]({{< ref "/docs/FAQ/_index.md" >}}) | `NodeExternalDNS` | Disabled | `Alpha` | 1.12.0 | | [Custom resync period for FleetAutoscaler](https://github.com/googleforgames/agones/issues/1955) | `CustomFasSyncInterval` | Disabled | `Alpha` | 1.17.0 | | [GameServer state filtering on GameServerAllocations](https://github.com/googleforgames/agones/issues/1239) | `StateAllocationFilter` | Disabled | `Alpha` | 1.14.0 | diff --git a/test/e2e/fleet_test.go b/test/e2e/fleet_test.go index bb70a5c489..62132f1ca0 100644 --- a/test/e2e/fleet_test.go +++ b/test/e2e/fleet_test.go @@ -297,20 +297,14 @@ func TestFleetRollingUpdate(t *testing.T) { maxSurge, err := intstr.GetValueFromIntOrPercent(flt.Spec.Strategy.RollingUpdate.MaxSurge, int(flt.Spec.Replicas), true) assert.Nil(t, err) - roundUp := true - if runtime.FeatureEnabled(runtime.FeatureRollingUpdateOnReady) { - roundUp = false - } + roundUp := false maxUnavailable, err := intstr.GetValueFromIntOrPercent(flt.Spec.Strategy.RollingUpdate.MaxUnavailable, int(flt.Spec.Replicas), roundUp) - shift := 0 - if runtime.FeatureEnabled(runtime.FeatureRollingUpdateOnReady) { - if maxUnavailable == 0 { - maxUnavailable = 1 - } - // This difference is inevitable, also could be seen with Deployments and ReplicaSets - shift = maxUnavailable + if maxUnavailable == 0 { + maxUnavailable = 1 } + // This difference is inevitable, also could be seen with Deployments and ReplicaSets + shift := maxUnavailable assert.Nil(t, err) expectedTotal := targetScale + maxSurge + maxUnavailable + shift @@ -349,9 +343,6 @@ func TestFleetRollingUpdate(t *testing.T) { } func TestUpdateFleetReplicaAndSpec(t *testing.T) { - if !runtime.FeatureEnabled(runtime.FeatureRollingUpdateOnReady) { - t.SkipNow() - } t.Parallel() client := framework.AgonesClient.AgonesV1()