LightOS driver: record volume project in provider_id - #28
Conversation
c775cef to
e97dca6
Compare
Retyping a volume between two volume types that name different LightOS projects left the source volume behind on the storage. It disappeared from OpenStack's view but kept consuming capacity. The only trace was a permission-denied or not-found error in the cinder-volume log. The trigger is any retype that changes lightos:project_name. A LightOS volume cannot move between projects, so Cinder copies the data into a new volume in the destination project and deletes the original afterwards. A LightOS volume is addressed by its project, so the driver has to know which project a volume lives in. It used to read that from the volume type on every call, and a volume type is mutable. Cinder deliberately leaves the destination volume type on the database record it uses to delete the source volume. So the driver asked the destination project to delete a volume that lived in the source project, and the delete failed. The project is now recorded on the volume when it is created, along with the LightOS volume UUID, and travels with whichever record addresses that volume. The volume type is only a fallback. Volumes created before this change are stamped when the volume service starts. Recording the UUID also drops a lookup: operations that need it no longer resolve it from the volume name first. A delete that finds nothing now counts as success, because the volume is addressed by a recorded identity rather than a derived one. If this recurs, look for a delete addressed to a project the volume was never created in. Assisted-by: Claude Opus 5 Signed-off-by: Yuval Brave <yuval@lightbitslabs.com>
A cross-project retype moved no data: the new volume was created in the source project, so the volume never reached the project the operator asked for. Caught on a live cluster - a retype from goose to fox left both volumes in goose and fox empty. Cinder builds the temporary volume for a migration by copying most fields off the source volume, and provider_id is one of them. The driver read the project back out of that inherited provider_id, which still named the source project. A volume type says where to create a volume. A provider_id records where a volume already lives. Creation now reads the volume type, and only operations on an existing volume read provider_id. Assisted-by: Claude Opus 5 Signed-off-by: Yuval Brave <yuval@lightbitslabs.com>
The backfill that records a project on volumes created before we stored one issued a lookup per volume. It runs from init_host, before the volume service accepts requests, so on a host with tens of thousands of volumes that alone kept cinder-volume out of service for minutes. A lookup that could not reach an API server raised, and nothing between here and init_host catches it, so a single unreachable server stopped the service from starting at all. Only the project is needed, and the volume type already carries it, so the backfill no longer talks to the cluster. provider_id now accepts a project on its own, and the UUID is resolved by name when something needs it. Volumes that already carry a project are skipped, so a restart of a stamped deployment produces no updates. Anything unexpected while resolving a project leaves that volume unstamped instead of aborting startup. It falls back to its volume type, as it did before being stamped. Assisted-by: Claude Opus 5 Signed-off-by: Yuval Brave <yuval@lightbitslabs.com>
e97dca6 to
0aad280
Compare
|
small clarifications to PR cover letter: A LightOS volume is addressed by its project, and the driver read that project So the driver asked the destination project to delete a volume living in the source project. -> A volume's project and LightOS UUID are now recorded in its provider_id at Possibly note that project field is inited for all existing volumes at driver startup, Three commits: record the project and resolve from it; create volumes in the |
ronen-lb
left a comment
ronen-lb
left a comment
There was a problem hiding this comment.
Review scope: full diff.
Review assisted by an AI agent; findings were checked by a human before posting.
|
|
||
| try: | ||
| project_name = self._get_volume_type_project_name(volume) | ||
| except Exception: |
There was a problem hiding this comment.
[major] _get_volume_type_project_name catches every exception itself and returns LIGHTOS_DEFAULT_PROJECT_NAME, so this handler cannot fire, and the two cases it is meant to separate arrive here identically: an untyped volume, where default is the right answer, and a typed volume whose type could not be read, where it is a guess. In the second case the volume is stamped default permanently — and since provider_id now wins over the volume type, a volume that lives elsewhere stops resolving to its real project even once the type is readable again.
Having the helper distinguish "no project specified" from "lookup failed" would let a failed lookup leave the volume unstamped, which is what the comment on the next line describes.
Motivation: the provider_id backfill guessed the default project when a volume's type could not be read, and recorded the guess in provider_id, where it outranks the volume type - a volume living elsewhere would never resolve to its real project again, even once its type became readable. Alongside it, three smaller review findings: the release note linked a tracker its readers cannot open; a delete answered NOT_FOUND counted as success with no trace, which is the only signal left when a delete is addressed to the wrong project; and the retype tests carried a parameter no caller used while duplicating the migration hand-over inline. Implementation: _get_volume_type_project_name resolves untyped volumes to the default project and lets a failed type read raise, so the backfill leaves such volumes unstamped and they keep resolving from their volume type as before. Runtime address resolution keeps the old default-project guess, now behind an explicit catch that logs a warning. A NOT_FOUND delete logs the project and UUID it was addressed to. The migration hand-over moved to a helper both retype tests share, and create_cloned_volume's returned model_update is now asserted, since a clone that stops recording its address would silently fall back to the volume type. Bug root cause: _get_volume_type_project_name caught every exception internally and returned the default project, so the backfill's except clause was dead code and a failed type read was indistinguishable from a volume that legitimately has no type. Issue: LBM1-44442 Assisted-by: Claude Fable 5 Change-Id: I43e73b8abefbd4541d16a76d551bb0d8d616e5ad Signed-off-by: Yuval Brave <yuval@lightbitslabs.com>
2 participants
PR description
Retyping a volume between two volume types that name different LightOS projects
left the source volume behind on the storage. It vanished from OpenStack's view
but kept consuming capacity, leaving only a permission-denied or not-found error
in the cinder-volume log.
A LightOS volume is addressed by its project, and the driver read that project
from the volume type on every call. A volume type is mutable. When a retype
needs the volume moved, Cinder deletes the source through a record that
deliberately keeps the destination volume type — a volume type is "not a key
for volume deletion" per
Volume.finish_volume_migration. So the driver askedthe destination project to delete a volume living in the source project.
A volume's project and LightOS UUID are now recorded in its
provider_idatcreation, and travel with whichever record addresses that volume, because
Cinder swaps
provider_idbetween the two records during the hand-over. Thevolume type stays as a fallback, so no data migration is needed.
Three commits: record the project and resolve from it; create volumes in the
project their volume type names (Cinder copies
provider_idonto thetemporary migration volume, where it still names the source project); and stamp
pre-existing volumes at service start. That backfill runs before the service
accepts requests, so it takes the project from the volume type and makes no
cluster calls —
provider_idaccepts a project on its own, and the UUID isresolved by name when needed.
How was the PR tested?
VolumeManager.update_migrated_volumeandfinish_volume_migrationandassert the source volume is gone. Others cover resolution, the volume-type
fallback, an unparseable
provider_id, creation ignoring an inherited one,and the backfill — that test fails on any cluster call, so "no IO" is
asserted rather than assumed. 44/44 pass; 5 fail without this change.
flake8 clean.
The source project kept only its untouched volume, the destination held the
migrated one, and the delete went to the source project. Before this change
it went to the destination and returned 404.
this branch. All stamped, no volume lookups, 940ms. A per-volume-lookup
approach took 6.8s for the same 100, which extrapolates to ~10 min at 10,000.
Three restarts of 100 already-stamped volumes took ~280ms each.
Not tested: the cross-project retype ran on commit 2 of 3, which the backfill
change does not touch. Test volumes were empty, so no migration copied real
data.
PR dependencies
Jira Ticket
Issue: LBM1-44442