Skip to content

Commit 49d210f

Browse files
kvapsclaude
andcommitted
refactor: drop the KV-store machinery entirely (Phase 10.4 closure)
Investigation of the production cozystack cluster + a grep of piraeusdatastore/linstor-csi confirms that linstor-csi has never written to a generic LINSTOR KV instance. Per-PVC CSI metadata lives on the RD as `Aux/csi-volume-annotations` / `Aux/csi-provisioning-completed-by` aux properties — not in any KV store. The only KV consumer in linstor-csi is `csi-backup-mapping`, used solely for the L2L (linstor-to- linstor) backup-remote feature. Clusters without explicit `linstor remote create linstor ...` invocations show an empty `linstor c kv list` output. `csi-snapshot-shippings` and `csi-volumes` KV instances are both fiction. Deleted: - api/v1alpha1/kventry_types.go + CRD/RBAC/sample manifests - pkg/store/k8s/kv_store.go - pkg/store/inmemory_volume_definition.go's inMemoryKVStore - store.KeyValueStore interface + Store.KeyValueStore() accessor (`pkg/store/store.go`) - storetest.RunKeyValueStore + every caller in both backends - effectiveprops.LegacyControllerProps + LegacyControllerPropsInstance const + the matching tests - pkg/rest/kv_store_test.go (mass test of CSI-volumes routing through KV — that routing was never load-bearing) - The csi-volumes annotation special-case routing in pkg/rest/kv_store.go (read/write per-RD annotations from POST/PUT) — linstor-csi doesn't actually write there. - The legacy KV fallback for the cluster passphrase in pkg/rest/encryption.go (production path went Secret-only in Phase 10.4; the fallback was scaffolding). - The Java→KVEntry-shaped controllerPropsInstance / passphraseKey consts (no longer reachable). Refactored: - pkg/rest/kv_store.go is now a minimal stub: GET → empty list / empty instance envelope, POST/PUT/DELETE → 200 no-op so `linstor c kv set` doesn't trip on 405 in interactive probes. - pkg/rest/controller_props.go rebased onto ControllerConfig.Spec.ExtraProps via the apiserver Client. Auto-creates the singleton ControllerConfig on first write. - pkg/rest/encryption.go drops the Store-backed fallback; requires the apiserver Client. - pkg/rest tests get a shared `newFakeRESTClient` helper so every server-with-store fixture gets a non-nil Client wired by default. Manifests regenerated (controller-gen + RBAC). The Secrets + ControllerConfig RBAC entries were re-added manually after controller-gen wiped them. PLAN ticks: 705 (csi-snapshot-shippings) + 707 (kventries drop) both close. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent 7df3eba commit 49d210f

27 files changed

Lines changed: 215 additions & 2056 deletions

