Repository navigation
fix(layer-stack): decide a definition's layer stack once and never format over data - #206
Draft
Andrei Kvapil (kvaps) wants to merge 14 commits into
Draft
Andrei Kvapil (kvaps) wants to merge 14 commits into
Andrei Kvapil (kvaps) wants to merge 14 commits into
Conversation
A definition that leaves its layer stack empty and sits in a group that sets one was read two ways: the REST and witness gates resolved the group's stack, while the satellite built the volume from the empty one, which is the default stack. In a LUKS group that put plaintext on disk under a definition every gate called encrypted. The stack is now decided once, by the controller, and written onto the definition, as upstream decides it at resource creation. A definition with nothing on disk takes the group's stack. One that already holds data keeps the stack its storage was built with: giving it the group's now would have the satellite build a different stack over that data. It is annotated with the group's stack it did not get, and a warning event names it, so such a volume reads as what it is and can be found. The controller-side resolver that walked the group on read had no callers left and is removed. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
The satellite read a definition's empty layer stack as the default and built the volume from it, even when the definition's group sets another stack. It now waits, on a short requeue, until the controller has written the group's stack onto the definition, and the intent builder refuses such a definition outright, so no path turns the empty stack into storage. A definition in no group, or in a group without a stack, keeps the default as before; a group that cannot be read applies nothing. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Format took "not a LUKS device" to mean "empty" and ran luksFormat. A volume that went to disk without LUKS and is later asked for it, by a layer stack that changed under it or a restore that brought data in, is not LUKS either, and luksFormat destroys it. Format now probes the device with `blkid -p` first and formats only one that carries no signature at all. DRBD internal metadata, a filesystem or a partition table refuses it with ErrDeviceHoldsData, naming what was found; a probe that cannot say the device is empty refuses it too. The apply of that replica fails with the reason, and nothing is written. A fresh thick LV that reuses extents can expose a leftover signature; the message says how to clear it. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
The layer stack pin looked only at what replicas had reported. A definition can hold data before any of them has: linstor-migrate adopts volumes that are already full and latches Initialized on them, and the latch is set once the controller has seen a real write. Either way the storage was built under the stack the definition carried, so the pin keeps it. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
The converter copied the resource definition's layer_stack column as is. LINSTOR resolves the stack per resource and often leaves that column empty, so the stack those volumes were built with lives only in LAYER_RESOURCE_IDS. A converted definition with an empty stack then took whatever blockstor resolved for it, not what is on disk: a LUKS volume lost its LUKS layer, and a definition in a group with another stack was read through that group's. An empty stack is now read off the replicas' layer chains. Replicas built in different ways leave it unset and are reported, as is a replica built otherwise than the stack its definition names, since the satellite would not bring it up as it is. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
The page said an RD with an empty stack inherits its group's at read time, which is what put plaintext on disk under encrypted definitions. It now says the stack is fixed once on the definition, by the REST create path or by the controller's pin, what happens to a definition that already holds data, what the operator sees, and how to find such definitions. The migration guide and the REST create path's comment say the same. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
A clone or restore wrote the source's stack onto the new definition as stored, reading an empty one as the default. Under a group that sets a stack, an empty one is not decided yet: the controller pins it to the group's stack or to the default depending on whether data is already there, and the satellite builds nothing until it does. The copy took the default regardless and kept it for good, so a shell of an empty definition in a LUKS group stayed plaintext with no data forcing it. The stack is now resolved through the group. An undecided source, leftover or prepared target is answered 503 (the CLI exits with an error) before anything is written, and a group that cannot be read is an error rather than the default. A volume-less clone copies no bytes, so it takes only a stack the source chose and is otherwise left for its own group to decide. A copy short of what its group asks for carries the unapplied-group-layer-stack annotation the controller puts on a definition pinned over data. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
A group that comes to set a layer stack leaves every definition under it with no stack of its own undecided: the satellite builds nothing for them, and clones and restores of them answer 503. Nothing enqueued them, so the pin waited for the periodic re-sync. Watch the groups and re-run the pin for those definitions when a group's stack is set or changed. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
The pin compared the stack it applied with the group's in order and case, while clone and restore compared them as sets. A group spelling its stack in another order or case flagged every definition holding data as short of it. Both now share one comparison. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
The recorder writes events.k8s.io events, which the stand's controller role did not grant, so the layer-stack warning was dropped. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
The unapplied-group-layer-stack annotation was set on any difference from the group's stack, so an encrypted copy in a plaintext group was flagged as if it were the plaintext volume the annotation exists to find. It is now set only when a layer the group asks for is missing. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
LINSTOR builds a diskless replica as DRBD over STORAGE whatever the definition's stack, so counting it made the replicas of every LUKS definition with a tiebreaker look divergent and left its stack unset. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
…group Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A definition that left its layer stack to its resource group was read two ways. The API and the placement gates took the group's stack, while the satellite built the volume from the definition's own empty stack. In a LUKS group that means a definition reported as encrypted with plaintext volumes under it.
Now the stack is decided once and stored on the definition:
blockstor.io/unapplied-group-layer-stackannotation and aLayerStackNotInheritedwarning event. Groups are watched, so definitions are picked up as soon as their group gets a stack.luksFormaton a device that carries anyblkidsignature. On a thick LV that reuses old extents this can stop a fresh volume untilwipefs.linstor-migratereads an empty definition stack from the layer chain of its diskful replicas and reports replicas that disagree.docs/layer-stack.mdsays where the stack is decided and how to find the affected definitions.The warning event needs
events.k8s.ioevents (create, patch) in the controller RBAC.Stacked on #190.