linode: attribute NodeBalancers by lke<id> label; tag volumes before every cluster delete - #1296
Conversation
…every delete Follow-up to #1287. Two teardown-leak paths survived that PR, found by auditing the live account 24h after deploy: 3 orphaned NodeBalancers and 32 orphaned volumes the reaper could not attribute. - NodeBalancers: the Linode CCM reconciles a NodeBalancer's tags back to its defaults, so the pmm-qa-run tag create-lke stamped on it does not survive -- the sweep never matched any NodeBalancer. Attribute by the immutable lke<clusterid>- label instead (the CCM bakes the cluster id into it); delete one only when its cluster is no longer live. - Volumes: provision-time tagging only covers volumes attached when create-lke exits. Clusters grow PVCs afterwards (scaled StatefulSet, backups), and those leak untagged. Tag the cluster's attached volumes again right before every cluster delete (destroy-lke and the reaper), while the nodes still exist, via a shared tag-lke-resources.sh -- also covers clusters provisioned before volume tagging shipped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DBhvZuEUQU8ym347Xat4Yy
|
Warning Review limit reachedNext included review available in 49 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
WalkthroughThe change adds Merge Risk: 🟡 Moderate · up to The volume-tagging change can miss volumes beyond the first API page and can remove unrelated volume tags, causing cleanup failures and metadata loss. These bounded correctness issues should be fixed before merging. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Team
Run ID: fb34483b-31d1-4c8b-bf8c-2d1fa9bf20ec
📒 Files selected for processing (6)
.claude/integrations/slack/relay/relay.js.claude/skills/linode-ha-provisioning/SKILL.md.claude/skills/linode-ha-provisioning/scripts/create-lke-pmm-ha.sh.claude/skills/linode-ha-provisioning/scripts/destroy-lke.sh.claude/skills/linode-ha-provisioning/scripts/prune-lke-orphans.sh.claude/skills/linode-ha-provisioning/scripts/tag-lke-resources.sh
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
percona/pmm-qa(manual)percona/pmm(manual)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
…sources Address CodeRabbit review on #1296: - Page through /volumes instead of reading only the first page, so a cluster's volumes are still found when the account holds more than one page. - Merge our tags with each volume's existing tags (PUT replaces the whole list) so unrelated tags are not dropped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DBhvZuEUQU8ym347Xat4Yy
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Gate gap, for the record: the only check that touched this diff is |
…comments Address review on #1296: - prune-lke-orphans.sh: gate NodeBalancer deletion on the NB being idle (no backend node up) in addition to its lke<id> cluster being gone. The CCM lets a live cluster adopt another cluster's NodeBalancer (keeping the original cluster id in its label), so a label-only rule could delete a live cluster's load balancer. An orphan has zero backends up once its cluster is gone; a reused or still-serving NB does not. Fail safe: keep the NB if its configs are unreadable. - Trim duplicated/inaccurate header and call-site comments across tag-lke-resources.sh, destroy-lke.sh, and relay.js. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DBhvZuEUQU8ym347Xat4Yy
3 participants
Problem
Follow-up to #1287. Auditing the live Linode account ~24h after that PR shipped turned up teardown leaks it still could not clean: 3 orphaned NodeBalancers and 32 orphaned Block Storage volumes the reaper was running past every 15 min but never deleting. Two distinct gaps in the positive-attribution model:
NodeBalancers were attributed by a tag the CCM strips.
create-lkestamped each cluster's NodeBalancer with itspmm-qa-run:<id>tag, but the Linode CCM reconciles a NodeBalancer's tags back to its defaults (["kubernetes"]) shortly after. So by teardown the tag is gone andprune-lke-orphans.shmatched no NodeBalancer, ever. (Confirmed on a torn-down test cluster whose NodeBalancer still existed 24h later with only["kubernetes"].) This also means linode: delete orphaned account tags, volumes, and NodeBalancers on teardown #1287's teardown verification passed falsely — it checked immediately, before the CCM had stripped the tag.Volumes created after provisioning were never tagged. The provision-time pass only tags volumes attached when
create-lkeexits. A cluster grows PVCs afterwards (a scaled StatefulSet, backups), and those later volumes leaked untagged. Clusters provisioned before linode: delete orphaned account tags, volumes, and NodeBalancers on teardown #1287 (or before the token had Volumes R/W) are untagged entirely.The 32 orphaned volumes were all from two clusters provisioned/destroyed a day before #1287 deployed — pure pre-fix backlog, cleared by hand.
Changes
prune-lke-orphans.sh— attribute NodeBalancers by their immutablelke<clusterid>-label (baked in by the CCM) instead of a tag. Delete one only when its cluster id is no longer in the live cluster list. Volume attribution (bypmm-qa-run:tag whose run has no live cluster) is unchanged.tag-lke-resources.sh(new, shared) — tags a cluster's currently-attached volumes with itspmm-qa-run:<id>. Resolves the run from the cluster's own tag/label when the caller only knows the cluster id. NodeBalancers are deliberately not tagged (see above).destroy-lke.shand the relay reaper (relay.js) — calltag-lke-resources.shright before deleting the cluster, while the nodes (and volume attachments) still exist, so post-provision and pre-fix volumes are attributed too.create-lke-pmm-ha.sh— its EXIT-trap tagging now delegates to the shared script (no behaviour change).SKILL.md— documents label-based NodeBalancer attribution and tag-before-delete.Validation
shellcheck -S warningclean on all four scripts;node --checkonrelay.js.PMM-13860had 16/16 volumes correctly tagged (so volume tagging works and is durable), while the orphans carried no usable attribution.🤖 Generated with Claude Code
https://claude.ai/code/session_01DBhvZuEUQU8ym347Xat4Yy
Generated by Claude Code