Agones: Not explicitly providing a fleet replacement strategy results in it being set to blank and redeployments failing

[ShironekoBen]

What happened:

If you create a fleet specification that does not include a "strategy" node, the replacement strategy gets set to be blank, which then results in re-deployments failing because applyDeploymentStrategy() in controller.go reports an error.

What you expected to happen:

The documentation (both the inline docs and the spec example at https://agones.dev/site/docs/reference/fleet/) indicates that the default option is "RollingUpdate", so I would have expected that not supplying a strategy would set it to that.

How to reproduce it (as minimally and precisely as possible):

~~1. Create a fleet spec without a strategy and deploy it.

1. Attempt to remove that deployment and you should get repeated errors logged from the Agones controller (and the pods associated with the deployment will not be shut down).~~

See the comment below from @aLekSer for a more accurate and detailed set of reproduction steps.

Possible cause:

I'm not a Go expert, but looking at ApplyDefaults() in fleet.go, I suspect the issue may be that the code checks for f.Spec.Strategy.Type being an empty string but in this instance it ends up as nil?

Environment:

• Agones version: 1.2.0
• Kubernetes version (use kubectl version): 1.13.12
• Install method (yaml/helm): YAML

[aLekSer]

I was not able to reproduce an issue just with removing Strategy, it is defaulted, for example if you create a fleet same way as in the docs:

kubectl apply -f https://raw.githubusercontent.com/googleforgames/agones/release-1.2.0/examples/simple-udp/fleet.yaml

But I was able to reproduce an issue with Updating a fleet:

1. Create an example fleet with a strategy:

wget https://raw.githubusercontent.com/googleforgames/agones/master/examples/fleet.yaml
kubectl apply -f ./fleet.yaml

1. Remove strategy and subsequent lines, all the way to the template:

vi ./fleet.yaml 
kubectl apply -f ./fleet.yaml

Now kubectl describe fleet fleet-example will show that fleet loose a strategy.
And when you change for instance a fleet label "foo:bar" to something different you would get such an error in controller logs:

{"error":"unexpected deployment strategy type: ","fleetKey":"default/fleet-example-46","message":"","queue":"agones.dev.FleetController","severity":"error","source":"*fleets.Controller","time":"2020-01-20T14:30:47.370704134Z"}
{"message":"unexpected deployment strategy type: ","severity":"error","stack":["agones.dev/agones/pkg/fleets.(*Controller).applyDeploymentStrategy\n\t/go/src/agones.dev/agones/pkg/fleets/controller.go:364","agones.dev/agones/pkg/fleets.(*Controller).syncFleet\n\t/go/src/agones.dev/agones/pkg/fleets/controller.go:289","agones.dev/agones/pkg/util/workerqueue.(*WorkerQueue).processNextWorkItem\n\t/go/src/agones.dev/agones/pkg/util/workerqueue/workerqueue.go:152","agones.dev/agones/pkg/util/workerqueue.(*WorkerQueue).runWorker\n\t/go/src/agones.dev/agones/pkg/util/workerqueue/workerqueue.go:128","k8s.io/apimachinery/pkg/util/wait.JitterUntil.func1\n\t/go/src/agones.dev/agones/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:133","k8s.io/apimachinery/pkg/util/wait.JitterUntil\n\t/go/src/agones.dev/agones/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:134","k8s.io/apimachinery/pkg/util/wait.Until\n\t/go/src/agones.dev/agones/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:88","agones.dev/agones/pkg/util/workerqueue.(*WorkerQueue).run\n\t/go/src/agones.dev/agones/pkg/util/workerqueue/workerqueue.go:180","runtime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1337"],"time":"2020-01-20T14:30:47.37079524Z"}

[ShironekoBen]

Ah, apologies for the confusion there - I'd been manipulating the specification through an intermediate script layer and hadn't spotted that it was updating rather than creating the setup anew. Many thanks for looking into that!

[ilkercelikyilmaz]

Can we close this issue ?

[ShironekoBen]

I don't believe so - as far as I'm aware the bug is still valid, it's just that the reproduction steps were slightly incorrect in that you need to update the fleet rather than just creating it. I'll try and edit the description to match.

[aLekSer]

Yes, I have a code which performs a validation on Fleet Update, need to add some tests.