diff --git a/charts/weka-operator/values.yaml b/charts/weka-operator/values.yaml index 79752cbab..de3a02d24 100644 --- a/charts/weka-operator/values.yaml +++ b/charts/weka-operator/values.yaml @@ -687,15 +687,70 @@ admissionPolicies: # # as 1 (operator's reconcile-time default). # cluster_drive_compute_core_ratio: default # strict: warn | relaxed: warn # - # # driveContainers × numDrives must not exceed the signed - # # (non-blocked) drives across matched drive-role nodes. + # # Hard floor: total compute cores must be at least total drive cores (1:1), or the plan is + # # infeasible — raise computeContainers/computeCores or lower the drive side. + # cluster_compute_drive_cores_floor: default # strict: error | relaxed: warn + # + # # A pinned driveCores/computeCores must not exceed capacityPlannerConstraints.maxCoresPerContainer (silent + # # when 0). A policy, not a schema max, so a cluster already pinned above it isn't blocked from + # # the very edit that would fix it. + # cluster_cores_per_container_limit: default # strict: error | relaxed: warn + # + # # numDrives+driveCapacity only: the per-drive capacity needs more drive cores than numDrives + # # allows (weka requires >=1 physical drive per drive core, and numDrives caps driveCores). + # # Reduces to driveCapacity > clusterCapacity.tlcCapacityPerCoreGiB, so raising numDrives never + # # helps — it scales the capacity and the core requirement together. Lower driveCapacity, or + # # switch to containerCapacity/clusterCapacity so cores are derived from the total instead. + # cluster_num_drives_below_required_cores: default # strict: error | relaxed: warn + # + # # An explicit driveCores below what the configured drive capacity (containerCapacity, or + # # numDrives+driveCapacity) requires — surfaced at admission instead of failing later as + # # DriveCapacityResourceShortfall on add-drive. Raising driveCores fixes it, which is why this + # # stays a warning. Silent in auto-full-drives mode: {numDrives: 4, driveCores: 3} is blessed + # # there (all four drives claimed, run on three cores), not a shortfall. + # cluster_drive_cores_below_capacity: default # strict: warn | relaxed: warn + # + # # driveContainers × numDrives must not exceed the signed (non-blocked) FULL drives across + # # matched drive-role nodes. Full-drives clusters only — drive-sharing's numDrives counts + # # capacity-carved virtual drives instead (see cluster_capacity_*). # cluster_signed_drives: default # strict: error | relaxed: warn # + # # Auto-full-drives ("acts as a daemonset", i.e. no container-count or capacity field set): a + # # pinned numDrives above a node's signed full drive count, or a pinned driveCores above it when + # # numDrives is unpinned (1 core per physical drive). Either makes the whole plan infeasible — + # # nothing is created anywhere. A driveCores pin BELOW the drive count is fine and not reported: + # # every drive is still claimed, just run on fewer cores. + # cluster_auto_full_drives_pin_exceeds_node_drives: default # strict: error | relaxed: warn + # + # # Auto-full-drives claims every signed drive, so compute hugepages (which scale with total + # # capacity) can exceed what any compute-eligible node has. Rejects at apply time instead of + # # after a failed formation, naming the shortfall and the compute-node count that would suffice. + # cluster_auto_full_drives_compute_hugepages: default # strict: error | relaxed: warn + # + # # Auto-full-drives places one container per eligible node, so a role nodeSelector matching fewer + # # nodes than the form-cluster floor (FORM_CLUSTER_MIN_{DRIVE,COMPUTE}_CONTAINERS, 5 by default) + # # can never form a cluster. The drive side is otherwise a silent hang on MinContainersNotReady; + # # the compute side surfaces as AutoFullDrivesInfeasible only after a failed formation. + # cluster_auto_full_drives_min_nodes: default # strict: error | relaxed: warn + # + # # No matched drive-role node carries the drive annotation this cluster's mode consumes + # # (weka-shared-drives for drive-sharing, weka-full-drives otherwise) — covers unsigned and + # # signed-in-the-other-mode alike, so the drive checks above are skipped. Advisory only: + # # applying before signing is a valid order of operations. + # cluster_drives_unsigned_advisory: default # strict: warn | relaxed: warn + # # # For each role, container count must not exceed the number of # # nodes matching that role's selector — a role hosts at most # # one container per node. # cluster_selected_nodes_count: default # strict: error | relaxed: warn # + # # A pinned driveContainers/computeContainers below the minimum weka needs to form a cluster at + # # all (FORM_CLUSTER_MIN_{DRIVE,COMPUTE}_CONTAINERS, 5 by default, 3 with ALLOW_SINGLE_PARITY). + # # Error in both modes: below the minimum, FormCluster refuses to proceed and the cluster loops on + # # MinContainersNotReady forever with its containers healthy but idle. Only explicit pins are + # # checked (auto-full-drives leaves both counts at 0 — see cluster_auto_full_drives_min_nodes). + # cluster_min_containers: default # strict: error | relaxed: error + # # # When driversDistService is an in-cluster Service URL, that # # Service must exist. External URLs (e.g. drivers.weka.io) # # are not validated. @@ -734,6 +789,20 @@ admissionPolicies: # # value produces a degenerate or unbootable cluster. # cluster_capacity_protection: default # strict: error | relaxed: error # + # # clusterCapacity only: its structural lower bound — the protection scheme's failure-domain floor + # # (stripeWidth+redundancyLevel+hotSpare) — sits below FORM_CLUSTER_MIN_DRIVE_CONTAINERS. The + # # drive-container count is derived from a capacity target, not a spec field, so nothing pinned + # # reaches cluster_min_containers while weka itself refuses to form and loops on + # # MinContainersNotReady. Warn-only: the floor is a bound, and the planner may still choose more. + # cluster_capacity_min_drive_containers: default # strict: warn | relaxed: warn + # + # # Greenfield clusterCapacity only: the target + driveTypesRatio cannot spread an active pool + # # across at least numFDmin failure domains of at least clusterCapacity.minChunkSizeGiB (384 GiB) + # # each — per pool, clusterCapacity × poolPart/(tlc+qlc) >= 384 × stripeWidth. TLC and QLC are + # # checked independently. Does not fire once TLC-bearing drive containers exist (an in-place + # # migration or an established cluster already clears the floor, and the planner only grows). + # cluster_capacity_chunk_feasibility: default # strict: error | relaxed: error + # # # When spec.overrides.skipDefaultFilesystemCreation is set, the operator # # stops managing features that hardcode the `default` filesystem: any # # configured telemetry export (filesystem-level audit is skipped; cluster- @@ -764,6 +833,14 @@ admissionPolicies: # # explicit value back to 0 (operator-derived). # cluster_cores_decrease: default # strict: error | relaxed: error # + # # An update that changes the cluster's DERIVED sizing mode (auto-full-drives vs explicit + # # container counts vs capacity) while drive containers already exist is denied — the two sizing + # # regimes would plan the same drives differently, and nothing ever removes the surplus. Two + # # switches ARE supported and stay allowed: explicit container counts -> auto-full-drives (the + # # running containers are adopted and grown in place), and containerCapacity/driveCapacity -> + # # clusterCapacity (the in-place migration). Revert anything else to keep the current mode. + # cluster_sizing_mode_flip: default # strict: error | relaxed: error + # # # Decreasing spec.numCores (or spec.dataServicesConfig.dataServicesFeCores) # # on a WekaContainer is denied for the same reason. spec.extraCores is not # # checked: it is never handed to weka, so shrinking it only narrows the diff --git a/internal/admission/defaults.go b/internal/admission/defaults.go index defc84f61..df0c9e166 100644 --- a/internal/admission/defaults.go +++ b/internal/admission/defaults.go @@ -12,13 +12,35 @@ var ( "cluster_cores_available": {Strict: Warn, Relaxed: Warn}, "cluster_hugepages_available": {Strict: Warn, Relaxed: Warn}, "cluster_signed_drives": {Strict: Error, Relaxed: Warn}, + "cluster_drives_unsigned_advisory": {Strict: Warn, Relaxed: Warn}, "cluster_network_ethdevice": {Strict: Warn, Relaxed: Warn}, "cluster_drive_compute_core_ratio": {Strict: Warn, Relaxed: Warn}, - "cluster_dataservices_fe_cores": {Strict: Error, Relaxed: Error}, - "cluster_capacity_protection": {Strict: Error, Relaxed: Error}, - "cluster_capacity_chunk_feasibility": {Strict: Error, Relaxed: Error}, - "cluster_skip_default_fs": {Strict: Warn, Relaxed: Warn}, - "cluster_podspec_syntax": {Strict: Error, Relaxed: Error}, + "cluster_compute_drive_cores_floor": {Strict: Error, Relaxed: Warn}, + "cluster_drive_cores_below_capacity": {Strict: Warn, Relaxed: Warn}, + // Stricter than cluster_drive_cores_below_capacity, which warns about a shortfall the operator can + // be told to fix by raising driveCores: here no legal driveCores exists, so the configured capacity + // is unreachable however the cluster is edited afterwards. Relaxed still warns — the containers do + // run, they just never finish adding drives. + "cluster_num_drives_below_required_cores": {Strict: Error, Relaxed: Warn}, + "cluster_cores_per_container_limit": {Strict: Error, Relaxed: Warn}, + // Both auto-full-drives policies describe a plan that never converges (the planner reports the + // whole thing infeasible and creates nothing), so strict rejects; relaxed warns so a fleet can + // still be applied and inspected. + "cluster_auto_full_drives_pin_exceeds_node_drives": {Strict: Error, Relaxed: Warn}, + "cluster_auto_full_drives_compute_hugepages": {Strict: Error, Relaxed: Warn}, + "cluster_auto_full_drives_min_nodes": {Strict: Error, Relaxed: Warn}, + // Error in BOTH modes: below the form-cluster minimum the cluster does not degrade, it never forms + // at all (waits forever on MinContainersNotReady), so relaxing this would only delay the failure. + "cluster_min_containers": {Strict: Error, Relaxed: Error}, + "cluster_dataservices_fe_cores": {Strict: Error, Relaxed: Error}, + "cluster_capacity_protection": {Strict: Error, Relaxed: Error}, + // Warn in both modes: unlike cluster_min_containers, which sees a definite pinned count below the + // minimum, a low protection floor only PERMITS an undersized derived plan — it does not prove one, + // since a large capacity target can still derive plenty of containers. + "cluster_capacity_min_drive_containers": {Strict: Warn, Relaxed: Warn}, + "cluster_capacity_chunk_feasibility": {Strict: Error, Relaxed: Error}, + "cluster_skip_default_fs": {Strict: Warn, Relaxed: Warn}, + "cluster_podspec_syntax": {Strict: Error, Relaxed: Error}, } wekaClientDefaults = map[string]PolicyDefaults{ @@ -30,6 +52,10 @@ var ( // of strict/relaxed mode — decreasing cores is never a safe operation. wekaClusterUpdateDefaults = map[string]PolicyDefaults{ "cluster_cores_decrease": {Strict: Error, Relaxed: Error}, + // Error in BOTH modes: flipping the derived sizing mode under a live cluster has no + // degraded-but-working outcome — two sizing regimes would fight over the same drives. The two + // switches the operator can actually carry over are allowlisted in the validator itself. + "cluster_sizing_mode_flip": {Strict: Error, Relaxed: Error}, } wekaContainerUpdateDefaults = map[string]PolicyDefaults{ "container_cores_decrease": {Strict: Error, Relaxed: Error}, diff --git a/internal/validation/cluster_auto_full_drives_compute_hugepages.go b/internal/validation/cluster_auto_full_drives_compute_hugepages.go new file mode 100644 index 000000000..614e02540 --- /dev/null +++ b/internal/validation/cluster_auto_full_drives_compute_hugepages.go @@ -0,0 +1,467 @@ +package validation + +import ( + "context" + "fmt" + "sort" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/weka/weka-operator/internal/capacityplanner" + "github.com/weka/weka-operator/internal/consts" + "github.com/weka/weka-operator/internal/controllers/allocator" + "github.com/weka/weka-operator/pkg/util" +) + +// clusterAutoFullDrivesComputeHugepages rejects an auto-full-drives ("acts as a daemonset") cluster +// whose compute containers can never be placed, because the hugepages each one needs exceeds what any +// compute-eligible node has. +// +// Why this mode specifically — and why the check is decidable at admission: nothing here is +// negotiable. Every signed drive is claimed, so the cluster's total TLC capacity is fixed by the +// hardware; each node's drive cores are min(effectiveDriveCount, maxCoresPerContainer) or the +// driveCores pin, and are NEVER traded away to make compute fit (that co-sizing search was deleted — +// it silently ran 48 drives on 8 cores). So the capacity-based share of a compute container's +// hugepages, totalTlcMiB / computeHugepagesTlcRatio / containerCount, and the core requirement that +// sits on top of it are both determined the moment the drives are signed. If no container count fits, +// the plan is infeasible up front and nothing is ever created. Catching it here turns a cluster that +// would never form into a kubectl-apply error. +// +// The only free variable is the container count, swept from the form-cluster floor up to the +// compute-eligible node count (the planner places at most one compute container per node), taking the +// fewest cores that still cover the requirement. A single fitting count anywhere in that sweep admits +// the cluster. This mirrors the worked example in doc/operator/deployment/act-as-daemonset.md; the two +// must agree number for number. +// +// Hyperconverged nodes are charged for their own drive container: a compute-eligible node that is also +// a signed drive-role node has cores × (HugepagesPerCoreMiB + DriveDpdkPerCoreMiB) subtracted from its +// allocatable hugepages before it is offered to compute. Skipping that would over-state headroom on +// exactly the fleet shape this mode is built for. +// +// Skipped when: some other sizing mode is in play; no drive-role node carries the full-drives +// annotation yet (nothing to project from — clusterDrivesUnsignedAdvisory owns that); the compute +// selector matches no node (clusterSelectedNodesCount owns that); or the form-cluster floor already +// exceeds the compute-eligible node count, which is a distinct infeasibility this message would +// misattribute to hugepages (clusterAutoFullDrivesMinNodes owns that one). +// +// Node headroom is otherwise read as ALLOCATABLE hugepages-2Mi, the same source +// clusterHugepagesAvailable uses, so foreign pods are not subtracted. That over-states what is free on +// a busy node, which is the safe direction for an Error policy: this only fires when the requirement +// does not fit even an idle fleet. +type clusterAutoFullDrivesComputeHugepages struct{} + +func (clusterAutoFullDrivesComputeHugepages) ID() string { + return "cluster_auto_full_drives_compute_hugepages" +} + +func (clusterAutoFullDrivesComputeHugepages) Validate(ctx context.Context, c client.Client, obj runtime.Object) field.ErrorList { + cluster, ok := obj.(*weka.WekaCluster) + if !ok { + return nil + } + // Nil dynamicTemplate is auto-full-drives mode (nothing was set), so no nil guard here. + config := cluster.Spec.Dynamic + if !config.UsesAutoFullDrives() { + return nil + } + + fldPath := field.NewPath("spec", "dynamicTemplate") + cons := allocator.ConstraintsForClusterSpec(&cluster.Spec) + + claim, errs := projectAutoFullDrivesClaim(ctx, c, cluster, config, cons, fldPath) + if errs != nil { + return errs + } + claimedTlcGiB, driveCores, annotatedNodes := claim.tlcGiB, claim.driveCores, claim.annotatedNodes + if annotatedNodes == 0 || claimedTlcGiB <= 0 || driveCores <= 0 { + return nil // pre-signing state; clusterDrivesUnsignedAdvisory owns it + } + + freeMiB, maxAllocatableMiB, errs := computeEligibleHugepagesMiB(ctx, c, cluster, claim.driveHugepagesByNode, fldPath) + if errs != nil { + return errs + } + if len(freeMiB) == 0 { + return nil + } + // Descending, so "the first n nodes" is the most generous set of size n. + sort.Sort(sort.Reverse(sort.IntSlice(freeMiB))) + largestFreeMiB := freeMiB[0] + + requiredComputeCores := capacityplanner.RequiredComputeCores(driveCores, 0, true, cons) + // deriveComputeLayout sweeps n from the form-cluster floor up to the placeable node count and + // rejects anything above it ("compute spreads one-per-node"), so those are this search's bounds. + minCount := max(1, cons.MinComputeContainers) + if minCount > len(freeMiB) { + // The floor alone is unreachable — a distinct infeasibility that has nothing to do with + // hugepages, and this message would misattribute it. clusterAutoFullDrivesMinNodes owns it, + // and reports on the same apply. + return nil + } + + // A pinned computeCores takes deriveComputeLayout's specCores branch at runtime, not its default + // sweep: cores is honored EXACTLY and count is derived from it, rather than cores being derived + // from a swept count. autoFullDrivesComputeHugepagesMiB below does the opposite (derives cores + // from count), so it would silently ignore the pin; this is a distinct code path, not an addition + // to the sweep. + if config != nil && config.ComputeCores > 0 { + return validateAutoFullDrivesPinnedComputeCores( + config, claim, cons, fldPath, claimedTlcGiB, driveCores, annotatedNodes, + requiredComputeCores, minCount, freeMiB, maxAllocatableMiB, + ) + } + + // A (count, cores) pair fits when the `count` most-capacious nodes each clear the per-container + // requirement. Cores are the smallest that still cover requiredComputeCores across `count` + // containers — larger cores only raise hugepages, so the minimum is the best case per count. + // Swept rather than evaluated at count == len(freeMiB) alone: on a heterogeneous fleet a smaller + // count can fit where the full spread does not, since fewer nodes have to clear the bar. + coresFit := false + for count := minCount; count <= len(freeMiB); count++ { + perContainerMiB, ok := autoFullDrivesComputeHugepagesMiB(claimedTlcGiB, requiredComputeCores, count, cons) + if !ok { + continue // too few containers to carry the cores; a larger count may still work + } + coresFit = true + if freeMiB[count-1] >= perContainerMiB { + return nil + } + } + + // Infeasible. Quantify against the most generous configuration available: every compute-eligible + // node hosting one container. + bestCount := len(freeMiB) + if !coresFit { + // Not a hugepages shortfall at all: even one container per compute-eligible node, each at the + // per-container core cap, cannot carry the required cores. Saying "the most any node has free is + // N MiB" here would name the wrong binding resource and send the operator after memory. + return field.ErrorList{field.Invalid(fldPath, "auto-full-drives", fmt.Sprintf( + "%s Compute spreads one container per node, at most %d core(s) each, so the "+ + "%d compute-eligible node(s) top out at %d compute core(s) — short of the requirement no "+ + "matter how much memory they have. The planner reports the plan infeasible "+ + "(AutoFullDrivesInfeasible) and creates nothing. Remedies: label more nodes for "+ + "spec.roleNodeSelector.compute; raise the maxCoresPerContainer Helm value (currently %d); "+ + "%sor pin spec.dynamicTemplate.numDrives lower so each node contributes less capacity.", + autoFullDrivesClaimPreamble(claimedTlcGiB, annotatedNodes, driveCores, requiredComputeCores), + cons.MaxCoresPerContainer, bestCount, bestCount*cons.MaxCoresPerContainer, + cons.MaxCoresPerContainer, driveCoresRemedyText(config, claim), + ))} + } + // coresFit is true, so the largest count is core-feasible: ceil(required/count) is non-increasing in + // count, so whichever count cleared the cap, bestCount clears it too. + requiredMiB, _ := autoFullDrivesComputeHugepagesMiB(claimedTlcGiB, requiredComputeCores, bestCount, cons) + + sufficient := autoFullDrivesSufficientComputeNodes(claimedTlcGiB, requiredComputeCores, largestFreeMiB, minCount, cons) + sufficientText := fmt.Sprintf("%d compute-eligible node(s) of that size would be needed", sufficient) + if sufficient == 0 { + sufficientText = "no number of nodes of that size is enough — even a single-core compute container " + + "does not fit, so the shortfall is per-node hugepages, not node count" + } + + // When the cap binds, the base is already clamped and raising the ratio changes nothing; say so + // rather than listing a remedy that cannot help. + capNote := "" + if cons.ComputeMaxHugepagesMiB > 0 && requiredMiB >= cons.ComputeMaxHugepagesMiB { + capNote = " (note: the per-container base is already clamped at computeMaxHugepagesMiB, so " + + "raising computeHugepagesTlcRatio will not move it — lowering the cap is the lever that does)" + } + + // Lowering driveCores is the operator's most direct lever now that the planner will not reduce + // cores on its own, and it costs zero drives — but only when the PER-CORE term is what binds. The + // floor of the per-container figure is its capacity share plus a single core; if even that clears + // the largest node outright, no core reduction of any size can rescue the fleet, and offering it + // as a remedy would send the operator down a dead end. + floorAtOneCore := capacityplanner.ComputeContainerHugepagesMiB(claimedTlcGiB, 0, bestCount, 1, cons) + driveCoresRemedy := driveCoresRemedyText(config, claim) + if floorAtOneCore > maxAllocatableMiB { + driveCoresRemedy = "" + capNote += fmt.Sprintf( + " Lowering driveCores will NOT help here: at %d container(s) the capacity share alone is "+ + "%d MiB even at one compute core, above the %d MiB the largest compute-eligible node has "+ + "allocatable — the binding term is capacity, not cores.", + bestCount, floorAtOneCore, maxAllocatableMiB, + ) + } + + // Remedies mirror the planner's runtime fixesAutoFullDrivesCompute, in the same order, so + // admission and the infeasibility report say the same thing. There is deliberately no + // computeContainers lever: setting one takes the cluster out of this mode entirely. + detail := fmt.Sprintf( + "%s Spread over all %d compute-eligible node(s) that is %d MiB of hugepages "+ + "per compute container, but the most any compute-eligible node has free is %d MiB, and %s. "+ + "Drive cores are never reduced to make compute fit, so no container-count/core combination "+ + "fits: the plan is reported infeasible (AutoFullDrivesInfeasible) and no container is ever "+ + "created.%s Remedies: add compute-eligible nodes (label more nodes for "+ + "spec.roleNodeSelector.compute); %sraise the hugepagesTlcRatio Helm value (currently "+ + "%d) so each GiB of capacity costs less hugepages; lower "+ + "the computeMaxHugepagesMiB Helm value (currently %d) to cap the per-container request; or "+ + "pin spec.dynamicTemplate.numDrives lower so each node contributes less capacity.", + autoFullDrivesClaimPreamble(claimedTlcGiB, annotatedNodes, driveCores, requiredComputeCores), + bestCount, requiredMiB, largestFreeMiB, sufficientText, capNote, driveCoresRemedy, + cons.ComputeHugepagesTlcRatio, cons.ComputeMaxHugepagesMiB, + ) + return field.ErrorList{field.Invalid(fldPath, "auto-full-drives", detail)} +} + +// validateAutoFullDrivesPinnedComputeCores mirrors deriveComputeLayout's specCores branch for a +// pinned spec.dynamicTemplate.computeCores in auto-full-drives mode: cores is honored exactly (never +// derived from count, unlike the unpinned sweep), count is the smallest that still meets +// requiredComputeCores at that fixed core size — max(floor, ceil(required/cores)) — and the pair is +// infeasible when that count exceeds the compute-eligible node count or the count-th most generous +// node cannot hold its hugepages. Same hugepages-only headroom fidelity as the unpinned sweep above +// (see the file doc comment): per-node core headroom is not part of either check. +// +// Not mirrored: deriveComputeLayout also caps cores against topNMin(nodeHeadroom, count, +// maxCoresPerContainer), the weakest of the chosen count nodes' REAL per-node core headroom, derived +// from raw CPU allocatable plus HT/FullPcpusOnly topology via capacityplanner's unexported +// physicalCPUToDataCores/dataCoresCapacityShared. This validator has never fetched that data and +// cannot call those functions from outside the capacityplanner package; reimplementing the +// conversion here would duplicate CPU accounting this file does not own and risk drifting from it. +// clusterCoresPerContainerLimit already rejects computeCores above the global maxCoresPerContainer +// cap in every mode, and clusterCoresAvailable checks a pin against raw per-node CPU for other +// roles — but its containers>0 guard never fires for AFD-mode compute (computeContainers is always 0 +// there), so the real per-node core fit specifically is left to the planner at reconcile time +// (AutoFullDrivesInfeasible) rather than caught here. +func validateAutoFullDrivesPinnedComputeCores( + config *weka.WekaClusterTemplate, + claim autoFullDrivesClaim, + cons *capacityplanner.CapacityConstraints, + fldPath *field.Path, + claimedTlcGiB, driveCores, annotatedNodes, requiredComputeCores, minCount int, + freeMiB []int, + maxAllocatableMiB int, +) field.ErrorList { + cores := config.ComputeCores + count := max(minCount, util.CeilDiv(requiredComputeCores, cores)) + + if count > len(freeMiB) { + detail := fmt.Sprintf( + "%s With spec.dynamicTemplate.computeCores pinned at %d, that takes %d "+ + "compute container(s) (one per node), but only %d node(s) are compute-eligible. The planner "+ + "reports the plan infeasible (AutoFullDrivesInfeasible) and creates nothing. Remedies: add "+ + "compute-eligible nodes (label more nodes for spec.roleNodeSelector.compute); raise "+ + "computeCores so fewer containers are needed; unpin computeCores to let it auto-derive; "+ + "%sor pin spec.dynamicTemplate.numDrives lower so each node contributes less capacity.", + autoFullDrivesClaimPreamble(claimedTlcGiB, annotatedNodes, driveCores, requiredComputeCores), + cores, count, len(freeMiB), + driveCoresRemedyText(config, claim), + ) + return field.ErrorList{field.Invalid(fldPath, "auto-full-drives", detail)} + } + + perContainerMiB := capacityplanner.ComputeContainerHugepagesMiB(claimedTlcGiB, 0, count, cores, cons) + // Descending, so index count-1 is the count-th most generous node — same convention as the + // unpinned sweep above. + if freeMiB[count-1] >= perContainerMiB { + return nil + } + + detail := fmt.Sprintf( + "%s With spec.dynamicTemplate.computeCores pinned at %d, that takes %d compute "+ + "container(s), each needing %d MiB of hugepages, but the %d-th most generous compute-eligible "+ + "node has only %d MiB free after drive placement (the largest has %d MiB allocatable). The "+ + "planner reports the plan infeasible (AutoFullDrivesInfeasible) and creates nothing. Remedies: "+ + "add compute-eligible nodes (label more nodes for spec.roleNodeSelector.compute); unpin "+ + "spec.dynamicTemplate.computeCores to let it auto-derive; raise the hugepagesTlcRatio Helm "+ + "value (currently %d) so each GiB of capacity costs less hugepages; lower the "+ + "computeMaxHugepagesMiB Helm value (currently %d) to cap the per-container request; %sor pin "+ + "spec.dynamicTemplate.numDrives lower so each node contributes less capacity.", + autoFullDrivesClaimPreamble(claimedTlcGiB, annotatedNodes, driveCores, requiredComputeCores), + cores, count, perContainerMiB, + count, freeMiB[count-1], maxAllocatableMiB, + cons.ComputeHugepagesTlcRatio, cons.ComputeMaxHugepagesMiB, driveCoresRemedyText(config, claim), + ) + return field.ErrorList{field.Invalid(fldPath, "auto-full-drives", detail)} +} + +// autoFullDrivesClaimPreamble is the sentence every infeasibility message below opens with: what the +// signed-drive claim commits the cluster to before compute is placed — total TLC, the drive-role nodes +// it came from, the drive cores that run it, and the compute cores that requirement translates to. +// Extracted so the four emission sites cannot drift from each other one word at a time; it ends in a +// period with no trailing space, so callers splice it in with "%s " followed by their own continuation. +func autoFullDrivesClaimPreamble(claimedTlcGiB, annotatedNodes, driveCores, requiredComputeCores int) string { + return fmt.Sprintf( + "auto-full-drives mode claims every signed drive, so this cluster would hold %d GiB of TLC "+ + "capacity across %d drive-role node(s), running on %d drive core(s) and therefore needing "+ + "%d compute core(s).", + claimedTlcGiB, annotatedNodes, driveCores, requiredComputeCores, + ) +} + +// driveCoresRemedyText renders the "lower driveCores" remedy sentence. When the operator pinned +// driveCores, the pin is a single number that applies to every node, so it is named directly. When it +// is unpinned, driveCores is derived per node from that node's own drive count, and on the +// heterogeneous fleets this mode exists to serve there is no single per-node figure to name — averaging +// claim.driveCores across claim.annotatedNodes would describe a value no actual node has, and calling +// it "current" would misrepresent a field that is unset. So the derived case reports the two real +// totals instead and lets the reader see how they relate, rather than fabricating a per-node average. +func driveCoresRemedyText(config *weka.WekaClusterTemplate, claim autoFullDrivesClaim) string { + const tail = " — every drive is still claimed, just run on fewer cores, which cuts both the " + + "compute-core requirement and each node's own drive-container reservation; " + if config != nil && config.DriveCores > 0 { + return fmt.Sprintf("lower spec.dynamicTemplate.driveCores (currently %d per node)%s", config.DriveCores, tail) + } + if claim.annotatedNodes == 0 { + return "" + } + return fmt.Sprintf( + "pin spec.dynamicTemplate.driveCores — it is currently derived per node from each node's drive "+ + "count (%d core(s) across %d node(s))%s", + claim.driveCores, claim.annotatedNodes, tail, + ) +} + +// autoFullDrivesComputeHugepagesMiB is one compute container's hugepages at `count` containers, using +// the fewest cores that still cover requiredComputeCores. Cores only ever raise the figure, so this is +// the best case for that count. Delegates the arithmetic to the planner so the two never drift. +// +// ok is false when `count` containers cannot carry requiredComputeCores at all, because the per-container +// share exceeds MaxCoresPerContainer. Clamping to the cap and reporting the clamped figure as a fit would +// price a layout that under-delivers cores no matter how much hugepages headroom the nodes have: +// deriveComputeLayout skips exactly those candidates (capVal < c) and ends in "cannot satisfy the +// compute:drive ratio", so treating one as feasible here admits a plan the planner never builds. +func autoFullDrivesComputeHugepagesMiB(claimedTlcGiB, requiredComputeCores, count int, cons *capacityplanner.CapacityConstraints) (hugepagesMiB int, ok bool) { + cores := max(1, util.CeilDiv(requiredComputeCores, count)) + if cons.MaxCoresPerContainer > 0 && cores > cons.MaxCoresPerContainer { + return 0, false + } + return capacityplanner.ComputeContainerHugepagesMiB(claimedTlcGiB, 0, count, cores, cons), true +} + +// autoFullDrivesSufficientComputeNodes returns the smallest compute-container count whose per-container +// hugepages fit in perNodeMiB, i.e. how many nodes of the fleet's best size would be enough. Returns 0 +// when no count works — the per-container figure is floored by the per-core terms, so a node too small +// for a 1-core container can never be satisfied by adding more of them. +func autoFullDrivesSufficientComputeNodes(claimedTlcGiB, requiredComputeCores, perNodeMiB, minCount int, cons *capacityplanner.CapacityConstraints) int { + // Bounded: past requiredComputeCores containers the cores term is pinned at 1 and the capacity + // share only shrinks, so if nothing has fit by then, nothing will. + limit := max(minCount, requiredComputeCores) + 1 + for count := max(1, minCount); count <= limit; count++ { + hugepagesMiB, ok := autoFullDrivesComputeHugepagesMiB(claimedTlcGiB, requiredComputeCores, count, cons) + if !ok { + continue // too few containers to carry the cores, whatever the memory + } + if hugepagesMiB <= perNodeMiB { + return count + } + } + return 0 +} + +// autoFullDrivesClaim is what the fleet's signed drives commit the cluster to, before compute is +// placed: total claimed TLC, the drive cores that runs on, how many nodes contributed, and each +// contributing node's own drive-container hugepages reservation. +type autoFullDrivesClaim struct { + tlcGiB int + driveCores int + annotatedNodes int + // driveHugepagesByNode: node name -> MiB its own drive container reserves. Charged against that + // node's headroom when it is also compute-eligible (hyperconverged). + driveHugepagesByNode map[string]int +} + +// projectAutoFullDrivesClaim totals what the cluster would claim: every signed non-blocked full drive +// on every drive-role node, or the numDrives largest when pinned. +// +// Drive cores are the FULL derived count — the driveCores pin, else +// min(effectiveDriveCount, maxCoresPerContainer). They are never projected at a reduced value: there +// is no co-sizing search to descend, and cores are never traded away to make compute fit. Projecting +// anything lower would under-state the compute requirement and admit a cluster the planner +// immediately declares infeasible, which is precisely what this policy exists to prevent. +func projectAutoFullDrivesClaim( + ctx context.Context, + c client.Client, + cluster *weka.WekaCluster, + config *weka.WekaClusterTemplate, + cons *capacityplanner.CapacityConstraints, + fldPath *field.Path, +) (autoFullDrivesClaim, field.ErrorList) { + claim := autoFullDrivesClaim{driveHugepagesByNode: map[string]int{}} + + nodes, errs := listDriveRoleNodes(ctx, c, cluster, fldPath) + if errs != nil { + return claim, errs + } + if len(nodes) == 0 { + return claim, nil + } + infos, errs := driveRoleNodeInfos(nodes, fldPath) + if errs != nil { + return claim, errs + } + + var pinnedNumDrives, pinnedDriveCores int + if config != nil { + pinnedNumDrives, pinnedDriveCores = config.NumDrives, config.DriveCores + } + for _, ni := range infos { + if _, full := ni.Node.Annotations[consts.AnnotationWekaFullDrives]; !full { + continue + } + caps := make([]int, 0, len(ni.Info.AvailableDrives)) + for _, d := range ni.Info.AvailableDrives { + caps = append(caps, d.CapacityGiB) + } + if len(caps) == 0 { + continue + } + claim.annotatedNodes++ + + // Largest-first, so a numDrives pin takes the biggest drives — what the planner does. + sort.Sort(sort.Reverse(sort.IntSlice(caps))) + effective := len(caps) + if pinnedNumDrives > 0 { + // A pin ABOVE the signed count is its own rejection + // (clusterAutoFullDrivesPinExceedsNodeDrives); clamp so this check still reports on the + // capacity the node could actually contribute. + effective = min(pinnedNumDrives, len(caps)) + } + for _, capGiB := range caps[:effective] { + claim.tlcGiB += capGiB + } + + nodeCores := pinnedDriveCores + if nodeCores <= 0 { + nodeCores = capacityplanner.FullDriveCores(effective, cons) + } + claim.driveCores += nodeCores + // The planner's own formula, not a mirror of it: a drive container reserves per CORE *and* per DRIVE, + // so a per-core-only figure under-reserves by 200 MiB per drive on any node holding more drives than + // cores — which is exactly the shape this mode creates — and would report room for compute that the + // drive container has already taken. + claim.driveHugepagesByNode[ni.Node.Name] = capacityplanner.DriveContainerHugepagesMiB(nodeCores, effective, cons) + } + return claim, nil +} + +// computeEligibleHugepagesMiB returns each compute-role node's hugepages-2Mi available to a COMPUTE +// container, in MiB: its allocatable, less the reservation of a drive container this cluster would +// place on the same node (hyperconverged nodes carry both). maxAllocatableMiB is the largest RAW +// allocatable across the same nodes, before any drive reservation — the absolute ceiling a compute +// container could ever have, used to tell a per-core shortfall from a capacity one. +func computeEligibleHugepagesMiB( + ctx context.Context, + c client.Client, + cluster *weka.WekaCluster, + driveHugepagesByNode map[string]int, + fldPath *field.Path, +) (freeMiB []int, maxAllocatableMiB int, errs field.ErrorList) { + selector := cluster.GetNodeSelectorForRole(weka.WekaContainerModeCompute) + var nodes corev1.NodeList + if err := c.List(ctx, &nodes, client.MatchingLabels(selector)); err != nil { + return nil, 0, field.ErrorList{field.InternalError(fldPath, fmt.Errorf("listing compute-role nodes: %w", err))} + } + hpResource := corev1.ResourceName(string(corev1.ResourceHugePagesPrefix) + "2Mi") + out := make([]int, 0, len(nodes.Items)) + for i := range nodes.Items { + qty := nodes.Items[i].Status.Allocatable[hpResource] + allocatable := int(qty.Value() / mib) + maxAllocatableMiB = max(maxAllocatableMiB, allocatable) + out = append(out, max(0, allocatable-driveHugepagesByNode[nodes.Items[i].Name])) + } + return out, maxAllocatableMiB, nil +} diff --git a/internal/validation/cluster_auto_full_drives_compute_hugepages_test.go b/internal/validation/cluster_auto_full_drives_compute_hugepages_test.go new file mode 100644 index 000000000..822be9c7a --- /dev/null +++ b/internal/validation/cluster_auto_full_drives_compute_hugepages_test.go @@ -0,0 +1,585 @@ +package validation + +import ( + "context" + "fmt" + "strings" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + globalconfig "github.com/weka/weka-operator/internal/config" +) + +// Distinct label KEYS, deliberately: a hyperconverged node carries both, and a single shared key +// (e.g. role=drive / role=compute) could only ever hold one of them. +var ( + afdDriveLabels = map[string]string{"afd-drive": "yes"} + afdComputeLabels = map[string]string{"afd-compute": "yes"} +) + +// setAutoFullDrivesHugepagesConfig pins every knob the validator reads to the operator's shipped +// defaults (LoadCapacityEnv isn't called in unit tests, so they'd otherwise be zero) and restores them +// on cleanup. +func setAutoFullDrivesHugepagesConfig(t *testing.T) { + t.Helper() + prevTlcRatio := globalconfig.Config.DriveSharing.HugepagesTlcRatio + prevMaxHP := globalconfig.Config.ComputeMaxHugepagesMiB + prevMaxCores := globalconfig.Config.CapacityPlanner.MaxCoresPerContainer + prevFullDrivesRatio := globalconfig.Config.CapacityPlanner.FullDrivesComputeToDriveCoreRatio + prevMinCompute := globalconfig.Consts.FormClusterMinComputeContainers + + globalconfig.Config.DriveSharing.HugepagesTlcRatio = 1000 + globalconfig.Config.ComputeMaxHugepagesMiB = 360000 + globalconfig.Config.CapacityPlanner.MaxCoresPerContainer = 19 + globalconfig.Config.CapacityPlanner.FullDrivesComputeToDriveCoreRatio = 2.0 + globalconfig.Consts.FormClusterMinComputeContainers = 5 + + t.Cleanup(func() { + globalconfig.Config.DriveSharing.HugepagesTlcRatio = prevTlcRatio + globalconfig.Config.ComputeMaxHugepagesMiB = prevMaxHP + globalconfig.Config.CapacityPlanner.MaxCoresPerContainer = prevMaxCores + globalconfig.Config.CapacityPlanner.FullDrivesComputeToDriveCoreRatio = prevFullDrivesRatio + globalconfig.Consts.FormClusterMinComputeContainers = prevMinCompute + }) +} + +// withHugepages stamps allocatable hugepages-2Mi onto a node. +func withHugepages(n *corev1.Node, allocatableMiB int) *corev1.Node { + n.Status.Allocatable = corev1.ResourceList{ + corev1.ResourceName(string(corev1.ResourceHugePagesPrefix) + "2Mi"): *resource.NewQuantity( + int64(allocatableMiB)*mib, resource.BinarySI), + } + return n +} + +// computeRoleNode builds a diskless compute-eligible node with the given allocatable hugepages-2Mi. +func computeRoleNode(name string, allocatableMiB int) *corev1.Node { + return withHugepages(&corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: name, Labels: afdComputeLabels}, + }, allocatableMiB) +} + +// afdCluster builds an auto-full-drives cluster with explicit drive and compute role selectors. +func afdCluster(dynamic *weka.WekaClusterTemplate) *weka.WekaCluster { + c := &weka.WekaCluster{} + c.Spec.Dynamic = dynamic + drive, compute := afdDriveLabels, afdComputeLabels + c.Spec.RoleNodeSelector.Drive = &drive + c.Spec.RoleNodeSelector.Compute = &compute + return c +} + +// afdHyperconvergedFleet is the fleet from doc/operator/deployment/act-as-daemonset.md's worked +// example: 8 HYPERCONVERGED nodes, each carrying 6 signed drives of 14307 GiB and `allocatableMiB` of +// hugepages, and each matched by BOTH role selectors. Every node therefore pays for its own drive +// container out of the same hugepages pool its compute container draws from — the shape the doc's +// "60,000 − 6 × 1664 = 50,016 MiB" step describes. +func afdHyperconvergedFleet(t *testing.T, nodeCount, allocatableMiB int) []*corev1.Node { + t.Helper() + labels := map[string]string{} + for k, v := range afdDriveLabels { + labels[k] = v + } + for k, v := range afdComputeLabels { + labels[k] = v + } + var nodes []*corev1.Node + for i := 0; i < nodeCount; i++ { + caps := make([]int, 6) + for j := range caps { + caps[j] = 14307 + } + nodes = append(nodes, withHugepages( + driveRoleNode(t, fmt.Sprintf("node-%d", i), labels, caps), allocatableMiB)) + } + return nodes +} + +// afdLabFleet is the same 8 drive nodes but with a SEPARATE, diskless pool of compute-eligible nodes, +// so the two populations never overlap and no node is charged for a drive container. +func afdLabFleet(t *testing.T, computeFreeMiB int, computeNodes int) []*corev1.Node { + t.Helper() + var nodes []*corev1.Node + for i := 0; i < 8; i++ { + caps := make([]int, 6) + for j := range caps { + caps[j] = 14307 + } + nodes = append(nodes, driveRoleNode(t, fmt.Sprintf("drive-%d", i), afdDriveLabels, caps)) + } + for i := 0; i < computeNodes; i++ { + nodes = append(nodes, computeRoleNode(fmt.Sprintf("compute-%d", i), computeFreeMiB)) + } + return nodes +} + +// TestAutoFullDrivesComputeHugepages_LabFleetGroundTruth reproduces the worked example in +// doc/operator/deployment/act-as-daemonset.md step by step. The doc is the specification for this +// policy, so every figure it prints is asserted here: if the two ever disagree, one of them is a bug. +// +// claimed = 8 × 6 × 14307 = 686,736 GiB +// drive cores = 8 × min(6, 19) = 48 ⇒ 2.0 × 48 = 96 compute cores +// per node = 60,000 − 6 × 1664 (own drive ctr) = 50,016 MiB for compute +// at 8 ctrs = 703,217/8 + 1700×12, +64×12 = 109,070 MiB ✗ +// at 18 ctrs = 703,217/18 + 1700×6, evened, +384 = 49,652 MiB ✓ (17 misses at 51,950) +func TestAutoFullDrivesComputeHugepages_LabFleetGroundTruth(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + + c := fakeClientWithNodes(t, afdHyperconvergedFleet(t, 8, 60000)...) + + errs := v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{})) + if len(errs) != 1 { + t.Fatalf("expected exactly one violation, got %v", errs) + } + detail := errs[0].Detail + for _, want := range []string{ + "686736 GiB", // step 1 + "48 drive core", // step 2 — the FULL derived count, never reduced to fit compute + "96 compute core", // step 3 + "109070 MiB of hugepages per compute container", // step 4 + "50016 MiB", // step 2, net of this node's own drive container + "18 compute-eligible node(s) of that size would be needed", // step 6 + "AutoFullDrivesInfeasible", + "Drive cores are never reduced to make compute fit", + // Step 5: even at one compute core the capacity share alone is 89,666 MiB, above the node's + // entire 60,000 MiB — so this fleet is capacity-bound and no core reduction can rescue it. + "89666 MiB even at one compute core", + "the binding term is capacity, not cores", + // The four remedies, in the doc's order. + "add compute-eligible nodes", + // Helm value names, asserted verbatim: both are top-level keys in values.yaml + "hugepagesTlcRatio Helm value", + "computeMaxHugepagesMiB Helm value", + "spec.dynamicTemplate.numDrives lower", + } { + if !strings.Contains(detail, want) { + t.Errorf("expected message to contain %q, got: %s", want, detail) + } + } + // Offering "lower driveCores" on a capacity-bound fleet would send the operator down a dead end. + if strings.Contains(detail, "lower spec.dynamicTemplate.driveCores") { + t.Errorf("must not offer the driveCores remedy when capacity binds, got: %s", detail) + } +} + +// TestAutoFullDrivesComputeHugepages_DriveCoresRemedyWhenCoresBind is the other side of that branch. +// The planner no longer reduces drive cores on its own, so on a fleet where the PER-CORE term is what +// binds, lowering driveCores is the operator's most direct lever and costs zero drives — the message +// must say so. Small drives (2000 GiB) keep the capacity share at 12,288 MiB, well inside the node's +// 20,000 MiB, so the shortfall is genuinely about cores. +func TestAutoFullDrivesComputeHugepages_DriveCoresRemedyWhenCoresBind(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + + bothRoles := map[string]string{} + for k, val := range afdDriveLabels { + bothRoles[k] = val + } + for k, val := range afdComputeLabels { + bothRoles[k] = val + } + var nodes []*corev1.Node + for i := 0; i < 8; i++ { + caps := make([]int, 6) + for j := range caps { + caps[j] = 2000 + } + nodes = append(nodes, withHugepages( + driveRoleNode(t, fmt.Sprintf("node-%d", i), bothRoles, caps), 20000)) + } + + errs := v.Validate(ctx, fakeClientWithNodes(t, nodes...), afdCluster(&weka.WekaClusterTemplate{})) + if len(errs) != 1 { + t.Fatalf("expected exactly one violation, got %v", errs) + } + detail := errs[0].Detail + if !strings.Contains(detail, "pin spec.dynamicTemplate.driveCores — it is currently derived per node "+ + "from each node's drive count (48 core(s) across 8 node(s))") { + t.Errorf("expected the driveCores remedy reporting the derived totals, got: %s", detail) + } + if strings.Contains(detail, "the binding term is capacity, not cores") { + t.Errorf("this fleet is core-bound, not capacity-bound, got: %s", detail) + } +} + +// TestAutoFullDrivesComputeHugepages_HyperconvergedNodePaysForItsDriveContainer isolates the headroom +// rule. Both fleets have the SAME claim (the same 8 signed drive nodes ⇒ 686,736 GiB, 48 drive cores, +// 96 compute cores) and the SAME 18 compute-eligible nodes at 55,000 MiB allocatable — the only +// difference is whether the 8 drive nodes are among those 18. +// +// Disjoint: 18 diskless nodes at 55,000 ≥ the 49,652 MiB an 18-container layout needs ⇒ admitted. +// Overlapping: the 8 drive nodes are charged 6 × 1664 = 9,984 MiB for their own drive container, +// leaving 45,016 — below 49,652 ⇒ rejected. Without the charge this fleet would be waved through. +func TestAutoFullDrivesComputeHugepages_HyperconvergedNodePaysForItsDriveContainer(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + + const allocatableMiB = 55000 + + signedDrives := func() []int { + caps := make([]int, 6) + for j := range caps { + caps[j] = 14307 + } + return caps + } + bothRoles := map[string]string{} + for k, v := range afdDriveLabels { + bothRoles[k] = v + } + for k, v := range afdComputeLabels { + bothRoles[k] = v + } + + t.Run("disjoint diskless compute pool is admitted", func(t *testing.T) { + var nodes []*corev1.Node + for i := 0; i < 8; i++ { + nodes = append(nodes, driveRoleNode(t, fmt.Sprintf("drive-%d", i), afdDriveLabels, signedDrives())) + } + for i := 0; i < 18; i++ { + nodes = append(nodes, computeRoleNode(fmt.Sprintf("compute-%d", i), allocatableMiB)) + } + if errs := v.Validate(ctx, fakeClientWithNodes(t, nodes...), afdCluster(&weka.WekaClusterTemplate{})); len(errs) != 0 { + t.Fatalf("18 diskless nodes at %d MiB should fit the 49652 MiB layout, got %v", allocatableMiB, errs) + } + }) + + t.Run("overlapping drive nodes are charged and rejected", func(t *testing.T) { + var nodes []*corev1.Node + for i := 0; i < 8; i++ { + nodes = append(nodes, withHugepages( + driveRoleNode(t, fmt.Sprintf("both-%d", i), bothRoles, signedDrives()), allocatableMiB)) + } + for i := 0; i < 10; i++ { + nodes = append(nodes, computeRoleNode(fmt.Sprintf("compute-%d", i), allocatableMiB)) + } + errs := v.Validate(ctx, fakeClientWithNodes(t, nodes...), afdCluster(&weka.WekaClusterTemplate{})) + if len(errs) != 1 { + t.Fatalf("expected rejection once the 8 drive nodes pay for their own drive container, got %v", errs) + } + // The claim is unchanged — only the headroom moved. + if !strings.Contains(errs[0].Detail, "686736 GiB") || !strings.Contains(errs[0].Detail, "48 drive core") { + t.Errorf("expected the same claim as the disjoint case, got: %s", errs[0].Detail) + } + }) +} + +// TestAutoFullDrivesComputeHugepages_Fits: the same fleet on nodes with room to spare is admitted. +func TestAutoFullDrivesComputeHugepages_Fits(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + + c := fakeClientWithNodes(t, afdLabFleet(t, 200000, 8)...) + + if errs := v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{})); len(errs) != 0 { + t.Errorf("expected no violation when the requirement fits, got %v", errs) + } +} + +// TestAutoFullDrivesComputeHugepages_CoreCapBindsBeforeHugepages guards the per-container core cap. +// The 8-node drive fleet needs 96 compute cores (48 drive cores × 2.0), and compute spreads one +// container per node at no more than MaxCoresPerContainer=19 — so 5 compute nodes top out at 95 cores +// and no layout can carry the requirement, however much memory the nodes have. Clamping cores to 19 and +// pricing THAT container's hugepages would find a comfortable fit at 200000 MiB per node (174159 MiB +// needed) and admit a plan deriveComputeLayout rejects outright. One more compute node closes the gap +// (ceil(96/6) = 16 cores, within the cap), which is the control below. +func TestAutoFullDrivesComputeHugepages_CoreCapBindsBeforeHugepages(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + + c := fakeClientWithNodes(t, afdLabFleet(t, 200000, 5)...) + + errs := v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{})) + if len(errs) != 1 { + t.Fatalf("expected a violation when the core cap alone makes the plan infeasible, got %v", errs) + } + detail := errs[0].Detail + // The message must name cores, not memory: the nodes have hugepages to spare, and sending the + // operator after memory here would be a dead end. + for _, want := range []string{"96 compute core(s)", "top out at 95 compute core(s)", "at most 19 core(s) each"} { + if !strings.Contains(detail, want) { + t.Errorf("detail missing %q, got: %s", want, detail) + } + } + if strings.Contains(detail, "the most any compute-eligible node has free") { + t.Errorf("core shortfall must not be reported as a hugepages shortfall, got: %s", detail) + } + + // Control: a sixth compute node brings ceil(96/6)=16 cores under the cap, and 200000 MiB per node + // covers the 145426 MiB that layout needs. + c = fakeClientWithNodes(t, afdLabFleet(t, 200000, 6)...) + if errs := v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{})); len(errs) != 0 { + t.Errorf("expected no violation once the cores fit under the cap, got %v", errs) + } +} + +// TestAutoFullDrivesComputeHugepages_PinnedComputeCoresHonoredVerbatim: UsesAutoFullDrives doesn't +// consult ComputeCores, so a pin coexists with AFD mode. deriveComputeLayout's specCores branch +// honors it exactly and derives count from it (see +// TestDeriveComputeLayout_AgreesWithAutoFullDrivesHugepagesValidator's pinned-cores case for the +// same numbers), rather than the unpinned sweep's opposite direction of deriving cores from count — +// so a pin must not be silently ignored. +func TestAutoFullDrivesComputeHugepages_PinnedComputeCoresHonoredVerbatim(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + + // 96 required compute cores (48 drive cores × 2.0). Pinned at 18 cores/container that's + // ceil(96/18)=6 containers, but only 5 compute-eligible nodes -- infeasible on node count, not + // hugepages, however much memory those 5 nodes have. + c := fakeClientWithNodes(t, afdLabFleet(t, 200000, 5)...) + errs := v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{ComputeCores: 18})) + if len(errs) != 1 { + t.Fatalf("expected a violation when the pin's derived count exceeds the compute node count, got %v", errs) + } + for _, want := range []string{"pinned at 18", "6 compute container(s)", "only 5 node(s) are compute-eligible"} { + if !strings.Contains(errs[0].Detail, want) { + t.Errorf("detail missing %q, got: %s", want, errs[0].Detail) + } + } + + // A sixth compute node supplies the 6th container the pin needs, and 200000 MiB covers its share. + c = fakeClientWithNodes(t, afdLabFleet(t, 200000, 6)...) + if errs = v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{ComputeCores: 18})); len(errs) != 0 { + t.Errorf("expected no violation once the pin's derived count fits the compute node count, got %v", errs) + } + + // Same 6-node fleet, but too little memory for 18 cores/container: a hugepages shortfall, not a + // node-count one, and the message must name memory instead. + c = fakeClientWithNodes(t, afdLabFleet(t, 10000, 6)...) + errs = v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{ComputeCores: 18})) + if len(errs) != 1 { + t.Fatalf("expected a violation when the pin's hugepages do not fit, got %v", errs) + } + if !strings.Contains(errs[0].Detail, "MiB free after drive placement") { + t.Errorf("expected a hugepages shortfall message, got: %s", errs[0].Detail) + } +} + +// TestAutoFullDrivesComputeHugepages_ProjectsFullDerivedCores is the regression guard for the +// projection rule, and it runs in the direction that matters. There is no co-sizing search: drive +// cores are min(drives, 19) and are never reduced to make compute fit. At 100000 MiB per node the +// fleet needs 109070 MiB per compute container at the full 6 cores/node and must be REJECTED — +// projecting any reduced core count (e.g. 1/node, which would ask only 91430 MiB) would wave through +// a cluster the planner immediately declares infeasible. +func TestAutoFullDrivesComputeHugepages_ProjectsFullDerivedCores(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + + c := fakeClientWithNodes(t, afdLabFleet(t, 100000, 8)...) + + errs := v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{})) + if len(errs) != 1 { + t.Fatalf("drive cores must be projected at the full derived count (48), not a reduced one; got %v", errs) + } + if !strings.Contains(errs[0].Detail, "48 drive core") { + t.Errorf("expected the full derived core total, got: %s", errs[0].Detail) + } +} + +// TestAutoFullDrivesComputeHugepages_PinnedDriveCoresHonoredVerbatim: a driveCores pin replaces the +// derived count outright, in both directions. Pinning 2 cores/node on the fleet above cuts compute +// demand from 96 to 32 cores and brings it within reach — every drive is still claimed, just run on +// fewer cores. +func TestAutoFullDrivesComputeHugepages_PinnedDriveCoresHonoredVerbatim(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + + c := fakeClientWithNodes(t, afdLabFleet(t, 100000, 8)...) + + if errs := v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{DriveCores: 2})); len(errs) != 0 { + t.Errorf("a lower driveCores pin must be honored verbatim and lower compute demand, got %v", errs) + } + // The same fleet at the derived 6 cores/node is rejected — see _ProjectsFullDerivedCores. + errs := v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{DriveCores: 6})) + if len(errs) != 1 { + t.Fatalf("expected a pin equal to the derived count to reject like the unpinned case, got %v", errs) + } + if !strings.Contains(errs[0].Detail, "48 drive core") { + t.Errorf("expected the pinned core total in the message, got: %s", errs[0].Detail) + } +} + +// TestAutoFullDrivesComputeHugepages_FloorAboveNodeCountSkipped: when the form-cluster floor already +// exceeds the compute-eligible node count the cluster is infeasible for a different reason, and this +// message would misattribute it to hugepages. +func TestAutoFullDrivesComputeHugepages_FloorAboveNodeCountSkipped(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) // FormClusterMinComputeContainers = 5 + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + + // 3 compute-eligible nodes, far too small — but the floor of 5 is unreachable first. + c := fakeClientWithNodes(t, afdLabFleet(t, 1000, 3)...) + + if errs := v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{})); len(errs) != 0 { + t.Errorf("expected no violation when the form-cluster floor is unreachable, got %v", errs) + } +} + +// TestAutoFullDrivesComputeHugepages_NumDrivesPinLowersTheClaim exercises the fourth remedy: pinning +// numDrives lower cuts the claimed capacity and with it the per-container hugepages share. +func TestAutoFullDrivesComputeHugepages_NumDrivesPinLowersTheClaim(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + + c := fakeClientWithNodes(t, afdLabFleet(t, 60000, 8)...) + + if errs := v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{NumDrives: 1})); len(errs) != 0 { + t.Errorf("expected numDrives=1 to bring the claim within reach, got %v", errs) + } +} + +// TestAutoFullDrivesComputeHugepages_NodeTooSmallForOneCore: when no node can host even a single-core +// compute container, the message must say the shortfall is per-node hugepages, not node count. +func TestAutoFullDrivesComputeHugepages_NodeTooSmallForOneCore(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + + c := fakeClientWithNodes(t, afdLabFleet(t, 1000, 8)...) + + errs := v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{})) + if len(errs) != 1 { + t.Fatalf("expected exactly one violation, got %v", errs) + } + if !strings.Contains(errs[0].Detail, "no number of nodes of that size is enough") { + t.Errorf("expected the per-node-shortfall wording, got: %s", errs[0].Detail) + } +} + +// TestAutoFullDrivesComputeHugepages_CapSaturatedNote: once computeMaxHugepagesMiB clamps the base, +// raising the TLC ratio cannot move it, so the message must say which lever still works rather than +// listing one that does nothing. +func TestAutoFullDrivesComputeHugepages_CapSaturatedNote(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + prev := globalconfig.Config.ComputeMaxHugepagesMiB + globalconfig.Config.ComputeMaxHugepagesMiB = 20000 + t.Cleanup(func() { globalconfig.Config.ComputeMaxHugepagesMiB = prev }) + + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + c := fakeClientWithNodes(t, afdLabFleet(t, 19000, 8)...) + + errs := v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{})) + if len(errs) != 1 { + t.Fatalf("expected exactly one violation, got %v", errs) + } + if !strings.Contains(errs[0].Detail, "already clamped at computeMaxHugepagesMiB") { + t.Errorf("expected the cap-saturation note, got: %s", errs[0].Detail) + } +} + +// TestAutoFullDrivesComputeHugepages_Skips covers every path that must stay silent. +func TestAutoFullDrivesComputeHugepages_Skips(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + + // Bootstrap: drive-role nodes exist but none is signed yet, so there is nothing to project from. + t.Run("pre-signing drive nodes", func(t *testing.T) { + nodes := []*corev1.Node{ + driveRoleNode(t, "drive-0", afdDriveLabels, nil), + driveRoleNode(t, "drive-1", afdDriveLabels, nil), + computeRoleNode("compute-0", 1000), + } + if errs := v.Validate(ctx, fakeClientWithNodes(t, nodes...), afdCluster(&weka.WekaClusterTemplate{})); len(errs) != 0 { + t.Errorf("expected no violation pre-signing, got %v", errs) + } + }) + + t.Run("no drive-role nodes matched", func(t *testing.T) { + c := fakeClientWithNodes(t, computeRoleNode("compute-0", 1000)) + if errs := v.Validate(ctx, c, afdCluster(&weka.WekaClusterTemplate{})); len(errs) != 0 { + t.Errorf("expected no violation, got %v", errs) + } + }) + + // clusterSelectedNodesCount owns "the compute selector matches nothing". + t.Run("no compute-eligible nodes matched", func(t *testing.T) { + var nodes []*corev1.Node + for i := 0; i < 8; i++ { + caps := make([]int, 6) + for j := range caps { + caps[j] = 14307 + } + nodes = append(nodes, driveRoleNode(t, fmt.Sprintf("drive-%d", i), afdDriveLabels, caps)) + } + if errs := v.Validate(ctx, fakeClientWithNodes(t, nodes...), afdCluster(&weka.WekaClusterTemplate{})); len(errs) != 0 { + t.Errorf("expected no violation, got %v", errs) + } + }) + + // Every other sizing mode is out of scope: the planner can shrink its capacity target there. + t.Run("other sizing modes", func(t *testing.T) { + c := fakeClientWithNodes(t, afdLabFleet(t, 60000, 8)...) + for name, dyn := range map[string]*weka.WekaClusterTemplate{ + "clusterCapacity": {ClusterCapacity: "500TiB"}, + "containerCapacity": {ContainerCapacity: 6000}, + "counts": {ComputeContainers: 6, DriveContainers: 6}, + } { + t.Run(name, func(t *testing.T) { + if errs := v.Validate(ctx, c, afdCluster(dyn)); len(errs) != 0 { + t.Errorf("expected no violation, got %v", errs) + } + }) + } + }) +} + +// TestAutoFullDrivesComputeHugepages_NilTemplate: a nil dynamicTemplate is the mode's default shape and +// must be evaluated, not skipped. +func TestAutoFullDrivesComputeHugepages_NilTemplate(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + v := &clusterAutoFullDrivesComputeHugepages{} + ctx := context.Background() + + c := fakeClientWithNodes(t, afdLabFleet(t, 60000, 8)...) + + cluster := afdCluster(nil) + if errs := v.Validate(ctx, c, cluster); len(errs) != 1 { + t.Fatalf("expected a nil template to be evaluated as auto-full-drives, got %v", errs) + } +} + +// A drive container reserves hugepages per CORE *and* per DRIVE (1400/core + 200/drive + DPDK/core, the +// figure its pod requests). This projection charged per core only, which under-reserved by 200 MiB per drive +// on exactly the shape this mode creates — a node holding more drives than cores — and so reported room for +// compute that the drive container had already taken. +// +// The fixture is the doc's 8-node hyperconverged fleet with driveCores pinned to 1, so each node holds 6 +// drives on 1 core. Correct accounting reserves 1400 + 6*200 + 64 = 2664 MiB per node; cores-only reserved +// 1664. At 93500 MiB allocatable the difference is decisive: the compute container needs 91430 MiB, which +// fits under the old figure and does not under the real one. Verified by mutation — restoring the cores-only +// arithmetic makes this fleet pass admission and then fail at runtime. +func TestAutoFullDrivesComputeHugepages_ChargesDriveReservationPerDrive(t *testing.T) { + setAutoFullDrivesHugepagesConfig(t) + ctx := context.Background() + v := clusterAutoFullDrivesComputeHugepages{} + pinned := &weka.WekaClusterTemplate{DriveCores: 1} + + // Just under the real threshold: must be rejected. + tooTight := fakeClientWithNodes(t, afdHyperconvergedFleet(t, 8, 93500)...) + if errs := v.Validate(ctx, tooTight, afdCluster(pinned)); len(errs) == 0 { + t.Error("fleet admitted at 93500 MiB — the drive reservation is being charged per core only, so " + + "admission is offering compute the drive container's per-drive hugepages") + } + + // Above it: must still be accepted, so the check is not simply rejecting everything. + roomy := fakeClientWithNodes(t, afdHyperconvergedFleet(t, 8, 94500)...) + if errs := v.Validate(ctx, roomy, afdCluster(pinned)); len(errs) != 0 { + t.Errorf("fleet rejected at 94500 MiB, where compute genuinely fits: %v", errs) + } +} diff --git a/internal/validation/cluster_auto_full_drives_min_nodes.go b/internal/validation/cluster_auto_full_drives_min_nodes.go new file mode 100644 index 000000000..38451a4cd --- /dev/null +++ b/internal/validation/cluster_auto_full_drives_min_nodes.go @@ -0,0 +1,109 @@ +package validation + +import ( + "context" + "fmt" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + globalconfig "github.com/weka/weka-operator/internal/config" +) + +// clusterAutoFullDrivesMinNodes rejects an auto-full-drives ("acts as a daemonset") cluster whose +// role nodeSelectors cannot yield the container counts weka needs to form a cluster at all. +// +// In this mode both container counts are 0 and the operator places exactly ONE container per eligible +// node, so the node count IS the container count — a selector matching fewer nodes than +// FormClusterMinDriveContainers / FormClusterMinComputeContainers (5 by default, 3 under +// ALLOW_SINGLE_PARITY) can never satisfy the floor, no matter how the fleet is sized. Because both +// counts are 0, clusterSelectedNodesCount (which iterates the pinned counts) and clusterMinContainers +// (which only checks explicit pins) both no-op here; without this policy nothing checks it. +// +// The two legs fail differently at runtime, which is why both are worth catching at apply time: +// - DRIVE: nothing reports it. The planner has no drive-container floor — it happily plans one +// container per signed node — so the plan is feasible, the pods run healthy, and the cluster loops +// on MinContainersNotReady (funcs_clusterization.go) every 15 seconds forever. A silent hang. +// - COMPUTE: the planner floors at MinComputeContainers and reports AutoFullDrivesInfeasible, so it +// is at least visible — but only after a failed formation, not at kubectl apply. +// +// Counts MATCHED nodes, not signed ones: labelling and drive-signing are independent, and a node that +// is labelled but not yet signed will still host a container once signing runs. That also keeps this +// check free of the partial-annotation bootstrap problem that gates the drive-capacity validators — +// there is no signing state to be mid-rollout on. +type clusterAutoFullDrivesMinNodes struct{} + +func (clusterAutoFullDrivesMinNodes) ID() string { return "cluster_auto_full_drives_min_nodes" } + +func (clusterAutoFullDrivesMinNodes) Validate(ctx context.Context, c client.Client, obj runtime.Object) field.ErrorList { + cluster, ok := obj.(*weka.WekaCluster) + if !ok { + return nil + } + // Nil dynamicTemplate is auto-full-drives mode (nothing was set), so no nil guard here. + if !cluster.Spec.Dynamic.UsesAutoFullDrives() { + return nil + } + + legs := []struct { + role string + // selectorField is the role-specific selector path named in the remedy. + selectorField string + min int + envVar string + // consequence describes what actually happens at runtime if this is left as-is. + consequence string + }{ + { + role: weka.WekaContainerModeDrive, + selectorField: "spec.roleNodeSelector.drive", + min: globalconfig.Consts.FormClusterMinDriveContainers, + envVar: "FORM_CLUSTER_MIN_DRIVE_CONTAINERS", + consequence: "Nothing reports this at runtime: the plan is feasible and the drive containers " + + "run healthy, but the cluster waits on MinContainersNotReady forever", + }, + { + role: weka.WekaContainerModeCompute, + selectorField: "spec.roleNodeSelector.compute", + min: globalconfig.Consts.FormClusterMinComputeContainers, + envVar: "FORM_CLUSTER_MIN_COMPUTE_CONTAINERS", + consequence: "The planner reports the whole plan infeasible (AutoFullDrivesInfeasible) and " + + "creates nothing", + }, + } + + var out field.ErrorList + for _, leg := range legs { + if leg.min <= 0 { // floor disabled by configuration — nothing to enforce + continue + } + selector := cluster.GetNodeSelectorForRole(leg.role) + fldPath := field.NewPath("spec", "roleNodeSelector", leg.role) + + var nodes corev1.NodeList + if err := c.List(ctx, &nodes, client.MatchingLabels(selector)); err != nil { + out = append(out, field.InternalError(fldPath, + fmt.Errorf("listing %s-role nodes: %w", leg.role, err))) + continue + } + matched := len(nodes.Items) + if matched >= leg.min { + continue + } + + detail := fmt.Sprintf( + "the %s-role nodeSelector (%s) matches %d node(s), below the %d %s container(s) weka needs "+ + "to form a cluster. This cluster sets no container counts, so it acts as a daemonset and "+ + "places exactly one %s container per eligible node — the matched node count IS the "+ + "container count, and no sizing field can raise it. %s. Label at least %d node(s) for "+ + "%s (it falls back to spec.nodeSelector when unset), or lower the floor via %s.", + leg.role, formatSelector(selector), matched, leg.min, leg.role, + leg.role, leg.consequence, leg.min, leg.selectorField, leg.envVar, + ) + out = append(out, field.Invalid(fldPath, matched, detail)) + } + return out +} diff --git a/internal/validation/cluster_auto_full_drives_min_nodes_test.go b/internal/validation/cluster_auto_full_drives_min_nodes_test.go new file mode 100644 index 000000000..082f4f3b8 --- /dev/null +++ b/internal/validation/cluster_auto_full_drives_min_nodes_test.go @@ -0,0 +1,263 @@ +package validation + +import ( + "context" + "errors" + "fmt" + "strings" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" +) + +var ( + minNodesDriveLabels = map[string]string{"afd-min-drive": "yes"} + minNodesComputeLabels = map[string]string{"afd-min-compute": "yes"} +) + +// minNodesCluster builds an auto-full-drives cluster with distinct drive and compute role selectors. +func minNodesCluster(dynamic *weka.WekaClusterTemplate) *weka.WekaCluster { + c := &weka.WekaCluster{} + c.Spec.Dynamic = dynamic + drive, compute := minNodesDriveLabels, minNodesComputeLabels + c.Spec.RoleNodeSelector.Drive = &drive + c.Spec.RoleNodeSelector.Compute = &compute + return c +} + +// plainNode builds a labelled node with no drive annotation — this policy counts labels, not signing. +func plainNode(name string, labels map[string]string) *corev1.Node { + return &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: name, Labels: labels}} +} + +// minNodesFleet builds `drives` drive-role nodes and `computes` compute-role nodes. +func minNodesFleet(drives, computes int) []*corev1.Node { + var nodes []*corev1.Node + for i := 0; i < drives; i++ { + nodes = append(nodes, plainNode(fmt.Sprintf("drive-%d", i), minNodesDriveLabels)) + } + for i := 0; i < computes; i++ { + nodes = append(nodes, plainNode(fmt.Sprintf("compute-%d", i), minNodesComputeLabels)) + } + return nodes +} + +// TestAutoFullDrivesMinNodes_BothRolesBelowFloor: each role is reported independently. +func TestAutoFullDrivesMinNodes_BothRolesBelowFloor(t *testing.T) { + withFormClusterMinContainers(t, 5, 5) + v := &clusterAutoFullDrivesMinNodes{} + ctx := context.Background() + + c := fakeClientWithNodes(t, minNodesFleet(3, 2)...) + + errs := v.Validate(ctx, c, minNodesCluster(&weka.WekaClusterTemplate{})) + if len(errs) != 2 { + t.Fatalf("expected one violation per role, got %v", errs) + } + + var sawDrive, sawCompute bool + for _, e := range errs { + switch { + case strings.Contains(e.Detail, "the drive-role nodeSelector"): + sawDrive = true + if e.Field != "spec.roleNodeSelector.drive" { + t.Errorf("expected the drive selector path, got %q", e.Field) + } + for _, want := range []string{ + "matches 3 node(s), below the 5 drive container(s)", + "afd-min-drive=yes", + "one drive container per eligible node", + // The drive side is the silent one — the message must say so. + "Nothing reports this at runtime", + "MinContainersNotReady", + "Label at least 5 node(s) for spec.roleNodeSelector.drive", + "FORM_CLUSTER_MIN_DRIVE_CONTAINERS", + } { + if !strings.Contains(e.Detail, want) { + t.Errorf("drive message missing %q, got: %s", want, e.Detail) + } + } + case strings.Contains(e.Detail, "the compute-role nodeSelector"): + sawCompute = true + if e.Field != "spec.roleNodeSelector.compute" { + t.Errorf("expected the compute selector path, got %q", e.Field) + } + for _, want := range []string{ + "matches 2 node(s), below the 5 compute container(s)", + "AutoFullDrivesInfeasible", + "FORM_CLUSTER_MIN_COMPUTE_CONTAINERS", + } { + if !strings.Contains(e.Detail, want) { + t.Errorf("compute message missing %q, got: %s", want, e.Detail) + } + } + } + } + if !sawDrive || !sawCompute { + t.Errorf("expected both roles reported, got %v", errs) + } +} + +// TestAutoFullDrivesMinNodes_RolesAreIndependent: a satisfied role must not mask a starved one. +func TestAutoFullDrivesMinNodes_RolesAreIndependent(t *testing.T) { + withFormClusterMinContainers(t, 5, 5) + v := &clusterAutoFullDrivesMinNodes{} + ctx := context.Background() + + t.Run("drive short, compute fine", func(t *testing.T) { + c := fakeClientWithNodes(t, minNodesFleet(4, 6)...) + errs := v.Validate(ctx, c, minNodesCluster(&weka.WekaClusterTemplate{})) + if len(errs) != 1 || !strings.Contains(errs[0].Detail, "the drive-role nodeSelector") { + t.Fatalf("expected only the drive violation, got %v", errs) + } + }) + + t.Run("compute short, drive fine", func(t *testing.T) { + c := fakeClientWithNodes(t, minNodesFleet(6, 4)...) + errs := v.Validate(ctx, c, minNodesCluster(&weka.WekaClusterTemplate{})) + if len(errs) != 1 || !strings.Contains(errs[0].Detail, "the compute-role nodeSelector") { + t.Fatalf("expected only the compute violation, got %v", errs) + } + }) +} + +// TestAutoFullDrivesMinNodes_AtTheFloorPasses: exactly the floor is enough. +func TestAutoFullDrivesMinNodes_AtTheFloorPasses(t *testing.T) { + withFormClusterMinContainers(t, 5, 5) + v := &clusterAutoFullDrivesMinNodes{} + ctx := context.Background() + + c := fakeClientWithNodes(t, minNodesFleet(5, 5)...) + if errs := v.Validate(ctx, c, minNodesCluster(&weka.WekaClusterTemplate{})); len(errs) != 0 { + t.Errorf("expected no violation at exactly the floor, got %v", errs) + } +} + +// TestAutoFullDrivesMinNodes_UnsignedNodesStillCount is the bootstrap path. This policy counts LABELS, +// not signed drives: applying before sign-drives runs is a valid order of operations, and those nodes +// will host containers once signing completes. A labelled-but-unsigned fleet must be admitted. +func TestAutoFullDrivesMinNodes_UnsignedNodesStillCount(t *testing.T) { + withFormClusterMinContainers(t, 5, 5) + v := &clusterAutoFullDrivesMinNodes{} + ctx := context.Background() + + // Six drive nodes with no weka-full-drives annotation at all, six compute nodes. + c := fakeClientWithNodes(t, minNodesFleet(6, 6)...) + if errs := v.Validate(ctx, c, minNodesCluster(&weka.WekaClusterTemplate{})); len(errs) != 0 { + t.Errorf("pre-signing fleets must be admitted — this policy counts labels, got %v", errs) + } +} + +// TestAutoFullDrivesMinNodes_SinglePartyLowersTheFloor: the floors track configuration +// (ALLOW_SINGLE_PARITY lowers both to 3), not a hard-coded 5. +func TestAutoFullDrivesMinNodes_SingleParityLowersTheFloor(t *testing.T) { + withFormClusterMinContainers(t, 3, 3) + v := &clusterAutoFullDrivesMinNodes{} + ctx := context.Background() + + c := fakeClientWithNodes(t, minNodesFleet(3, 3)...) + if errs := v.Validate(ctx, c, minNodesCluster(&weka.WekaClusterTemplate{})); len(errs) != 0 { + t.Errorf("expected no violation at the single-parity floor of 3, got %v", errs) + } +} + +// TestAutoFullDrivesMinNodes_FloorOfZeroDisablesTheLeg: a non-positive floor switches the check off, +// per role, rather than substituting a default. +func TestAutoFullDrivesMinNodes_FloorOfZeroDisablesTheLeg(t *testing.T) { + withFormClusterMinContainers(t, 0, 5) + v := &clusterAutoFullDrivesMinNodes{} + ctx := context.Background() + + c := fakeClientWithNodes(t, minNodesFleet(1, 1)...) + errs := v.Validate(ctx, c, minNodesCluster(&weka.WekaClusterTemplate{})) + if len(errs) != 1 || !strings.Contains(errs[0].Detail, "the compute-role nodeSelector") { + t.Fatalf("expected the drive leg disabled and only compute reported, got %v", errs) + } +} + +// TestAutoFullDrivesMinNodes_ZeroMatchedNodesIsReported: an empty selector match is the loudest form +// of this misconfiguration, and nothing else catches it in this mode — clusterSelectedNodesCount +// iterates the pinned counts, which are 0 here. +func TestAutoFullDrivesMinNodes_ZeroMatchedNodesIsReported(t *testing.T) { + withFormClusterMinContainers(t, 5, 5) + v := &clusterAutoFullDrivesMinNodes{} + ctx := context.Background() + + c := fakeClientWithNodes(t, plainNode("unrelated", map[string]string{"other": "label"})) + errs := v.Validate(ctx, c, minNodesCluster(&weka.WekaClusterTemplate{})) + if len(errs) != 2 { + t.Fatalf("expected both roles reported when the selectors match nothing, got %v", errs) + } + // Guard the assumption this policy rests on: the pinned-count validator really is inert here. + if other := (&clusterSelectedNodesCount{}).Validate(ctx, c, minNodesCluster(&weka.WekaClusterTemplate{})); len(other) != 0 { + t.Errorf("clusterSelectedNodesCount was expected to no-op in this mode, got %v", other) + } +} + +// TestAutoFullDrivesMinNodes_NilTemplate: a nil dynamicTemplate is the mode's default shape and must +// be evaluated, not skipped. +func TestAutoFullDrivesMinNodes_NilTemplate(t *testing.T) { + withFormClusterMinContainers(t, 5, 5) + v := &clusterAutoFullDrivesMinNodes{} + ctx := context.Background() + + c := fakeClientWithNodes(t, minNodesFleet(2, 2)...) + if errs := v.Validate(ctx, c, minNodesCluster(nil)); len(errs) != 2 { + t.Errorf("expected a nil template to be evaluated as auto-full-drives, got %v", errs) + } +} + +// TestAutoFullDrivesMinNodes_OtherSizingModesSkipped: with explicit counts or a capacity target the +// node count is not the container count, and clusterSelectedNodesCount / clusterMinContainers own it. +func TestAutoFullDrivesMinNodes_OtherSizingModesSkipped(t *testing.T) { + withFormClusterMinContainers(t, 5, 5) + v := &clusterAutoFullDrivesMinNodes{} + ctx := context.Background() + + c := fakeClientWithNodes(t, minNodesFleet(1, 1)...) + for name, dyn := range map[string]*weka.WekaClusterTemplate{ + "counts": {ComputeContainers: 6, DriveContainers: 6}, + "clusterCapacity": {ClusterCapacity: "500TiB"}, + "containerCapacity": {ContainerCapacity: 6000}, + } { + t.Run(name, func(t *testing.T) { + if errs := v.Validate(ctx, c, minNodesCluster(dyn)); len(errs) != 0 { + t.Errorf("expected no violation outside auto-full-drives mode, got %v", errs) + } + }) + } +} + +// TestAutoFullDrivesMinNodes_ListFailureSurfaces: a List error must not be silently admitted. +func TestAutoFullDrivesMinNodes_ListFailureSurfaces(t *testing.T) { + withFormClusterMinContainers(t, 5, 5) + v := &clusterAutoFullDrivesMinNodes{} + ctx := context.Background() + + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme: %v", err) + } + c := fake.NewClientBuilder().WithScheme(scheme).WithInterceptorFuncs(interceptor.Funcs{ + List: func(ctx context.Context, cl client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + return errors.New("boom") + }, + }).Build() + + errs := v.Validate(ctx, c, minNodesCluster(&weka.WekaClusterTemplate{})) + if len(errs) != 2 { + t.Fatalf("expected one internal error per role, got %v", errs) + } + for _, e := range errs { + if e.Type != field.ErrorTypeInternal { + t.Errorf("expected an InternalError, got %v", e.Type) + } + } +} diff --git a/internal/validation/cluster_auto_full_drives_pin_exceeds_node_drives.go b/internal/validation/cluster_auto_full_drives_pin_exceeds_node_drives.go new file mode 100644 index 000000000..ce3e1e830 --- /dev/null +++ b/internal/validation/cluster_auto_full_drives_pin_exceeds_node_drives.go @@ -0,0 +1,149 @@ +package validation + +import ( + "context" + "fmt" + "sort" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/weka/weka-operator/internal/consts" +) + +// clusterAutoFullDrivesPinExceedsNodeDrives checks the two per-node pins of auto-full-drives +// ("acts as a daemonset") mode against what each drive-role node actually has signed. Both legs +// describe a plan that never converges: the planner reports the whole cluster infeasible and creates +// nothing, so this is an Error under strict mode. +// +// - driveCores ABOVE a node's EFFECTIVE drive count: weka runs at most one drive core per physical +// drive, so the pin cannot be satisfied there. Names the FEWEST-drives node. Skipped when +// numDrives is pinned — CEL (numDrives >= driveCores) already covers that exact comparison, and +// reporting it twice would only duplicate the rejection. +// - numDrives ABOVE a node's signed drive count: the pin selects that many largest drives per node +// and cannot be honored where fewer exist. Names the FEWEST-drives node. +// +// A driveCores pin BELOW the drive count is deliberately NOT reported: drives are decoupled from +// cores, so the node keeps every drive and simply runs them on fewer cores. That is a supported, +// lossless configuration. +type clusterAutoFullDrivesPinExceedsNodeDrives struct{} + +func (clusterAutoFullDrivesPinExceedsNodeDrives) ID() string { + return "cluster_auto_full_drives_pin_exceeds_node_drives" +} + +func (clusterAutoFullDrivesPinExceedsNodeDrives) Validate(ctx context.Context, c client.Client, obj runtime.Object) field.ErrorList { + cluster, ok := obj.(*weka.WekaCluster) + if !ok { + return nil + } + // No nil guard on Spec.Dynamic: a nil template IS auto-full-drives mode, it just carries no pins, + // so both legs fall through on their own (DriveCores/NumDrives read as 0). + config := cluster.Spec.Dynamic + if !config.UsesAutoFullDrives() { + return nil + } + var driveCores, numDrives int + if config != nil { + driveCores, numDrives = config.DriveCores, config.NumDrives + } + if driveCores <= 0 && numDrives <= 0 { + return nil + } + + fldPath := field.NewPath("spec", "dynamicTemplate") + + nodes, errs := listDriveRoleNodes(ctx, c, cluster, fldPath) + if errs != nil { + return errs + } + if len(nodes) == 0 { + return nil + } + + // Only the full-drives annotation carries the information needed; no annotated node means + // sign-drives hasn't run yet — a pre-signing state clusterDrivesUnsignedAdvisory already owns. + anyAnnotated := false + for i := range nodes { + if _, full := nodes[i].Annotations[consts.AnnotationWekaFullDrives]; full { + anyAnnotated = true + break + } + } + if !anyAnnotated { + return nil + } + + // Sort by name so the "worst node" pick is deterministic on ties. + sort.Slice(nodes, func(i, j int) bool { return nodes[i].Name < nodes[j].Name }) + + infos, errs := driveRoleNodeInfos(nodes, fldPath) + if errs != nil { + return errs + } + + // worst* track the fewest-drives offender per leg; -1 = no offender seen yet. + coresAffected, coresWorstCount := 0, -1 + var coresWorstNode string + drivesAffected, drivesWorstCount := 0, -1 + var drivesWorstNode string + + for _, ni := range infos { + if _, full := ni.Node.Annotations[consts.AnnotationWekaFullDrives]; !full { + continue + } + signed := len(ni.Info.AvailableDrives) + if signed == 0 { + continue + } + + if numDrives > 0 && numDrives > signed { + drivesAffected++ + if drivesWorstCount == -1 || signed < drivesWorstCount { + drivesWorstCount = signed + drivesWorstNode = ni.Node.Name + } + } + + // Effective drive count is the pin when set, else everything the node signed. Skipped + // entirely under a numDrives pin — CEL owns the numDrives >= driveCores comparison. + if driveCores > 0 && numDrives <= 0 && driveCores > signed { + coresAffected++ + if coresWorstCount == -1 || signed < coresWorstCount { + coresWorstCount = signed + coresWorstNode = ni.Node.Name + } + } + } + + var out field.ErrorList + if drivesAffected > 0 { + detail := fmt.Sprintf( + "spec.dynamicTemplate.numDrives (%d) exceeds node %q's %d signed full drive(s) — the worst "+ + "(fewest-drives) of %d affected node(s). numDrives pins how many of each node's largest "+ + "drives the cluster takes, so it cannot be honored where fewer are signed, and the whole "+ + "plan is reported infeasible (AutoFullDrivesInfeasible) — no drive or compute container is "+ + "created anywhere. Lower numDrives to at most %d, drop the pin so each node contributes "+ + "every drive it has signed, sign more drives on that node, or remove it from "+ + "spec.roleNodeSelector.drive.", + numDrives, drivesWorstNode, drivesWorstCount, drivesAffected, drivesWorstCount, + ) + out = append(out, field.Invalid(fldPath.Child("numDrives"), numDrives, detail)) + } + if coresAffected > 0 { + detail := fmt.Sprintf( + "spec.dynamicTemplate.driveCores (%d) exceeds node %q's %d signed full drive(s) — the worst "+ + "(fewest-drives) of %d affected node(s). Full-drives mode runs at most one drive core per "+ + "physical drive, so this pin cannot be satisfied there and the whole plan is reported "+ + "infeasible (AutoFullDrivesInfeasible) — no drive or compute container is created "+ + "anywhere. Lower driveCores to at most %d, drop the pin so cores are derived per node, or "+ + "switch to drive-sharing mode (containerCapacity or clusterCapacity) to run more cores "+ + "than physical drives.", + driveCores, coresWorstNode, coresWorstCount, coresAffected, coresWorstCount, + ) + out = append(out, field.Invalid(fldPath.Child("driveCores"), driveCores, detail)) + } + return out +} diff --git a/internal/validation/cluster_auto_full_drives_pin_exceeds_node_drives_test.go b/internal/validation/cluster_auto_full_drives_pin_exceeds_node_drives_test.go new file mode 100644 index 000000000..76068052f --- /dev/null +++ b/internal/validation/cluster_auto_full_drives_pin_exceeds_node_drives_test.go @@ -0,0 +1,319 @@ +package validation + +import ( + "context" + "fmt" + "strings" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" +) + +// autoFullDrivesPinCluster builds an auto-full-drives WekaCluster (no container-count or capacity +// field set) with the given driveCores/numDrives pins, spec.nodeSelector, and optionally +// spec.roleNodeSelector.drive. +func autoFullDrivesPinCluster(driveCores, numDrives int, selector, roleDriveSelector map[string]string) *weka.WekaCluster { + c := &weka.WekaCluster{} + c.Spec.Dynamic = &weka.WekaClusterTemplate{DriveCores: driveCores, NumDrives: numDrives} + c.Spec.NodeSelector = selector + if roleDriveSelector != nil { + c.Spec.RoleNodeSelector.Drive = &roleDriveSelector + } + return c +} + +// drivesOfCount returns a driveCapacitiesGiB slice of the given LENGTH for driveRoleNode: only len(...) +// matters here since this validator compares drive COUNT, not capacity. +func drivesOfCount(n int) []int { + out := make([]int, n) + for i := range out { + out[i] = 100 + } + return out +} + +// TestAutoFullDrivesPin_DriveCoresAboveDriveCount covers a driveCores pin exceeding a node's drive +// count: unsatisfiable in full-drives mode. Must reject, name the node, and report its drive count. +func TestAutoFullDrivesPin_DriveCoresAboveDriveCount(t *testing.T) { + v := &clusterAutoFullDrivesPinExceedsNodeDrives{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + c := fakeClientWithNodes(t, driveRoleNode(t, "big-node", labels, drivesOfCount(3))) + + errs := v.Validate(ctx, c, autoFullDrivesPinCluster(5, 0, labels, nil)) + if len(errs) != 1 { + t.Fatalf("expected exactly one violation, got %v", errs) + } + if got, want := errs[0].Field, "spec.dynamicTemplate.driveCores"; got != want { + t.Errorf("expected field %q, got %q", want, got) + } + detail := errs[0].Detail + for _, want := range []string{"big-node", "at most 3", "AutoFullDrivesInfeasible"} { + if !strings.Contains(detail, want) { + t.Errorf("expected message to contain %q, got: %s", want, detail) + } + } +} + +// TestAutoFullDrivesPin_DriveCoresBelowDriveCountIsLossless is the inverted case of the deleted +// "too low" leg: with drives decoupled from cores, a pin below the drive count keeps every drive and +// runs it on fewer cores. It must NOT be reported. +func TestAutoFullDrivesPin_DriveCoresBelowDriveCountIsLossless(t *testing.T) { + v := &clusterAutoFullDrivesPinExceedsNodeDrives{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + c := fakeClientWithNodes(t, driveRoleNode(t, "big-node", labels, drivesOfCount(10))) + + errs := v.Validate(ctx, c, autoFullDrivesPinCluster(2, 0, labels, nil)) + if len(errs) != 0 { + t.Errorf("a driveCores pin below the drive count is lossless and supported; got %v", errs) + } +} + +// TestAutoFullDrivesPin_NumDrivesAboveSignedCount covers the third leg: numDrives pinned above what a +// node has signed cannot be honored there. +func TestAutoFullDrivesPin_NumDrivesAboveSignedCount(t *testing.T) { + v := &clusterAutoFullDrivesPinExceedsNodeDrives{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + c := fakeClientWithNodes(t, + driveRoleNode(t, "n-small", labels, drivesOfCount(4)), + driveRoleNode(t, "n-large", labels, drivesOfCount(30)), + ) + + errs := v.Validate(ctx, c, autoFullDrivesPinCluster(0, 10, labels, nil)) + if len(errs) != 1 { + t.Fatalf("expected exactly one violation, got %v", errs) + } + if got, want := errs[0].Field, "spec.dynamicTemplate.numDrives"; got != want { + t.Errorf("expected field %q, got %q", want, got) + } + detail := errs[0].Detail + for _, want := range []string{"n-small", "at most 4", "1 affected node"} { + if !strings.Contains(detail, want) { + t.Errorf("expected message to contain %q, got: %s", want, detail) + } + } + if strings.Contains(detail, "n-large") { + t.Errorf("expected only the worst node to be named, got: %s", detail) + } +} + +// TestAutoFullDrivesPin_NumDrivesPinnedSkipsDriveCoresLeg: with numDrives pinned, CEL already enforces +// numDrives >= driveCores, so this validator must not re-report the same comparison. +func TestAutoFullDrivesPin_NumDrivesPinnedSkipsDriveCoresLeg(t *testing.T) { + v := &clusterAutoFullDrivesPinExceedsNodeDrives{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + // 30 signed drives, numDrives=5 (satisfiable), driveCores=8 > the effective 5. CEL owns it. + c := fakeClientWithNodes(t, driveRoleNode(t, "n1", labels, drivesOfCount(30))) + + errs := v.Validate(ctx, c, autoFullDrivesPinCluster(8, 5, labels, nil)) + if len(errs) != 0 { + t.Errorf("expected the driveCores leg to be skipped under a numDrives pin, got %v", errs) + } +} + +// TestAutoFullDrivesPin_BothLegsAtOnce: a numDrives pin above one node's count while driveCores also +// exceeds the effective count. Only the numDrives leg fires (the driveCores leg is CEL's). +func TestAutoFullDrivesPin_BothLegsAtOnce(t *testing.T) { + v := &clusterAutoFullDrivesPinExceedsNodeDrives{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + c := fakeClientWithNodes(t, driveRoleNode(t, "n-small", labels, drivesOfCount(2))) + + errs := v.Validate(ctx, c, autoFullDrivesPinCluster(6, 8, labels, nil)) + if len(errs) != 1 { + t.Fatalf("expected exactly one violation (numDrives only), got %v", errs) + } + if got, want := errs[0].Field, "spec.dynamicTemplate.numDrives"; got != want { + t.Errorf("expected field %q, got %q", want, got) + } +} + +// TestAutoFullDrivesPin_Adequate covers a driveCores pin that exactly matches every node's drive +// count: no violation. +func TestAutoFullDrivesPin_Adequate(t *testing.T) { + v := &clusterAutoFullDrivesPinExceedsNodeDrives{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, drivesOfCount(4)), + driveRoleNode(t, "n2", labels, drivesOfCount(4)), + ) + + errs := v.Validate(ctx, c, autoFullDrivesPinCluster(4, 0, labels, nil)) + if len(errs) != 0 { + t.Errorf("expected no violation, got %v", errs) + } +} + +// TestAutoFullDrivesPin_WorstPick covers several nodes tripping the SAME leg: the fewest-drives node +// must be named and the affected count must include all of them. +func TestAutoFullDrivesPin_WorstPick(t *testing.T) { + v := &clusterAutoFullDrivesPinExceedsNodeDrives{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + c := fakeClientWithNodes(t, + driveRoleNode(t, "n-small", labels, drivesOfCount(2)), + driveRoleNode(t, "n-medium", labels, drivesOfCount(4)), + driveRoleNode(t, "n-large", labels, drivesOfCount(8)), + ) + + // driveCores=8 exceeds n-small(2) and n-medium(4), matches n-large(8) exactly. + errs := v.Validate(ctx, c, autoFullDrivesPinCluster(8, 0, labels, nil)) + if len(errs) != 1 { + t.Fatalf("expected exactly one violation, got %v", errs) + } + detail := errs[0].Detail + if !strings.Contains(detail, "n-small") { + t.Errorf("expected the worst (fewest-drives) node n-small to be named, got: %s", detail) + } + if strings.Contains(detail, "n-medium") || strings.Contains(detail, "n-large") { + t.Errorf("expected only the worst node to be named, got: %s", detail) + } + if !strings.Contains(detail, "2 affected node") { + t.Errorf("expected 2 affected nodes (n-small, n-medium), got: %s", detail) + } +} + +// TestAutoFullDrivesPin_NotAutoFullDrives: silent when a container count or capacity field puts the +// cluster in another sizing mode — clusterDriveCoresBelowCapacity owns those. +func TestAutoFullDrivesPin_NotAutoFullDrives(t *testing.T) { + v := &clusterAutoFullDrivesPinExceedsNodeDrives{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + c := fakeClientWithNodes(t, driveRoleNode(t, "n1", labels, drivesOfCount(3))) + + for name, dyn := range map[string]*weka.WekaClusterTemplate{ + "containerCapacity": {ContainerCapacity: 6000, DriveCores: 9}, + "clusterCapacity": {ClusterCapacity: "500TiB", DriveCores: 9}, + "counts": {ComputeContainers: 6, DriveContainers: 6, DriveCores: 9}, + } { + t.Run(name, func(t *testing.T) { + cluster := &weka.WekaCluster{} + cluster.Spec.Dynamic = dyn + cluster.Spec.NodeSelector = labels + if errs := v.Validate(ctx, c, cluster); len(errs) != 0 { + t.Errorf("expected no violation outside auto-full-drives mode, got %v", errs) + } + }) + } +} + +// TestAutoFullDrivesPin_NoPins covers the mode's default shape — a nil or empty dynamicTemplate, which +// IS auto-full-drives but carries no pins to check. Must not panic and must stay silent. +func TestAutoFullDrivesPin_NoPins(t *testing.T) { + v := &clusterAutoFullDrivesPinExceedsNodeDrives{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + c := fakeClientWithNodes(t, driveRoleNode(t, "n1", labels, drivesOfCount(3))) + + t.Run("nil dynamicTemplate", func(t *testing.T) { + cluster := &weka.WekaCluster{} + cluster.Spec.NodeSelector = labels + if errs := v.Validate(ctx, c, cluster); len(errs) != 0 { + t.Errorf("expected no violation for a nil template, got %v", errs) + } + }) + t.Run("empty dynamicTemplate", func(t *testing.T) { + if errs := v.Validate(ctx, c, autoFullDrivesPinCluster(0, 0, labels, nil)); len(errs) != 0 { + t.Errorf("expected no violation for an empty template, got %v", errs) + } + }) +} + +// TestAutoFullDrivesPin_NoSignedDrives covers the pre-signing bootstrap case: no node has a populated +// full-drives annotation — silent, not a violation. +func TestAutoFullDrivesPin_NoSignedDrives(t *testing.T) { + v := &clusterAutoFullDrivesPinExceedsNodeDrives{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + c := fakeClientWithNodes(t, driveRoleNode(t, "n1", labels, nil)) + + if errs := v.Validate(ctx, c, autoFullDrivesPinCluster(9, 0, labels, nil)); len(errs) != 0 { + t.Errorf("expected no violation pre-signing, got %v", errs) + } + if errs := v.Validate(ctx, c, autoFullDrivesPinCluster(0, 9, labels, nil)); len(errs) != 0 { + t.Errorf("expected no violation pre-signing, got %v", errs) + } +} + +// TestAutoFullDrivesPin_NoMatchingNodes covers nodes existing but none matching the selector: silent. +func TestAutoFullDrivesPin_NoMatchingNodes(t *testing.T) { + v := &clusterAutoFullDrivesPinExceedsNodeDrives{} + ctx := context.Background() + + c := fakeClientWithNodes(t, driveRoleNode(t, "n1", map[string]string{"other": "label"}, drivesOfCount(2))) + + errs := v.Validate(ctx, c, autoFullDrivesPinCluster(9, 0, map[string]string{"role": "drive"}, nil)) + if len(errs) != 0 { + t.Errorf("expected no violation when selector matches nothing, got %v", errs) + } +} + +// TestAutoFullDrivesPin_RoleSelectorFallback covers the role-selector fallback: unset uses +// spec.nodeSelector; set, it uses that instead and ignores the cluster-wide match. +func TestAutoFullDrivesPin_RoleSelectorFallback(t *testing.T) { + v := &clusterAutoFullDrivesPinExceedsNodeDrives{} + ctx := context.Background() + + clusterWideSelector := map[string]string{"pool": "general"} + roleSelector := map[string]string{"drive-role": "yes"} + + const ( + roleNodeDrives = 20 + clusterNodeDrives = 2 + pinnedDriveCores = 21 // above both, so whichever node is matched trips the too-high leg + ) + + // n1 matches only the role-specific selector; n2 matches only the cluster-wide selector. + nodeRoleOnly := driveRoleNode(t, "role-only-node", roleSelector, drivesOfCount(roleNodeDrives)) + nodeClusterOnly := driveRoleNode(t, "cluster-only-node", clusterWideSelector, drivesOfCount(clusterNodeDrives)) + + t.Run("roleNodeSelector.drive unset falls back to spec.nodeSelector", func(t *testing.T) { + c := fakeClientWithNodes(t, nodeRoleOnly, nodeClusterOnly) + errs := v.Validate(ctx, c, autoFullDrivesPinCluster(pinnedDriveCores, 0, clusterWideSelector, nil)) + if len(errs) != 1 { + t.Fatalf("expected exactly one violation, got %v", errs) + } + detail := errs[0].Detail + if !strings.Contains(detail, "cluster-only-node") { + t.Errorf("expected fallback to name cluster-only-node, got: %s", detail) + } + if strings.Contains(detail, "role-only-node") { + t.Errorf("expected fallback to ignore role-only-node, got: %s", detail) + } + if !strings.Contains(detail, fmt.Sprintf("%d signed full drive", clusterNodeDrives)) { + t.Errorf("expected drive count %d, got: %s", clusterNodeDrives, detail) + } + }) + + t.Run("roleNodeSelector.drive set overrides spec.nodeSelector", func(t *testing.T) { + c := fakeClientWithNodes(t, nodeRoleOnly, nodeClusterOnly) + errs := v.Validate(ctx, c, autoFullDrivesPinCluster(pinnedDriveCores, 0, clusterWideSelector, roleSelector)) + if len(errs) != 1 { + t.Fatalf("expected exactly one violation, got %v", errs) + } + detail := errs[0].Detail + if !strings.Contains(detail, "role-only-node") { + t.Errorf("expected role selector to name role-only-node, got: %s", detail) + } + if strings.Contains(detail, "cluster-only-node") { + t.Errorf("expected role selector to ignore cluster-only-node, got: %s", detail) + } + if !strings.Contains(detail, fmt.Sprintf("%d signed full drive", roleNodeDrives)) { + t.Errorf("expected drive count %d, got: %s", roleNodeDrives, detail) + } + }) +} diff --git a/internal/validation/cluster_capacity_chunk_feasibility_test.go b/internal/validation/cluster_capacity_chunk_feasibility_test.go index cacec1b83..bed69c899 100644 --- a/internal/validation/cluster_capacity_chunk_feasibility_test.go +++ b/internal/validation/cluster_capacity_chunk_feasibility_test.go @@ -14,8 +14,6 @@ import ( "github.com/weka/weka-operator/internal/pkg/domain" ) -const tibV = 1024 // GiB per TiB - func ccCluster(uid string, cap string, sw, rl, hs int, ratio *weka.DriveTypesRatio) *weka.WekaCluster { c := &weka.WekaCluster{ ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: "ns", UID: types.UID(uid)}, diff --git a/internal/validation/cluster_capacity_min_drive_containers.go b/internal/validation/cluster_capacity_min_drive_containers.go new file mode 100644 index 000000000..475789969 --- /dev/null +++ b/internal/validation/cluster_capacity_min_drive_containers.go @@ -0,0 +1,70 @@ +package validation + +import ( + "context" + "fmt" + + wekav1alpha1 "github.com/weka/weka-k8s-api/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/weka/weka-operator/internal/capacityplanner" + globalconfig "github.com/weka/weka-operator/internal/config" + "github.com/weka/weka-operator/internal/controllers/allocator" +) + +// clusterCapacityMinDriveContainers warns when clusterCapacity's structural lower bound — the protection +// scheme's failure-domain floor (StripeWidth+RedundancyLevel+HotSpare) — sits below +// FormClusterMinDriveContainers. clusterCapacity derives the drive-container count from a capacity +// target rather than a spec field, so a plan as small as that floor is legitimate; if the floor is +// below the form-cluster minimum, the planner may hand admission nothing pinned to check while weka +// itself refuses to form the cluster, and it loops on MinContainersNotReady forever with healthy but +// idle containers — the same silent hang cluster_min_containers and cluster_auto_full_drives_min_nodes +// exist to prevent, in the one mode neither covers. +type clusterCapacityMinDriveContainers struct{} + +func (clusterCapacityMinDriveContainers) ID() string { return "cluster_capacity_min_drive_containers" } + +func (clusterCapacityMinDriveContainers) Validate(_ context.Context, _ client.Client, obj runtime.Object) field.ErrorList { + cluster, ok := obj.(*wekav1alpha1.WekaCluster) + if !ok || cluster.Spec.Dynamic == nil || !cluster.Spec.Dynamic.UsesClusterCapacity() { + return nil + } + minDrive := globalconfig.Consts.FormClusterMinDriveContainers + if minDrive <= 0 { // minimum disabled by configuration — nothing to enforce + return nil + } + if cluster.Spec.Dynamic.DriveContainers > 0 { // pinned counts are covered (as an Error) by cluster_min_containers + return nil + } + + specSW, specRL, specHS := cluster.Spec.StripeWidth, cluster.Spec.RedundancyLevel, cluster.Spec.HotSpare + sw, rl, hs := globalconfig.Config.DriveSharing.EffectiveProtection(specSW, specRL, specHS) + // Below the protection floor the failure-domain math is degenerate — the shipped chart leaves + // PROTECTION_* at 0, so an unset spec resolves to 0+0+0 and would be reported as a "floor of 0 drive + // containers" with a remedy to lower the minimum to 0. clusterCapacityProtection already rejects + // those schemes outright; same guard as clusterCapacityChunkFeasibility. + minSW, minRL, minHS := allocator.MinProtectionFloor() + if sw < minSW || rl < minRL || hs < minHS { + return nil + } + minFd := capacityplanner.ProtectionScheme{StripeWidth: sw, RedundancyLevel: rl, HotSpare: hs}.MinFdNum() + if minFd >= minDrive { + return nil + } + + // Report the raw spec value as the bad value (what the API client set), but check and message the + // effective one — same convention as cluster_capacity_protection. + return field.ErrorList{field.Invalid(field.NewPath("spec", "stripeWidth"), specSW, fmt.Sprintf( + "clusterCapacity derives the drive-container count from the capacity target, not from a spec field; "+ + "its only structural lower bound is the protection scheme's failure-domain floor. With the "+ + "effective protection stripeWidth=%d, redundancyLevel=%d, hotSpare=%d, that floor is %d drive "+ + "container(s), below FormClusterMinDriveContainers=%d — the planner may legitimately derive a plan "+ + "as small as %d drive containers, and the cluster would then wait forever on MinContainersNotReady "+ + "with its containers running but idle. Raise stripeWidth/redundancyLevel/hotSpare (or their "+ + "PROTECTION_STRIPE_WIDTH/PROTECTION_REDUNDANCY_LEVEL/PROTECTION_HOT_SPARE Helm defaults) so the "+ + "floor reaches %d, or pin spec.dynamicTemplate.driveContainers to at least %d, or lower "+ + "FORM_CLUSTER_MIN_DRIVE_CONTAINERS to at most %d.", + sw, rl, hs, minFd, minDrive, minFd, minDrive, minDrive, minFd))} +} diff --git a/internal/validation/cluster_capacity_min_drive_containers_test.go b/internal/validation/cluster_capacity_min_drive_containers_test.go new file mode 100644 index 000000000..9247d04ee --- /dev/null +++ b/internal/validation/cluster_capacity_min_drive_containers_test.go @@ -0,0 +1,140 @@ +package validation + +import ( + "context" + "strings" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + + globalconfig "github.com/weka/weka-operator/internal/config" +) + +// withFormClusterMinDriveContainers sets FormClusterMinDriveContainers for the duration of the test, +// restoring the original via t.Cleanup. +func withFormClusterMinDriveContainers(t *testing.T, min int) { + t.Helper() + prev := globalconfig.Consts.FormClusterMinDriveContainers + globalconfig.Consts.FormClusterMinDriveContainers = min + t.Cleanup(func() { + globalconfig.Consts.FormClusterMinDriveContainers = prev + }) +} + +// TestClusterCapacityMinDriveContainers covers clusterCapacity's structural lower bound (the protection +// scheme's failure-domain floor) against the form-cluster minimum, in the one mode where the +// drive-container count isn't a spec field cluster_min_containers can check directly. +func TestClusterCapacityMinDriveContainers(t *testing.T) { + v := &clusterCapacityMinDriveContainers{} + ctx := context.Background() + + tests := []struct { + name string + minDrive int + sw, rl, hs int // protection defaults (0 lets the spec value, also 0 in most cases, win) + dynamic *weka.WekaClusterTemplate + wantN int + wantSubs []string + }{ + { + name: "auto-full-drives mode skipped", + minDrive: 7, + sw: 3, rl: 2, hs: 0, + dynamic: &weka.WekaClusterTemplate{}, + }, + { + name: "count-based mode skipped", + minDrive: 7, + sw: 3, rl: 2, hs: 0, + dynamic: &weka.WekaClusterTemplate{DriveContainers: 3, ComputeContainers: 3}, + }, + { + name: "pinned driveContainers skipped even with a low floor", + minDrive: 7, + sw: 3, rl: 2, hs: 0, + dynamic: &weka.WekaClusterTemplate{ClusterCapacity: "500TiB", DriveContainers: 3}, + }, + { + name: "minimum disabled skipped", + minDrive: 0, + sw: 3, rl: 2, hs: 0, + dynamic: &weka.WekaClusterTemplate{ClusterCapacity: "500TiB"}, + }, + { + // The shipped configuration: 3+2+0 floor (5) meets the default minimum (5) exactly. This must + // NOT warn. + name: "floor equal to the minimum is allowed", + minDrive: 5, + sw: 3, rl: 2, hs: 0, + dynamic: &weka.WekaClusterTemplate{ClusterCapacity: "500TiB"}, + }, + { + name: "floor above the minimum is allowed", + minDrive: 4, + sw: 3, rl: 2, hs: 0, + dynamic: &weka.WekaClusterTemplate{ClusterCapacity: "500TiB"}, + }, + { + name: "floor below a raised minimum is warned", + minDrive: 7, + sw: 3, rl: 2, hs: 0, + dynamic: &weka.WekaClusterTemplate{ClusterCapacity: "500TiB"}, + wantN: 1, + wantSubs: []string{ + "stripeWidth=3, redundancyLevel=2, hotSpare=0", + "floor is 5 drive container(s)", + "FormClusterMinDriveContainers=7", + "as small as 5 drive containers", + "MinContainersNotReady", + "floor reaches 7", + "driveContainers to at least 7", + "FORM_CLUSTER_MIN_DRIVE_CONTAINERS to at most 5", + }, + }, + { + // Protection left at 0 in the spec falls back to the Helm DriveSharing defaults. + name: "protection taken from Helm defaults when the spec leaves them 0", + minDrive: 7, + sw: 3, rl: 2, hs: 0, + dynamic: &weka.WekaClusterTemplate{ClusterCapacity: "500TiB"}, + wantN: 1, + wantSubs: []string{"stripeWidth=3, redundancyLevel=2, hotSpare=0"}, + }, + { + // The SHIPPED chart leaves protection.stripeWidth/redundancyLevel at 0, so an unset spec + // resolves to 0+0+0 and MinFdNum() is 0. Reporting "a floor of 0 drive container(s)" with a + // remedy to lower the minimum to 0 is nonsense; clusterCapacityProtection already rejects the + // scheme outright, so this rule must stay quiet below the floor. + name: "protection below the floor is left to cluster_capacity_protection", + minDrive: 5, + sw: 0, rl: 0, hs: 0, + dynamic: &weka.WekaClusterTemplate{ClusterCapacity: "500TiB"}, + }, + { + name: "partially sub-floor protection is also left alone", + minDrive: 7, + sw: 3, rl: 1, hs: 0, + dynamic: &weka.WekaClusterTemplate{ClusterCapacity: "500TiB"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + withFormClusterMinDriveContainers(t, tt.minDrive) + withDefaultProtection(t, tt.sw, tt.rl, tt.hs) + + cluster := &weka.WekaCluster{Spec: weka.WekaClusterSpec{Dynamic: tt.dynamic}} + // Leave spec protection fields at 0 so the Helm defaults set above take effect. + + errs := v.Validate(ctx, nil, cluster) + if len(errs) != tt.wantN { + t.Fatalf("got %d finding(s), want %d: %v", len(errs), tt.wantN, errs) + } + for _, sub := range tt.wantSubs { + if !strings.Contains(errs[0].Detail, sub) { + t.Errorf("detail missing %q, got: %s", sub, errs[0].Detail) + } + } + }) + } +} diff --git a/internal/validation/cluster_capacity_protection.go b/internal/validation/cluster_capacity_protection.go index 7f9e0cefd..593eb092d 100644 --- a/internal/validation/cluster_capacity_protection.go +++ b/internal/validation/cluster_capacity_protection.go @@ -13,12 +13,8 @@ import ( "github.com/weka/weka-operator/internal/controllers/allocator" ) -// clusterCapacityProtection rejects WekaCluster specs that use clusterCapacity -// with a protection scheme below the production 3+2+0 minimum (stripeWidth>=3, -// redundancyLevel>=2, hotSpare>=0 / hot spare optional). Any lower stripeWidth or -// redundancyLevel produces a degenerate or unbootable cluster. The floor is relaxed -// to single-parity 2+1+0 when the operator-level AllowSingleParity flag is set -// (QA/test only) — see allocator.MinProtectionFloor. +// clusterCapacityProtection rejects clusterCapacity specs below the production 3+2+0 protection floor +// (stripeWidth/redundancyLevel/hotSpare); AllowSingleParity relaxes this to 2+1+0 for QA (see allocator.MinProtectionFloor). type clusterCapacityProtection struct{} func (clusterCapacityProtection) ID() string { return "cluster_capacity_protection" } @@ -32,11 +28,7 @@ func (clusterCapacityProtection) Validate(_ context.Context, _ client.Client, ob specSW, specRL, specHS := cluster.Spec.StripeWidth, cluster.Spec.RedundancyLevel, cluster.Spec.HotSpare sw, rl, hs := globalconfig.Config.DriveSharing.EffectiveProtection(specSW, specRL, specHS) minSW, minRL, minHS := allocator.MinProtectionFloor() - // Compare against the *effective* protection (spec value, else the PROTECTION_* Helm - // default) but report the raw spec value as the field.Invalid "bad value" so kubectl/API - // clients see the value that actually exists in the object (not a default-resolved one). - // The message carries the effective value so a 0-in-spec-resolved-from-default case is - // still explained and points at both sources. + // Report the raw spec value as the bad value (what the API client set), but check and message the effective one. if sw < minSW { errs = append(errs, field.Invalid(field.NewPath("spec", "stripeWidth"), specSW, fmt.Sprintf("clusterCapacity requires stripeWidth >= %d (effective value %d; raise spec.stripeWidth to >= %d, or leave spec.stripeWidth=0 to fall back to the PROTECTION_STRIPE_WIDTH default — which must itself be >= %d)", minSW, sw, minSW, minSW))) diff --git a/internal/validation/cluster_compute_drive_cores_floor.go b/internal/validation/cluster_compute_drive_cores_floor.go new file mode 100644 index 000000000..ea6f0669b --- /dev/null +++ b/internal/validation/cluster_compute_drive_cores_floor.go @@ -0,0 +1,68 @@ +package validation + +import ( + "context" + "fmt" + + wekav1alpha1 "github.com/weka/weka-k8s-api/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/weka/weka-operator/internal/controllers/allocator" +) + +// clusterComputeDriveCoresFloor enforces the hard 1:1 floor between total compute and drive cores +// (compute containers front every drive); capacity planners enforce the same floor. See +// clusterDriveComputeCoreRatio for the softer recommended-ratio advisory check. +// +// Planner-managed templates are out of scope: cores there are assigned by the planner, not by +// GetWekaContainerCores, so the numbers this check reads are not the ones the cluster runs on. +type clusterComputeDriveCoresFloor struct{} + +func (clusterComputeDriveCoresFloor) ID() string { + return "cluster_compute_drive_cores_floor" +} + +func (clusterComputeDriveCoresFloor) Validate(_ context.Context, _ client.Client, obj runtime.Object) field.ErrorList { + cluster, ok := obj.(*wekav1alpha1.WekaCluster) + if !ok { + return nil + } + if cluster.Spec.Dynamic == nil { + return nil + } + + // clusterCapacity sizes both sides through the planner: computeCores/driveCores left at 0 mean + // "auto-derive" there (funcs_fd_planning.go), and the compute containers are built from + // plan.ComputeCores/plan.ComputeLayout rather than the template. Reading GetWekaContainerCores' + // static-template defaults (unset -> 1 core) would compare numbers the cluster never uses and reject + // a plan the planner sizes correctly. Auto full drives is excluded by the count guard below. + if allocator.IsPlannerManaged(cluster.Spec.Dynamic) { + return nil + } + + driveContainers := cluster.Spec.Dynamic.DriveContainers + computeContainers := cluster.Spec.Dynamic.ComputeContainers + if driveContainers <= 0 || computeContainers <= 0 { + return nil + } + + cores := allocator.GetWekaContainerCores(cluster.Spec.Dynamic) + driveSide := driveContainers * cores.Drive + computeSide := computeContainers * cores.Compute + + if computeSide >= driveSide { + return nil + } + + detail := fmt.Sprintf( + "total compute cores (%d) is below total drive cores (%d). A cluster must have at least one "+ + "compute core per drive core; raise computeContainers or computeCores (or lower the drive "+ + "side). Capacity planners enforce the same floor and will report the plan infeasible.", + computeSide, driveSide, + ) + return field.ErrorList{ + field.Invalid(field.NewPath("spec", "dynamicTemplate"), computeSide, detail), + } +} diff --git a/internal/validation/cluster_compute_drive_cores_floor_test.go b/internal/validation/cluster_compute_drive_cores_floor_test.go new file mode 100644 index 000000000..74b144c7a --- /dev/null +++ b/internal/validation/cluster_compute_drive_cores_floor_test.go @@ -0,0 +1,102 @@ +package validation + +import ( + "context" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" +) + +func TestClusterComputeDriveCoresFloor(t *testing.T) { + v := &clusterComputeDriveCoresFloor{} + ctx := context.Background() + + tests := []struct { + name string + dynamic *weka.WekaClusterTemplate + wantErr bool + }{ + { + name: "no dynamic template skipped", + dynamic: nil, + wantErr: false, + }, + { + name: "zero drive containers skipped", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 0, ComputeContainers: 4, + }, + wantErr: false, + }, + { + name: "zero compute containers skipped", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 4, ComputeContainers: 0, + }, + wantErr: false, + }, + { + name: "compute cores equal to drive cores clears the floor", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 2, ComputeContainers: 2, + DriveCores: 2, ComputeCores: 2, + }, + wantErr: false, + }, + { + name: "compute cores above drive cores clears the floor", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 1, ComputeContainers: 4, + DriveCores: 1, ComputeCores: 1, + }, + wantErr: false, + }, + { + name: "compute cores below drive cores violates the 1:1 floor", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 4, ComputeContainers: 1, + DriveCores: 2, ComputeCores: 1, + }, + wantErr: true, + }, + { + name: "unset cores default to 1:1 per container, floor cleared", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 1, ComputeContainers: 1, + }, + wantErr: false, + }, + { + // The planner derives compute cores when computeCores is unset (funcs_fd_planning.go passes + // 0 == auto-derive), so the template's 1-core default describes a cluster that never runs. + // Reading it here would reject a spec the planner sizes correctly. + name: "clusterCapacity with more drive than compute containers skipped", + dynamic: &weka.WekaClusterTemplate{ + ClusterCapacity: "1PiB", + DriveContainers: 8, ComputeContainers: 5, + }, + wantErr: false, + }, + { + name: "clusterCapacity with a pinned driveCores skipped", + dynamic: &weka.WekaClusterTemplate{ + ClusterCapacity: "1PiB", + DriveContainers: 6, ComputeContainers: 6, + DriveCores: 4, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + errs := v.Validate(ctx, nil, ratioCluster(tt.dynamic)) + if tt.wantErr && len(errs) == 0 { + t.Errorf("expected an error, got none") + } + if !tt.wantErr && len(errs) != 0 { + t.Errorf("expected no error, got %v", errs) + } + }) + } +} diff --git a/internal/validation/cluster_cores_available.go b/internal/validation/cluster_cores_available.go index 0681f5be1..b5a99a216 100644 --- a/internal/validation/cluster_cores_available.go +++ b/internal/validation/cluster_cores_available.go @@ -11,14 +11,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// clusterCoresAvailable warns per-role on two failure modes: total -// requested cores exceed total Allocatable[cpu] across matched nodes -// (capacity), or the smallest matched node can't fit a single -// container (single-fit). Skips when *Cores is 0 (operator-derived) or -// when no matched nodes (clusterSelectedNodesCount covers). -// -// Allocatable[cpu] is the kubelet view; it doesn't reflect Weka's -// isolcpus pinning. Bin-packing across roles is not attempted. +// clusterCoresAvailable checks per-role, against matched nodes' Allocatable[cpu] (the kubelet view, +// which doesn't reflect Weka's isolcpus pinning): total requested cores must fit total allocatable +// (no bin-packing across roles attempted), and the smallest node must fit at least one container. +// Skipped when *Cores is 0 (operator-derived) or no nodes match (clusterSelectedNodesCount covers that). type clusterCoresAvailable struct{} func (clusterCoresAvailable) ID() string { diff --git a/internal/validation/cluster_cores_decrease.go b/internal/validation/cluster_cores_decrease.go index d170fb10e..b28c22aba 100644 --- a/internal/validation/cluster_cores_decrease.go +++ b/internal/validation/cluster_cores_decrease.go @@ -10,13 +10,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// clusterCoresDecrease rejects WekaCluster updates that reduce any cores -// field in spec.dynamicTemplate to a smaller EXPLICIT (positive) value. Unsetting a -// field — new == 0 for plain ints, or nil for nullable *int — means "revert -// to operator-derived sizing" and is ALLOWED (e.g. when migrating from -// containerCapacity to clusterCapacity, where the planner derives cores). -// Only an explicit positive decrease (e.g. 4 -> 2) is blocked, since that can -// destabilize a running cluster. +// clusterCoresDecrease rejects updates that explicitly decrease a spec.dynamicTemplate cores field +// (e.g. 4 -> 2), since that can destabilize a running cluster. Unsetting a field (new == 0, or nil +// for *int) means "revert to operator-derived sizing" and is allowed. type clusterCoresDecrease struct{} func (clusterCoresDecrease) ID() string { return "cluster_cores_decrease" } @@ -34,8 +30,7 @@ func (clusterCoresDecrease) ValidateUpdate(_ context.Context, _ client.Client, o return nil // nothing to compare against } o := oldCluster.Spec.Dynamic - // When new.Dynamic is nil treat all cores as 0 (unset) — i.e. revert to - // operator-derived sizing, which is allowed by the new==0 rule below. + // A nil new.Dynamic treats all cores as 0 (unset), allowed by the new==0 rule below. var emptyDynamic wekav1alpha1.WekaClusterTemplate n := newCluster.Spec.Dynamic if n == nil { @@ -58,8 +53,6 @@ func (clusterCoresDecrease) ValidateUpdate(_ context.Context, _ client.Client, o var errs field.ErrorList for _, ch := range checks { - // new == 0 means the field is unset (revert to operator-derived - // sizing) — allowed. Only block an explicit positive decrease. if ch.new != 0 && ch.new < ch.old { errs = append(errs, field.Forbidden( field.NewPath("spec", "dynamicTemplate", ch.fieldName), @@ -70,8 +63,7 @@ func (clusterCoresDecrease) ValidateUpdate(_ context.Context, _ client.Client, o } } - // Nullable *int: a nil new value means unset (revert to operator-derived) - // and is allowed. Only block an explicit smaller value. + // *int field: nil new value means unset and is allowed. if o.DataServicesFeCores != nil && n.DataServicesFeCores != nil && *n.DataServicesFeCores < *o.DataServicesFeCores { errs = append(errs, field.Forbidden( diff --git a/internal/validation/cluster_cores_per_container_limit.go b/internal/validation/cluster_cores_per_container_limit.go new file mode 100644 index 000000000..92d1b9f28 --- /dev/null +++ b/internal/validation/cluster_cores_per_container_limit.go @@ -0,0 +1,71 @@ +package validation + +import ( + "context" + "fmt" + + wekav1alpha1 "github.com/weka/weka-k8s-api/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + globalconfig "github.com/weka/weka-operator/internal/config" +) + +// clusterCoresPerContainerLimit rejects driveCores/computeCores above +// CapacityPlanner.MaxCoresPerContainer (default 19), the most cores a weka container may hold. This is +// an admission policy rather than a CEL/schema Maximum bound: a schema bound would block any edit to an +// already-over-limit running cluster, and can't track the Helm-configurable limit the planners use. +// MaxCoresPerContainer <= 0 disables the cap, matching planner behavior. +type clusterCoresPerContainerLimit struct{} + +func (clusterCoresPerContainerLimit) ID() string { + return "cluster_cores_per_container_limit" +} + +func (clusterCoresPerContainerLimit) Validate(_ context.Context, _ client.Client, obj runtime.Object) field.ErrorList { + cluster, ok := obj.(*wekav1alpha1.WekaCluster) + if !ok { + return nil + } + if cluster.Spec.Dynamic == nil { + return nil + } + + limit := globalconfig.Config.CapacityPlanner.MaxCoresPerContainer + if limit <= 0 { + return nil + } + + config := cluster.Spec.Dynamic + // Only the two planner-managed roles are checked; protocol roles are out of scope until confirmed to apply the same way. + checks := []struct { + field string + cores int + }{ + {"driveCores", config.DriveCores}, + {"computeCores", config.ComputeCores}, + } + + var out field.ErrorList + for _, c := range checks { + if c.cores <= limit { // also covers unset (0) + continue + } + detail := fmt.Sprintf( + "spec.dynamicTemplate.%s (%d) exceeds the per-container core limit of %d — a single weka "+ + "container cannot hold more cores than that. Lower %s to at most %d, or add containers "+ + "(raise %s) to spread the cores across more of them.", + c.field, c.cores, limit, c.field, limit, containerCountFieldFor(c.field), + ) + out = append(out, field.Invalid(field.NewPath("spec", "dynamicTemplate", c.field), c.cores, detail)) + } + return out +} + +func containerCountFieldFor(coresField string) string { + if coresField == "driveCores" { + return "driveContainers" + } + return "computeContainers" +} diff --git a/internal/validation/cluster_cores_per_container_limit_test.go b/internal/validation/cluster_cores_per_container_limit_test.go new file mode 100644 index 000000000..fd034a29c --- /dev/null +++ b/internal/validation/cluster_cores_per_container_limit_test.go @@ -0,0 +1,104 @@ +package validation + +import ( + "context" + "strings" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + + globalconfig "github.com/weka/weka-operator/internal/config" +) + +// withMaxCoresPerContainer sets the config value the validator reads and restores it after the test. +func withMaxCoresPerContainer(t *testing.T, limit int) { + t.Helper() + prev := globalconfig.Config.CapacityPlanner.MaxCoresPerContainer + globalconfig.Config.CapacityPlanner.MaxCoresPerContainer = limit + t.Cleanup(func() { globalconfig.Config.CapacityPlanner.MaxCoresPerContainer = prev }) +} + +func TestClusterCoresPerContainerLimit(t *testing.T) { + v := &clusterCoresPerContainerLimit{} + ctx := context.Background() + + tests := []struct { + name string + limit int + dynamic *weka.WekaClusterTemplate + wantN int + wantSubs []string + }{ + { + name: "no dynamic template skipped", + limit: 19, + dynamic: nil, + }, + { + name: "both unset skipped", + limit: 19, + dynamic: &weka.WekaClusterTemplate{}, + }, + { + name: "both within the limit", + limit: 19, + dynamic: &weka.WekaClusterTemplate{DriveCores: 19, ComputeCores: 1}, + }, + { + name: "driveCores above the limit", + limit: 19, + dynamic: &weka.WekaClusterTemplate{DriveCores: 20}, + wantN: 1, + wantSubs: []string{"driveCores", "20", "limit of 19", "driveContainers"}, + }, + { + name: "computeCores above the limit", + limit: 19, + dynamic: &weka.WekaClusterTemplate{ComputeCores: 24}, + wantN: 1, + wantSubs: []string{"computeCores", "24", "limit of 19", "computeContainers"}, + }, + { + name: "both above the limit reported separately", + limit: 19, + dynamic: &weka.WekaClusterTemplate{DriveCores: 20, ComputeCores: 21}, + wantN: 2, + }, + { + // 0 disables the cap in the planners; admission must agree, not fall back to 19. + name: "limit of zero disables the check", + limit: 0, + dynamic: &weka.WekaClusterTemplate{DriveCores: 200, ComputeCores: 200}, + }, + { + // Limit is Helm-configurable; must not be hard-coded to 19. + name: "honors a lowered configured limit", + limit: 8, + dynamic: &weka.WekaClusterTemplate{DriveCores: 12}, + wantN: 1, + wantSubs: []string{"limit of 8"}, + }, + { + name: "honors a raised configured limit", + limit: 32, + dynamic: &weka.WekaClusterTemplate{DriveCores: 24}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + withMaxCoresPerContainer(t, tt.limit) + cluster := &weka.WekaCluster{Spec: weka.WekaClusterSpec{Dynamic: tt.dynamic}} + + errs := v.Validate(ctx, nil, cluster) + if len(errs) != tt.wantN { + t.Fatalf("got %d violation(s), want %d: %v", len(errs), tt.wantN, errs) + } + for _, sub := range tt.wantSubs { + if !strings.Contains(errs[0].Detail, sub) { + t.Errorf("detail missing %q, got: %s", sub, errs[0].Detail) + } + } + }) + } +} diff --git a/internal/validation/cluster_drive_compute_core_ratio.go b/internal/validation/cluster_drive_compute_core_ratio.go index 6f83fa874..5b6cbe802 100644 --- a/internal/validation/cluster_drive_compute_core_ratio.go +++ b/internal/validation/cluster_drive_compute_core_ratio.go @@ -3,23 +3,24 @@ package validation import ( "context" "fmt" + "math" wekav1alpha1 "github.com/weka/weka-k8s-api/api/v1alpha1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" + + globalconfig "github.com/weka/weka-operator/internal/config" + "github.com/weka/weka-operator/internal/controllers/allocator" ) -// clusterDriveComputeCoreRatio warns when the drive:compute core ratio -// exceeds the recommended maximum of 1:2 — i.e. drive cores > -// compute cores / 2. Compute does more CPU-bound work per I/O than -// drive (filesystem, RAID, client multiplexing); past 1:2 the cluster -// saturates the front end under load while drives sit idle. -// -// Cores are evaluated using max(*Cores, 1) to mirror the operator's -// allocator.GetWekaContainerCores() default (zero → 1), so the webhook -// sees the same effective ratio the reconciler will commit to. Skipped -// when either side has zero containers. +// clusterDriveComputeCoreRatio warns when the drive:compute core ratio exceeds the recommended maximum +// for the cluster's mode (globalconfig.Config.CapacityPlanner.{ComputeToTlcDriveCoreRatio, +// FullDrivesComputeToDriveCoreRatio}) — past that, compute-bound work saturates the front end while +// drives sit idle. Cores are resolved via allocator.GetWekaContainerCores(), matching the reconciler, so +// auto-derived drive cores count too — except under clusterCapacity/auto-full-drives, where the planner +// assigns both sides and the template's numbers are not the ones the cluster runs on. Skips cases +// already below clusterComputeDriveCoresFloor's hard 1:1 floor, which owns those exclusively. type clusterDriveComputeCoreRatio struct{} func (clusterDriveComputeCoreRatio) ID() string { @@ -35,49 +36,63 @@ func (clusterDriveComputeCoreRatio) Validate(_ context.Context, _ client.Client, return nil } - driveContainers := cluster.Spec.Dynamic.DriveContainers - computeContainers := cluster.Spec.Dynamic.ComputeContainers + config := cluster.Spec.Dynamic + // Same exclusion as clusterComputeDriveCoresFloor: under clusterCapacity the planner assigns both + // sides, so GetWekaContainerCores' template defaults describe a cluster that never exists. + if allocator.IsPlannerManaged(config) { + return nil + } + + driveContainers := config.DriveContainers + computeContainers := config.ComputeContainers if driveContainers <= 0 || computeContainers <= 0 { return nil } - driveCores := coresOrOne(cluster.Spec.Dynamic.DriveCores) - computeCores := coresOrOne(cluster.Spec.Dynamic.ComputeCores) - driveSide := driveContainers * driveCores - computeSide := computeContainers * computeCores + cores := allocator.GetWekaContainerCores(config) + driveSide := driveContainers * cores.Drive + computeSide := computeContainers * cores.Compute + + // clusterComputeDriveCoresFloor already owns and reports this case exclusively. + if computeSide < driveSide { + return nil + } + + // Auto-full-drives mode is not reachable here: it requires both counts unset, and both being set + // is what makes this function run at all. + exclusiveFullDrives := config.NumDrives > 0 && config.DriveCapacity == 0 + var ratio float64 + if exclusiveFullDrives { + ratio = globalconfig.Config.CapacityPlanner.FullDrivesComputeToDriveCoreRatio + } else { + ratio = globalconfig.Config.CapacityPlanner.ComputeToTlcDriveCoreRatio + } + if ratio <= 0 { + return nil + } - if 2*driveSide <= computeSide { + required := int(math.Ceil(ratio * float64(driveSide))) + if computeSide >= required { return nil } n, m := reduceRatio(driveSide, computeSide) - ratio := fmt.Sprintf("%d:%d", n, m) + actualRatio := fmt.Sprintf("%d:%d", n, m) detail := fmt.Sprintf( - "drive:compute core ratio exceeds the recommended maximum of 1:2 "+ - "(drive containers: %d, compute cores: %d, ratio: %s). "+ - "Adjust driveContainers or compute core count to restore a "+ - "valid ratio.", - driveContainers, computeCores, ratio, + "drive:compute core ratio is below the recommended 1:%g (total drive cores: %d, total compute "+ + "cores: %d, actual ratio: %s). Adjust driveContainers/driveCores or computeContainers/computeCores "+ + "to restore a ratio closer to recommended.", + ratio, driveSide, computeSide, actualRatio, ) return field.ErrorList{ field.Invalid( field.NewPath("spec", "dynamicTemplate"), - ratio, + actualRatio, detail, ), } } -// coresOrOne mirrors util.GetNonZeroOrDefault(_, 1) used by the operator's -// allocator.GetWekaContainerCores() — a 0 spec value becomes 1 at -// reconcile time. -func coresOrOne(cores int) int { - if cores <= 0 { - return 1 - } - return cores -} - // reduceRatio divides both sides by their gcd so the message reads // `1:2` rather than `6:12`. func reduceRatio(a, b int) (reducedA, reducedB int) { diff --git a/internal/validation/cluster_drive_compute_core_ratio_test.go b/internal/validation/cluster_drive_compute_core_ratio_test.go new file mode 100644 index 000000000..0dd7a5511 --- /dev/null +++ b/internal/validation/cluster_drive_compute_core_ratio_test.go @@ -0,0 +1,242 @@ +package validation + +import ( + "context" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + + globalconfig "github.com/weka/weka-operator/internal/config" +) + +func ratioCluster(dynamic *weka.WekaClusterTemplate) *weka.WekaCluster { + c := &weka.WekaCluster{} + c.Spec.Dynamic = dynamic + return c +} + +// setRatioConfig sets the ratio knobs the validator reads (zero by default since LoadCapacityEnv isn't +// called in unit tests) and restores them on cleanup. +func setRatioConfig(t *testing.T, tlcRatio, fullDrivesRatio float64) { + prevTlc := globalconfig.Config.CapacityPlanner.ComputeToTlcDriveCoreRatio + prevFullDrives := globalconfig.Config.CapacityPlanner.FullDrivesComputeToDriveCoreRatio + globalconfig.Config.CapacityPlanner.ComputeToTlcDriveCoreRatio = tlcRatio + globalconfig.Config.CapacityPlanner.FullDrivesComputeToDriveCoreRatio = fullDrivesRatio + t.Cleanup(func() { + globalconfig.Config.CapacityPlanner.ComputeToTlcDriveCoreRatio = prevTlc + globalconfig.Config.CapacityPlanner.FullDrivesComputeToDriveCoreRatio = prevFullDrives + }) +} + +func TestClusterDriveComputeCoreRatio(t *testing.T) { + // Ratio pinned to 2.0, matching the old hardcoded 1:2 behavior this test mirrors. + setRatioConfig(t, 2.0, 2.0) + + v := &clusterDriveComputeCoreRatio{} + ctx := context.Background() + + tests := []struct { + name string + dynamic *weka.WekaClusterTemplate + wantErr bool + }{ + { + name: "no dynamic template skipped", + dynamic: nil, + wantErr: false, + }, + { + name: "zero drive containers skipped", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 0, ComputeContainers: 4, + }, + wantErr: false, + }, + { + name: "explicit cores within recommended ratio passes", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 2, ComputeContainers: 2, + DriveCores: 1, ComputeCores: 4, + }, + wantErr: false, + }, + { + // driveSide=2, computeSide=2: clears the floor but required=ceil(2.0*2)=4 -> flagged. + name: "explicit cores at the 1:1 floor but below recommended ratio flagged", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 2, ComputeContainers: 2, + DriveCores: 1, ComputeCores: 1, + }, + wantErr: true, + }, + { + // Same shape as the flagged case above, but under clusterCapacity the planner assigns both + // sides, so the template's numbers are not the ones the cluster runs on. + name: "clusterCapacity skipped even below the recommended ratio", + dynamic: &weka.WekaClusterTemplate{ + ClusterCapacity: "1PiB", + DriveContainers: 2, ComputeContainers: 2, + DriveCores: 1, ComputeCores: 1, + }, + wantErr: false, + }, + { + name: "unset cores on both sides default to 1:1 per container, within ratio", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 1, ComputeContainers: 2, + }, + wantErr: false, + }, + { + // driveSide=8, computeSide=1: below floor, owned exclusively by cluster_compute_drive_cores_floor. + name: "computeSide below driveSide is owned by cluster_compute_drive_cores_floor, skipped here", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 4, ComputeContainers: 1, + DriveCores: 2, ComputeCores: 1, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + errs := v.Validate(ctx, nil, ratioCluster(tt.dynamic)) + if tt.wantErr && len(errs) == 0 { + t.Errorf("expected an error, got none") + } + if !tt.wantErr && len(errs) != 0 { + t.Errorf("expected no error, got %v", errs) + } + }) + } +} + +// TestClusterDriveComputeCoreRatio_ZeroRatioDisables verifies a ratio of 0 (also the default, since +// LoadCapacityEnv isn't called in unit tests) disables the advisory entirely. +func TestClusterDriveComputeCoreRatio_ZeroRatioDisables(t *testing.T) { + setRatioConfig(t, 0, 0) + + v := &clusterDriveComputeCoreRatio{} + ctx := context.Background() + + dynamic := &weka.WekaClusterTemplate{ + DriveContainers: 1, ComputeContainers: 1, + DriveCores: 1, ComputeCores: 1, ContainerCapacity: 1000, + } + errs := v.Validate(ctx, nil, ratioCluster(dynamic)) + if len(errs) != 0 { + t.Errorf("expected no error with ratio disabled, got %v", errs) + } +} + +// TestClusterDriveComputeCoreRatio_FullDrivesUsesFullDrivesRatio verifies exclusive full-drives mode +// (numDrives>0 && driveCapacity==0) reads FullDrivesComputeToDriveCoreRatio instead of +// ComputeToTlcDriveCoreRatio. Auto-full-drives mode cannot reach this validator at all: it requires +// both container counts unset, and both being set is the precondition for running. +func TestClusterDriveComputeCoreRatio_FullDrivesUsesFullDrivesRatio(t *testing.T) { + // tlc ratio 1.0 and full-drives ratio 2.0 disagree on 2 drive / 2 compute cores, so which applies is observable. + setRatioConfig(t, 1.0, 2.0) + + v := &clusterDriveComputeCoreRatio{} + ctx := context.Background() + + tests := []struct { + name string + dynamic *weka.WekaClusterTemplate + wantErr bool + }{ + { + name: "exclusive full-drives (numDrives>0, driveCapacity==0) uses full-drives ratio, flagged", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 2, ComputeContainers: 2, + DriveCores: 1, ComputeCores: 1, NumDrives: 4, DriveCapacity: 0, + }, + wantErr: true, + }, + { + name: "numDrives+driveCapacity>0 is drive-sharing, not exclusive full-drives, uses tlc ratio", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 2, ComputeContainers: 2, + DriveCores: 1, ComputeCores: 1, NumDrives: 4, DriveCapacity: 2000, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + errs := v.Validate(ctx, nil, ratioCluster(tt.dynamic)) + if tt.wantErr && len(errs) == 0 { + t.Errorf("expected an error, got none") + } + if !tt.wantErr && len(errs) != 0 { + t.Errorf("expected no error, got %v", errs) + } + }) + } +} + +// TestClusterDriveComputeCoreRatio_AutoDerivedCores verifies an auto-derived drive-core count (e.g. from +// containerCapacity) is reflected in the ratio check too, not just the user-set spec value. +func TestClusterDriveComputeCoreRatio_AutoDerivedCores(t *testing.T) { + setRatioConfig(t, 1.0, 2.0) + + // Per-core capacity caps set deterministically; mirrors internal/controllers/allocator/templates_test.go. + prevTlc := globalconfig.Config.ClusterCapacity.TlcCapacityPerCoreGiB + prevQlc := globalconfig.Config.ClusterCapacity.QlcCapacityPerCoreGiB + globalconfig.Config.ClusterCapacity.TlcCapacityPerCoreGiB = 5 * 1024 // 5120 GiB/core + globalconfig.Config.ClusterCapacity.QlcCapacityPerCoreGiB = 50 * 1024 // 51200 GiB/core + t.Cleanup(func() { + globalconfig.Config.ClusterCapacity.TlcCapacityPerCoreGiB = prevTlc + globalconfig.Config.ClusterCapacity.QlcCapacityPerCoreGiB = prevQlc + }) + + v := &clusterDriveComputeCoreRatio{} + ctx := context.Background() + + tests := []struct { + name string + dynamic *weka.WekaClusterTemplate + wantErr bool + }{ + { + // 6000 GiB needs ceil(6000/5120)=2 drive cores; driveSide=2, computeSide=3 >= required=2. + name: "containerCapacity forces 2 drive cores, still within 1:1 ratio", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 1, ComputeContainers: 3, + ContainerCapacity: 6000, + }, + wantErr: false, + }, + { + // driveSide=2, computeSide=2: exactly the 1:1 floor, and ratio required is also 2. + name: "containerCapacity forces 2 drive cores, computeSide equals driveSide", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 1, ComputeContainers: 2, + ContainerCapacity: 6000, + }, + wantErr: false, + }, + { + // 5000 <= 5120 derives to 1 core. + name: "containerCapacity within one core's worth unaffected", + dynamic: &weka.WekaClusterTemplate{ + DriveContainers: 1, ComputeContainers: 3, + ContainerCapacity: 5000, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + errs := v.Validate(ctx, nil, ratioCluster(tt.dynamic)) + if tt.wantErr && len(errs) == 0 { + t.Errorf("expected an error, got none") + } + if !tt.wantErr && len(errs) != 0 { + t.Errorf("expected no error, got %v", errs) + } + }) + } +} diff --git a/internal/validation/cluster_drive_cores_below_capacity.go b/internal/validation/cluster_drive_cores_below_capacity.go new file mode 100644 index 000000000..e9a4a7f04 --- /dev/null +++ b/internal/validation/cluster_drive_cores_below_capacity.go @@ -0,0 +1,67 @@ +package validation + +import ( + "context" + "fmt" + + wekav1alpha1 "github.com/weka/weka-k8s-api/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/weka/weka-operator/internal/controllers/allocator" +) + +// clusterDriveCoresBelowCapacity warns when an explicit driveCores is below what the configured drive +// capacity (containerCapacity, or numDrives+driveCapacity) requires — getDriveCores no longer clamps it +// up, so this surfaces the shortfall at admission instead of failing later via +// DriveCapacityResourceShortfall on add-drive. Warn-only. +// +// Silent in auto-full-drives mode by construction: DerivedDriveCores has no capacity basis there (no +// containerCapacity, and numDrives without driveCapacity is a drive COUNT, not capacity), so it +// returns ok=false and this returns nil. That is deliberate — {numDrives: 4, driveCores: 3} is a +// blessed configuration in that mode (all four drives claimed, run on three cores), not a shortfall. +// See clusterAutoFullDrivesPinExceedsNodeDrives for the pins that ARE checked there. +type clusterDriveCoresBelowCapacity struct{} + +func (clusterDriveCoresBelowCapacity) ID() string { + return "cluster_drive_cores_below_capacity" +} + +func (clusterDriveCoresBelowCapacity) Validate(_ context.Context, _ client.Client, obj runtime.Object) field.ErrorList { + cluster, ok := obj.(*wekav1alpha1.WekaCluster) + if !ok { + return nil + } + if cluster.Spec.Dynamic == nil { + return nil + } + + config := cluster.Spec.Dynamic + if config.DriveCores <= 0 { + return nil + } + + derived, ok := allocator.DerivedDriveCores(config) + if !ok { + return nil + } + if config.DriveCores >= derived { + return nil + } + + detail := fmt.Sprintf( + "spec.dynamicTemplate.driveCores (%d) is below the %d core(s) that the configured drive "+ + "capacity requires. The operator will honor the explicit value as set, but drive adds "+ + "will be deferred with DriveCapacityResourceShortfall until driveCores is raised to at "+ + "least %d or the configured capacity is reduced.", + config.DriveCores, derived, derived, + ) + return field.ErrorList{ + field.Invalid( + field.NewPath("spec", "dynamicTemplate", "driveCores"), + config.DriveCores, + detail, + ), + } +} diff --git a/internal/validation/cluster_drive_cores_below_capacity_test.go b/internal/validation/cluster_drive_cores_below_capacity_test.go new file mode 100644 index 000000000..2aeddb78d --- /dev/null +++ b/internal/validation/cluster_drive_cores_below_capacity_test.go @@ -0,0 +1,115 @@ +package validation + +import ( + "context" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + + globalconfig "github.com/weka/weka-operator/internal/config" +) + +func TestClusterDriveCoresBelowCapacity(t *testing.T) { + // Per-core capacity caps set deterministically; mirrors internal/controllers/allocator/templates_test.go. + prevTlc := globalconfig.Config.ClusterCapacity.TlcCapacityPerCoreGiB + prevQlc := globalconfig.Config.ClusterCapacity.QlcCapacityPerCoreGiB + globalconfig.Config.ClusterCapacity.TlcCapacityPerCoreGiB = 5 * 1024 // 5120 GiB/core + globalconfig.Config.ClusterCapacity.QlcCapacityPerCoreGiB = 50 * 1024 // 51200 GiB/core + t.Cleanup(func() { + globalconfig.Config.ClusterCapacity.TlcCapacityPerCoreGiB = prevTlc + globalconfig.Config.ClusterCapacity.QlcCapacityPerCoreGiB = prevQlc + }) + + v := &clusterDriveCoresBelowCapacity{} + ctx := context.Background() + + tests := []struct { + name string + dynamic *weka.WekaClusterTemplate + wantErr bool + }{ + { + name: "no dynamic template skipped", + dynamic: nil, + wantErr: false, + }, + { + name: "driveCores unset skipped", + dynamic: &weka.WekaClusterTemplate{ + ContainerCapacity: 6000, // would derive to 2 cores + }, + wantErr: false, + }, + { + name: "containerCapacity mode: explicit below derived warns", + dynamic: &weka.WekaClusterTemplate{ + ContainerCapacity: 6000, // ceil(6000/5120)=2 + DriveCores: 1, + }, + wantErr: true, + }, + { + name: "containerCapacity mode: explicit equal to derived no warning", + dynamic: &weka.WekaClusterTemplate{ + ContainerCapacity: 6000, // ceil(6000/5120)=2 + DriveCores: 2, + }, + wantErr: false, + }, + { + name: "containerCapacity mode: explicit above derived no warning", + dynamic: &weka.WekaClusterTemplate{ + ContainerCapacity: 6000, // ceil(6000/5120)=2 + DriveCores: 3, + }, + wantErr: false, + }, + { + name: "numDrives+driveCapacity mode: explicit below derived warns", + dynamic: &weka.WekaClusterTemplate{ + NumDrives: 4, + DriveCapacity: 2000, // 8000 GiB, ceil(8000/5120)=2 + DriveCores: 1, + }, + wantErr: true, + }, + { + name: "numDrives+driveCapacity mode: explicit equal to derived no warning", + dynamic: &weka.WekaClusterTemplate{ + NumDrives: 4, + DriveCapacity: 2000, // 8000 GiB, ceil(8000/5120)=2 + DriveCores: 2, + }, + wantErr: false, + }, + { + name: "numDrives+driveCapacity mode: explicit above derived no warning", + dynamic: &weka.WekaClusterTemplate{ + NumDrives: 4, + DriveCapacity: 2000, // 8000 GiB, ceil(8000/5120)=2 + DriveCores: 3, + }, + wantErr: false, + }, + { + name: "no capacity basis to derive from: no warning regardless of driveCores", + dynamic: &weka.WekaClusterTemplate{ + NumDrives: 4, // pure full-drives mode, no driveCapacity + DriveCores: 1, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + errs := v.Validate(ctx, nil, ratioCluster(tt.dynamic)) + if tt.wantErr && len(errs) == 0 { + t.Errorf("expected an error, got none") + } + if !tt.wantErr && len(errs) != 0 { + t.Errorf("expected no error, got %v", errs) + } + }) + } +} diff --git a/internal/validation/cluster_drivers_dist_service_exists.go b/internal/validation/cluster_drivers_dist_service_exists.go index 72b6f7e3a..05666395a 100644 --- a/internal/validation/cluster_drivers_dist_service_exists.go +++ b/internal/validation/cluster_drivers_dist_service_exists.go @@ -14,15 +14,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// clusterDriversDistServiceExists verifies that -// spec.driversDistService — when set to an in-cluster Kubernetes Service -// URL — points to a Service that actually exists. Hostnames are -// classified as in-cluster when they either contain a `svc` segment -// (e.g. `name.ns.svc(.cluster.local)`) or are a single label -// (`weka-driver`, treated as a Service in the WekaCluster's namespace). -// Empty values (operator auto-resolves via WekaPolicy) and other hosts -// (multi-segment names without a `svc` segment) are skipped silently. -// Malformed URLs always fail. +// clusterDriversDistServiceExists verifies that spec.driversDistService, when set to an in-cluster +// Service URL, points to a Service that actually exists. A hostname is in-cluster if it has a `svc` +// segment (`name.ns.svc(.cluster.local)`) or is a single label (e.g. `weka-driver`, the WekaCluster's +// namespace). Empty values (auto-resolved via WekaPolicy) and other hosts are skipped silently; +// malformed URLs always fail. type clusterDriversDistServiceExists struct{} func (clusterDriversDistServiceExists) ID() string { diff --git a/internal/validation/cluster_drives_unsigned_advisory.go b/internal/validation/cluster_drives_unsigned_advisory.go new file mode 100644 index 000000000..34bd3329d --- /dev/null +++ b/internal/validation/cluster_drives_unsigned_advisory.go @@ -0,0 +1,122 @@ +package validation + +import ( + "context" + "fmt" + "sort" + "strings" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/weka/weka-operator/internal/consts" +) + +// clusterDrivesUnsignedAdvisory warns when no node matching the drive-role nodeSelector carries the +// drive annotation this cluster's mode consumes (shared-drives vs full-drives are disjoint, so a node +// signed the other way gets a distinct re-signing message). Without this, unsigned nodes silently +// bypass clusterSignedDrives and the auto-full-drives projection, admitting a misconfigured apply +// unnoticed. Warn-only. +type clusterDrivesUnsignedAdvisory struct{} + +func (clusterDrivesUnsignedAdvisory) ID() string { + return "cluster_drives_unsigned_advisory" +} + +func (clusterDrivesUnsignedAdvisory) Validate(ctx context.Context, c client.Client, obj runtime.Object) field.ErrorList { + cluster, ok := obj.(*weka.WekaCluster) + if !ok { + return nil + } + // A nil dynamicTemplate is no longer out of scope: it is auto-full-drives mode (one drive container + // per eligible node), which needs signed drives just as much as an explicit template does. + // clusterMinDrivesFeasibility already rejects auto-full-drives + positive minNumDrives with zero + // signed drives; staying silent there avoids double-reporting the same condition. + if cluster.Spec.Dynamic.UsesAutoFullDrives() && cluster.Spec.GetStartIoConditions().MinNumDrives > 0 { + return nil + } + + selector := cluster.GetNodeSelectorForRole(weka.WekaContainerModeDrive) + nodes, errs := listDriveRoleNodes(ctx, c, cluster, field.NewPath("spec", "nodeSelector")) + if errs != nil { + return errs + } + if len(nodes) == 0 { + // clusterSelectedNodesCount owns "the selector matches nothing". + return nil + } + + // Which annotation matters is a property of the cluster, not of what happens to be on the nodes. + wantAnn, wantMode := consts.AnnotationWekaFullDrives, "full-drives" + otherAnn, otherMode := consts.AnnotationSharedDrives, "drive-sharing" + if cluster.IsDriveSharing() { + wantAnn, wantMode, otherAnn, otherMode = otherAnn, otherMode, wantAnn, wantMode + } + + otherModeNodes := 0 + for i := range nodes { + ann := nodes[i].Annotations + if _, ok := ann[wantAnn]; ok { + return nil + } + if _, ok := ann[otherAnn]; ok { + otherModeNodes++ + } + } + + detail := fmt.Sprintf( + "none of the %d node(s) matching the drive-role nodeSelector (%s) has drives signed for this "+ + "cluster's %s mode — no %s annotation on %s. Drive containers cannot claim a drive until "+ + "sign-drives runs there, and the drive-count and capacity checks are skipped in the "+ + "meantime, so a misconfigured spec would be admitted unnoticed. Sign drives on the "+ + "matched nodes in %s mode.", + len(nodes), formatSelector(selector), wantMode, wantAnn, + formatNodeNames(nodes), wantMode, + ) + if otherModeNodes > 0 { + detail = fmt.Sprintf( + "%d of the %d node(s) matching the drive-role nodeSelector (%s) are signed in %s mode "+ + "(%s), but this cluster is %s mode and consumes %s — the two are disjoint, so those "+ + "drives are unusable here and no drive container will be able to claim one. "+ + "Re-sign the matched nodes (%s) in %s mode, or change the cluster's sizing to match "+ + "how the nodes are signed.", + otherModeNodes, len(nodes), formatSelector(selector), otherMode, otherAnn, + wantMode, wantAnn, formatNodeNames(nodes), wantMode, + ) + } + return field.ErrorList{ + field.Invalid(field.NewPath("spec", "nodeSelector"), formatSelector(selector), detail), + } +} + +// formatSelector renders a label selector deterministically as "k=v,k=v" for message text. +func formatSelector(selector map[string]string) string { + if len(selector) == 0 { + return "" + } + parts := make([]string, 0, len(selector)) + for k, v := range selector { + parts = append(parts, fmt.Sprintf("%s=%s", k, v)) + } + sort.Strings(parts) + return strings.Join(parts, ",") +} + +// formatNodeNames lists up to three node names, so the warning names something the user can act on +// without pasting an entire fleet into an admission response. +func formatNodeNames(nodes []corev1.Node) string { + const maxNamed = 3 + names := make([]string, 0, len(nodes)) + for i := range nodes { + names = append(names, nodes[i].Name) + } + // Sort before truncating so the named subset is stable across List orderings. + sort.Strings(names) + if len(names) > maxNamed { + return fmt.Sprintf("%s and %d more", strings.Join(names[:maxNamed], ", "), len(names)-maxNamed) + } + return strings.Join(names, ", ") +} diff --git a/internal/validation/cluster_drives_unsigned_advisory_test.go b/internal/validation/cluster_drives_unsigned_advisory_test.go new file mode 100644 index 000000000..0d4d95bd7 --- /dev/null +++ b/internal/validation/cluster_drives_unsigned_advisory_test.go @@ -0,0 +1,209 @@ +package validation + +import ( + "context" + "errors" + "strings" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + "github.com/weka/weka-operator/internal/consts" +) + +// sharedDriveRoleNode builds a proxy-mode node: matched by the drive-role selector and signed via +// weka-shared-drives rather than weka-full-drives. +func sharedDriveRoleNode(name string, labels map[string]string) *corev1.Node { + return &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: labels, + Annotations: map[string]string{ + consts.AnnotationSharedDrives: `[["550e8400-e29b-41d4-a716-446655440000","S1",7000,"/dev/nvme0n1"]]`, + }, + }, + } +} + +func TestClusterDrivesUnsignedAdvisory(t *testing.T) { + v := &clusterDrivesUnsignedAdvisory{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + withSelector := func(dynamic *weka.WekaClusterTemplate, minNumDrives int) *weka.WekaCluster { + c := minDrivesCluster(dynamic, minNumDrives) + c.Spec.NodeSelector = labels + return c + } + sized := &weka.WekaClusterTemplate{DriveContainers: 2, NumDrives: 4} + autoFullDrives := &weka.WekaClusterTemplate{} + // Capacity-based sizing is what makes IsDriveSharing() true. + sharing := &weka.WekaClusterTemplate{ClusterCapacity: "100TiB"} + + t.Run("all matched nodes unsigned warns", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, nil), + driveRoleNode(t, "n2", labels, nil), + ) + errs := v.Validate(ctx, c, withSelector(sized, 0)) + if len(errs) != 1 { + t.Fatalf("expected 1 advisory, got %v", errs) + } + if d := errs[0].Detail; !strings.Contains(d, "role=drive") || !strings.Contains(d, "n1, n2") { + t.Errorf("expected selector and node names in the message, got %q", d) + } + }) + + // The whole point of generalizing: auto-full-drives clusters have no drive count to check, so + // nothing else would say anything here. + t.Run("auto-full-drives without minNumDrives warns", func(t *testing.T) { + c := fakeClientWithNodes(t, driveRoleNode(t, "n1", labels, nil)) + if errs := v.Validate(ctx, c, withSelector(autoFullDrives, 0)); len(errs) != 1 { + t.Fatalf("expected 1 advisory, got %v", errs) + } + }) + + // A nil dynamicTemplate is the default shape of auto-full-drives mode, not an out-of-scope + // cluster: it needs signed drives just as much as an explicit template does. + t.Run("nil dynamicTemplate warns", func(t *testing.T) { + c := fakeClientWithNodes(t, driveRoleNode(t, "n1", labels, nil)) + if errs := v.Validate(ctx, c, withSelector(nil, 0)); len(errs) != 1 { + t.Fatalf("expected 1 advisory for a nil template, got %v", errs) + } + }) + + // clusterMinDrivesFeasibility rejects this exact state; warning too would double-report it. + t.Run("auto-full-drives with minNumDrives defers to the feasibility error", func(t *testing.T) { + c := fakeClientWithNodes(t, driveRoleNode(t, "n1", labels, nil)) + cluster := withSelector(autoFullDrives, 10) + if errs := v.Validate(ctx, c, cluster); len(errs) != 0 { + t.Errorf("expected no advisory, got %v", errs) + } + // Guard the assumption the suppression rests on: something else does report it. + if errs := (&clusterMinDrivesFeasibility{}).Validate(ctx, c, cluster); len(errs) == 0 { + t.Errorf("suppressed the advisory but clusterMinDrivesFeasibility stayed silent too") + } + }) + + t.Run("one full-signed node silences", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, []int{1000}), + driveRoleNode(t, "n2", labels, nil), + ) + if errs := v.Validate(ctx, c, withSelector(sized, 0)); len(errs) != 0 { + t.Errorf("expected no advisory, got %v", errs) + } + }) + + // Mode-awareness: shared-drives signing does not satisfy a full-drives cluster. The populations + // are disjoint, so this is a mode mismatch, not a signed node. + t.Run("shared-signed nodes do not satisfy a full-drives cluster", func(t *testing.T) { + c := fakeClientWithNodes(t, + sharedDriveRoleNode("n1", labels), + driveRoleNode(t, "n2", labels, nil), + ) + errs := v.Validate(ctx, c, withSelector(sized, 0)) + if len(errs) != 1 { + t.Fatalf("expected 1 advisory, got %v", errs) + } + if d := errs[0].Detail; !strings.Contains(d, "1 of the 2") || !strings.Contains(d, "Re-sign") { + t.Errorf("expected the mode-mismatch message, got %q", d) + } + }) + + t.Run("drive-sharing cluster is satisfied by shared-signed nodes", func(t *testing.T) { + c := fakeClientWithNodes(t, + sharedDriveRoleNode("n1", labels), + driveRoleNode(t, "n2", labels, nil), + ) + if errs := v.Validate(ctx, c, withSelector(sharing, 0)); len(errs) != 0 { + t.Errorf("expected no advisory, got %v", errs) + } + }) + + t.Run("full-signed nodes do not satisfy a drive-sharing cluster", func(t *testing.T) { + c := fakeClientWithNodes(t, driveRoleNode(t, "n1", labels, []int{1000})) + errs := v.Validate(ctx, c, withSelector(sharing, 0)) + if len(errs) != 1 { + t.Fatalf("expected 1 advisory, got %v", errs) + } + d := errs[0].Detail + if !strings.Contains(d, consts.AnnotationSharedDrives) { + t.Errorf("expected the shared-drives annotation to be named as required, got %q", d) + } + if !strings.Contains(d, "full-drives mode") { + t.Errorf("expected the node's actual mode to be named, got %q", d) + } + }) + + t.Run("drive-sharing cluster with fully unsigned nodes warns", func(t *testing.T) { + c := fakeClientWithNodes(t, driveRoleNode(t, "n1", labels, nil)) + errs := v.Validate(ctx, c, withSelector(sharing, 0)) + if len(errs) != 1 { + t.Fatalf("expected 1 advisory, got %v", errs) + } + if d := errs[0].Detail; strings.Contains(d, "Re-sign") { + t.Errorf("expected the not-signed message, not the mismatch one: %q", d) + } + }) + + t.Run("unmatched unsigned node ignored", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, []int{1000}), + driveRoleNode(t, "other", map[string]string{"role": "compute"}, nil), + ) + if errs := v.Validate(ctx, c, withSelector(sized, 0)); len(errs) != 0 { + t.Errorf("expected no advisory, got %v", errs) + } + }) + + t.Run("no matched nodes skipped", func(t *testing.T) { + if errs := v.Validate(ctx, fakeClientWithNodes(t), withSelector(sized, 0)); len(errs) != 0 { + t.Errorf("expected no advisory, got %v", errs) + } + }) + + t.Run("node List failure surfaces as an internal error, not silently admitted", func(t *testing.T) { + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme: %v", err) + } + listErr := errors.New("boom") + c := fake.NewClientBuilder().WithScheme(scheme).WithInterceptorFuncs(interceptor.Funcs{ + List: func(ctx context.Context, cl client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + return listErr + }, + }).Build() + errs := v.Validate(ctx, c, withSelector(sized, 0)) + if len(errs) != 1 { + t.Fatalf("expected the List failure to surface as one error, got %v", errs) + } + if errs[0].Type != field.ErrorTypeInternal { + t.Errorf("expected an InternalError, got %v", errs[0].Type) + } + }) + + t.Run("many unsigned nodes truncates the name list", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, nil), + driveRoleNode(t, "n2", labels, nil), + driveRoleNode(t, "n3", labels, nil), + driveRoleNode(t, "n4", labels, nil), + driveRoleNode(t, "n5", labels, nil), + ) + errs := v.Validate(ctx, c, withSelector(sized, 0)) + if len(errs) != 1 { + t.Fatalf("expected 1 advisory, got %v", errs) + } + if d := errs[0].Detail; !strings.Contains(d, "n1, n2, n3 and 2 more") { + t.Errorf("expected a truncated node list, got %q", d) + } + }) +} diff --git a/internal/validation/cluster_hugepages_available.go b/internal/validation/cluster_hugepages_available.go index 07c8627ae..aa3eaf5e2 100644 --- a/internal/validation/cluster_hugepages_available.go +++ b/internal/validation/cluster_hugepages_available.go @@ -16,11 +16,9 @@ const mib = int64(1) << 20 // clusterHugepagesAvailable warns per-role on capacity and single-fit // failures, mirroring clusterCoresAvailable but for hugepages-2Mi. // -// Unit gap: *Hugepages fields are MiB (pod.go formats them as "%dMi"); -// Allocatable[hugepages-2Mi].Value() is bytes. Multiply MiB × mib to -// compare. Skipped when *Hugepages is 0 (operator-derived from drive -// capacity). Role mapping isn't 1:1 with cores: s3/nfs/smbw use the -// *Frontend* fields. +// Unit gap: *Hugepages fields are MiB (pod.go formats them "%dMi"); Allocatable[hugepages-2Mi] is +// bytes — multiply MiB × mib to compare. Skipped when *Hugepages is 0 (operator-derived from drive +// capacity). Role mapping isn't 1:1 with cores: s3/nfs/smbw use the *Frontend* fields. type clusterHugepagesAvailable struct{} func (clusterHugepagesAvailable) ID() string { diff --git a/internal/validation/cluster_min_containers.go b/internal/validation/cluster_min_containers.go new file mode 100644 index 000000000..5ab2947fe --- /dev/null +++ b/internal/validation/cluster_min_containers.go @@ -0,0 +1,86 @@ +package validation + +import ( + "context" + "fmt" + + wekav1alpha1 "github.com/weka/weka-k8s-api/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + globalconfig "github.com/weka/weka-operator/internal/config" +) + +// clusterMinContainers rejects a pinned driveContainers/computeContainers below the minimum weka needs to +// form a cluster at all (FormClusterMinDriveContainers/FormClusterMinComputeContainers, 5 by default, 3 +// with ALLOW_SINGLE_PARITY). This is an Error in both modes: below the minimum, FormCluster refuses to +// proceed and the cluster loops on MinContainersNotReady forever with its containers healthy but idle — +// a plan the planner happily accepts, e.g. clusterCapacity alongside a single pinned count of 3. +// +// Only explicit pins are checked, which is also why auto-full-drives mode never reaches the body: both +// counts are 0 there by definition (setting either one is what leaves the mode), so the "unset" skip +// fires first. An auto-full-drives cluster whose nodeSelector matches too few nodes stalls the same +// way; that is caught at runtime instead. +type clusterMinContainers struct{} + +func (clusterMinContainers) ID() string { + return "cluster_min_containers" +} + +func (clusterMinContainers) Validate(_ context.Context, _ client.Client, obj runtime.Object) field.ErrorList { + cluster, ok := obj.(*wekav1alpha1.WekaCluster) + if !ok { + return nil + } + if cluster.Spec.Dynamic == nil { + return nil + } + + config := cluster.Spec.Dynamic + + checks := []struct { + field string + count int + min int + // role is the wording used in the remedy, and differs from field for readability. + role string + }{ + {"driveContainers", config.DriveContainers, globalconfig.Consts.FormClusterMinDriveContainers, "drive"}, + {"computeContainers", config.ComputeContainers, globalconfig.Consts.FormClusterMinComputeContainers, "compute"}, + } + + var out field.ErrorList + for _, c := range checks { + if c.min <= 0 { // minimum disabled by configuration — nothing to enforce + continue + } + if c.count <= 0 { // unset: derived elsewhere (see type doc) + continue + } + if c.count >= c.min { + continue + } + remedy := fmt.Sprintf("raise %s to at least %d", c.field, c.min) + detail := fmt.Sprintf( + "spec.dynamicTemplate.%s (%d) is below the %d %s container(s) weka needs to form a cluster — "+ + "the cluster would never be created, it would wait forever on MinContainersNotReady with its "+ + "containers running but idle. %s. The minimum is configurable via FORM_CLUSTER_MIN_%s_CONTAINERS.", + c.field, c.count, c.min, c.role, remedy, upperRole(c.role), + ) + out = append(out, field.Invalid(field.NewPath("spec", "dynamicTemplate", c.field), c.count, detail)) + } + return out +} + +// upperRole renders a role name for the env-var hint in the message ("drive" -> "DRIVE"). +func upperRole(role string) string { + switch role { + case "drive": + return "DRIVE" + case "compute": + return "COMPUTE" + default: + return role + } +} diff --git a/internal/validation/cluster_min_containers_test.go b/internal/validation/cluster_min_containers_test.go new file mode 100644 index 000000000..9030b3cdb --- /dev/null +++ b/internal/validation/cluster_min_containers_test.go @@ -0,0 +1,151 @@ +package validation + +import ( + "context" + "strings" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + + globalconfig "github.com/weka/weka-operator/internal/config" +) + +// withFormClusterMinContainers sets the two minimums the validator reads and restores them after the test. +func withFormClusterMinContainers(t *testing.T, drive, compute int) { + t.Helper() + prevD := globalconfig.Consts.FormClusterMinDriveContainers + prevC := globalconfig.Consts.FormClusterMinComputeContainers + globalconfig.Consts.FormClusterMinDriveContainers = drive + globalconfig.Consts.FormClusterMinComputeContainers = compute + t.Cleanup(func() { + globalconfig.Consts.FormClusterMinDriveContainers = prevD + globalconfig.Consts.FormClusterMinComputeContainers = prevC + }) +} + +// TestClusterMinContainers covers pinned counts below the form-cluster minimum: a feasible plan whose +// pods run, yet the cluster never forms. +func TestClusterMinContainers(t *testing.T) { + v := &clusterMinContainers{} + ctx := context.Background() + + tests := []struct { + name string + minDrive int + minCompute int + dynamic *weka.WekaClusterTemplate + wantN int + wantSubs []string + wantNotSubs []string + }{ + { + name: "no dynamic template skipped", + minDrive: 5, + minCompute: 5, + dynamic: nil, + }, + { + // Unset counts are derived elsewhere and default to these minimums, so admission must not object. + name: "both unset skipped", + minDrive: 5, + minCompute: 5, + dynamic: &weka.WekaClusterTemplate{}, + }, + { + name: "at the minimum is allowed", + minDrive: 5, + minCompute: 5, + dynamic: &weka.WekaClusterTemplate{DriveContainers: 5, ComputeContainers: 5}, + }, + { + name: "above the minimum is allowed", + minDrive: 5, + minCompute: 5, + dynamic: &weka.WekaClusterTemplate{DriveContainers: 8, ComputeContainers: 6}, + }, + { + // clusterCapacity alongside a single pinned count: the planner accepts it, the pods run, + // the cluster never forms. + name: "driveContainers below the minimum is rejected", + minDrive: 5, + minCompute: 5, + dynamic: &weka.WekaClusterTemplate{ClusterCapacity: "500TiB", DriveContainers: 3}, + wantN: 1, + wantSubs: []string{ + "driveContainers", "(3)", "below the 5 drive container(s)", "MinContainersNotReady", + "raise driveContainers to at least 5", "FORM_CLUSTER_MIN_DRIVE_CONTAINERS", + }, + }, + { + name: "computeContainers below the minimum is rejected", + minDrive: 5, + minCompute: 5, + dynamic: &weka.WekaClusterTemplate{ComputeContainers: 2}, + wantN: 1, + wantSubs: []string{"computeContainers", "(2)", "below the 5 compute container(s)", "FORM_CLUSTER_MIN_COMPUTE_CONTAINERS"}, + // The auto-full-drives remedies are gone: they were unreachable (both counts are 0 in that + // mode, so the "unset" skip fires first) and their wording was wrong. + wantNotSubs: []string{"caps how many eligible nodes are used", "remove the pin"}, + }, + { + name: "both below the minimum reported separately", + minDrive: 5, + minCompute: 5, + dynamic: &weka.WekaClusterTemplate{DriveContainers: 1, ComputeContainers: 2}, + wantN: 2, + }, + { + // ALLOW_SINGLE_PARITY lowers the minimum to 3; the validator tracks config, not a hard-coded 5. + name: "honors a lowered minimum (single parity)", + minDrive: 3, + minCompute: 3, + dynamic: &weka.WekaClusterTemplate{DriveContainers: 3, ComputeContainers: 3}, + }, + { + name: "honors a raised minimum", + minDrive: 7, + minCompute: 7, + dynamic: &weka.WekaClusterTemplate{DriveContainers: 5}, + wantN: 1, + wantSubs: []string{"below the 7 drive container(s)"}, + }, + { + // A non-positive minimum switches the check off; admission must agree, not substitute a default. + name: "minimum of zero disables the check", + minDrive: 0, + minCompute: 0, + dynamic: &weka.WekaClusterTemplate{DriveContainers: 1, ComputeContainers: 1}, + }, + { + // The two roles are independent: a drive minimum of 0 must not suppress the compute violation. + name: "roles are independent", + minDrive: 0, + minCompute: 5, + dynamic: &weka.WekaClusterTemplate{DriveContainers: 1, ComputeContainers: 1}, + wantN: 1, + wantSubs: []string{"computeContainers"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + withFormClusterMinContainers(t, tt.minDrive, tt.minCompute) + cluster := &weka.WekaCluster{Spec: weka.WekaClusterSpec{Dynamic: tt.dynamic}} + + errs := v.Validate(ctx, nil, cluster) + if len(errs) != tt.wantN { + t.Fatalf("got %d violation(s), want %d: %v", len(errs), tt.wantN, errs) + } + for _, sub := range tt.wantSubs { + if !strings.Contains(errs[0].Detail, sub) { + t.Errorf("detail missing %q, got: %s", sub, errs[0].Detail) + } + } + for _, sub := range tt.wantNotSubs { + if strings.Contains(errs[0].Detail, sub) { + t.Errorf("detail must not contain %q, got: %s", sub, errs[0].Detail) + } + } + }) + } +} diff --git a/internal/validation/cluster_min_drives_feasibility.go b/internal/validation/cluster_min_drives_feasibility.go index 14831e552..c8daf90f9 100644 --- a/internal/validation/cluster_min_drives_feasibility.go +++ b/internal/validation/cluster_min_drives_feasibility.go @@ -4,37 +4,47 @@ import ( "context" "fmt" - wekav1alpha1 "github.com/weka/weka-k8s-api/api/v1alpha1" + weka "github.com/weka/weka-k8s-api/api/v1alpha1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/weka/weka-operator/internal/consts" ) -// clusterMinDrivesFeasibility rejects WekaClusters whose -// spec.startIoConditions.minNumDrives exceeds driveContainers × numDrives. -// The IO-start condition would never be satisfied — WaitForDrivesAdd() -// polls forever. Skipped when driveContainers or numDrives is 0 -// (operator-derived; webhook can't predict the eventual total). +// clusterMinDrivesFeasibility rejects minNumDrives exceeding the total drive count the cluster can +// ever reach — WaitForDrivesAdd() would poll forever. Total is driveContainers×numDrives, or in +// auto-full-drives mode, the signed non-blocked full drives each drive-role-matched node contributes +// (capped per node by a numDrives pin). Unlike clusterSignedDrives, auto-full-drives has no bootstrap +// skip: zero signed drives is rejected as a real infeasibility. type clusterMinDrivesFeasibility struct{} func (clusterMinDrivesFeasibility) ID() string { return "cluster_min_drives_feasibility" } -func (clusterMinDrivesFeasibility) Validate(_ context.Context, _ client.Client, obj runtime.Object) field.ErrorList { - cluster, ok := obj.(*wekav1alpha1.WekaCluster) +func (clusterMinDrivesFeasibility) Validate(ctx context.Context, c client.Client, obj runtime.Object) field.ErrorList { + cluster, ok := obj.(*weka.WekaCluster) if !ok { return nil } - if cluster.Spec.Dynamic == nil { - return nil - } minNumDrives := cluster.Spec.GetStartIoConditions().MinNumDrives if minNumDrives <= 0 { return nil } + fldPath := field.NewPath("spec", "startIoConditions", "minNumDrives") + + // Checked before the nil guard below: a nil dynamicTemplate IS auto-full-drives mode, so it takes + // the per-node branch rather than falling through as "nothing configured". + if cluster.Spec.Dynamic.UsesAutoFullDrives() { + return validateMinDrivesAutoFullDrives(ctx, c, cluster, minNumDrives, fldPath) + } + if cluster.Spec.Dynamic == nil { + return nil + } + driveContainers := cluster.Spec.Dynamic.DriveContainers numDrives := cluster.Spec.Dynamic.NumDrives if driveContainers <= 0 || numDrives <= 0 { @@ -52,10 +62,69 @@ func (clusterMinDrivesFeasibility) Validate(_ context.Context, _ client.Client, minNumDrives, driveContainers, numDrives, total, ) return field.ErrorList{ - field.Invalid( - field.NewPath("spec", "startIoConditions", "minNumDrives"), - minNumDrives, - detail, - ), + field.Invalid(fldPath, minNumDrives, detail), + } +} + +// validateMinDrivesAutoFullDrives totals signed, non-blocked full drives (AvailableDrives only — +// full-drives mode never picks up SharedDrives, a disjoint drive-sharing population) across +// drive-role-matched nodes. A pinned numDrives caps each node's contribution: the mode takes that many +// of a node's largest drives and leaves the rest, so summing the raw counts would over-state the +// reachable total and let an unsatisfiable minNumDrives through. +func validateMinDrivesAutoFullDrives(ctx context.Context, c client.Client, cluster *weka.WekaCluster, minNumDrives int, fldPath *field.Path) field.ErrorList { + nodes, errs := listDriveRoleNodes(ctx, c, cluster, fldPath) + if errs != nil { + return errs + } + if len(nodes) == 0 { + return nil + } + + infos, errs := driveRoleNodeInfos(nodes, fldPath) + if errs != nil { + return errs + } + perNodeCap := 0 // 0 = unpinned, take everything the node signed + if cluster.Spec.Dynamic != nil { + perNodeCap = cluster.Spec.Dynamic.NumDrives + } + var total int + for _, ni := range infos { + n := len(ni.Info.AvailableDrives) + if perNodeCap > 0 { + n = min(n, perNodeCap) + } + total += n + } + + if minNumDrives <= total { + return nil + } + + // total == 0: sign-drives hasn't run yet. Still a genuine infeasibility, but name the actual + // cause rather than the generic "exceeds N drives" wording. + pinNote := "" + if perNodeCap > 0 { + pinNote = fmt.Sprintf(", each node capped at the pinned numDrives=%d", perNodeCap) + } + detail := fmt.Sprintf( + "spec.startIoConditions.minNumDrives (%d) exceeds the total signed, non-blocked full drives "+ + "the cluster can claim across %d matched drive-role node(s)%s (%d). The cluster will never "+ + "satisfy the IO-start condition. Reduce minNumDrives, raise or unset numDrives, sign more "+ + "drives, or label more nodes.", + minNumDrives, len(nodes), pinNote, total, + ) + if total == 0 { + detail = fmt.Sprintf( + "spec.startIoConditions.minNumDrives (%d) cannot be satisfied: none of the %d matched "+ + "drive-role node(s) has any signed, non-blocked full drive (no %s annotation, or "+ + "every drive is blocked), so the cluster has no drives to consume and the IO-start "+ + "condition would never be met. Sign drives on the drive-role nodes before applying "+ + "the cluster, label more nodes, or unset minNumDrives.", + minNumDrives, len(nodes), consts.AnnotationWekaFullDrives, + ) + } + return field.ErrorList{ + field.Invalid(fldPath, minNumDrives, detail), } } diff --git a/internal/validation/cluster_min_drives_feasibility_test.go b/internal/validation/cluster_min_drives_feasibility_test.go new file mode 100644 index 000000000..6c6a943b0 --- /dev/null +++ b/internal/validation/cluster_min_drives_feasibility_test.go @@ -0,0 +1,180 @@ +package validation + +import ( + "context" + "strings" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" +) + +func minDrivesCluster(dynamic *weka.WekaClusterTemplate, minNumDrives int) *weka.WekaCluster { + c := &weka.WekaCluster{} + c.Spec.Dynamic = dynamic + if minNumDrives > 0 { + c.Spec.StartIoConditions = &weka.StartIoConditions{MinNumDrives: minNumDrives} + } + return c +} + +// Count-based behavior — regression guard for the driveContainers × numDrives check (nil client: this +// path never touches it). Every case here sets a field that leaves auto-full-drives mode, so none of +// them reaches the node-listing branch. +func TestClusterMinDrivesFeasibility_CountBased(t *testing.T) { + v := &clusterMinDrivesFeasibility{} + ctx := context.Background() + + tests := []struct { + name string + dynamic *weka.WekaClusterTemplate + minNum int + wantErr bool + }{ + { + name: "minNumDrives unset skipped", + dynamic: &weka.WekaClusterTemplate{DriveContainers: 2, NumDrives: 2}, + minNum: 0, + wantErr: false, + }, + { + name: "driveContainers/numDrives unset (operator-derived) skipped", + dynamic: &weka.WekaClusterTemplate{ContainerCapacity: 6000}, + minNum: 10, + wantErr: false, + }, + { + name: "minNumDrives within total passes", + dynamic: &weka.WekaClusterTemplate{DriveContainers: 2, NumDrives: 4}, + minNum: 8, + wantErr: false, + }, + { + name: "minNumDrives exceeding total flagged", + dynamic: &weka.WekaClusterTemplate{DriveContainers: 2, NumDrives: 4}, + minNum: 9, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + errs := v.Validate(ctx, nil, minDrivesCluster(tt.dynamic, tt.minNum)) + if tt.wantErr && len(errs) == 0 { + t.Errorf("expected an error, got none") + } + if !tt.wantErr && len(errs) != 0 { + t.Errorf("expected no error, got %v", errs) + } + }) + } +} + +// Auto-full-drives behavior: both container counts are 0 in this mode, so the total is derived from +// signed full drives on drive-role-matched nodes instead. +func TestClusterMinDrivesFeasibility_AutoFullDrives(t *testing.T) { + v := &clusterMinDrivesFeasibility{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + autoCluster := func(dynamic *weka.WekaClusterTemplate, minNumDrives int) *weka.WekaCluster { + c := minDrivesCluster(dynamic, minNumDrives) + c.Spec.NodeSelector = labels + return c + } + emptyTemplate := func(minNumDrives int) *weka.WekaCluster { + return autoCluster(&weka.WekaClusterTemplate{}, minNumDrives) + } + + t.Run("sufficient signed drives passes", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, []int{1000, 1000, 1000}), // 3 drives + driveRoleNode(t, "n2", labels, []int{1000, 1000}), // 2 drives + ) + errs := v.Validate(ctx, c, emptyTemplate(5)) + if len(errs) != 0 { + t.Errorf("expected no error, got %v", errs) + } + }) + + t.Run("minNumDrives exceeding total signed flagged", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, []int{1000, 1000, 1000}), // 3 drives + driveRoleNode(t, "n2", labels, []int{1000, 1000}), // 2 drives + ) + errs := v.Validate(ctx, c, emptyTemplate(6)) + if len(errs) == 0 { + t.Errorf("expected an error, got none") + } + }) + + // A nil dynamicTemplate is auto-full-drives mode, not "nothing configured": it must take this + // branch rather than being skipped, or an unsatisfiable minNumDrives sails through. + t.Run("nil dynamicTemplate takes the auto-full-drives branch", func(t *testing.T) { + c := fakeClientWithNodes(t, driveRoleNode(t, "n1", labels, []int{1000, 1000})) + if errs := v.Validate(ctx, c, autoCluster(nil, 2)); len(errs) != 0 { + t.Errorf("expected no error at minNumDrives=2, got %v", errs) + } + if errs := v.Validate(ctx, c, autoCluster(nil, 3)); len(errs) == 0 { + t.Errorf("expected an error at minNumDrives=3 with only 2 signed drives, got none") + } + }) + + // The over-count bug: a pinned numDrives caps each node's contribution, so the reachable total is + // min(signed, numDrives) per node, not the raw sum. + t.Run("pinned numDrives caps each node's contribution", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, []int{1000, 1000, 1000, 1000, 1000}), // 5 signed + driveRoleNode(t, "n2", labels, []int{1000, 1000, 1000, 1000, 1000}), // 5 signed + ) + pinned := func(min int) *weka.WekaCluster { + return autoCluster(&weka.WekaClusterTemplate{NumDrives: 2}, min) + } + // numDrives=2 means 2 per node = 4 reachable, not 10. + if errs := v.Validate(ctx, c, pinned(4)); len(errs) != 0 { + t.Errorf("expected no error at minNumDrives=4, got %v", errs) + } + errs := v.Validate(ctx, c, pinned(5)) + if len(errs) != 1 { + t.Fatalf("expected an error at minNumDrives=5 (only 4 reachable under the pin), got %v", errs) + } + if !strings.Contains(errs[0].Detail, "pinned numDrives=2") { + t.Errorf("expected the message to name the pin as the cause, got %q", errs[0].Detail) + } + }) + + // No bootstrap skip: unsigned nodes mean the mode has nothing to consume, so any positive + // minNumDrives is rejected rather than silently admitted. + t.Run("pre-signing (no annotations) rejected", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, nil), + driveRoleNode(t, "n2", labels, nil), + ) + errs := v.Validate(ctx, c, emptyTemplate(100)) + if len(errs) != 1 { + t.Fatalf("expected 1 error, got %v", errs) + } + if !strings.Contains(errs[0].Detail, "has any signed") { + t.Errorf("expected the unsigned-specific message, got %q", errs[0].Detail) + } + }) + + t.Run("partially signed counts only what is signed", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, []int{1000, 1000}), // 2 drives + driveRoleNode(t, "n2", labels, nil), // not signed yet + ) + if errs := v.Validate(ctx, c, emptyTemplate(2)); len(errs) != 0 { + t.Errorf("expected no error at minNumDrives=2, got %v", errs) + } + if errs := v.Validate(ctx, c, emptyTemplate(3)); len(errs) == 0 { + t.Errorf("expected an error at minNumDrives=3, got none") + } + }) + + t.Run("no matched nodes skipped", func(t *testing.T) { + c := fakeClientWithNodes(t) + errs := v.Validate(ctx, c, emptyTemplate(100)) + if len(errs) != 0 { + t.Errorf("expected no error (no matched nodes), got %v", errs) + } + }) +} diff --git a/internal/validation/cluster_num_drives_below_required_cores.go b/internal/validation/cluster_num_drives_below_required_cores.go new file mode 100644 index 000000000..3793fcdb0 --- /dev/null +++ b/internal/validation/cluster_num_drives_below_required_cores.go @@ -0,0 +1,84 @@ +package validation + +import ( + "context" + "fmt" + + wekav1alpha1 "github.com/weka/weka-k8s-api/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + globalconfig "github.com/weka/weka-operator/internal/config" + "github.com/weka/weka-operator/internal/controllers/allocator" +) + +// clusterNumDrivesBelowRequiredCores rejects a numDrives+driveCapacity template whose configured +// capacity needs more drive cores than numDrives allows. In this mode each drive core needs at least +// one virtual drive, so CEL (wekacluster_types.go) caps driveCores at numDrives — meaning a per-drive +// driveCapacity above the per-core TLC capacity is unreachable at EVERY legal driveCores, not just at +// the one the operator picks. +// +// Nothing else catches it. getDriveCores clamps the derived count to numDrives, so the container is +// built with too few cores for the capacity it is told to hold and drive adds are deferred with +// DriveCapacityResourceShortfall forever; clusterDriveCoresBelowCapacity only fires on an explicit +// driveCores and compares against that same clamped figure, so it stays silent here. +// +// The check reduces exactly to driveCapacity <= TlcCapacityPerCoreGiB (required = ceil(numDrives × +// driveCapacity / perCore) is <= numDrives iff driveCapacity <= perCore), which is why raising +// numDrives is NOT offered as a remedy: it scales capacity and requirement together and never closes +// the gap. +type clusterNumDrivesBelowRequiredCores struct{} + +func (clusterNumDrivesBelowRequiredCores) ID() string { + return "cluster_num_drives_below_required_cores" +} + +func (clusterNumDrivesBelowRequiredCores) Validate(_ context.Context, _ client.Client, obj runtime.Object) field.ErrorList { + cluster, ok := obj.(*wekav1alpha1.WekaCluster) + if !ok { + return nil + } + config := cluster.Spec.Dynamic + if config == nil { + return nil + } + // Planner-managed templates size their own cores, so the numDrives ceiling this rule reasons about + // does not apply. CEL already makes clusterCapacity mutually exclusive with numDrives/driveCapacity, + // so this is unreachable through the API — but the guard keeps the rule honest for callers that + // evaluate a template directly, and matches how every other core-sizing rule scopes itself. + if allocator.IsPlannerManaged(config) { + return nil + } + // The only remaining mode that pins a drive count and a per-drive capacity independently. + // containerCapacity sets a whole-container figure with no drive count to bound the cores. + if config.NumDrives <= 0 || config.DriveCapacity <= 0 { + return nil + } + + required, ok := allocator.RequiredDriveCoresForTemplate(config) + if !ok || required <= config.NumDrives { + return nil + } + + perCoreGiB := globalconfig.Config.ClusterCapacity.TlcCapacityPerCoreGiB + detail := fmt.Sprintf( + "spec.dynamicTemplate.driveCapacity (%d GiB per drive × numDrives %d = %d GiB) needs %d drive "+ + "core(s), but numDrives caps driveCores at %d — weka requires at least one drive per drive "+ + "core in this mode, so the capacity is unreachable at every legal driveCores and drive adds "+ + "would be deferred with DriveCapacityResourceShortfall indefinitely. Raising numDrives does "+ + "not help: it raises total capacity by the same factor. Lower driveCapacity to at most %d "+ + "GiB (the per-core TLC capacity), switch to spec.dynamicTemplate.containerCapacity so the "+ + "operator sizes cores from the total instead of per drive, or raise the "+ + "CLUSTER_CAPACITY_TLC_CAPACITY_PER_CORE_GIB Helm value (currently %d).", + config.DriveCapacity, config.NumDrives, config.DriveCapacity*config.NumDrives, + required, config.NumDrives, perCoreGiB, perCoreGiB, + ) + return field.ErrorList{ + field.Invalid( + field.NewPath("spec", "dynamicTemplate", "driveCapacity"), + config.DriveCapacity, + detail, + ), + } +} diff --git a/internal/validation/cluster_num_drives_below_required_cores_test.go b/internal/validation/cluster_num_drives_below_required_cores_test.go new file mode 100644 index 000000000..e1709ebc3 --- /dev/null +++ b/internal/validation/cluster_num_drives_below_required_cores_test.go @@ -0,0 +1,111 @@ +package validation + +import ( + "context" + "strings" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + + globalconfig "github.com/weka/weka-operator/internal/config" +) + +func TestClusterNumDrivesBelowRequiredCores(t *testing.T) { + prevTlc := globalconfig.Config.ClusterCapacity.TlcCapacityPerCoreGiB + globalconfig.Config.ClusterCapacity.TlcCapacityPerCoreGiB = 5 * 1024 // 5120 GiB/core + t.Cleanup(func() { + globalconfig.Config.ClusterCapacity.TlcCapacityPerCoreGiB = prevTlc + }) + + v := &clusterNumDrivesBelowRequiredCores{} + ctx := context.Background() + + tests := []struct { + name string + dynamic *weka.WekaClusterTemplate + wantErr bool + wantSubs []string + }{ + { + name: "no dynamic template skipped", + dynamic: nil, + wantErr: false, + }, + { + name: "containerCapacity mode skipped: no drive count to bound cores", + dynamic: &weka.WekaClusterTemplate{ + ContainerCapacity: 40000, // ceil(40000/5120)=8, but there is no numDrives to compare against + }, + wantErr: false, + }, + { + name: "pure full-drives mode skipped: no driveCapacity", + dynamic: &weka.WekaClusterTemplate{ + NumDrives: 4, + }, + wantErr: false, + }, + { + name: "clusterCapacity skipped: planner assigns cores itself", + dynamic: &weka.WekaClusterTemplate{ + ClusterCapacity: "1PiB", + NumDrives: 4, + DriveCapacity: 8000, + }, + wantErr: false, + }, + { + // 4 drives x 2000 GiB = 8000 GiB, ceil(8000/5120)=2 <= numDrives=4: reachable, no finding. + name: "required cores at or below numDrives is reachable", + dynamic: &weka.WekaClusterTemplate{ + NumDrives: 4, + DriveCapacity: 2000, + }, + wantErr: false, + }, + { + // 4 drives x 1280 GiB = 5120 GiB, ceil(5120/5120)=1: the boundary case, still reachable. + name: "required cores exactly at the per-core capacity is reachable", + dynamic: &weka.WekaClusterTemplate{ + NumDrives: 4, + DriveCapacity: 1280, + }, + wantErr: false, + }, + { + // 4 drives x 8000 GiB = 32000 GiB, ceil(32000/5120)=7 > numDrives=4: unreachable at any legal + // driveCores, since CEL caps driveCores at numDrives. + name: "required cores above numDrives is unreachable", + dynamic: &weka.WekaClusterTemplate{ + NumDrives: 4, + DriveCapacity: 8000, + }, + wantErr: true, + wantSubs: []string{ + "driveCapacity (8000 GiB per drive × numDrives 4 = 32000 GiB) needs 7 drive core(s)", + "numDrives caps driveCores at 4", + "Raising numDrives does not help", + "Lower driveCapacity to at most 5120 GiB", + "CLUSTER_CAPACITY_TLC_CAPACITY_PER_CORE_GIB Helm value (currently 5120)", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cluster := &weka.WekaCluster{Spec: weka.WekaClusterSpec{Dynamic: tt.dynamic}} + errs := v.Validate(ctx, nil, cluster) + if tt.wantErr && len(errs) == 0 { + t.Fatalf("expected an error, got none") + } + if !tt.wantErr && len(errs) != 0 { + t.Fatalf("expected no error, got %v", errs) + } + for _, sub := range tt.wantSubs { + if !strings.Contains(errs[0].Detail, sub) { + t.Errorf("detail missing %q, got: %s", sub, errs[0].Detail) + } + } + }) + } +} diff --git a/internal/validation/cluster_selected_nodes_count_test.go b/internal/validation/cluster_selected_nodes_count_test.go new file mode 100644 index 000000000..23e05cc07 --- /dev/null +++ b/internal/validation/cluster_selected_nodes_count_test.go @@ -0,0 +1,74 @@ +package validation + +import ( + "context" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" +) + +func selectedNodesCluster(dynamic *weka.WekaClusterTemplate, selector map[string]string) *weka.WekaCluster { + c := &weka.WekaCluster{} + c.Spec.Dynamic = dynamic + c.Spec.NodeSelector = selector + return c +} + +// Count-based behavior — unchanged regression guard. +func TestClusterSelectedNodesCount_CountBased(t *testing.T) { + v := &clusterSelectedNodesCount{} + ctx := context.Background() + labels := map[string]string{"role": "any"} + + t.Run("containers within matched node count passes", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, nil), + driveRoleNode(t, "n2", labels, nil), + ) + dynamic := &weka.WekaClusterTemplate{DriveContainers: 2} + errs := v.Validate(ctx, c, selectedNodesCluster(dynamic, labels)) + if len(errs) != 0 { + t.Errorf("expected no error, got %v", errs) + } + }) + + t.Run("containers exceeding matched node count flagged", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, nil), + ) + dynamic := &weka.WekaClusterTemplate{DriveContainers: 2} + errs := v.Validate(ctx, c, selectedNodesCluster(dynamic, labels)) + if len(errs) == 0 { + t.Errorf("expected an error, got none") + } + }) +} + +// Auto-full-drives: driveContainers is always 0 in that mode, so the drive-role branch is structurally +// inert — this locks in that such a cluster is never flagged on the drive role, even with zero matched +// nodes. A pinned computeContainers leaves the mode, and that role keeps behaving as before. +func TestClusterSelectedNodesCount_AutoFullDrivesDriveRoleInert(t *testing.T) { + v := &clusterSelectedNodesCount{} + ctx := context.Background() + labels := map[string]string{"role": "any"} + + t.Run("auto-full-drives with zero matched drive nodes: not flagged", func(t *testing.T) { + c := fakeClientWithNodes(t) // no nodes at all + dynamic := &weka.WekaClusterTemplate{} + errs := v.Validate(ctx, c, selectedNodesCluster(dynamic, labels)) + if len(errs) != 0 { + t.Errorf("expected no error (driveContainers is always 0 in auto-full-drives mode), got %v", errs) + } + }) + + t.Run("a pinned compute role is still enforced", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, nil), + ) + dynamic := &weka.WekaClusterTemplate{ComputeContainers: 3} + errs := v.Validate(ctx, c, selectedNodesCluster(dynamic, labels)) + if len(errs) == 0 { + t.Errorf("expected an error: 3 compute containers pinned against 1 matched node") + } + }) +} diff --git a/internal/validation/cluster_signed_drives.go b/internal/validation/cluster_signed_drives.go index d5a2eda11..55f854df2 100644 --- a/internal/validation/cluster_signed_drives.go +++ b/internal/validation/cluster_signed_drives.go @@ -5,19 +5,19 @@ import ( "fmt" weka "github.com/weka/weka-k8s-api/api/v1alpha1" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/weka/weka-operator/internal/consts" - "github.com/weka/weka-operator/internal/controllers/allocator" ) -// clusterSignedDrives rejects WekaClusters where driveContainers × numDrives -// exceeds the count of signed, non-blocked drives across matched drive-role -// nodes. Bootstrap-skipped when no node has either weka-full-drives or -// weka-shared-drives annotation set (sign-drives hasn't run yet). +// clusterSignedDrives rejects full-drives clusters where driveContainers × numDrives exceeds signed, +// non-blocked full drives across matched drive-role nodes (bootstrap-skipped until any node carries +// weka-full-drives). Only exclusive full drives count — shared drives are carved by capacity instead. +// Out of scope by construction: auto-full-drives mode (no fixed container count — drives are claimed +// per node; clusterAutoFullDrivesPinExceedsNodeDrives owns its pins) and drive sharing (numDrives +// counts virtual drives — a category error to compare; cluster_capacity_* owns feasibility there). type clusterSignedDrives struct{} func (clusterSignedDrives) ID() string { @@ -29,9 +29,14 @@ func (clusterSignedDrives) Validate(ctx context.Context, c client.Client, obj ru if !ok { return nil } + // A nil template is auto-full-drives mode, which the next check drops anyway; the explicit guard + // keeps the field reads below obviously safe. if cluster.Spec.Dynamic == nil { return nil } + if cluster.Spec.Dynamic.UsesAutoFullDrives() || cluster.IsDriveSharing() { + return nil + } driveContainers := cluster.Spec.Dynamic.DriveContainers numDrives := cluster.Spec.Dynamic.NumDrives @@ -41,23 +46,19 @@ func (clusterSignedDrives) Validate(ctx context.Context, c client.Client, obj ru fldPath := field.NewPath("spec", "dynamicTemplate").Child("numDrives") - selector := cluster.GetNodeSelectorForRole("drive") - var nodes corev1.NodeList - if err := c.List(ctx, &nodes, client.MatchingLabels(selector)); err != nil { - return field.ErrorList{ - field.InternalError(fldPath, fmt.Errorf("listing drive-role nodes: %w", err)), - } + nodes, errs := listDriveRoleNodes(ctx, c, cluster, fldPath) + if errs != nil { + return errs } - if len(nodes.Items) == 0 { + if len(nodes) == 0 { return nil } + // Only the full-drives annotation gates the bootstrap skip — a drive-sharing-signed node is not + // signed for this cluster. clusterDrivesUnsignedAdvisory covers both mismatch states. anyAnnotated := false - for i := range nodes.Items { - n := &nodes.Items[i] - _, full := n.Annotations[consts.AnnotationWekaFullDrives] - _, shared := n.Annotations[consts.AnnotationSharedDrives] - if full || shared { + for i := range nodes { + if _, full := nodes[i].Annotations[consts.AnnotationWekaFullDrives]; full { anyAnnotated = true break } @@ -66,17 +67,13 @@ func (clusterSignedDrives) Validate(ctx context.Context, c client.Client, obj ru return nil } - getter := allocator.NewK8sNodeInfoGetter(c) + infos, errs := driveRoleNodeInfos(nodes, fldPath) + if errs != nil { + return errs + } var available int - for i := range nodes.Items { - name := nodes.Items[i].Name - info, err := getter(ctx, weka.NodeName(name)) - if err != nil { - return field.ErrorList{ - field.InternalError(fldPath, fmt.Errorf("reading drive info for node %q: %w", name, err)), - } - } - available += len(info.AvailableDrives) + len(info.SharedDrives) + for _, ni := range infos { + available += len(ni.Info.AvailableDrives) } requested := driveContainers * numDrives @@ -86,10 +83,10 @@ func (clusterSignedDrives) Validate(ctx context.Context, c client.Client, obj ru detail := fmt.Sprintf( "spec.dynamicTemplate.driveContainers × numDrives (%d × %d = %d) exceeds the "+ - "total signed and non-blocked drives across %d matched drive node(s) (%d). "+ + "total signed and non-blocked full drives across %d matched drive node(s) (%d). "+ "Some drive containers will not be able to claim a drive. Reduce numDrives, "+ "reduce driveContainers, sign more drives, or label more nodes.", - driveContainers, numDrives, requested, len(nodes.Items), available, + driveContainers, numDrives, requested, len(nodes), available, ) return field.ErrorList{ field.Invalid(fldPath, numDrives, detail), diff --git a/internal/validation/cluster_signed_drives_test.go b/internal/validation/cluster_signed_drives_test.go new file mode 100644 index 000000000..476bcc144 --- /dev/null +++ b/internal/validation/cluster_signed_drives_test.go @@ -0,0 +1,150 @@ +package validation + +import ( + "context" + "fmt" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + corev1 "k8s.io/api/core/v1" +) + +func signedDrivesCluster(dynamic *weka.WekaClusterTemplate, selector map[string]string) *weka.WekaCluster { + c := &weka.WekaCluster{} + c.Spec.Dynamic = dynamic + c.Spec.NodeSelector = selector + return c +} + +// Count-based behavior — regression guard for the driveContainers × numDrives vs. signed-drives check. +func TestClusterSignedDrives_CountBased(t *testing.T) { + v := &clusterSignedDrives{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + t.Run("sufficient signed drives passes", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, []int{1000, 1000, 1000, 1000}), // 4 drives + ) + dynamic := &weka.WekaClusterTemplate{DriveContainers: 1, NumDrives: 4} + errs := v.Validate(ctx, c, signedDrivesCluster(dynamic, labels)) + if len(errs) != 0 { + t.Errorf("expected no error, got %v", errs) + } + }) + + t.Run("requested exceeding signed flagged", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, []int{1000, 1000}), // 2 drives + ) + dynamic := &weka.WekaClusterTemplate{DriveContainers: 1, NumDrives: 4} + errs := v.Validate(ctx, c, signedDrivesCluster(dynamic, labels)) + if len(errs) == 0 { + t.Errorf("expected an error, got none") + } + }) + + t.Run("pre-signing (no annotations) skipped, not rejected", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, nil), + ) + dynamic := &weka.WekaClusterTemplate{DriveContainers: 1, NumDrives: 4} + errs := v.Validate(ctx, c, signedDrivesCluster(dynamic, labels)) + if len(errs) != 0 { + t.Errorf("expected no error (bootstrap skip), got %v", errs) + } + }) + + t.Run("driveContainers or numDrives unset skipped", func(t *testing.T) { + c := fakeClientWithNodes(t) + dynamic := &weka.WekaClusterTemplate{DriveContainers: 0, NumDrives: 4} + errs := v.Validate(ctx, c, signedDrivesCluster(dynamic, labels)) + if len(errs) != 0 { + t.Errorf("expected no error, got %v", errs) + } + }) + + // Shared drives are a disjoint population and must not pad the availability count into a false pass. + t.Run("shared drives do not count toward a full-drives request", func(t *testing.T) { + c := fakeClientWithNodes(t, + driveRoleNode(t, "n1", labels, []int{1000}), // 1 full drive, signed + sharedDriveRoleNode("n2", labels), // proxy-signed; unusable here + ) + dynamic := &weka.WekaClusterTemplate{DriveContainers: 1, NumDrives: 2} + errs := v.Validate(ctx, c, signedDrivesCluster(dynamic, labels)) + if len(errs) == 0 { + t.Errorf("expected an error: 2 requested vs 1 usable full drive") + } + }) + + // Bootstrap skip keys on the full-drives annotation only; proxy-signed nodes aren't "signed" here. + t.Run("only shared-signed nodes bootstrap-skipped", func(t *testing.T) { + c := fakeClientWithNodes(t, sharedDriveRoleNode("n1", labels)) + dynamic := &weka.WekaClusterTemplate{DriveContainers: 1, NumDrives: 4} + if errs := v.Validate(ctx, c, signedDrivesCluster(dynamic, labels)); len(errs) != 0 { + t.Errorf("expected no error (bootstrap skip), got %v", errs) + } + advisory := (&clusterDrivesUnsignedAdvisory{}).Validate(ctx, c, signedDrivesCluster(dynamic, labels)) + if len(advisory) != 1 { + t.Errorf("expected the unsigned advisory to cover this state, got %v", advisory) + } + }) +} + +// Drive-sharing clusters: numDrives counts virtual drives, so comparing to physical count is a category +// error. Skipped entirely — cluster_capacity_* owns feasibility there. +func TestClusterSignedDrives_DriveSharingSkipped(t *testing.T) { + v := &clusterSignedDrives{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + // 8×6=48 virtual drives against 1 physical shared drive: counting here would wrongly reject. + c := fakeClientWithNodes(t, sharedDriveRoleNode("n1", labels)) + dynamic := &weka.WekaClusterTemplate{DriveContainers: 6, NumDrives: 8, DriveCapacity: 100} + + cluster := signedDrivesCluster(dynamic, labels) + if !cluster.IsDriveSharing() { + t.Fatalf("test premise broken: driveCapacity cluster is not IsDriveSharing()") + } + if errs := v.Validate(ctx, c, cluster); len(errs) != 0 { + t.Errorf("expected drive-sharing to be skipped, got %v", errs) + } +} + +// Auto-full-drives: driveContainers is always 0 in that mode, so this validator always skips +// regardless of how few drives are signed. Locks in the explicit UsesAutoFullDrives() early-return. +func TestClusterSignedDrives_AutoFullDrivesAlwaysSkipped(t *testing.T) { + v := &clusterSignedDrives{} + ctx := context.Background() + labels := map[string]string{"role": "drive"} + + tests := []struct { + name string + nodes []int // per-node signed drive counts; nil node list when empty + }{ + {name: "no matched nodes"}, + {name: "matched nodes, none signed yet", nodes: []int{0}}, + {name: "matched nodes with signed drives", nodes: []int{4, 2}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var nodes []*corev1.Node + for i, count := range tt.nodes { + var caps []int + if count > 0 { + caps = make([]int, count) + for j := range caps { + caps[j] = 1000 + } + } + nodes = append(nodes, driveRoleNode(t, fmt.Sprintf("n%d", i), labels, caps)) + } + c := fakeClientWithNodes(t, nodes...) + errs := v.Validate(ctx, c, signedDrivesCluster(&weka.WekaClusterTemplate{}, labels)) + if len(errs) != 0 { + t.Errorf("expected no error in auto-full-drives mode, got %v", errs) + } + }) + } +} diff --git a/internal/validation/cluster_sizing_mode_flip.go b/internal/validation/cluster_sizing_mode_flip.go new file mode 100644 index 000000000..98f00049a --- /dev/null +++ b/internal/validation/cluster_sizing_mode_flip.go @@ -0,0 +1,184 @@ +package validation + +import ( + "context" + "fmt" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/weka/weka-operator/internal/pkg/domain" +) + +// clusterSizingModeFlip rejects an update that changes the cluster's DERIVED sizing mode while drive +// containers already exist. The mode is implicit — it follows from which of +// computeContainers/driveContainers/clusterCapacity/containerCapacity/driveCapacity are set — so +// nothing else makes the flip loud. Left unchecked, adding driveContainers to a live auto-full-drives +// ("acts as a daemonset") cluster starts creating count-based single-drive containers alongside the +// node-sized ones already running, and the two sizing regimes fight over the same drives forever. +// +// It covers EVERY transition, not only the ones touching auto-full-drives, because nothing else does: +// the cluster_capacity_* policies are create-shaped and never see the old object, and +// cluster_capacity_chunk_feasibility deliberately disarms itself once drive containers exist. The +// capacity modes fail the same way in their own idiom — see modeFlipConsequence for each. +// +// Two switches ARE supported and are allowlisted in modeSwitchSupported. +// +// Error in BOTH modes: unlike a capacity check, there is no degraded-but-working outcome here. +// Deliberately scoped to clusters that already HAVE drive containers — before any exist the mode is +// still free to change, which is what makes fixing a mistyped spec possible. +type clusterSizingModeFlip struct{} + +func (clusterSizingModeFlip) ID() string { return "cluster_sizing_mode_flip" } + +func (clusterSizingModeFlip) ValidateUpdate(ctx context.Context, c client.Client, oldObj, newObj runtime.Object) field.ErrorList { + oldCluster, ok := oldObj.(*weka.WekaCluster) + if !ok { + return nil + } + newCluster, ok := newObj.(*weka.WekaCluster) + if !ok { + return nil + } + + oldMode := derivedSizingMode(oldCluster.Spec.Dynamic) + newMode := derivedSizingMode(newCluster.Spec.Dynamic) + if oldMode == newMode { + return nil + } + if modeSwitchSupported(oldMode, newMode) { + return nil + } + // Note the ordering: nothing above this point touches the API server, so an unchanged mode — the + // overwhelmingly common edit — and a supported switch are never exposed to a List failure. Only an + // update that genuinely flips the mode to one we cannot carry over can be blocked by one. + hasDriveContainers, err := clusterHasDriveContainer(ctx, c, newCluster) + if err != nil { + // Fail CLOSED. Treating "could not list" as "no containers exist" would let an apiserver blip + // during a kubectl edit wave through the exact change this policy blocks, and the damage — + // count-based containers created alongside the running node-pinned ones — outlasts the blip. + // A spurious rejection costs a retry; a spurious admission costs the cluster's topology. + return field.ErrorList{field.InternalError( + field.NewPath("spec", "dynamicTemplate"), + fmt.Errorf("this update changes the cluster's derived sizing mode from %s to %s, which is "+ + "not a supported switch once drive containers exist — but listing them failed, so "+ + "the change could not be validated and is rejected rather than risked: %w. Retry the "+ + "edit; if it keeps failing, check the operator's access to WekaContainer resources in "+ + "namespace %q", oldMode, newMode, err, newCluster.Namespace), + )} + } + if !hasDriveContainers { + return nil + } + + detail := fmt.Sprintf( + "this update changes the cluster's derived sizing mode from %s to %s while drive containers "+ + "already exist. The mode is not a field — it follows from which sizing fields are set (%s) — "+ + "so the operator would start planning the running drive containers under different rules: "+ + "%s. Once drive containers exist the only supported switches are unsetting both "+ + "spec.dynamicTemplate.computeContainers and spec.dynamicTemplate.driveContainers, which "+ + "adopts the daemonset mode by growing the existing drive containers in place, and moving a "+ + "drive-sharing cluster to spec.dynamicTemplate.clusterCapacity.", + oldMode, newMode, sizingModeFields, modeFlipConsequence(oldMode, newMode), + ) + return field.ErrorList{ + field.Forbidden(field.NewPath("spec", "dynamicTemplate"), detail), + } +} + +// Derived sizing modes, in the same precedence order UsesAutoFullDrives/IsDriveSharing use. +const ( + sizingModeAutoFullDrives = "auto-full-drives (acts as a daemonset)" + sizingModeClusterCapacity = "clusterCapacity" + sizingModeDriveSharing = "drive-sharing (containerCapacity/driveCapacity)" + sizingModeCounts = "explicit container counts" + + sizingModeFields = "computeContainers, driveContainers, clusterCapacity, containerCapacity, driveCapacity" +) + +// derivedSizingMode names the sizing regime a template selects. Nil-safe: a nil template sets none of +// the fields, so it is auto-full-drives, exactly as UsesAutoFullDrives reports. +func derivedSizingMode(d *weka.WekaClusterTemplate) string { + switch { + case d.UsesAutoFullDrives(): + return sizingModeAutoFullDrives + case d.UsesClusterCapacity(): + return sizingModeClusterCapacity + case d.ContainerCapacity > 0 || d.DriveCapacity > 0: + return sizingModeDriveSharing + default: + return sizingModeCounts + } +} + +// modeSwitchSupported lists the (old -> new) pairs that are safe on a cluster that already has drive +// containers, because the new mode's planner can carry the running containers over. Every other pair +// is rejected: without adoption, and with no scale-down path anywhere in the operator, the two sizing +// regimes end up planning the same drives under different rules. +func modeSwitchSupported(oldMode, newMode string) bool { + switch { + case oldMode == sizingModeCounts && newMode == sizingModeAutoFullDrives: + // Both are exclusive full-drives modes over the same physical drives. The auto-full-drives + // planner resolves an existing container's node through GetNodeAffinity(), which falls back to + // Status.NodeAffinity, so a scheduled count-based container — which carries no Spec.NodeAffinity + // — is still matched to its node and grown in place to that node's full drive set, rather than + // joined by a second population. + return true + case oldMode == sizingModeDriveSharing && newMode == sizingModeClusterCapacity: + // The documented in-place migration (doc/operator/deployment/cluster-capacity.md). Both hold + // virtual drives, and inventory.DriveContainerCapacities reads containerCapacity and + // driveCapacity alike, so the planner grows from the running set instead of planning a fresh one. + return true + } + return false +} + +// modeFlipConsequence states, in the user's terms, what the operator would start doing after the flip. +// One branch per way the flip goes wrong; the supported pairs never reach here. +func modeFlipConsequence(oldMode, newMode string) string { + switch { + case oldMode == sizingModeAutoFullDrives: + return "the per-node containers sized from each node's own signed drives would be joined by a " + + "second, differently sized population planned from the new fields" + case newMode == sizingModeAutoFullDrives: + return "the running containers hold virtual drives that the full-drives inventory accounts for " + + "on neither side, so they could be neither adopted nor grown, and every eligible node would " + + "additionally get a new container sized from its own signed drives" + case oldMode == sizingModeCounts && newMode == sizingModeClusterCapacity: + return "the running full-drives containers report no capacity to the capacity planner, so it " + + "would plan a fresh set covering the whole target on top of them — and would then fail to " + + "write containerCapacity onto a container that has numDrives set, wedging reconciliation on " + + "every pass" + case oldMode == sizingModeClusterCapacity: + return "the planner-placed, node-pinned containers would be abandoned — counted toward the new " + + "container count without being resized, with unpinned count-based containers created for any " + + "shortfall, and nothing removed if there is a surplus, since the operator never auto-shrinks" + default: + return "the running containers would keep the drive layout their own family gave them while the " + + "operator sizes new ones under the other family's rules, and nothing reconciles the two" + } +} + +// clusterHasDriveContainer reports whether the cluster already has any drive container. A List failure +// is returned as an error, NOT swallowed into "none" — the caller must decide, and for a mode flip the +// safe decision is to reject. +// +// A missing UID does resolve to "none", and that is a real answer rather than a fallback: containers +// are labelled with the cluster UID, so an object that was never persisted cannot have any. On an +// UPDATE admission request the UID is always populated anyway. +func clusterHasDriveContainer(ctx context.Context, c client.Client, cluster *weka.WekaCluster) (bool, error) { + uid := string(cluster.GetUID()) + if uid == "" { + return false, nil + } + var containers weka.WekaContainerList + if err := c.List(ctx, &containers, client.InNamespace(cluster.Namespace), client.MatchingLabels{ + domain.WekaLabelClusterId: uid, + domain.WekaLabelMode: weka.WekaContainerModeDrive, + }); err != nil { + return false, fmt.Errorf("listing the cluster's drive containers: %w", err) + } + return len(containers.Items) > 0, nil +} diff --git a/internal/validation/cluster_sizing_mode_flip_test.go b/internal/validation/cluster_sizing_mode_flip_test.go new file mode 100644 index 000000000..764f3fe75 --- /dev/null +++ b/internal/validation/cluster_sizing_mode_flip_test.go @@ -0,0 +1,318 @@ +package validation + +import ( + "context" + "errors" + "strings" + "testing" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + "github.com/weka/weka-operator/internal/pkg/domain" +) + +const modeFlipClusterUID = "cluster-uid-1" + +// modeFlipCluster builds a WekaCluster with a stable UID, so the drive-container lookup can match. +func modeFlipCluster(dynamic *weka.WekaClusterTemplate) *weka.WekaCluster { + c := &weka.WekaCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: "ns", UID: types.UID(modeFlipClusterUID)}, + } + c.Spec.Dynamic = dynamic + return c +} + +// modeFlipClient seeds a fake client with `n` drive containers belonging to modeFlipCluster. +func modeFlipClient(t *testing.T, driveContainers int) client.Client { + t.Helper() + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme(core): %v", err) + } + if err := weka.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme(weka): %v", err) + } + b := fake.NewClientBuilder().WithScheme(scheme) + for i := 0; i < driveContainers; i++ { + wc := &weka.WekaContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "drive-" + string(rune('a'+i)), + Namespace: "ns", + Labels: map[string]string{ + domain.WekaLabelClusterId: modeFlipClusterUID, + domain.WekaLabelMode: weka.WekaContainerModeDrive, + }, + }, + } + b = b.WithObjects(wc) + } + return b.Build() +} + +// modeFlipFailingClient returns a client whose every List fails, so a test can prove the validator +// either does or does not reach the API server. +func modeFlipFailingClient(t *testing.T) client.Client { + t.Helper() + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme(core): %v", err) + } + if err := weka.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme(weka): %v", err) + } + return fake.NewClientBuilder().WithScheme(scheme).WithInterceptorFuncs(interceptor.Funcs{ + List: func(ctx context.Context, cl client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + return errors.New("etcdserver: request timed out") + }, + }).Build() +} + +// TestSizingModeFlip_LeavingTheMode is the case the policy exists for: adding container counts to a +// live daemonset cluster would start a second, differently sized drive-container population. +func TestSizingModeFlip_LeavingTheMode(t *testing.T) { + v := &clusterSizingModeFlip{} + ctx := context.Background() + c := modeFlipClient(t, 3) + + old := modeFlipCluster(&weka.WekaClusterTemplate{}) + updated := modeFlipCluster(&weka.WekaClusterTemplate{ComputeContainers: 6, DriveContainers: 6}) + + errs := v.ValidateUpdate(ctx, c, old, updated) + if len(errs) != 1 { + t.Fatalf("expected exactly one violation, got %v", errs) + } + detail := errs[0].Detail + for _, want := range []string{ + "auto-full-drives (acts as a daemonset)", + "explicit container counts", + "drive containers already exist", + "second, differently sized population", + // The message must name the switches that ARE available, so the one-way rule is discoverable + // from the rejection alone. + "the only supported switches are", + } { + if !strings.Contains(detail, want) { + t.Errorf("expected message to contain %q, got: %s", want, detail) + } + } + // The user explicitly vetoed recreate-the-cluster as a remedy. + for _, banned := range []string{"delete", "recreate", "re-create"} { + if strings.Contains(strings.ToLower(detail), banned) { + t.Errorf("message must not suggest deleting/recreating the cluster, got: %s", detail) + } + } +} + +// TestSizingModeFlip_SupportedSwitches covers the two transitions the operator can carry over on a +// live cluster: counts -> daemonset (existing containers are adopted via Status.NodeAffinity and grown +// in place) and drive-sharing -> clusterCapacity (the documented in-place migration). +func TestSizingModeFlip_SupportedSwitches(t *testing.T) { + v := &clusterSizingModeFlip{} + ctx := context.Background() + c := modeFlipClient(t, 3) + + cases := map[string][2]*weka.WekaClusterTemplate{ + "counts -> daemonset": { + {ComputeContainers: 6, DriveContainers: 6}, + {}, + }, + "counts -> daemonset, template removed entirely": { + {ComputeContainers: 6, DriveContainers: 6}, + nil, + }, + "counts -> daemonset, keeping the numDrives pin": { + {ComputeContainers: 6, DriveContainers: 6, NumDrives: 4}, + {NumDrives: 4}, + }, + "containerCapacity -> clusterCapacity": { + {ContainerCapacity: 6000, DriveContainers: 6, ComputeContainers: 6}, + {ClusterCapacity: "500TiB"}, + }, + "numDrives+driveCapacity -> clusterCapacity": { + {NumDrives: 6, DriveCapacity: 3500}, + {ClusterCapacity: "500TiB"}, + }, + } + for name, pair := range cases { + t.Run(name, func(t *testing.T) { + errs := v.ValidateUpdate(ctx, c, modeFlipCluster(pair[0]), modeFlipCluster(pair[1])) + if len(errs) != 0 { + t.Errorf("expected a supported switch to be admitted, got %v", errs) + } + }) + } +} + +// TestSizingModeFlip_SupportedSwitchNeverListsContainers: the allowlist is consulted before the +// drive-container lookup, so a switch we support is never hostage to apiserver health. Without this +// ordering, an etcd blip would reject the one mode change users are told to make. +func TestSizingModeFlip_SupportedSwitchNeverListsContainers(t *testing.T) { + v := &clusterSizingModeFlip{} + ctx := context.Background() + c := modeFlipFailingClient(t) + + old := modeFlipCluster(&weka.WekaClusterTemplate{ComputeContainers: 6, DriveContainers: 6}) + updated := modeFlipCluster(&weka.WekaClusterTemplate{}) + + if errs := v.ValidateUpdate(ctx, c, old, updated); len(errs) != 0 { + t.Errorf("a supported switch must not touch the API server at all, got %v", errs) + } +} + +// TestSizingModeFlip_RejectedSwitches walks every transition the operator cannot carry over. The +// capacity pairs matter as much as the auto-full-drives ones: nothing else guards them — the +// cluster_capacity_* policies never see the old object, and chunk_feasibility disarms itself once +// drive containers exist. +func TestSizingModeFlip_RejectedSwitches(t *testing.T) { + v := &clusterSizingModeFlip{} + ctx := context.Background() + c := modeFlipClient(t, 3) + + auto := &weka.WekaClusterTemplate{} + counts := &weka.WekaClusterTemplate{ComputeContainers: 6, DriveContainers: 6} + capacity := &weka.WekaClusterTemplate{ClusterCapacity: "500TiB"} + sharing := &weka.WekaClusterTemplate{ContainerCapacity: 6000, DriveContainers: 6, ComputeContainers: 6} + + cases := map[string]struct { + old, updated *weka.WekaClusterTemplate + wantConsequence string + }{ + "daemonset -> counts": {auto, counts, "second, differently sized population"}, + "daemonset -> clusterCapacity": {auto, capacity, "second, differently sized population"}, + "daemonset -> drive-sharing": {auto, sharing, "second, differently sized population"}, + "clusterCapacity -> daemonset": {capacity, auto, "neither adopted nor grown"}, + "drive-sharing -> daemonset": {sharing, auto, "neither adopted nor grown"}, + "counts -> clusterCapacity": {counts, capacity, "wedging reconciliation"}, + "clusterCapacity -> counts": {capacity, counts, "never auto-shrinks"}, + "clusterCapacity -> drive-shar.": {capacity, sharing, "never auto-shrinks"}, + "counts -> drive-sharing": {counts, sharing, "nothing reconciles the two"}, + "drive-sharing -> counts": {sharing, counts, "nothing reconciles the two"}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + errs := v.ValidateUpdate(ctx, c, modeFlipCluster(tc.old), modeFlipCluster(tc.updated)) + if len(errs) != 1 { + t.Fatalf("expected exactly one violation, got %v", errs) + } + detail := errs[0].Detail + if !strings.Contains(detail, tc.wantConsequence) { + t.Errorf("expected the %q consequence, got: %s", tc.wantConsequence, detail) + } + oldMode := derivedSizingMode(tc.old) + newMode := derivedSizingMode(tc.updated) + if !strings.Contains(detail, oldMode) || !strings.Contains(detail, newMode) { + t.Errorf("expected message to name both modes (%s -> %s), got: %s", oldMode, newMode, detail) + } + }) + } +} + +// TestSizingModeFlip_NilTemplateIsTheSameMode: unsetting the whole template from an already +// auto-full-drives cluster is not a flip — nil and {} are the same mode. +func TestSizingModeFlip_NilTemplateIsTheSameMode(t *testing.T) { + v := &clusterSizingModeFlip{} + ctx := context.Background() + c := modeFlipClient(t, 3) + + if errs := v.ValidateUpdate(ctx, c, modeFlipCluster(&weka.WekaClusterTemplate{}), modeFlipCluster(nil)); len(errs) != 0 { + t.Errorf("{} -> nil is the same mode, got %v", errs) + } + if errs := v.ValidateUpdate(ctx, c, modeFlipCluster(nil), modeFlipCluster(&weka.WekaClusterTemplate{NumDrives: 4})); len(errs) != 0 { + t.Errorf("numDrives is a per-node override, not a mode change, got %v", errs) + } +} + +// TestSizingModeFlip_NoDriveContainersYet: before any drive container exists the mode is still free to +// change, which is what makes fixing a mistyped spec possible. +func TestSizingModeFlip_NoDriveContainersYet(t *testing.T) { + v := &clusterSizingModeFlip{} + ctx := context.Background() + c := modeFlipClient(t, 0) + + old := modeFlipCluster(&weka.WekaClusterTemplate{}) + updated := modeFlipCluster(&weka.WekaClusterTemplate{ComputeContainers: 6, DriveContainers: 6}) + + if errs := v.ValidateUpdate(ctx, c, old, updated); len(errs) != 0 { + t.Errorf("expected no violation before any drive container exists, got %v", errs) + } +} + +// TestSizingModeFlip_ListFailureFailsClosed: if the drive containers cannot be listed, the mode change +// cannot be validated and must be REJECTED, not admitted. Swallowing the error into "no containers +// exist" would let an apiserver blip during a kubectl edit wave through the exact change this policy +// blocks — and unlike the blip, the resulting two-population topology persists. +func TestSizingModeFlip_ListFailureFailsClosed(t *testing.T) { + v := &clusterSizingModeFlip{} + ctx := context.Background() + c := modeFlipFailingClient(t) + + old := modeFlipCluster(&weka.WekaClusterTemplate{}) + updated := modeFlipCluster(&weka.WekaClusterTemplate{ComputeContainers: 6, DriveContainers: 6}) + + errs := v.ValidateUpdate(ctx, c, old, updated) + if len(errs) != 1 { + t.Fatalf("a List failure must reject, not admit; got %v", errs) + } + if errs[0].Type != field.ErrorTypeInternal { + t.Errorf("expected an InternalError, got %v", errs[0].Type) + } + for _, want := range []string{ + "could not be validated and is rejected rather than risked", + "etcdserver: request timed out", + "Retry the edit", + } { + if !strings.Contains(errs[0].Detail, want) { + t.Errorf("expected message to contain %q, got: %s", want, errs[0].Detail) + } + } +} + +// TestSizingModeFlip_ListFailureIrrelevantWhenModeUnchanged: the container lookup happens only after +// the mode comparison, so an ordinary edit that does not flip the mode is never exposed to a List +// failure. Without this, every edit to a live cluster would be hostage to apiserver health. +func TestSizingModeFlip_ListFailureIrrelevantWhenModeUnchanged(t *testing.T) { + v := &clusterSizingModeFlip{} + ctx := context.Background() + c := modeFlipFailingClient(t) + + old := modeFlipCluster(&weka.WekaClusterTemplate{}) + updated := modeFlipCluster(&weka.WekaClusterTemplate{NumDrives: 4}) // same mode, per-node override + + if errs := v.ValidateUpdate(ctx, c, old, updated); len(errs) != 0 { + t.Errorf("a same-mode edit must not touch the API server at all, got %v", errs) + } +} + +// TestSizingModeFlip_SameModeIgnored: resizing within a mode is never this validator's business. +func TestSizingModeFlip_SameModeIgnored(t *testing.T) { + v := &clusterSizingModeFlip{} + ctx := context.Background() + c := modeFlipClient(t, 3) + + cases := map[string][2]*weka.WekaClusterTemplate{ + "same mode, different counts": { + {ComputeContainers: 6, DriveContainers: 6}, + {ComputeContainers: 8, DriveContainers: 8}, + }, + "same mode, different clusterCapacity": { + {ClusterCapacity: "500TiB"}, + {ClusterCapacity: "800TiB"}, + }, + } + for name, pair := range cases { + t.Run(name, func(t *testing.T) { + errs := v.ValidateUpdate(ctx, c, modeFlipCluster(pair[0]), modeFlipCluster(pair[1])) + if len(errs) != 0 { + t.Errorf("expected no violation, got %v", errs) + } + }) + } +} diff --git a/internal/validation/drive_node_test_helpers_test.go b/internal/validation/drive_node_test_helpers_test.go new file mode 100644 index 000000000..3a00b20d0 --- /dev/null +++ b/internal/validation/drive_node_test_helpers_test.go @@ -0,0 +1,53 @@ +package validation + +import ( + "encoding/json" + "fmt" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/weka/weka-operator/internal/consts" + "github.com/weka/weka-operator/internal/pkg/domain" +) + +// driveRoleNode builds a Node with the given labels (for nodeSelector matching). When +// driveCapacitiesGiB is non-nil, the weka.io/weka-full-drives annotation is set with one synthetic +// signed drive entry per capacity value; driveCapacitiesGiB == nil means "sign-drives hasn't run on +// this node yet" (no annotation at all). +func driveRoleNode(t *testing.T, name string, labels map[string]string, driveCapacitiesGiB []int) *corev1.Node { + t.Helper() + n := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: name, Labels: labels}, + } + if driveCapacitiesGiB != nil { + entries := make([]domain.DriveEntry, 0, len(driveCapacitiesGiB)) + for i, capGiB := range driveCapacitiesGiB { + entries = append(entries, domain.DriveEntry{Serial: fmt.Sprintf("%s-d%d", name, i), CapacityGiB: capGiB}) + } + b, err := json.Marshal(entries) + if err != nil { + t.Fatalf("marshal drive entries: %v", err) + } + n.Annotations = map[string]string{consts.AnnotationWekaFullDrives: string(b)} + } + return n +} + +// fakeClientWithNodes builds a fake client.Client seeded with the given Nodes. +func fakeClientWithNodes(t *testing.T, nodes ...*corev1.Node) client.Client { + t.Helper() + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme: %v", err) + } + b := fake.NewClientBuilder().WithScheme(scheme) + for _, n := range nodes { + b = b.WithObjects(n) + } + return b.Build() +} diff --git a/internal/validation/drive_role_nodes.go b/internal/validation/drive_role_nodes.go new file mode 100644 index 000000000..0547e9819 --- /dev/null +++ b/internal/validation/drive_role_nodes.go @@ -0,0 +1,45 @@ +package validation + +import ( + "context" + "fmt" + + weka "github.com/weka/weka-k8s-api/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/weka/weka-operator/internal/controllers/allocator" +) + +// listDriveRoleNodes lists nodes matching cluster's drive-role nodeSelector — the List step shared by +// every drive validator. On failure it returns a field.InternalError against fldPath; whether to +// surface or discard that error is the caller's choice, not a parameter here. +func listDriveRoleNodes(ctx context.Context, c client.Client, cluster *weka.WekaCluster, fldPath *field.Path) ([]corev1.Node, field.ErrorList) { + selector := cluster.GetNodeSelectorForRole(weka.WekaContainerModeDrive) + var nodes corev1.NodeList + if err := c.List(ctx, &nodes, client.MatchingLabels(selector)); err != nil { + return nil, field.ErrorList{field.InternalError(fldPath, fmt.Errorf("listing drive-role nodes: %w", err))} + } + return nodes.Items, nil +} + +// driveRoleNodeInfo pairs a drive-role node with its parsed AllocatorNodeInfo. +type driveRoleNodeInfo struct { + Node *corev1.Node + Info *allocator.AllocatorNodeInfo +} + +// driveRoleNodeInfos parses AllocatorNodeInfo for each already-fetched node, using +// ParseAllocatorNodeInfo directly instead of re-fetching each one by name. +func driveRoleNodeInfos(nodes []corev1.Node, fldPath *field.Path) ([]driveRoleNodeInfo, field.ErrorList) { + out := make([]driveRoleNodeInfo, 0, len(nodes)) + for i := range nodes { + info, err := allocator.ParseAllocatorNodeInfo(&nodes[i]) + if err != nil { + return nil, field.ErrorList{field.InternalError(fldPath, fmt.Errorf("reading drive info for node %q: %w", nodes[i].Name, err))} + } + out = append(out, driveRoleNodeInfo{Node: &nodes[i], Info: info}) + } + return out, nil +} diff --git a/internal/validation/registry.go b/internal/validation/registry.go index 6d6326195..2d9c5c6be 100644 --- a/internal/validation/registry.go +++ b/internal/validation/registry.go @@ -13,10 +13,20 @@ var ( &clusterCoresAvailable{}, &clusterHugepagesAvailable{}, &clusterSignedDrives{}, + &clusterDrivesUnsignedAdvisory{}, &clusterNetworkEthdevice{}, &clusterDriveComputeCoreRatio{}, + &clusterComputeDriveCoresFloor{}, + &clusterDriveCoresBelowCapacity{}, + &clusterNumDrivesBelowRequiredCores{}, + &clusterAutoFullDrivesPinExceedsNodeDrives{}, + &clusterAutoFullDrivesComputeHugepages{}, + &clusterAutoFullDrivesMinNodes{}, + &clusterCoresPerContainerLimit{}, + &clusterMinContainers{}, &clusterDataservicesFeCores{}, &clusterCapacityProtection{}, + &clusterCapacityMinDriveContainers{}, &clusterCapacityChunkFeasibility{}, &clusterSkipDefaultFs{}, &clusterPodspecSyntax{}, @@ -29,6 +39,7 @@ var ( // Update-only registries: validators that require both old and new objects. WekaClusterUpdate = []UpdateValidator{ &clusterCoresDecrease{}, + &clusterSizingModeFlip{}, } WekaContainerUpdate = []UpdateValidator{ &containerCoresDecrease{},