Skip to content

Add support for PDB with the teleport-cluster helm chart#7138

Merged
webvictim merged 18 commits intogravitational:masterfrom
stefansedich:teleport-cluster-pdb
Aug 19, 2021
Merged

Add support for PDB with the teleport-cluster helm chart#7138
webvictim merged 18 commits intogravitational:masterfrom
stefansedich:teleport-cluster-pdb

Conversation

@stefansedich
Copy link
Copy Markdown
Contributor

@stefansedich stefansedich commented Jun 2, 2021

This PR adds support for a PDB as part of the teleport-cluster chart, for the sole reason that if we truly want to support HA then we should likely configure a PDB.

I have disabled it by default and have made it an opt-in thing, we can also enable it when replicaCount>1 so am open to suggestions.

Comment thread examples/chart/teleport-cluster/templates/pdb.yaml
@russjones russjones added the helm label Jun 3, 2021
@russjones
Copy link
Copy Markdown
Contributor

@webvictim Can you review this?

Comment on lines +38 to +42
# If enabled will create a Pod Disruption Budget
# https://kubernetes.io/docs/concepts/workloads/pods/disruptions/
podDisruptionBudget:
enabled: false
minAvailable: 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that this should be under the highAvailability section as it's only relevant for that type of deployment.

Copy link
Copy Markdown
Contributor

@webvictim webvictim left a comment

Choose a reason for hiding this comment

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

Added some feedback.

Also, please add this value to the Helm chart reference at https://github.com/gravitational/teleport/blob/master/docs/pages/kubernetes-access/helm/reference.mdx so it's kept up to date.

@stefansedich
Copy link
Copy Markdown
Contributor Author

Added some feedback.

Also, please add this value to the Helm chart reference at https://github.com/gravitational/teleport/blob/master/docs/pages/kubernetes-access/helm/reference.mdx so it's kept up to date.

@webvictim feedback implemented and added to the docs.

Comment thread examples/chart/teleport-cluster/values.schema.json Outdated
@webvictim
Copy link
Copy Markdown
Contributor

@stefansedich Thanks! One final thing - could you please add a sample values file examples/chart/teleport-cluster/.lint/pdb.yaml to cover this new addition which will ensure that the new template gets automatically evaluated on PRs?

See something like https://github.com/gravitational/teleport/blob/master/examples/chart/teleport-cluster/.lint/volumes.yaml for an example.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Docs-wise this is great thanks for the helpful reference update!

@stefansedich
Copy link
Copy Markdown
Contributor Author

@stefansedich Thanks! One final thing - could you please add a sample values file examples/chart/teleport-cluster/.lint/pdb.yaml to cover this new addition which will ensure that the new template gets automatically evaluated on PRs?

See something like https://github.com/gravitational/teleport/blob/master/examples/chart/teleport-cluster/.lint/volumes.yaml for an example.

@webvictim done, hopefully I got that right 😄

@stefansedich stefansedich force-pushed the teleport-cluster-pdb branch from eb65f38 to 7ede544 Compare June 29, 2021 16:54
@webvictim
Copy link
Copy Markdown
Contributor

@stefansedich Yep, looks good. Let's see what the linter says.

Comment thread examples/chart/teleport-cluster/values.schema.json Outdated
Comment thread examples/chart/teleport-cluster/.lint/pdb.yaml
@stefansedich
Copy link
Copy Markdown
Contributor Author

@webvictim what was needed now to get this in?

@webvictim webvictim enabled auto-merge (squash) July 29, 2021 14:44
@webvictim
Copy link
Copy Markdown
Contributor

Looks fine now. @klizhentas @r0mant @russjones Please codeowner review.

@stefansedich
Copy link
Copy Markdown
Contributor Author

@webvictim @klizhentas @r0mant @russjones any chance of getting this one in?

@webvictim
Copy link
Copy Markdown
Contributor

This is fine by me, just needs a rubber stamp from @russjones @klizhentas or @r0mant.

Copy link
Copy Markdown
Contributor

@russjones russjones left a comment

Choose a reason for hiding this comment

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

Bot.

@russjones
Copy link
Copy Markdown
Contributor

@webvictim Please backport this to branch/v7 as well, that's where we will be releasing Helm charts from soon.

@webvictim webvictim merged commit 3219103 into gravitational:master Aug 19, 2021
webvictim added a commit that referenced this pull request Aug 20, 2021
* Add support for PDB with the teleport-cluster helm chart

Co-authored-by: Gus Luxton <gus@goteleport.com>
webvictim added a commit that referenced this pull request Aug 26, 2021
* Add abilty to configure postStart handler for teleport-cluster chart (#7168)

* Add support for PDB with the teleport-cluster helm chart (#7138)

* Allow teleport-cluster-agent chart to use an existing volume for the data directory (#7096)

* Mount teleport-tls to the init container for the teleport-cluster helm chart (#7166)

* add support for running agent helm chart on persistent volume (#7123)

* helm: Make auth type configurable (#7508)

* Add imagePullSecrets in kube-agent chart (#6941)

Co-authored-by: Gus Luxton <gus@goteleport.com>
Co-authored-by: Stefan Sedich <stefan.sedich@gmail.com>
Co-authored-by: Kevin Nisbet <kevin@gravitational.com>
Co-authored-by: Antonio Gurgel <66754096+antonio-te@users.noreply.github.com>
Co-authored-by: Aymen Memni <7290430+amemni@users.noreply.github.com>
zmb3 pushed a commit that referenced this pull request Sep 23, 2021
* Add support for PDB with the teleport-cluster helm chart

Co-authored-by: Gus Luxton <gus@goteleport.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants