Skip to content

storagecluster/storagenodeset: stop exposing sbcli-private tuning params via CRD spec #379

Description

@boddumanohar

Background

#152 removed DistrBs/DistrChunkBs hardcoding from ClusterAddParams because sbcli's cli-reference.yaml marks --distr-bs/--distr-chunk-bs as private: true — internal/advanced backend knobs that sbcli deliberately hides from CLI users. Since sbcli itself doesn't consider these user-facing, the operator shouldn't expose them as CRD spec fields either.

A follow-up audit of every field the operator sends to sbcli's API against simplyblock_cli/cli-reference.yaml's private: true markers found four more fields with the exact same problem: they're marked private in sbcli but are directly settable via the operator's CRD spec.

Violations found

CRD field sbcli flag (private) Wiring
StorageClusterSpec.PageSizeInBlocks (operator/api/v1alpha1/storagecluster_types.go:243) --page_size — private at cli-reference.yaml:852 (create) & :1084 (add) operator/internal/controller/simplyblockstoragecluster_controller.go:246
StorageClusterSpec.MaxQueueSize (storagecluster_types.go:246) --max-queue-size — private at :993/:1182 simplyblockstoragecluster_controller.go:256
StorageClusterSpec.InflightIOThreshold (storagecluster_types.go:249) --inflight-io-threshold — private at :999/:1188 simplyblockstoragecluster_controller.go:257
JournalManagerSpec.PercentPerDevice (operator/api/v1alpha1/storagenodeset_types.go:32) --jm-percent — private at :195 operator/internal/controller/storagenode_controller.go:364, via journalManagerPercentPerDeviceFromSpec (:495-499)

Proposed fix

Same pattern as the DistrBs/DistrChunkBs fix (commit 0677de0): drop these fields from the CRD spec (or stop sourcing them from it) and let the backend default apply, rather than tracking sbcli-private params through the CRD surface.

Judgment call (not included above, needs a decision)

StorageNode(Set)Spec.SpdkImage / SpdkProxyImage also map to sbcli-private flags (--spdk-image, --spdk-proxy-image), but unlike the tuning knobs above, these are container image references — arguably legitimate for a Kubernetes-native operator to own directly (e.g. for phased rollouts), since there's no other mechanism to select the image. Flagging for a separate decision on whether these should also be locked down.

Out of scope / noted in passing

ClusterUpdateParams (operator/internal/utils/types.go:53) is unused anywhere in the controllers, and doesn't match the real /api/v2/clusters/{id} PUT contract (which only accepts name). Separate cleanup, not part of this issue.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions