Skip to content

fix(linstor-migrate): report an unreadable layer stack - #226

Open
Andrei Kvapil (kvaps) wants to merge 1 commit into
mainfrom
fix/migrate-report-broken-layer-stack
Open

Andrei Kvapil (kvaps) wants to merge 1 commit into
mainfrom
fix/migrate-report-broken-layer-stack

Conversation

@kvaps

Copy link
Copy Markdown
Member

Follow-up to #223. The stack check read the RD row's layer_stack and ignored the parse error, so a column that is not a JSON list counted as no stack at all.

When the replicas then refused the definition, the warning said it had "no layer stack of its own", and the value itself never showed up. When the replicas agreed, the only trace was the generic unparsable list ... dropped, with nothing about which stack the definition got.

Now:

  • a refusal names the unreadable value, e.g. an unreadable layer_stack "[\"DRBD\"," and its replicas were built as ...;
  • a definition that converts says it took the stack its replicas were built with and which one, or that it has no diskful replica to read one from and is left unset.

Each path reports it once.

I did not make an unreadable column refuse on its own. With every diskful replica chained and all chains agreeing, the chains are what is on disk, and that is the same evidence #223 already trusts for an empty column. Taking them means the satellite adopts each volume as it was built. Anything weaker (replicas built two ways, a replica without a chain) is refused as before, now with the right reason.

The site's migration page still described the stack warnings from before #223 ("left unset; set spec.layerStack before adopting"). It now lists the refusal and the new warning. docs/linstor-migration.md gets one sentence.

Tests: TestAnUnreadableRDStackIsReportedOnce covers agreeing replicas, disagreeing ones, a replica without a chain and no replicas. Reverting each part of the fix turns its case red.

@kvaps
Andrei Kvapil (kvaps) marked this pull request as ready for review November 2, 2026 21:31
@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

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 10 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
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 95f7f83f-49a7-4066-9050-b6874a632bde
Commits

Reviewing files that changed from the base of the PR and between e49b274 and ea5433b.

Files selected for processing: 4
  • docs/linstor-migration.md
  • pkg/linstormigrate/convert.go
  • pkg/linstormigrate/unreadable_layer_stack_test.go
  • site/content/en/docs/migration.md
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

The stack check read the RD row's layer_stack without looking at the
parse error, so an unreadable column counted as no stack. When the
replicas then refused the definition, the warning said it had "no
layer stack of its own" and the value was never shown; when they
agreed, the only trace was a generic "unparsable list dropped".

The refusal now names the unreadable value, and a definition that
converts says which stack it took from its replicas, or that it has
none to take. Either way it is reported once.

The site's migration page still described the warnings from before
the refusal; it now lists the refusal and this warning.

Assisted-by: LLM
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant