@@ -222,123 +222,187 @@ func TestFleetScaleUpEditAndScaleDown(t *testing.T) {
222222func TestFleetRollingUpdate (t * testing.T ) {
223223 t .Parallel ()
224224 ctx := context .Background ()
225- // Use scaleFleetPatch (true) or scaleFleetSubresource (false)
226- fixtures := []bool {true , false }
227- maxSurge := []string {"25%" , "10%" }
225+ fixtures := []struct {
226+ // Use scaleFleetPatch (true) or scaleFleetSubresource (false)
227+ usePatch bool
228+ maxSurge string
229+ // If true, create and cycle GS Allocations when triggering a rolling update:
230+ // - Repeatedly allocate and shutdown GameServers (keeping ~50% of the Fleet in an Allocated state).
231+ // - Once 50% of the Fleet is Allocated, trigger the rolling update.
232+ // - Keep allocating/shutting down GameServers, to allocate in both the old and new GSSets.
233+ // - Verify the rolling update completes.
234+ // This simulates updating a Fleet that is live/in-use, and reproduces an issue where allocated GameServers
235+ // causes a rolling update to get stuck and keep the old GameServerSet up.
236+ cycleAllocations bool
237+ }{
238+ {
239+ usePatch : true ,
240+ maxSurge : "25%" ,
241+ cycleAllocations : false ,
242+ },
243+ {
244+ usePatch : true ,
245+ maxSurge : "10%" ,
246+ cycleAllocations : false ,
247+ },
248+ {
249+ usePatch : false ,
250+ maxSurge : "25%" ,
251+ cycleAllocations : false ,
252+ },
253+ {
254+ usePatch : false ,
255+ maxSurge : "10%" ,
256+ cycleAllocations : false ,
257+ },
258+ {
259+ usePatch : true ,
260+ maxSurge : "25%" ,
261+ cycleAllocations : true ,
262+ },
263+ }
264+ for i := range fixtures {
265+ fixture := fixtures [i ]
266+ t .Run (fmt .Sprintf ("Use fleet Patch %t %s cycle %t" , fixture .usePatch , fixture .maxSurge , fixture .cycleAllocations ), func (t * testing.T ) {
267+ t .Parallel ()
228268
229- for _ , usePatch := range fixtures {
230- for _ , maxSurgeParam := range maxSurge {
231- usePatch := usePatch
232- maxSurgeParam := maxSurgeParam
233- t .Run (fmt .Sprintf ("Use fleet Patch %t %s" , usePatch , maxSurgeParam ), func (t * testing.T ) {
234- t .Parallel ()
235-
236- client := framework .AgonesClient .AgonesV1 ()
237-
238- flt := defaultFleet (framework .Namespace )
239- flt .ApplyDefaults ()
240- flt .Spec .Replicas = 1
241- rollingUpdatePercent := intstr .FromString (maxSurgeParam )
242- flt .Spec .Strategy .RollingUpdate .MaxSurge = & rollingUpdatePercent
243- flt .Spec .Strategy .RollingUpdate .MaxUnavailable = & rollingUpdatePercent
244-
245- flt , err := client .Fleets (framework .Namespace ).Create (ctx , flt , metav1.CreateOptions {})
246- if assert .Nil (t , err ) {
247- defer client .Fleets (framework .Namespace ).Delete (ctx , flt .ObjectMeta .Name , metav1.DeleteOptions {}) // nolint:errcheck
248- }
269+ client := framework .AgonesClient .AgonesV1 ()
249270
250- assert .Equal (t , int32 (1 ), flt .Spec .Replicas )
251- assert .Equal (t , maxSurgeParam , flt .Spec .Strategy .RollingUpdate .MaxSurge .StrVal )
252- assert .Equal (t , maxSurgeParam , flt .Spec .Strategy .RollingUpdate .MaxUnavailable .StrVal )
271+ flt := defaultFleet (framework .Namespace )
272+ flt .ApplyDefaults ()
273+ flt .Spec .Replicas = 1
274+ rollingUpdatePercent := intstr .FromString (fixture .maxSurge )
275+ flt .Spec .Strategy .RollingUpdate .MaxSurge = & rollingUpdatePercent
276+ flt .Spec .Strategy .RollingUpdate .MaxUnavailable = & rollingUpdatePercent
253277
254- framework .AssertFleetCondition (t , flt , e2e .FleetReadyCount (flt .Spec .Replicas ))
278+ flt , err := client .Fleets (framework .Namespace ).Create (ctx , flt , metav1.CreateOptions {})
279+ require .NoError (t , err )
280+ defer client .Fleets (framework .Namespace ).Delete (ctx , flt .ObjectMeta .Name , metav1.DeleteOptions {}) // nolint:errcheck
255281
256- // scale up
257- const targetScale = 8
258- if usePatch {
259- flt = scaleFleetPatch (ctx , t , flt , targetScale )
260- assert .Equal (t , int32 (targetScale ), flt .Spec .Replicas )
261- } else {
262- flt = scaleFleetSubresource (ctx , t , flt , targetScale )
263- }
282+ assert .Equal (t , int32 (1 ), flt .Spec .Replicas )
283+ assert .Equal (t , fixture .maxSurge , flt .Spec .Strategy .RollingUpdate .MaxSurge .StrVal )
284+ assert .Equal (t , fixture .maxSurge , flt .Spec .Strategy .RollingUpdate .MaxUnavailable .StrVal )
264285
265- framework .AssertFleetCondition (t , flt , e2e .FleetReadyCount (targetScale ))
286+ framework .AssertFleetCondition (t , flt , e2e .FleetReadyCount (flt . Spec . Replicas ))
266287
267- flt , err = client .Fleets (framework .Namespace ).Get (ctx , flt .ObjectMeta .GetName (), metav1.GetOptions {})
268- assert .NoError (t , err )
288+ // scale up
289+ const targetScale = 8
290+ if fixture .usePatch {
291+ flt = scaleFleetPatch (ctx , t , flt , targetScale )
292+ assert .Equal (t , int32 (targetScale ), flt .Spec .Replicas )
293+ } else {
294+ flt = scaleFleetSubresource (ctx , t , flt , targetScale )
295+ }
296+
297+ framework .AssertFleetCondition (t , flt , e2e .FleetReadyCount (targetScale ))
298+
299+ flt , err = client .Fleets (framework .Namespace ).Get (ctx , flt .ObjectMeta .GetName (), metav1.GetOptions {})
300+ require .NoError (t , err )
301+
302+ cycleCtx , cancelCycle := context .WithCancel (ctx )
303+ defer cancelCycle ()
304+ if fixture .cycleAllocations {
305+ // Repeatedly cycle allocations to keep ~half of the GameServers Allocated. Repeatedly Allocate and
306+ // delete such that both the old and new GSSet contain allocated GameServers.
307+ const halfScale = targetScale / 2
308+ const period = 3 * time .Second
309+ go framework .CycleAllocations (cycleCtx , t , flt , period , period * halfScale )
310+
311+ // Wait for at least half of the fleet to have be cycled (either Allocated or shutting down)
312+ // before updating the fleet.
313+ err = framework .WaitForFleetCondition (t , flt , func (entry * logrus.Entry , fleet * agonesv1.Fleet ) bool {
314+ return fleet .Status .ReadyReplicas < halfScale
315+ })
316+ }
269317
270- // Change ContainerPort to trigger creating a new GSSet
318+ // Change ContainerPort to trigger creating a new GSSet. Retry in case of a conflict.
319+ fltName := flt .GetName ()
320+ require .Eventually (t , func () bool {
321+ flt , err = client .Fleets (framework .Namespace ).Get (ctx , fltName , metav1.GetOptions {})
322+ require .NoError (t , err )
271323 fltCopy := flt .DeepCopy ()
272324 fltCopy .Spec .Template .Spec .Ports [0 ].ContainerPort ++
273325 flt , err = client .Fleets (framework .Namespace ).Update (ctx , fltCopy , metav1.UpdateOptions {})
274- assert .NoError (t , err )
326+ return err == nil
327+ }, time .Minute , time .Second )
275328
276- selector := labels .SelectorFromSet (labels.Set {agonesv1 .FleetNameLabel : flt .ObjectMeta .Name })
277- // New GSS was created
278- err = wait .PollImmediate (1 * time .Second , 30 * time .Second , func () (bool , error ) {
279- gssList , err := framework .AgonesClient .AgonesV1 ().GameServerSets (framework .Namespace ).List (ctx ,
280- metav1.ListOptions {LabelSelector : selector .String ()})
281- if err != nil {
282- return false , err
283- }
284- return len (gssList .Items ) == 2 , nil
285- })
286- assert .NoError (t , err )
287- // Check that total number of gameservers in the system does not exceed the RollingUpdate
288- // parameters (creating no more than maxSurge, deleting maxUnavailable servers at a time)
289- // Wait for old GSSet to be deleted
290- err = wait .PollImmediate (1 * time .Second , 5 * time .Minute , func () (bool , error ) {
291- list , err := framework .AgonesClient .AgonesV1 ().GameServers (framework .Namespace ).List (ctx ,
292- metav1.ListOptions {LabelSelector : selector .String ()})
293- if err != nil {
294- return false , err
295- }
329+ selector := labels .SelectorFromSet (labels.Set {agonesv1 .FleetNameLabel : flt .ObjectMeta .Name })
330+ // New GSS was created
331+ err = wait .PollImmediate (1 * time .Second , 30 * time .Second , func () (bool , error ) {
332+ gssList , err := framework .AgonesClient .AgonesV1 ().GameServerSets (framework .Namespace ).List (ctx ,
333+ metav1.ListOptions {LabelSelector : selector .String ()})
334+ if err != nil {
335+ return false , err
336+ }
337+ return len (gssList .Items ) == 2 , nil
338+ })
339+ assert .NoError (t , err )
340+ // Check that total number of gameservers in the system does not exceed the RollingUpdate
341+ // parameters (creating no more than maxSurge, deleting maxUnavailable servers at a time)
342+ // Wait for old GSSet to be deleted
343+ err = wait .PollImmediate (1 * time .Second , 5 * time .Minute , func () (bool , error ) {
344+ list , err := framework .AgonesClient .AgonesV1 ().GameServers (framework .Namespace ).List (ctx ,
345+ metav1.ListOptions {LabelSelector : selector .String ()})
346+ if err != nil {
347+ return false , err
348+ }
296349
297- maxSurge , err := intstr .GetValueFromIntOrPercent (flt .Spec .Strategy .RollingUpdate .MaxSurge , int (flt .Spec .Replicas ), true )
298- assert .Nil (t , err )
350+ maxSurge , err := intstr .GetValueFromIntOrPercent (flt .Spec .Strategy .RollingUpdate .MaxSurge , int (flt .Spec .Replicas ), true )
351+ assert .Nil (t , err )
299352
300- roundUp := false
301- maxUnavailable , err := intstr .GetValueFromIntOrPercent (flt .Spec .Strategy .RollingUpdate .MaxUnavailable , int (flt .Spec .Replicas ), roundUp )
353+ roundUp := false
354+ maxUnavailable , err := intstr .GetValueFromIntOrPercent (flt .Spec .Strategy .RollingUpdate .MaxUnavailable , int (flt .Spec .Replicas ), roundUp )
302355
303- if maxUnavailable == 0 {
304- maxUnavailable = 1
305- }
306- // This difference is inevitable, also could be seen with Deployments and ReplicaSets
307- shift := maxUnavailable
308- assert .Nil (t , err )
356+ if maxUnavailable == 0 {
357+ maxUnavailable = 1
358+ }
359+ // This difference is inevitable, also could be seen with Deployments and ReplicaSets
360+ shift := maxUnavailable
361+ assert .Nil (t , err )
309362
310- expectedTotal := targetScale + maxSurge + maxUnavailable + shift
311- if len (list .Items ) > expectedTotal {
312- err = fmt .Errorf ("new replicas should be less than target + maxSurge + maxUnavailable + shift. Replicas: %d, Expected: %d" , len (list .Items ), expectedTotal )
313- }
314- if err != nil {
315- return false , err
316- }
317- gssList , err := framework .AgonesClient .AgonesV1 ().GameServerSets (framework .Namespace ).List (ctx ,
318- metav1.ListOptions {LabelSelector : selector .String ()})
319- if err != nil {
320- return false , err
363+ // Ignore any GameServers that are shutting down (resulting from Allocation cycling).
364+ shuttingDown := 0
365+ for _ , gs := range list .Items {
366+ if gs .IsBeingDeleted () {
367+ shuttingDown ++
321368 }
322- return len (gssList .Items ) == 1 , nil
323- })
369+ }
370+ expectedTotal := targetScale + maxSurge + maxUnavailable + shift + shuttingDown
371+ if len (list .Items ) > expectedTotal {
372+ err = fmt .Errorf ("new replicas should be less than target + maxSurge + maxUnavailable + shift + shuttingDown. Replicas: %d, Expected: %d" , len (list .Items ), expectedTotal )
373+ }
374+ if err != nil {
375+ return false , err
376+ }
377+ gssList , err := framework .AgonesClient .AgonesV1 ().GameServerSets (framework .Namespace ).List (ctx ,
378+ metav1.ListOptions {LabelSelector : selector .String ()})
379+ if err != nil {
380+ return false , err
381+ }
382+ return len (gssList .Items ) == 1 , nil
383+ })
324384
325- assert .NoError (t , err )
385+ assert .NoError (t , err )
326386
327- // scale down, with allocation
328- const scaleDownTarget = 1
329- if usePatch {
330- flt = scaleFleetPatch (ctx , t , flt , scaleDownTarget )
331- } else {
332- flt = scaleFleetSubresource (ctx , t , flt , scaleDownTarget )
333- }
387+ // Stop cycling Allocations.
388+ // The AssertFleetConditions below will wait until the Allocation cycling has
389+ // fully stopped (when all Allocated GameServers are shut down).
390+ cancelCycle ()
391+
392+ // scale down, with allocation
393+ const scaleDownTarget = 1
394+ if fixture .usePatch {
395+ flt = scaleFleetPatch (ctx , t , flt , scaleDownTarget )
396+ } else {
397+ flt = scaleFleetSubresource (ctx , t , flt , scaleDownTarget )
398+ }
334399
335- framework .AssertFleetCondition (t , flt , e2e .FleetReadyCount (1 ))
400+ framework .AssertFleetCondition (t , flt , e2e .FleetReadyCount (1 ))
336401
337- framework .AssertFleetCondition (t , flt , func (log * logrus.Entry , fleet * agonesv1.Fleet ) bool {
338- return fleet .Status .AllocatedReplicas == 0
339- })
402+ framework .AssertFleetCondition (t , flt , func (log * logrus.Entry , fleet * agonesv1.Fleet ) bool {
403+ return fleet .Status .AllocatedReplicas == 0
340404 })
341- }
405+ })
342406 }
343407}
344408
0 commit comments