Skip to content

feat(storage): add the dormant Longhorn worker preparation step - #5686

Merged
devantler merged 1 commit into
mainfrom
claude/longhorn-worker-prep-5655
Nov 1, 2026
Merged

devantler merged 1 commit into
mainfrom
claude/longhorn-worker-prep-5655

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

Moving storage to Longhorn V2 needs each permanent worker's local disk rebuilt, and that is the most dangerous step of the migration. The large change that first held this step never converged in review, because a finding anywhere restarted review of everything, so this step now gets its own review.

What

Adds the guarded step that empties a worker, wipes only its temporary partition and brings it back, with a saved record so an interrupted run recovers instead of repeating the wipe. It also lets a deploy pass a worker that this step is deliberately holding. Nothing runs on the cluster from this change: the step refuses before writing anything until the new disk layout, the V2 capacity and the host settings land in later changes.

Part of #5655

Slice 4a of the split of the V2 preparation change. Adds the guarded
prepare-worker operation, its journal and the deploy gate's allowance for
a cordon that journal holds. Talos layout changes stay in the next slice,
so this has no live effect: a dispatch holds before any write.

Part of #5655

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor
Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 54 minutes.

@coderabbitai

coderabbitai Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 54 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 184e6e8 and 7b5e824.

Files selected for processing: 27
  • .github/workflows/prepare-longhorn-v2-node.yaml
  • scripts/migrate-longhorn-v2/README.md
  • scripts/migrate-longhorn-v2/api.go
  • scripts/migrate-longhorn-v2/authorization.go
  • scripts/migrate-longhorn-v2/authorization_test.go
  • scripts/migrate-longhorn-v2/budget.go
  • scripts/migrate-longhorn-v2/budget_test.go
  • scripts/migrate-longhorn-v2/capacity.go
  • scripts/migrate-longhorn-v2/capacity_test.go
  • scripts/migrate-longhorn-v2/client.go
  • scripts/migrate-longhorn-v2/client_test.go
  • scripts/migrate-longhorn-v2/config.go
  • scripts/migrate-longhorn-v2/config_test.go
  • scripts/migrate-longhorn-v2/hugepages.go
  • scripts/migrate-longhorn-v2/journal.go
  • scripts/migrate-longhorn-v2/journal_test.go
  • scripts/migrate-longhorn-v2/main.go
  • scripts/migrate-longhorn-v2/node.go
  • scripts/migrate-longhorn-v2/node_test.go
  • scripts/migrate-longhorn-v2/pools.go
  • scripts/migrate-longhorn-v2/pools_test.go
  • scripts/migrate-longhorn-v2/preflight.go
  • scripts/migrate-longhorn-v2/preflight_test.go
  • scripts/migrate-longhorn-v2/prepare.go
  • scripts/migrate-longhorn-v2/prepare_test.go
  • scripts/tests/test-talos-schematic-readback.sh
  • scripts/verify-talos-schematic-readback.sh

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.

Share

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@codex review for safety of the destructive worker reset path: journal recovery, what may run before the reset, and whether any dispatch can write on today's configuration

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-01T15:01:19.719724Z 7b5e824 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 7b5e824d50

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Evaluation at 7b5e824d50e738e8988e10a255d14c3b852184f6.

  • CI: CI 41318618343 and the workflow's own test job 41318618330 succeeded at this head, 50 checks settled with none failing. Codex found no major issues at this head; CodeRabbit was rate limited.
  • Dormant by test: a test reads the real production config and requires the migration pool lookup to fail. The tool does that lookup before any preparation step, so today a dispatch stops with no write. Whoever adds the V2 pool has to replace that test.
  • Driven against a fake cluster and host, not the real one: a whole preparation run, including the journal-first order, one reset only, and resume after an interrupted reset.
  • Mutants: nine safety checks disabled one at a time. Four survived at first, now all nine fail a test; a compile failure was not counted.
  • Deploy gate: the readback test still refuses a cordon with no journal, a journal for another Node UID, a completed journal, a second cordon and a non-worker cordon.

Not proven: nothing here has run on the cluster, and no real Talos volume read has been made through these checks. The one shared-path change is that deploy gate, which only widens when a matching journal exists, and none does.

@devantler
devantler marked this pull request as ready for review November 1, 2026 15:06
@devantler
devantler added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit 41dc946 Nov 1, 2026
50 checks passed
@devantler
devantler deleted the claude/longhorn-worker-prep-5655 branch November 1, 2026 18:35
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

1 participant