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.

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

Update Helm Charts to Support Multi Node Etcd Cluster #813

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

anveshreddy18
Copy link
Contributor

@anveshreddy18 anveshreddy18 commented Dec 18, 2024

What this PR does / why we need it:

This PR updates the outdated helm charts required to setup a single member etcd cluster with etcd-wrapper and etcd-backup-restore as containers in a pod.

The changes include:

  • updating charts to run etcd-wrapper with distroless etcd-backup-restore
  • correcting TLS paths and secrets
  • updating cloud provider related parameters
  • introduce serviceAccount, role, rolebinding templates
  • adding emulator support, etc

Which issue(s) this PR fixes:
Partially Fixes #708

Special notes for your reviewer:

Currently tested with the following cloud providers & their emulators:

  • GCP
  • AWS
  • AZURE

The remaining providers are to be tested yet.

Release note:

update the helm charts for deploying single & multi node etcd cluster

@anveshreddy18 anveshreddy18 requested a review from a team as a code owner December 18, 2024 10:00
@gardener-robot gardener-robot added needs/review Needs review size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Dec 18, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Dec 18, 2024
@ishan16696
Copy link
Member

Can you also capture somewhere in doc that right now, only nonHA etcd cluster can be deployed by helm charts for following providers.

@anveshreddy18
Copy link
Contributor Author

Sure @ishan16696, will mention that.

Copy link
Collaborator

@shreyas-s-rao shreyas-s-rao left a comment

Choose a reason for hiding this comment

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

@anveshreddy18 thanks a lot for taking up the long-pending task of updating the helm charts! Few comments from my side, PTAL:

  • Overall, I would like the charts to support multinode etcd clusters as well, as a static configuration. Since you've already introduced many peer TLS related artefacts in the chart, it should be easy to support multinode etcd clusters too. Moreover, it would be beneficial to have etcd-backup-restore charts support multinode etcd clusters, which are more widely used than single-node clusters.
  • please add newlines at the end of files - it's just good practice.
  • Please add etcd-peer-ca-secret.yaml and etcd-peer-server-tls-secret.yaml files, to support multinode etcd clusters

chart/etcd-backup-restore/templates/etcd-configmap.yaml Outdated Show resolved Hide resolved
chart/etcd-backup-restore/templates/etcd-configmap.yaml Outdated Show resolved Hide resolved
chart/etcd-backup-restore/templates/etcd-configmap.yaml Outdated Show resolved Hide resolved
chart/etcd-backup-restore/templates/etcd-statefulset.yaml Outdated Show resolved Hide resolved
chart/etcd-backup-restore/templates/etcd-statefulset.yaml Outdated Show resolved Hide resolved
chart/etcd-backup-restore/templates/etcd-statefulset.yaml Outdated Show resolved Hide resolved
chart/etcd-backup-restore/templates/etcd-statefulset.yaml Outdated Show resolved Hide resolved
@gardener-robot gardener-robot added the needs/changes Needs (more) changes label Dec 31, 2024
@shreyas-s-rao shreyas-s-rao added this to the v0.33.0 milestone Dec 31, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 3, 2025
@anveshreddy18 anveshreddy18 changed the title Update Helm Charts for Single Member Etcd Cluster Update Helm Charts to Support Multi Node Etcd Cluster Jan 3, 2025
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 3, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/changes Needs (more) changes needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review needs/second-opinion Needs second review by someone else size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Helm charts of backup-restore is not updated
6 participants