bug: upsertStorageClass is create-only — Pool StorageClassParameters edits on an existing Pool silently no-op

[boddumanohar]

Summary

upsertStorageClass (operator/internal/controller/simplyblockpool_controller.go:352-397) is create-only, not create-or-update, despite its name. If a Pool's generated StorageClass already exists, editing the Pool's storageClassParameters has zero effect — silently, with no error, Event, or status condition.

Root cause

if err := r.Create(ctx, sc); err != nil && !apierrors.IsAlreadyExists(err) {
    return err
}
return nil

The function only ever calls Create. When the StorageClass already exists, Create returns AlreadyExists, which is caught and swallowed. The function returns success having changed nothing.

Reproduction

1. Create a Pool CR. The reconciler creates its StorageClass with some set of parameters/allowedTopologies.
2. Edit the Pool's spec.storageClassParameters (any field — e.g. flip a boolean, change a string param).
3. The reconciler recomputes the correct new parameters/allowedTopologies and calls upsertStorageClass.
4. Create hits AlreadyExists → swallowed → no-op.
5. Nothing happens. The live StorageClass keeps serving with its original parameters indefinitely. No error, no Kubernetes Event, no status condition — no signal to the admin that their change had no effect.

Why this matters beyond any single feature

This isn't specific to one StorageClass parameter — it affects any field under StorageClassParameters, present or future. Any admin workflow of "edit an existing Pool to change how its volumes are provisioned" silently fails today. It was found while designing client-side compression/dedup (issue #277), where an admin flipping clientCompression/clientDeduplication on an existing pool would hit exactly this — but the bug predates and is independent of that design.

Complicating factor

Kubernetes StorageClasses are largely immutable via the API — parameters and allowedTopologies cannot be patched on an already-existing object. So a literal "update in place" fix isn't available; the real fix has to either detect-and-surface the mismatch, or replace-and-recreate (with the blast-radius implications that has for already-bound PVCs referencing the old StorageClass by name).

Suggested directions (not mutually exclusive)

• (a) Detect and surface loudly: compare the Pool spec's intended parameters/allowedTopologies against the live StorageClass's actual values; on mismatch, emit a Kubernetes Event and/or a status condition on the Pool telling the admin the change was ignored and why (e.g. "requires a new Pool/StorageClass — existing StorageClasses are immutable").
• (b) Document prominently at the field level: at minimum, every StorageClassParameters field's kubebuilder doc comment should state that changing it after the StorageClass already exists has no effect, so this is visible wherever the CRD/API is read — not just in a design doc.

References

Found during design work for #277, see PR #398 (operator/docs/designs/design-issue-277-client-side-compression.md, Section 13 "Open Questions and Discussion").