Skip to content

Conversation

rfranzke
Copy link
Member

@rfranzke rfranzke commented May 9, 2025

How to categorize this PR?

/area ipcei
/kind enhancement

What this PR does / why we need it:
This PR prepares for etcd-druid taking over management of etcd (this itself is not yet part of this PR and will follow later).

Which issue(s) this PR fixes:
Part of #2906

Special notes for your reviewer:
/cc @ScheererJ @timebertt

Release note:

NONE

@gardener-prow gardener-prow bot requested review from ScheererJ and timebertt May 9, 2025 10:26
@gardener-prow gardener-prow bot added area/ipcei IPCEI (Important Project of Common European Interest) kind/enhancement Enhancement, improvement, extension cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 9, 2025
@rfranzke rfranzke changed the title [GEP-28] Preparings for etcd-druid taking over management of etcd [GEP-28] Preparations for etcd-druid taking over management of etcd May 9, 2025
@rfranzke rfranzke force-pushed the gep28/etcd-prep branch from 5afa1e1 to c9ddd65 Compare May 9, 2025 10:31
@ScheererJ
Copy link
Member

/assign

Copy link
Member

@ScheererJ ScheererJ left a comment

Choose a reason for hiding this comment

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

Thanks for preparing the etcd take-over.

It looks good already, but I would like to understand the reasoning for the special image vector override.

@rfranzke rfranzke force-pushed the gep28/etcd-prep branch from 8067eec to a902893 Compare May 9, 2025 11:40
@rfranzke rfranzke requested a review from ScheererJ May 9, 2025 12:24
@gardener-prow gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 9, 2025
@gardener-prow gardener-prow bot added cla: no Indicates the PR's author has not signed the cla-assistant.io CLA. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. cla: no Indicates the PR's author has not signed the cla-assistant.io CLA. labels May 12, 2025
rfranzke added 7 commits May 13, 2025 09:14
This is not allowed for mirror pods
We just mount a path from the host instead.
- change names from `etcd-<role>-0` to `etcd-bootstrap-<role>` to make
  it more clear when `kubectl get pod`
- use the same data directory that etcd managed by etcd-druid would use
  later (it shall just take over the existing data dir)
- With gardener@f9f6a46
  (part of gardener#11853) we only
  skipped static pods during the bootstrap phase. However, we actually
  never want GRM webhooks to react on static pod creations, even if we
  are beyond the bootstrap phase. Otherwise, the kubelet might fail to
  create mirror pods for control plane components if the GRM webhook is
  unavailable. This can lead to undesired behaviour.
- Otherwise, we might get issues when etcd-druid takes over etcd
  management later
- Review this commit with `git diff -w` (ignoring whitespaces)
- `yq` seems to enforce indentation, so let's live with it

Output:

$ go generate ./imagevector/imagevector.go
containers.go
Cloning etcd-druid...
Found etcd-wrapper tag: v0.4.4
Cloning etcd-wrapper at tag v0.4.4...
Found etcd version string: v0.0.0-20240911181550-c123b3ea3db3
Extracted commit hash: c123b3ea3db3
Cloning etcd repo to resolve tag...
Resolved etcd tag: v3.4.34
Updating containers.yaml with etcd v3.4.34 tag
charts.go
@gardener-prow gardener-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2025
@rfranzke
Copy link
Member Author

@ScheererJ Any further feedback?

Copy link
Member

@ScheererJ ScheererJ left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label May 13, 2025
Copy link
Contributor

gardener-prow bot commented May 13, 2025

LGTM label has been added.

Git tree hash: c3740a0683d900e7295917f4e612ef65d1c56d8b

Copy link
Contributor

gardener-prow bot commented May 13, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ScheererJ

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 13, 2025
@rfranzke
Copy link
Member Author

/retest

@gardener-prow gardener-prow bot merged commit 91df6e7 into gardener:master May 13, 2025
19 checks passed
@rfranzke rfranzke deleted the gep28/etcd-prep branch May 13, 2025 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/ipcei IPCEI (Important Project of Common European Interest) cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants