Repository navigation
[etcd] Update etcd application (fix resources and headless services) - #1128
Conversation
|
Warning Rate limit exceededAndrei Kvapil (@kvaps) has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 14 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughThis update modifies the etcd Helm chart by incrementing its version, updating resource configuration and certificate DNS names, simplifying endpoint specification, and adjusting the version mapping. The resource configuration is streamlined, and certificate specifications now include additional DNS names for improved service discovery. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HelmChart
participant Kubernetes
participant EtcdService
User->>HelmChart: Deploy etcd chart (v2.9.0)
HelmChart->>Kubernetes: Render manifests with sanitized resources and updated endpoints
Kubernetes->>EtcdService: Create etcd pods with new resource config and certificates
EtcdService->>Kubernetes: Register with single endpoint and expanded DNS names
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
b4d7b05 to
1b59390
Compare
Andrei Kvapil (kvaps)
changed the title
Andrei Kvapil (kvaps)
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/extra/etcd/templates/datastore.yaml (1)
48-49: Nit: unchanged annotation flagged as diff
Line 48 is identical to the previous revision; remove the stray change to keep git history clean.packages/extra/etcd/templates/etcd-cluster.yaml (2)
155-157: Add full-qualified service DNS to certificate
etcd.<ns>.svc.cluster.local(or custom cluster-domain) is still missing.
Many clients use the FQDN returned by the Kubernetes API; without it TLS verification may fail.
Recommend appending the cluster-domain variant, e.g.:dnsNames: - etcd - etcd.{{ $.Release.Namespace }}.svc + - etcd.{{ $.Release.Namespace }}.svc.cluster.local
191-193: Same FQDN omission for peer certificate
Mirror the addition here to keep server & peer certs symmetric.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/extra/etcd/Chart.yaml(1 hunks)packages/extra/etcd/templates/datastore.yaml(2 hunks)packages/extra/etcd/templates/etcd-cluster.yaml(3 hunks)packages/extra/etcd/values.yaml(1 hunks)packages/extra/versions_map(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (5)
packages/extra/versions_map (1)
15-16: Pin-sha looks good – please verify SHA and remember to lock 2.9.0 later
2.8.0now points to4369b031, removing the transientHEAD, 👍.
Before merging, double-check that the hash exists in the repo history and represents the exact chart state you tagged.
For2.9.0we still keepHEAD; don’t forget to replace it with a real commit once the version is cut, otherwise reproducible builds will break.packages/extra/etcd/Chart.yaml (1)
6-6: Version bump LGTM
Nothing else changed in chart metadata, semantic-version ordering preserved.packages/extra/etcd/templates/datastore.yaml (1)
8-9: Single-service endpoint may break multi-replica fail-over
kamajiDataStore previously received the full replica list; switching to one ClusterIP (etcd.<ns>.svc:2379) relies on kube-proxy load-balancing.
Points to verify:
- Does the Etcd operator always create the non-headless
Servicenamedetcd(in addition to the headless one)?- If the ClusterIP is down (e.g. because no endpoints yet), will control-plane consumers retry until ready, or should we keep at least one pod FQDN?
- Some etcd client libraries still perform member-list calls that return pod names; make sure certificates include those (they do), but also ensure DNS can resolve them.
Consider leaving the explicit pod endpoints or providing both for safer roll-outs.
packages/extra/etcd/values.yaml (1)
11-14: Flatresourcesstructure – confirm library compatibility
cozy-lib.resources.sanitizenow receivescpu: 4 memory: 1GiVerify that the helper understands the flat layout and converts it into both
requestsandlimits(or whatever policy you expect).
If it still expects the classicrequests/limitsmap, this will silently drop all resource settings.packages/extra/etcd/templates/etcd-cluster.yaml (1)
50-50: Good move to centralised sanitation
Delegating to the shared helper reduces chart-specific logic.
1b59390 to
80541d3
Compare
Signed-off-by: Andrei Kvapil <[email protected]>
80541d3 to
a5ff9bf
Compare
3 participants
ref to #1127, clastix/kamaji#856 and cozystack/etcd-operator#291
Summary by CodeRabbit