-
Notifications
You must be signed in to change notification settings - Fork 813
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GameServer: Implement (immutable) scale subresource, add pdb #2807
Conversation
Build Failed 😱 Build Id: 96836bd3-0d19-4719-b0b9-3e769c4056f9 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
4df070f
to
6af80d0
Compare
Build Failed 😱 Build Id: ce7f46fc-2661-4ad0-bad6-7397c6906856 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
6af80d0
to
e0adeab
Compare
Build Succeeded 👏 Build Id: e3921135-a01f-4154-a28c-c6b10cf97e00 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
e0adeab
to
0aef7c4
Compare
Build Succeeded 👏 Build Id: 1b3f716a-74ae-4043-9512-7dd9fa15cb0d The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
0aef7c4
to
adb85f3
Compare
Build Failed 😱 Build Id: 2c21c2b2-781c-4e6d-b50a-28d8dc19e05c To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
adb85f3
to
ce28721
Compare
Build Succeeded 👏 Build Id: 6511a1b8-7685-4812-baf2-01ce8e29655b The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
ce28721
to
2d5e9f4
Compare
Build Succeeded 👏 Build Id: 525c9e71-a0e8-426d-b4c2-ec777bae6f66 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
This looks great! And I think it might be the first example of using a feature gate in the helm templating, which is really cool. One last thing before we merge: Can you update https://agones.dev/site/docs/guides/feature-stages/ to mention the new feature gate? I also want to make sure that we capture this appropriately in the release notes so I'm wondering if we should change the PR title to mention that this is an alpha feature? WDYT? |
Since we are feature gating, please add the feature gate to: Line 247 in 4ab41be
and Line 70 in 4ab41be
Which will make it easier to test locally, and also will run under the e2e tests with it turned on and off. (I swore I wrote these steps in features.go, but now cannot find them). You'll have to also add your gate here: agones/pkg/util/runtime/features.go Lines 27 to 67 in 4ab41be
Otherwise the parsing will fail, as it validates flags. |
2d5e9f4
to
7954fac
Compare
Ok! I made this more into a formal mechanism for feature gates in Helm and added more documentation of the process. PTAL! |
Build Succeeded 👏 Build Id: 2b75ab5f-7149-4939-8955-829170350a80 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
7954fac
to
fc602c3
Compare
… pdb This PR, under the `LifecycleContract` feature gate suggested in googleforgames#2794 * Adds a scale resource to `GameServer` by adding an `immutableReplicas` field, which has a `default`, `min` and `max` of 1. Having a `scale` subresource lets us define a `PodDisruptionBudget` that can be set to `maxUnavailable: 0%` [1]. * Adds a PDB per namespace with label selector `agones.dev/safe-to-evict: "false"`, which nothing yet adds. * Adds a mechanism to get feature gate values in Helm by using: {{- $featureGates := include "agones.featureGates" . | fromYaml }} * Cleanup / documentation of feature gate mechanisms After this PR, it's possible to define a fleet with the label and have all `GameServer` pods protected by a `PodDisruptionBudget`, e.g.: ``` $ kubectl scale fleet/fleet-example --replicas=5 fleet.agones.dev/fleet-example scaled $ kubectl describe pdb Name: agones-gameserver-safe-to-evict-false Namespace: default Max unavailable: 0% Selector: agones.dev/safe-to-evict=false Status: Allowed disruptions: 0 Current: 4 Desired: 5 Total: 5 Events: <none> ``` Additionally, because min/max/default are 1, Kubernetes enforces the immutability for us: ``` $ kubectl scale gs/fleet-example-k6dfs-6m5nq --replicas=1 gameserver.agones.dev/fleet-example-k6dfs-6m5nq scaled $ kubectl scale gs/fleet-example-k6dfs-6m5nq --replicas=2 The GameServer "fleet-example-k6dfs-6m5nq" is invalid: spec.immutableReplicas: Invalid value: 2: spec.immutableReplicas in body should be less than or equal to 1 $ kubectl scale gs/fleet-example-k6dfs-6m5nq --replicas=0 The GameServer "fleet-example-k6dfs-6m5nq" is invalid: spec.immutableReplicas: Invalid value: 0: spec.immutableReplicas in body should be greater than or equal to 1 ``` The only artifact of this addition is a new field in the Spec/Status named `immutableReplicas`, in the Kubernetes object. This field is not present in the in-memory representation for `GameServer`, nor is it present in `etcd` (by defaulting rules). The field is visible on `describe` or `get -oyaml`, but is otherwise ignored. [1] https://kubernetes.io/docs/tasks/run-application/configure-pdb/#identify-an-application-to-protect
fc602c3
to
23ebcc6
Compare
Fixed, PTAL! |
Build Failed 😱 Build Id: 26887251-551b-496a-b3f8-f6c1f15bad9e To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: a2936572-794e-41ff-8603-2563203790da The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: roberthbailey, zmerlynn The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR:
Adds a scale resource to
GameServer
by adding animmutableReplicas
field, which has adefault
,min
andmax
of 1. Having ascale
subresource lets us define aPodDisruptionBudget
that can be set tomaxUnavailable: 0%
. (Without thescale
subresource onGameServer
, we can only setminAvailable
, and it applies across the label selector - we would be forced to use one PDB per pod.)immutableReplicas
, in the Kubernetes object. This field is not present in the in-memory representation forGameServer
, nor is it present inetcd
(by defaulting rules). The field is visible ondescribe
orget -oyaml
, but is otherwise ignored.Adds a PDB per namespace with label selector
agones.dev/safe-to-evict: "false"
.safe-to-evict
, but this PR currently has no effect except allowing the use of the PDB by explicit label in the Spec.After this PR, it's possible to define a fleet with the
agones.dev/safe-to-evict: "false"
label and have allGameServer
pods protected by aPodDisruptionBudget
, e.g.:Additionally, because min/max/default are 1, Kubernetes enforces the immutability for us:
Closes #2806
See also: #553
/kind feature