PLAN.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -713,9 +713,9 @@ on the affected clusters.
713713
- [x] **`ControllerProps` instance → typed singleton CRD** (2026-05-10). `ControllerConfig` cluster-scoped CRD with `Spec.DRBDOptions` + `Spec.PassphraseSecretRef` + `Spec.ExtraProps`; canonical name `default`. ResourceReconciler.resolveEffectiveProps feeds the typed scope into `drbd.ResolveDRBDOptions` (was nil before) and folds ExtraProps into the output Props bag. Legacy KVEntry-shaped controllerProps() reader stays as forward-compat fallback for pre-migration clusters; both paths converge on the same wire shape.
714714
- [x] **Cluster passphrase → native `Secret`** (2026-05-10). REST endpoints (`/v1/encryption/passphrase` POST/PATCH/PUT) now route through a native Secret when `rest.Server` is wired with a controller-runtime `Client` + `Namespace` (the production path). Secret name resolved from `ControllerConfig.Spec.PassphraseSecretRef.Name` with default `blockstor-cluster-passphrase`; data key `passphrase`. Namespace picked from `--controller-namespace` → `$POD_NAMESPACE` → fallback `blockstor-system`. Legacy ControllerProps KV path stays available when `Client` is nil (in-memory-store tests, pre-migration clusters). Pinned by 4 new unit tests (Secret create, PATCH match/mismatch via Secret, rotation lands in Secret, ControllerConfig override honoured). RBAC manifest extended with `secrets` (create/get/list/patch/update/watch) and `controllerconfigs` (get/list/watch).
715715
- [x] **`csi-volumes` instance → ResourceDefinition annotation** (2026-05-10). REST handler routes `/v1/key-value-store/csi-volumes` traffic onto `ResourceDefinition.metadata.annotations["blockstor.io/csi-volume-data"]`. Each `OverrideProps` entry's key matches an RD name; the per-key value lands on that RD's annotation. GET assembles the inverse view by walking every RD. Per-key `DeleteProps` clears just one annotation; whole-instance DELETE strips the annotation from every RD that had it. NotFound on the RD lookup is soft-skipped so racy provision/deprovision sequences don't fail the batch. Other KV instances flow through the legacy KVEntry-backed store unchanged. Lands on RD (per-PVC) rather than Resource (per-replica) because csi-volumes is PVC-scoped — RD is the natural home; the original PLAN wording was off-by-one. Pinned by 3 contract tests.
716-
- [~] **`csi-snapshot-shippings` instance → per-Snapshot annotation** (2026-05-10, partial). Wire shape (`apiv1.Snapshot.Annotations`) + k8s store round-trip plumbing landed alongside the csi-volumes migration; the REST routing handler is deferred until the actual KV-key shape (`<snapName>` vs `<rdName>/<snapName>`) is verified against a real linstor-csi-snapshot-shipper trace — getting it wrong silently breaks the shipper. Current state: KV instance still flows through KVEntry-backed store, ready to swap in routing logic via the same pattern as csi-volumes.
716+
- [x] ~~`csi-snapshot-shippings` instance → per-Snapshot annotation~~ (2026-05-11, retired). The `csi-snapshot-shippings` KV instance is fiction — linstor-csi has never written to it. A grep of [piraeusdatastore/linstor-csi](https://github.com/piraeusdatastore/linstor-csi) shows only one KV consumer: `csi-backup-mapping`, used exclusively for the L2L (linstor-to-linstor) backup-remote feature when operators wire `linstor remote create linstor ...`. CSI-side per-PVC metadata lives on the RD via `Aux/csi-volume-annotations` / `Aux/csi-provisioning-completed-by` Aux properties, not in any KV instance. Production cozystack clusters show empty `linstor c kv list` output. Migration moot; the whole KV-store machinery in blockstor is now a stubbed compatibility surface — see line 707.
717717
- [x] **REST `/v1/key-value-store/{instance}` rewritten** on top of the new homes (2026-05-10). `pkg/rest/kv_store.go.handleKVGet/handleKVSet/handleKVDelete` special-case the `csi-volumes` instance: GET assembles the map by listing ResourceDefinitions and reading each one's `blockstor.io/csi-volume-data` annotation (`readCSIVolumesAnnotations`), POST writes each entry onto the matching RD (`applyCSIVolumesAnnotations`), DELETE strips the annotation from every RD that carries it. golinstor's `linstor c kv set/get/delete csi-volumes` sees the same wire shape it always did. Other KV instances continue to flow through the KVEntry-backed store until their migration lands (csi-snapshot-shippings is the only remaining one — line 705 [~]).
718-
- [ ] **Drop `kventries.blockstor.io.blockstor.io` CRD** type registration, `api/v1alpha1/kventry_types.go`, `pkg/store/k8s/kv_store.go`, the `KeyValueStore` interface in `pkg/store/store.go`.
718+
- [x] **Drop `kventries.blockstor.io.blockstor.io` CRD + KeyValueStore machinery** (2026-05-11). Deleted: `api/v1alpha1/kventry_types.go` + its CRD/RBAC manifests, `pkg/store/k8s/kv_store.go`, the `KeyValueStore` interface + `Store.KeyValueStore()` accessor in `pkg/store/store.go`, the `inMemoryKVStore` in `pkg/store/inmemory_volume_definition.go`, the `RunKeyValueStore` shared test in `pkg/store/storetest/storetest.go` + every caller, `pkg/effectiveprops.LegacyControllerProps` + the matching tests, the `Aux/csi-volume-annotations` special-case routing in `pkg/rest/kv_store.go`, the per-RD annotation read/write paths, the legacy KV fallback for the cluster passphrase in `pkg/rest/encryption.go`. `pkg/rest/kv_store.go` now exposes a minimal stub (GET → empty list / empty instance; POST/PUT/DELETE → 200 no-op) so `linstor c kv list` returns the same empty answer the production cluster already returns from upstream LINSTOR. `pkg/rest/controller_props.go` rebased onto `ControllerConfig.Spec.ExtraProps` via the apiserver Client. Net: about 2 200 lines + 5 manifests removed.
719719

720720
### 10.5 — `ApplyStoragePools` made non-stub (absorbs the existing architectural-debt item)
721721

api/v1alpha1/kventry_types.go

Lines changed: 0 additions & 88 deletions
This file was deleted.

api/v1alpha1/zz_generated.deepcopy.go

Lines changed: 0 additions & 96 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

config/crd/bases/blockstor.io.blockstor.io_kventries.yaml

Lines changed: 0 additions & 129 deletions
This file was deleted.

config/rbac/kustomization.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,6 @@ resources:
2222
# default, aiding admins in cluster management. Those roles are
2323
# not used by the blockstor itself. You can comment the following lines
2424
# if you do not want those helpers be installed with your Project.
25-
- kventry_admin_role.yaml
26-
- kventry_editor_role.yaml
27-
- kventry_viewer_role.yaml
2825
- snapshot_admin_role.yaml
2926
- snapshot_editor_role.yaml
3027
- snapshot_viewer_role.yaml

config/rbac/kventry_admin_role.yaml

Lines changed: 0 additions & 27 deletions
This file was deleted.

0 commit comments

Comments
 (0)