Skip to content
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.

Sign up for GitHub

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

Implemented PodDisruptionBudget on relevant deployments #2740

Merged
merged 3 commits into from
Nov 18, 2022
Merged

Implemented PodDisruptionBudget on relevant deployments #2740

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0894d7b
Implemented PodDisruptionBudget on relevant deployments
Sep 14, 2022
fd19f47
Merge branch 'main' into implements-pdb
roberthbailey Nov 9, 2022
12f90ad
Merge branch 'main' into implements-pdb
roberthbailey Nov 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions install/helm/agones/templates/ping.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,22 @@ spec:
{{- if .Values.agones.image.controller.pullSecret }}
imagePullSecrets:
- name: {{.Values.agones.image.controller.pullSecret}}
{{- end }}
{{- if .Values.agones.ping.pdb.enabled }}
---
apiVersion: policy/v1
kind: PodDisruptionBudget
metadata:
name: agones-ping-pdb
spec:
minAvailable: {{ .Values.agones.ping.pdb.minAvailable }}
maxUnavailable: {{ .Values.agones.ping.pdb.maxUnavailable }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://kubernetes.io/docs/tasks/run-application/configure-pdb/#specifying-a-poddisruptionbudget the above two fields are mutually exclusive (e.g. you can only set one or the other).

It isn't clear to me what happens if the spec looks like:

spec:
  minAvailable: 2
  maxUnavailable:

which is what will be produced if the boolean to enable this is toggled to true. Is that spec accepted by k8s or rejected?

Does the template here need to be more complicated so that we only set a single value?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious, so I tested this. It works fine, and sends the JSON you might expect: "spec":{"maxUnavailable":null,"minAvailable":0, [...]}.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @zmerlynn for checking that out. Do you think this is ok to submit as-is then?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should still be documented, but we could also merge and come back to that later.

selector:
matchLabels:
agones.dev/role: ping
app: {{ template "agones.name" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
{{- end }}
{{- if .Values.agones.ping.http.expose }}
---
Expand Down
17 changes: 16 additions & 1 deletion install/helm/agones/templates/service/allocation.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,22 @@ spec:
imagePullSecrets:
- name: {{.Values.agones.image.controller.pullSecret}}
{{- end }}

{{- if .Values.agones.allocator.pdb.enabled }}
---
apiVersion: policy/v1
kind: PodDisruptionBudget
metadata:
name: agones-allocator-pdb
spec:
minAvailable: {{ .Values.agones.allocator.pdb.minAvailable }}
maxUnavailable: {{ .Values.agones.allocator.pdb.maxUnavailable }}
selector:
matchLabels:
multicluster.agones.dev/role: allocator
app: {{ template "agones.name" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
{{- end }}
---
# Create a ClusterRole in that grants access to the agones allocation api
apiVersion: rbac.authorization.k8s.io/v1
Expand Down
6 changes: 6 additions & 0 deletions install/helm/agones/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ agones:
allocationBatchWaitTime: 500ms
ping:
install: true
pdb:
enabled: false
minAvailable: 1
updateStrategy: {}
resources: {}
# requests:
Expand Down Expand Up @@ -140,6 +143,9 @@ agones:
timeoutSeconds: 1
allocator:
install: true
pdb:
enabled: false
minAvailable: 1
updateStrategy: {}
apiServerQPS: 400
apiServerQPSBurst: 500
Expand Down
15 changes: 11 additions & 4 deletions site/content/en/docs/Installation/Install Agones/helm.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,20 @@ The following tables lists the configurable parameters of the Agones chart and t
| `gameservers.podPreserveUnknownFields` | Disable [field pruning][pruning] and schema validation on the Pod template for a [GameServer][gameserver] definition | `false` |
| `helm.installTests` | Add an ability to run `helm test agones` to verify the installation | `false` |
| `agones.controller.allocationBatchWaitTime` | Wait time between each allocation batch when performing allocations in controller mode | `500ms` |
| `agones.allocator.updateStrategy` | The [strategy](https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy) to apply to the ping deployment | `{}` |
| `agones.ping.updateStrategy` | The [strategy](https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy) to apply to the allocator deployment | `{}` |
| `agones.allocator.updateStrategy` | The [strategy](https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy) to apply to the ping deployment | `{}` |
| `agones.ping.updateStrategy` | The [strategy](https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy) to apply to the allocator deployment | `{}` |

{{% feature publishVersion="1.28.0" %}}
**New Configuration Features:**
| Parameter | Description | Default |
| -------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------- | ---------------------- |

| Parameter | Description | Default |
|---------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------|
| `agones.allocator.pdb.enabled` | Set to `true` to enable the creation of a [PodDisruptionBudget](https://kubernetes.io/docs/tasks/run-application/configure-pdb/) for the allocator deployment | `false` |
| `agones.allocator.pdb.minAvailable` | Description of the number of pods from that set that must still be available after the eviction, even in the absence of the evicted pod. Can be either an absolute number or a percentage. | `1` |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In each of the lines for minAvailable and maxUnavailable can you note that the two fields are mutually exclusive? Following the link to PDBs you can find this but it would be nice to highlight it here as well.

| `agones.allocator.pdb.maxUnavailable` | Description of the number of pods from that set that can be unavailable after the eviction. It can be either an absolute number or a percentage. | \`\` |
| `agones.ping.pdb.enabled` | Set to `true` to enable the creation of a [PodDisruptionBudget](https://kubernetes.io/docs/tasks/run-application/configure-pdb/) for the ping deployment | `false` |
| `agones.ping.pdb.minAvailable` | Description of the number of pods from that set that must still be available after the eviction, even in the absence of the evicted pod. Can be either an absolute number or a percentage. | `1` |
| `agones.ping.pdb.maxUnavailable` | Description of the number of pods from that set that can be unavailable after the eviction. It can be either an absolute number or a percentage. | \`\` |
{{% /feature %}}

[toleration]: https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/
Expand Down