refactor(api)!: move scheduling fields under spec.scheduling

[jdheyburn]

This PR closes #300

Summary

Group affinity, nodeSelector, tolerations, topologySpreadConstraints, and priorityClassName into a new spec.scheduling block (SchedulingSpec) on ValkeyClusterSpec instead of leaving them at the top level. This is purely a relocation: the controller reads the same values through cluster.Spec.Scheduling (nil-guarded) and propagates them to each ValkeyNode unchanged. ValkeyNode itself keeps its own top-level fields.

Features / Behaviour Changes

No behaviour changes here, this is just an API change to set up for future PRs under #299.

Moves these fields from ValkeyCluster.spec:

spec:
  affinity:
  tolerations:
  topologySpreadConstraints:
  nodeSelector:
  priorityClassName:

to:

spec:
  scheduling:
    affinity:
    tolerations:
    topologySpreadConstraints:
    nodeSelector:
    priorityClassName:    

Checklist

Before submitting the PR make sure the following are checked:

• This Pull Request is related to one issue.
• Commit message explains what changed and why
• Tests are added or updated.
• Documentation files are updated.
• I have run pre-commit locally (pre-commit run --all-files or hooks on commit)

[melancholictheory]

clean relocation, and the nil-guard on cluster.Spec.Scheduling reads right (zero-value SchedulingSpec when unset, so nodes get the same empty placement as before). the shard-aware TSC augmentation still works too, since it reads node.Spec.TopologySpreadConstraints, which stays flat.

one thing worth calling out loudly on a breaking move like this: removing the top-level fields doesn't make old manifests error, the apiserver just prunes the now-unknown keys. so someone upgrading with top-level affinity/nodeSelector/tolerations set won't get a rejection, those values silently drop and their pods can reschedule on the next roll. since there's no hard failure to catch it, i'd put it in the release notes / an upgrade note rather than rely on people noticing. we've been bitten by exactly this kind of silent prune on an alpha CRD field move.

heads up that this is now DIRTY against main since #271 landed. on the rebase, the buildClusterValkeyNode ValkeyNodeSpec literal needs to keep terminationGracePeriodSeconds and podSecurityContext at the top level, they aren't scheduling fields, so they stay flat on both specs. i hit that exact literal a few times rebasing #271 past #288 and #294.

minor: ValkeyNodeSpec keeps affinity/etc flat while ValkeyClusterSpec groups them under scheduling. fine for an internal CRD, just wondering whether the plan is to converge ValkeyNode later or keep the asymmetry deliberately.

[greptile-apps]

Greptile Summary

This PR moves ValkeyCluster pod scheduling settings into a nested API block. The main changes are:

• Added spec.scheduling for cluster-level scheduling fields.
• Regenerated the ValkeyCluster CRD and deepcopy code.
• Updated controller propagation to copy scheduling fields from the nested block.
• Updated docs, samples, and tests for the new API shape.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code.

T-Rex Logs

What T-Rex did

• Ran the scheduling relocation API test and confirmed ok valkey.io/valkey-operator/api/v1alpha1 with EXIT_CODE: 0.
• Generated diffs for the scheduling relocation manifests and completed the diff check after controller-gen ran for manifests and object generation, with EXIT_CODE: 0.
• Rendered the CRD definitions with kustomize and verified successful output, with EXIT_CODE: 0.
• Rendered the kustomize samples and verified successful output, with EXIT_CODE: 0.
• Executed focused scheduling/topology-spread tests and confirmed all tests passed, while the optional envtest environment blocker occurred with kube-apiserver startup timeout, resulting in 0 of 73 specs run and EXIT_CODE: 1.

_{Ran code and verified through T-Rex}

Important Files Changed

Filename	Overview
api/v1alpha1/valkeycluster_types.go	Adds the nested SchedulingSpec API shape for ValkeyCluster.
api/v1alpha1/zz_generated.deepcopy.go	Regenerates deepcopy handling for the nested scheduling spec.
config/crd/bases/valkey.io_valkeyclusters.yaml	Updates the CRD schema to expose scheduling fields under spec.scheduling.
internal/controller/valkeycluster_controller.go	Reads cluster scheduling values from Spec.Scheduling before creating ValkeyNode specs.
docs/valkeycluster.md	Documents the new spec.scheduling layout.

_{Reviews (2): Last reviewed commit: "Add test assertion" | Re-trigger Greptile}

[jdheyburn]

@melancholictheory

removing the top-level fields doesn't make old manifests error, the apiserver just prunes the now-unknown keys.

Agree - we'll mark this as a breaking change in the release notes. I wonder if there is a way to make the API error on unknown fields.

heads up that this is now DIRTY against main since #271 landed.

I fixed it in the rebase, does it look okay now?

minor: ValkeyNodeSpec keeps affinity/etc flat while ValkeyClusterSpec groups them under scheduling. fine for an internal CRD, just wondering whether the plan is to converge ValkeyNode later or keep the asymmetry deliberately.

The idea is that ValkeyCluster and ValkeyCell build up the low-level affinity, topologySpreadConstraints, etc. from the scheduling abstraction that get passed through to ValkeyNode. ValkeyNode doesn't need the global view, it just needs to know the spec to pass to the podSpec.

[melancholictheory]

rebase looks right, i checked the literal: scheduling.{nodeSelector,affinity,tolerations,priorityClassName,topologySpreadConstraints} now come off the nil-guarded copy, and podSecurityContext and terminationGracePeriodSeconds stay top-level where they belong.

I wonder if there is a way to make the API error on unknown fields.

for the apply path, yes, that already exists: server-side field validation (fieldValidation=Strict) rejects unknown fields. it's been GA since 1.25 and kubectl defaults to strict for apply/create, so someone re-applying an old manifest with a modern kubectl gets an unknown field "spec.affinity" error without you doing anything. tools that don't opt in (older kubectl, some GitOps controllers depending on config) still get the silent prune.

the case i'd worry about isn't the apply path though, it's the clusters that are already stored. a ValkeyCluster created before the refactor has spec.affinity sitting in etcd; once the CRD schema drops the field, the operator's typed client simply doesn't see it any more. nobody re-applied anything, so there's no request to validate and nothing to reject, and the next reconcile templates pods without the affinity. no fieldValidation setting catches that, because it isn't an apply, it's a schema change over existing data.

if you want a loud signal for those, the cheap version is to keep the old top-level fields in the schema for one release marked deprecated, have the operator prefer spec.scheduling but fall back to them, and set a warning condition when the legacy ones are populated. drop them the release after. otherwise the minimum is an upgrade note telling people to migrate their CRs before rolling the CRD, since after that point the values are gone from the operator's view whether or not they're still in etcd.

ValkeyNode doesn't need the global view, it just needs the spec to pass to the podSpec

that makes sense, the abstraction lives where the cluster-wide knowledge is. no objection.

[jdheyburn]

I'll be honest and say I don't know what the standard is for alpha CRDs (re deprecated fields). I'll bring it up on the weekly call later.

[jdheyburn]

@melancholictheory I am going to propose a breaking change note in the upgrade notes for this effort.

[jdheyburn]

@sandeepkunusoth Bjorn is OOO this week, would you be able to review please?

[melancholictheory]

sounds good. the one line that makes it land for already-created clusters: tell people to migrate their CRs before rolling the new CRD, since after the schema drops the fields the values are gone from the operator's view even though they're still in etcd. the apply-time case takes care of itself with strict field validation.