diff --git a/docs/design/wandb_v2/infra_version_enforcement.md b/docs/design/wandb_v2/infra_version_enforcement.md new file mode 100644 index 00000000..857be83d --- /dev/null +++ b/docs/design/wandb_v2/infra_version_enforcement.md @@ -0,0 +1,182 @@ +# Infrastructure Version Enforcement + +**Status:** Proposed — seeking review +**Author:** Daniel Panzella +**Last updated:** 2026-04-29 + +## 1. Problem + +The `WeightsAndBiases` CR pairs a single W&B server version with five infrastructure components: MySQL, Redis, ClickHouse, Kafka, and object store. Each W&B release is built and tested against a known set of component versions, but the operator does not currently encode any relationship between the two: + +- For **managed** infrastructure (the operator deploys it via vendor operators), no image is pinned. The deployed version drifts to whatever the vendor operator's defaults happen to be at install time, which can be older than what the W&B release was tested against, or newer in ways that introduce protocol incompatibilities. +- For **external** infrastructure (the user supplies connection details), the operator only stores a connection secret and never observes what version is actually running. A user can upgrade W&B against a database that no longer supports the SQL features the new release relies on. + +In both cases, the failure is not visible until the new W&B server image starts up and begins issuing traffic — and even then the symptoms (truncated columns, missing topics, ACL errors, broken aggregations) often look like configuration bugs rather than version mismatches. The problem is most acute during W&B upgrades, because that is when previously-working pairings can silently transition into broken ones. + +## 2. Goals + +- **Prevent unsupported W&B upgrades from being accepted by the API server.** A user attempting to bump `spec.wandb.version` to a release that does not support their currently-running external infra should receive a clear, synchronous rejection. +- **Make managed-infra versioning predictable.** The operator should deploy a known-good version per W&B release, and that version should advance automatically when the W&B release advances. +- **Source of truth lives with the W&B release.** Each server-manifest YAML is authored alongside the W&B build it describes, so it is the natural home for the compatibility statement. +- **Minimal new schema.** Reuse existing manifest sections and CRD status sub-objects; avoid new top-level types. +- **No regressions for existing deployments.** Older manifests without the new fields must continue to work; the operator falls back to today's behavior. + +## 3. Non-goals + +- **Auto-upgrading external infrastructure.** The operator never modifies user-supplied infra. If an external component is too old, the operator surfaces it; the user upgrades. +- **Bounding external infra at an upper version.** Newer-than-target external infra is intentionally allowed. Most users run shared databases that are upgraded on a different cadence than W&B. +- **Per-feature compatibility matrices.** Each component has a single minimum, not a feature-by-feature matrix. Refinements can be added later if needed. +- **Cross-component compatibility (e.g. "this MySQL version requires this Kafka version").** Out of scope. +- **Webhook-time probing of external infra.** Admission must be synchronous and fast; live probes do not fit. The reconciler is the actor that talks to external infra. + +## 4. Design + +### 4.1 Two version concepts, one source + +Each per-component section in the W&B server manifest gains two optional fields: + +- `minVersion` — the floor required of *external* infrastructure. Used by the validating webhook. +- `targetVersion` — the version *managed* infrastructure is deployed at. Used by the operator's vendor-spec builders. + +These are separate fields because they answer different questions ("is this user's infra acceptable?" vs. "what should we deploy?"), even though in practice they are often equal. Letting them differ gives W&B release engineering room to advance the deployed target ahead of the supported floor (e.g., raise `targetVersion` for new deployments while keeping `minVersion` low so that long-lived clusters are not forced to rev immediately). + +```yaml +# Example excerpt from 0.79.0.yaml +mysql: + default: + minVersion: "8.0.36" + targetVersion: "8.0.36" + sizing: { ... } +clickhouse: + default: + minVersion: "24.3" + targetVersion: "24.8" + sizing: { ... } +``` + +Both fields are optional. A manifest that sets neither preserves today's behavior: managed infra is deployed at the vendor-operator default, and external infra is unchecked. + +### 4.2 Three actors, three responsibilities + +```mermaid +flowchart LR + subgraph User + CR[WeightsAndBiases CR] + end + + subgraph Webhook[Validating Webhook] + VU[ValidateUpdate] + end + + subgraph Reconciler + Probe[External probe] + Pin[Managed pin] + Status[Status writer] + end + + subgraph Manifest[Server manifest YAML] + Min[minVersion] + Target[targetVersion] + end + + CR -- update spec.wandb.version --> VU + VU -- read --> Min + VU -- read --> CR + VU -- accept/reject --> CR + + Reconciler -- read --> Manifest + Probe -- detect --> Status + Pin -- read targetVersion --> Manifest + Pin -- write to vendor CR --> Status + Status -- patch --> CR +``` + +**Reconciler — managed components.** When the manifest's `targetVersion` is set, the operator's vendor-spec builders translate it into the appropriate field on the underlying vendor CR (e.g., `InnoDBCluster.Spec.Version`, `Kafka.Spec.Kafka.Version`, image tags on the Altinity / Opstree / MinIO Tenant CRs). The deployed version is then echoed back into `status.{component}Status.version`. Managed-side version is operator-owned: there is no user override. The only existing user-facing knob, `ManagedClickHouseSpec.Version`, is removed as part of this change. v2 is still in alpha, so the field is dropped outright with no deprecation window. This avoids two competing pins. + +**Reconciler — external components.** During each reconcile loop, after reading the connection secret the user supplied, the operator opens a short-lived client (using the component's native protocol), queries the running version, and writes it to `status.{component}Status.version`. Probes are best-effort — failures populate a `VersionProbeReady=False` condition with the error and leave the version field empty rather than failing the whole reconcile. + +**Validating webhook.** Only `ValidateUpdate` participates. When `spec.wandb.version` actually changes, the webhook fetches the *new* manifest (using the existing `manifest.GetServerManifest`, which already caches OCI pulls to a local volume), reads each component's `minVersion`, and compares against the **status** version the reconciler last cached. The webhook is purely a status reader — it does not probe, does not call out to external systems, and runs in the time budget of an admission request. + +Three outcomes per external component: + +| State | Behavior | +|---|---| +| Manifest `minVersion` empty | Skip — no policy. | +| Status `version` empty | Reject — the operator hasn't observed a version yet, so we can't make a claim. The user should wait for the next reconcile cycle. | +| Status `version` < `minVersion` | Reject with both versions named. | +| Status `version` >= `minVersion` | Accept. No upper bound. | + +Failure to fetch the manifest (network error, malformed YAML) is fail-closed: the webhook rejects the update. We would rather block an upgrade than silently allow one we cannot verify. + +### 4.3 First-apply edge case + +When a user first creates a `WeightsAndBiases` CR, `status` is empty — the webhook has no version to compare against. We deliberately do not block creates: most users would hit this on day one, and the legitimate fix (upgrade the database) is one the user can already see in the surfaced not-ready conditions. Instead, the reconciler emits a `VersionBelowMinimum` condition on the affected component, and the W&B application stays not-ready until the user fixes their infra. Subsequent updates do go through the webhook, so the strict path is enforced for the operation that actually motivates this work: upgrades. + +### 4.4 Failure modes and observability + +| Failure | Where | Observable | +|---|---|---| +| External infra unreachable from operator | Reconciler probe | `status.{x}Status.conditions[VersionProbeReady] = False`, error in message | +| External infra version unparseable | Reconciler probe | Same as above; `version` left empty | +| Status version below `minVersion` on update | Webhook | `apierrors.NewInvalid` with both versions named | +| Status version empty on update | Webhook | `apierrors.NewInvalid` asking the user to wait for reconciliation | +| Manifest fetch fails on update | Webhook | `apierrors.NewInvalid` — fail closed | +| `targetVersion` < `minVersion` in manifest | Manifest loader | Logged warning; operator continues. Treated as a release-engineering bug, not a user-actionable error. | + +## 5. Schema additions + +Two small additions; nothing renamed or removed except the deprecated managed-clickhouse version field. + +**Manifest** (in `pkg/wandb/manifest/manifest.go`): + +```go +type ComponentVersions struct { + MinVersion string `yaml:"minVersion,omitempty"` + TargetVersion string `yaml:"targetVersion,omitempty"` +} +// Embedded in InfraConfig and KafkaConfig. +``` + +**Status** (in `api/v2/weightsandbiases_types.go`): + +```go +type WBInfraStatus struct { + Ready bool `json:"ready"` + State string `json:"state,omitempty" default:"Unknown"` + Version string `json:"version,omitempty"` // <- new + Conditions []metav1.Condition `json:"conditions,omitempty"` +} +``` + +**Removal:** `ManagedClickHouseSpec.Version` is removed outright (v2 is still in alpha, so no deprecation window is needed). + +## 6. Backward compatibility + +- **Older manifests** without `minVersion` / `targetVersion` deserialize cleanly; the operator behaves exactly as today. +- **Older CRs with `spec.clickhouse.managedClickhouse.version` set** will fail validation under the new CRD schema. v2 is still alpha, so this is acceptable; affected users update their CR to drop the field. +- **The new `status.{x}Status.version` field is additive.** Older operator versions reading the same CRD will simply not see it. +- **Validating webhook is opt-in by manifest content.** Manifests authored before this change have empty `minVersion`, so the webhook accepts everything (today's behavior). + +## 7. Alternatives considered + +**Top-level `infraVersionRequirements:` block in the manifest.** Rejected: separates the version statement from the rest of the per-component config it relates to, and forces a parallel structure (sized component lookups by component name + instance). Inlining keeps one section per component. + +**One field instead of two (just `minVersion`, with managed deploys reading from it).** Rejected: conflates "what the W&B release supports" with "what we deploy by default." Two fields let release engineering advance the deployed target ahead of the supported floor without forcing existing clusters to upgrade their infra in lockstep. + +**Bounded `[minVersion, maxVersion]` window for external infra.** Considered and rejected by the design owner. Most external databases are operated independently and may legitimately run ahead of any version W&B has tested. An upper bound creates pointless rejections for the common case where newer infra is fine. If specific newer versions break, that's a `minVersion` bump for the *next* release, not a ceiling on the current one. + +**Probing in the webhook.** Rejected: admission requests must be fast and cannot reliably reach customer-private databases. The reconciler is the right actor; the webhook reads the cached observation. + +**Hardcoding the requirements in operator code.** Rejected: would couple every requirement bump to an operator release. The manifest is already the per-W&B-release artifact and is the natural home. + +**Letting users override the managed-infra version.** Rejected by the design owner. Two pins create ambiguity, drift, and support burden. If a user needs a specific managed version, the path is to update the W&B server manifest for the release they are deploying. + +## 8. Open questions + +- **MinIO/S3 version reporting.** Real S3 has no version. For MinIO we can read the `Server` header; for AWS/GCS we should record a marker (e.g. `"s3"` or `"gcs"`) and skip the `minVersion` check. The exact marker scheme should be agreed before implementation. +- **Kafka version detection.** `ApiVersions` returns supported API ranges, not a clean broker version string. We will likely have to derive a representative version from the highest supported APIs, or use vendor-specific endpoints. Open as to which library to use. +- **Probe credentials.** The probe uses the same connection secret the W&B applications use. Some users may want to provide a read-only credential specifically for version probing — out of scope for v1, but a possible extension. + +## 9. Implementation status + +A detailed implementation plan with exact file paths and dependencies lives in [plan-infra-version-enforcement.md](../../plan-infra-version-enforcement.md). diff --git a/docs/design/wandb_v2/multi_instance_infra.md b/docs/design/wandb_v2/multi_instance_infra.md new file mode 100644 index 00000000..02c32b96 --- /dev/null +++ b/docs/design/wandb_v2/multi_instance_infra.md @@ -0,0 +1,245 @@ +# Multi-Instance Infrastructure (MySQL, Redis, ObjectStore, ClickHouse) + +## Status + +Implemented (alpha). Kafka is intentionally **out of scope** and remains +single-instance. + +## Context + +The W&B v2 API originally modeled exactly **one** instance of each backing +infrastructure type. `Spec.MySQL`, `Spec.Redis`, `Spec.ObjectStore`, and +`Spec.ClickHouse` were each a single struct, and each had a single matching +status field carrying one connection. + +We want a single W&B deployment to provision and connect to **multiple** +instances of each of these four types — for example a primary MySQL plus a +separate analytics MySQL — and to route individual applications to the correct +instance. This document describes the design that delivers that capability and +the conventions future revisions should preserve. + +## Goals + +- Allow N instances per infra type (MySQL, Redis, ObjectStore, ClickHouse). +- Let the **server manifest** decide which application connects to which + instance. +- Guarantee a safe fallback: an app that requests an unprovisioned instance + resolves to a well-known **default** instance instead of failing. +- Keep the change surface to the operator; no new external coordination. + +## Non-goals + +- Kafka multi-instance (deliberately deferred). +- Backwards-compatible data migration of existing v2 CRs. The project is in + alpha, so the v2 schema change is allowed to be breaking. (The v1→v2 + conversion webhook still compiles and maps the single v1 infra into the + `default` instance.) + +## Design overview + +### The map model + +Each of the four infra types becomes a **map keyed by instance name**. A +reserved key, `apiv2.DefaultInstanceName` (`"default"`), identifies the fallback +instance. + +```go +// api/v2/weightsandbiases_types.go +type WeightsAndBiasesSpec struct { + MySQL map[string]MySQLSpec `json:"mysql,omitempty"` + Redis map[string]RedisSpec `json:"redis,omitempty"` + Kafka KafkaSpec `json:"kafka,omitempty"` // unchanged + ObjectStore map[string]ObjectStoreSpec `json:"objectStore,omitempty"` + ClickHouse map[string]ClickHouseSpec `json:"clickhouse,omitempty"` + // ... +} +``` + +Status mirrors the spec, keyed by the same instance names: + +```go +type WeightsAndBiasesStatus struct { + MySQLStatus map[string]MysqlInfraStatus `json:"mysqlStatus,omitempty"` + RedisStatus map[string]RedisInfraStatus `json:"redisStatus,omitempty"` + KafkaStatus KafkaInfraStatus `json:"kafkaStatus,omitempty"` // unchanged + ObjectStoreStatus map[string]ObjectStoreInfraStatus `json:"objectStoreStatus,omitempty"` + ClickHouseStatus map[string]ClickHouseInfraStatus `json:"clickhouseStatus,omitempty"` + // ... +} +``` + +`Status.Wandb.MySQLInit` likewise became `map[string]MigrationJobStatus`, since +the database-init job now runs per managed MySQL instance. + +### The `default` instance and fallback + +The defaulting webhook guarantees that any infra type with at least one instance +also has a `default` instance (and seeds a managed `default` when the map is +empty). The validating webhook rejects a CR that defines instances for a type +but omits `default`. This invariant is what makes fallback always resolvable. + +Resolution is centralized in a single generic helper: + +```go +// api/v2/weightsandbiases_types.go +func ResolveInstance[T any](m map[string]T, key string) (T, bool) { + if key == "" { key = DefaultInstanceName } + if v, ok := m[key]; ok { return v, true } + if v, ok := m[DefaultInstanceName]; ok { return v, true } // fallback + var zero T; return zero, false +} +``` + +### Application → instance mapping (server manifest) + +The mapping of an application's env var to a specific infra instance lives in the +**server manifest**, carried in the existing `EnvSource.Name` field. For the +`mysql`/`redis`/`clickhouse`/`bucket` source types this field was previously +unused, so no manifest schema change was required. + +```yaml +# server manifest env var: route this app to the "analytics" MySQL instance +- name: ANALYTICS_MYSQL + sources: + - type: mysql + name: analytics # instance key; empty => "default" +``` + +`resolveEnvvars` (in `internal/controller/reconciler/pods.go`) reads +`src.Name`, looks up the instance status via `ResolveInstance`, and emits the +connection secret reference exactly as before. An empty or unknown `name` +resolves to `default`. + +```mermaid +flowchart LR + Manifest[Manifest EnvSource
type=mysql, name=analytics] + Resolve["ResolveInstance(Status.MySQLStatus, "analytics")"] + Named[MySQLStatus[analytics]] + Default[MySQLStatus[default]] + Env[Container EnvVar
ValueFrom: analytics-conn secret] + + Manifest --> Resolve + Resolve -->|present| Named + Resolve -->|absent| Default + Named --> Env + Default --> Env +``` + +## Reconciliation + +The top-level flow (`reconcile_v2.go`) is unchanged in shape — finalize → write +→ read → infer status → reconcile manifest — but each infra phase now **loops +over the instance map** instead of acting on a single struct. + +```mermaid +flowchart TD + subgraph Write[Write Infra State] + direction LR + WM["for key, spec := range Spec.MySQL"] + end + subgraph Read[Read Infra State] + direction LR + RM["for key, spec := range Spec.MySQL"] + end + subgraph Infer[Infer Status] + direction LR + IM["Status.MySQLStatus[key] = ..."] + end + Write --> Read --> Infer +``` + +Per-type orchestration (`mysql.go`, `redis.go`, `objectstore.go`, +`clickhouse.go`) follows a consistent pattern: + +- `xWriteState` / `xReadState` return `map[string]…` keyed by instance. +- `xInferStatus` writes each instance's status into `Status.Status[key]` and + consolidates the per-instance `ctrl.Result`s via `consolidateResults`. +- A per-type `runXRetentionFinalizer(ctx, c, wandb, key, spec)` applies the + configured retention policy to each managed/external instance during deletion. + +The lower-level vendor packages (moco, opstree, altinity, seaweedfs) already +operated on an explicit `(spec, NamespacedName)`; they now receive the managed +instance spec as a parameter rather than reading the singular field off the CR. + +### Readiness + +Overall readiness aggregates across **all** instances of every map-based type +(Kafka remains a single check). Two helpers express the two slightly different +semantics already present in the codebase: + +- `allInstancesReady` — used for requeue gating; every instance (managed or + external) must report `Ready`. +- `managedInstancesReady` — used by `inferState` for the top-level + `Status.Ready`; only **managed** instances gate readiness (external/absent are + treated as ready), preserving prior behavior. + +A type with no instances is trivially ready. + +## Resource naming + +To keep multiple instances from colliding while preserving the existing +single-instance resource names, the **default instance keeps the historical +name** and non-default instances are suffixed with their key. + +| Concern | Default instance | Named instance (`analytics`) | +| --- | --- | --- | +| Managed resource name (defaulter) | `-mysql` | `-mysql-analytics` | +| External connection secret | `wandb-mysql-connection` | `wandb-mysql-connection-analytics` | +| MySQL init job | `-mysql-moco-init` | `-mysql-analytics-moco-init` | +| Infra HTTPRoute suffix | `` | `analytics-` | + +> Note: the MySQL init job name changed from `-moco-init` to +> `-moco-init` (i.e. `-mysql-moco-init` for the default instance), +> because it is now per managed instance. + +## Sizing + +`ApplyInfraSizing` iterates each managed-infra map and applies the resolved +`SizingConfig`. The manifest's per-type sizing map is consulted with the same +fallback rule: prefer the manifest config matching the CR instance key, falling +back to the manifest `default` config (`infraSizingConfig`). + +## Validation & defaulting summary + +| Webhook | Behavior | +| --- | --- | +| Defaulter | Seeds a managed `default` when a type's map is empty; per instance, ensures `ManagedX` when no `ExternalX`, and defaults `Name`/`Namespace`. Redis sets Sentinel when `Size != dev`; ObjectStore defaults `AccessKey`. | +| Validator | Per-instance mutual-exclusion (`Managed` XOR `External`); rejects any type that defines instances but no `default` key; per-instance Redis change-immutability checks. | + +## Key files + +| Area | File | +| --- | --- | +| Types + `ResolveInstance` | `api/v2/weightsandbiases_types.go` | +| Defaulter / validator | `internal/webhook/v2/weightsandbiases_webhook.go` | +| Top-level reconcile + readiness | `internal/controller/reconciler/reconcile_v2.go` | +| Per-type orchestration | `internal/controller/reconciler/{mysql,redis,objectstore,clickhouse}.go` | +| Env var routing | `internal/controller/reconciler/pods.go` | +| Sizing | `internal/controller/reconciler/sizing.go` | +| Infra networking | `internal/controller/reconciler/{gateway,infra_routes}.go` | +| External connection secrets | `internal/controller/infra/external/*/*.go` | +| v1→v2 conversion (→ `default`) | `api/v1/weightsandbiases_conversion_mapping.go` | + +## Trade-offs & alternatives considered + +- **Full map vs. "singular default + additional map".** We chose a full map with + a reserved `default` key. It is a breaking schema change but yields a single, + uniform model with no special-case "primary" field. Acceptable because v2 is + alpha and no data migration is required. +- **Instance selection in the manifest vs. the CR.** We put the + application→instance mapping in the server manifest (`EnvSource.Name`) so the + application bundle owns its own routing, rather than requiring cluster admins + to maintain an app→instance table in the CR. +- **Kafka excluded.** Kafka topic/broker provisioning has more cross-cutting + logic; multi-instance Kafka was deferred to keep this change focused. + +## Future considerations + +- If Kafka multi-instance is needed, mirror this pattern: map-ify + `Spec.Kafka`/`Status.KafkaStatus`, loop the orchestration, and extend + `resolveEnvvars`' `kafka` case to honor `src.Name`. +- The manifest's per-type sizing/config maps and the CR instance maps are + currently correlated only by key with a `default` fallback. A future revision + may want first-class per-instance manifest config keyed to CR instance names. +- Legacy MinIO cleanup is scoped to the CR (pre-multi-instance) and therefore + only runs for the `default` ObjectStore instance. diff --git a/docs/design/wandb_v2/version_compatibility.md b/docs/design/wandb_v2/version_compatibility.md new file mode 100644 index 00000000..3ede5650 --- /dev/null +++ b/docs/design/wandb_v2/version_compatibility.md @@ -0,0 +1,265 @@ +# Operator ↔ Server-Manifest Version Compatibility + +**Status:** Draft / proposed +**Area:** `pkg/wandb/manifest`, `internal/webhook/v2`, operator version embedding + +## Summary + +The operator and the W&B server manifest evolve independently and are released +on different cadences. Certain operator changes require a newer manifest, and +certain manifest changes require a newer operator. Today nothing enforces this: +the manifest carries a `requiredOperatorVersion` field and a `manifestVersion` +field, but the operator parses neither (`manifestVersion` isn't even on the +struct) and validates neither. + +This document proposes a **two-sided compatibility contract** enforced at +**admission time** by the validating webhook, **fail-closed**, using both +existing fields: + +- `requiredOperatorVersion` — the manifest declares which operator versions can + process it (semver constraint). +- `manifestVersion` — a coarse, monotonic integer schema-contract version; the + operator embeds the **explicit set** of manifest versions it supports. + +## Problem statement + +Compatibility breaks in **both directions**: + +1. **Operator needs a newer manifest.** We ship an operator change that depends + on manifest content/shape that older manifests don't have. Users must not be + able to pin an older manifest against the new operator. +2. **Manifest needs a newer operator.** Upstream makes a backwards-incompatible + change to the manifest. Older operators must refuse to install it rather than + mis-reconcile. + +A single one-directional field cannot express both — each side must be able to +state a requirement about the other. + +### Three version axes (don't conflate them) + +| Axis | Example | Owned by | Changes | +|------|---------|----------|---------| +| **Operator version** | `2.1.0` | this repo (git tag / chart `appVersion`) | per operator release | +| **W&B server version** (`spec.wandb.version`) | `0.79.0` | upstream W&B releases | frequently | +| **Manifest schema version** (`manifestVersion`) | `3` | upstream manifest generator | rarely (only on structural breaks) | + +Compatibility is gated on **operator version ↔ manifest schema version**, *not* +on the W&B server version. Server versions churn constantly; we deliberately do +**not** want to maintain a per-server-version compatibility table. The manifest +schema version is the slow-moving structural contract — that is the correct +thing to gate on. + +## Goals + +- Express and enforce compatibility in both directions. +- Fail fast and legibly at `kubectl apply` time (admission rejection with a + human-readable reason), so a bad version pin never reaches reconciliation. +- Keep the common case zero-friction: once upstream cuts compatible manifests, + nothing special is required of users. +- Avoid maintaining a per-server-version matrix. + +## Non-goals + +- Gating on the W&B **server** version itself. +- Auto-selecting a compatible version for the user (we reject; we don't rewrite). +- Runtime/reconcile-time enforcement as the primary gate (see "Enforcement"). + +## Design + +### The two-sided contract + +Two complementary fields, with a clear division of labor. + +#### `manifestVersion` — monotonic integer schema-contract version + +- Type: **integer** (`1`, `2`, `3`, …). Replaces the current stringly-typed + `v1alpha1` value seen in test manifests. +- Bumped **only** on a backwards-incompatible change to the manifest *structure* + — a new required field the operator must understand, a renamed/removed field, + a changed meaning. These are expected to be **rare**. +- The operator embeds an **explicit set** of supported versions, e.g. + `CompatibleManifestVersions = {2, 3}` — *not* a floor/ceiling range. A discrete + set is simple, exact, and avoids implying support for versions we never tested. + It also lets us drop support for an old version (remove `2` from the set) + independently of adding a new one. + +This single field handles **both directions at the structural-break granularity**: + +- Manifest `manifestVersion` ∉ operator's set, and it's **higher** than anything + we know → manifest is too new, operator too old → reject (direction 2). +- Manifest `manifestVersion` ∉ operator's set, and it's **lower** than our + minimum → operator dropped support for that shape → reject (direction 1). + +#### `requiredOperatorVersion` — semver constraint on the operator + +- Type: **semver constraint string** (e.g. `>=2.3.0 <3.0.0`), parsed with + `github.com/Masterminds/semver/v3` (already in `go.mod`). +- The manifest declares the operator versions that can correctly process it; the + operator checks its **own embedded version** against the constraint. +- This is the **fine-grained** lever. Not every "you need a newer operator" + requirement is a structural manifest break. Example: the operator fixes how it + provisions Kafka and a manifest relies on the new behavior — the manifest + *structure* is unchanged (`manifestVersion` stays the same) but we still want + to require operator `>=2.4.0`. `manifestVersion` can't express that; + `requiredOperatorVersion` can. + +#### Why both + +| Concern | Field | Granularity | Owned by | +|---------|-------|-------------|----------| +| Structural contract of the manifest document | `manifestVersion` | coarse, integer set | operator (the set) + manifest (its value) | +| Minimum/range operator *behavior* a manifest depends on | `requiredOperatorVersion` | fine, semver | manifest | + +Both checks must pass (logical AND). + +### Operator self-version + +The binary currently embeds no version. Introduce a small package, e.g. +`internal/version` (or `pkg/version`), exposing: + +```go +package version + +// Version is the operator's semantic version, injected at build time. +// Falls back to a sentinel for `go run` / tests. +var Version = "0.0.0-dev" +``` + +Injected via linker flag in the Dockerfile/Makefile, driven by the chart +`appVersion` or git tag: + +``` +go build -ldflags "-X github.com/wandb/operator/internal/version.Version=${VERSION}" ./cmd/manager +``` + +Both the webhook and (later, if desired) the reconciler read +`version.Version`. The same value should be surfaced in logs at startup. + +### Prerelease handling (important) + +`github.com/Masterminds/semver/v3` does **not** match prereleases against range +constraints by default: `2.0.0-alpha.2` does **not** satisfy `^2.0.0` or +`>=2.0.0`. The operator chart is currently `2.0.0-alpha.2`, so a naive check +would reject every manifest today. + +**Policy:** compare on the **release version with prerelease/build metadata +stripped**. `2.0.0-alpha.2` is treated as `2.0.0` for the purpose of the +`requiredOperatorVersion` check. Implementation: parse the operator version, +take `Major/Minor/Patch`, and check that finalized version against the +constraint (or use a constraint built with prerelease-inclusive options). This +keeps alpha/rc builds usable during development while preserving range +semantics. Document this clearly so upstream constraint authors know prereleases +are floored to their release version. + +### Strict / fail-closed semantics + +Both fields are **required and must be valid**. The manifest is rejected if: + +- `manifestVersion` is absent, ≤ 0, or non-integer. +- `requiredOperatorVersion` is absent or not a parseable semver constraint. +- `manifestVersion` ∉ `CompatibleManifestVersions`. +- the operator's (prerelease-floored) version does not satisfy + `requiredOperatorVersion`. + +This means **every currently-published manifest must be re-cut** to carry both +fields before it will admit. Given v2 is still in alpha, this is acceptable and +is called out in Migration below. + +## Enforcement: validating webhook (admission) + +Enforcement lives in the existing validating webhook +(`internal/webhook/v2/weightsandbiases_webhook.go`), invoked on **create and +update** — which is exactly when a user pins or changes `spec.wandb.version` / +`spec.wandb.manifestRepository`. A new `validateManifestCompatibility(ctx, wandb)` +is added to the `validateSpec` orchestrator. Returning an `error` rejects +admission and the message is shown directly to the user at `kubectl apply`. + +### Why webhook-only is feasible here + +The obvious objection to admission-time enforcement is that it must **fetch the +OCI manifest** to read the two fields. That is acceptable because: + +- The webhook runs **in-process** in the operator pod — same network egress and + registry credentials as the reconciler. +- `manifest.GetServerManifest` already **caches** pulled artifacts into a local + OCI store (`/tmp/server-manifest`). The first admission of a given + `(repository, version)` pulls; subsequent admissions hit the cache. +- Reconciliation pulls the same manifest moments later, so the webhook fetch + warms the cache it will use anyway. + +### Failure modes + +Because we are fail-closed: + +| Situation | Result | Message | +|-----------|--------|---------| +| Operator version not in `requiredOperatorVersion` | **reject** | "manifest \ requires operator \; this operator is \. Upgrade the operator or select a compatible manifest version." | +| `manifestVersion` not in operator's set | **reject** | "manifest \ uses schema version N; this operator supports {…}. …" | +| Field missing / unparseable | **reject** | field-path error via `field.Invalid` | +| **Registry unreachable / pull fails** | **reject** | "could not fetch manifest \:\ to validate compatibility: \" | + +The last row is the operationally significant one: a registry outage will +**block CR applies/updates** (including unrelated spec edits, since the webhook +fires on every update). Mitigations: + +- The local OCI cache means previously-validated versions still admit offline. +- Apply a bounded timeout to the webhook fetch so admission fails fast with a + clear message rather than hanging. +- Existing CRs continue reconciling regardless — admission only gates *new + writes*, not the running deployment. + +This trade-off is inherent to the "webhook-only" decision and is accepted; it is +documented here so the on-call behavior is not surprising. + +## Implementation plan + +1. **`pkg/wandb/manifest/manifest.go`** — add `ManifestVersion int` to the + `Manifest` struct (key `manifestVersion`; matched case-insensitively by + `sigs.k8s.io/yaml`). Handle it in `mergeSimple` (take the first non-zero, and + detect conflicting values across merged files). +2. **`internal/version/version.go`** — new package with the build-injected + `Version` var. +3. **Build wiring** — `-ldflags -X` in `Dockerfile`/`Dockerfile.cross` and the + relevant `Makefile` build targets, sourced from chart `appVersion` or git tag. +4. **`pkg/wandb/manifest/compat.go`** (new) — the compatibility logic: + - `CompatibleManifestVersions` set (the operator-owned source of truth). + - `CheckCompatibility(operatorVersion string, m Manifest) error` doing both + checks with prerelease flooring. Pure and unit-testable, no I/O. +5. **`internal/webhook/v2/weightsandbiases_webhook.go`** — add + `validateManifestCompatibility`: fetch via `GetServerManifest`, then call + `manifest.CheckCompatibility(version.Version, m)`; wire into `validateSpec`. +6. **Re-cut local fixtures** under `hack/testing-manifests/server-manifest/*` + with integer `manifestVersion` and a valid `requiredOperatorVersion`. +7. **Tests** — table-driven unit tests for `CheckCompatibility` (in-set, + out-of-set high/low, satisfied/unsatisfied constraint, prerelease operator, + missing/garbage fields); webhook tests for accept/reject and the + registry-unreachable path. +8. **Docs** — `docs/config-api.md` and an upstream note in wandb/core's + `onprem/server-manifest` generator describing the contract and the integer + `manifestVersion` bump policy. + +## Worked examples + +- **Operator `2.4.0`, manifest `manifestVersion: 3`, `requiredOperatorVersion: ">=2.4.0 <3.0.0"`**, operator set `{2,3}` → admit. +- **Operator `2.3.0`** with the same manifest → reject: operator below + `requiredOperatorVersion` floor (direction 2, fine-grained). +- **Operator `2.4.0` supporting `{3,4}`, manifest `manifestVersion: 2`** → reject: + schema too old, support dropped (direction 1). +- **Operator `2.4.0` supporting `{2,3}`, manifest `manifestVersion: 4`** → reject: + schema too new for this operator (direction 2, structural). +- **Manifest missing `manifestVersion`** → reject (fail-closed). + +## Open questions + +1. **Source of the build version** — git tag vs chart `appVersion`. They should + not drift; pick one as canonical (recommend git tag, with CI asserting the + chart matches). +2. **`manifestVersion` set maintenance** — where does the canonical set live and + how is dropping an old version reviewed? Proposal: a single constant in + `compat.go` with a comment block documenting each version's meaning. +3. **Conversion / older OCI tags** — do we backfill the two fields into already + published manifests, or only enforce for versions cut after this lands? With + fail-closed, un-backfilled old versions become unschedulable; confirm that's + intended. +4. **Webhook timeout value** — what bound on the OCI fetch keeps admission + responsive without flaking on a cold cache + slow registry? diff --git a/docs/plan-infra-version-enforcement.md b/docs/plan-infra-version-enforcement.md new file mode 100644 index 00000000..f664ad7c --- /dev/null +++ b/docs/plan-infra-version-enforcement.md @@ -0,0 +1,266 @@ +# Plan: Enforce minimum infrastructure versions on W&B upgrades + +## Context + +When a user bumps `spec.wandb.version` on a `WeightsAndBiases` CR, the new W&B server image requires its dependent infrastructure (MySQL, Redis, ClickHouse, Kafka, MinIO/object store) to be at or above a known minimum version, and managed deployments need a concrete pin so that the operator's own infrastructure rolls forward in lockstep with W&B. Today the operator has no notion of either: + +- **Managed infra** (operator-deployed) is created from vendor-spec builders with no image pin — versions float to whatever the underlying vendor operator defaults to. +- **External infra** (user-supplied) is stored as connection secrets only; the operator never knows what version the user is actually pointing at. + +This means an upgrade can succeed at apply time and then fail at runtime with cryptic SQL/Redis/Kafka errors. + +The goal is to make the W&B server manifest the source of truth for two per-component versions so that: + +1. For **managed** components, the reconciler deploys the manifest's **target version** (the version the W&B release is built against). This keeps managed deployments predictable and lets W&B advance the floor by simply publishing a new manifest. +2. For **external** components, the reconciler probes the live component each loop and writes the detected version into `status.{component}Status.version`. The validating webhook then compares `status` against the new manifest's **minimum version** on `spec.wandb.version` updates and **hard-rejects** the update if the user-supplied infra is below it. There is no upper bound on user-supplied infra: a user running a newer-than-target version is intentionally supported. (The webhook can't probe — admission must be fast and synchronous — so it relies on the version the reconciler last cached into status.) + +There is one edge case the webhook can't catch: the very first apply, where status is empty. That case is left to the reconciler, which surfaces a not-ready condition with a clear message. + +## Design summary + +| Concern | Where versions live | +|---|---| +| Minimum supported version per component, per W&B release | New `minVersion` field in each per-component manifest section | +| Managed deployed version (image tag) | New `targetVersion` field in each per-component manifest section, consumed by spec builders | +| External detected version | New `version` field on each `*InfraStatus` in the CRD status | +| Enforcement on update | Validating webhook compares status `version >= minVersion` | + +Key decisions: + +- Requirements live **on the existing per-component sections** in the manifest (`Bucket`, `Clickhouse`, `Mysql`, `Redis`, `Kafka`), not in a new top-level section. +- Two fields per component: **`minVersion`** (the floor enforced for external infra) and **`targetVersion`** (what managed infra is deployed at). +- External version detection uses **active protocol probes** during reconcile. +- Managed components deploy the **manifest's `targetVersion`** unconditionally; user overrides on the CR are not honored. +- Webhook **hard-rejects** updates whose external infra version is below `minVersion`. No upper bound — newer is always allowed for external infra. + +`github.com/Masterminds/semver/v3` is already a direct dependency — used for parsing and comparison. + +--- + +## 1. Manifest changes — `pkg/wandb/manifest/manifest.go` + +Add two optional version fields, grouped in a small embedded struct, to each existing component config: + +- **`MinVersion`** is the floor enforced for external infra (webhook check). Empty means "no minimum". +- **`TargetVersion`** is the version managed infra is deployed at (spec builders consume it). Empty preserves today's behavior of letting the underlying vendor operator pick a default. + +```go +// ComponentVersions declares the version policy for an infrastructure +// component for a given W&B release. MinVersion is the floor required of +// external/user-supplied components; the validating webhook rejects +// upgrades when the live external version is below it. TargetVersion is +// the version managed deployments are pinned to by the spec builders. +// Both are exact semver strings (e.g. "8.0.36"); pre-release suffixes +// are allowed. Either may be empty to disable that role. +type ComponentVersions struct { + MinVersion string `yaml:"minVersion,omitempty"` + TargetVersion string `yaml:"targetVersion,omitempty"` +} + +type InfraConfig struct { + Sizing map[v2.Size]SizingConfig `yaml:"sizing"` + Ingress *AppIngressSpec `yaml:"ingress,omitempty"` + ComponentVersions `yaml:",inline"` +} + +type KafkaConfig struct { + Sizing map[v2.Size]KafkaSizingConfig `yaml:"sizing"` + Topics []KafkaTopic `yaml:"topics"` + ComponentVersions `yaml:",inline"` +} +``` + +Update `mergeSimple` / `mergeInfraConfigs` to carry `MinVersion` and `TargetVersion` through (preserve dst when set, else copy from src — same precedence as today). + +A typical manifest entry for `0.79.0.yaml` will gain: + +```yaml +mysql: + default: + minVersion: "8.0.36" + targetVersion: "8.0.36" + sizing: {...} +clickhouse: + default: + minVersion: "24.3" + targetVersion: "24.8" + sizing: {...} +kafka: + minVersion: "3.6" + targetVersion: "3.7.1" + sizing: {...} +``` + +No new top-level field, no new collections. Backward compatible — empty fields mean "no policy" and older manifests deserialize cleanly. Sanity check: `TargetVersion` should be `>= MinVersion` if both are set; the manifest loader logs a warning if not (the manifest is W&B-authored, so this is a release-engineering smell, not a user-actionable error). + +--- + +## 2. CRD status changes — `api/v2/weightsandbiases_types.go` + +Add a `Version` field to the shared base `WBInfraStatus` so it appears on every component status: + +```go +type WBInfraStatus struct { + Ready bool `json:"ready"` + State string `json:"state,omitempty" default:"Unknown"` + Version string `json:"version,omitempty"` + Conditions []metav1.Condition `json:"conditions,omitempty"` +} +``` + +Single, semver-formatted string (e.g. `"8.0.36"`, `"3.6.1"`, `"24.3.1.2305"`). Empty means "not yet detected" or "probe failed" (the reconciler will set a condition explaining why). + +This field is populated: +- For **managed** components: from the deployed image tag (the spec builder already knows it). +- For **external** components: from the live protocol probe. + +The `+kubebuilder:printcolumn` markers stay as they are; we don't need a column for version. + +Run `make manifests generate` to regenerate the CRD YAML and `zz_generated.deepcopy.go`. + +Also: **remove the existing `Version` field on `ManagedClickHouseSpec`** at [api/v2/weightsandbiases_types.go:459](api/v2/weightsandbiases_types.go:459). It is the only managed spec with such a field today, and once the manifest is the source of truth, leaving it would create two competing pins. v2 is still in alpha, so we delete it outright with no deprecation window. + +--- + +## 3. Reconciler changes + +### 3a. Managed: pin the deployed version to the manifest's target + +The reconciler already fetches the manifest at [internal/controller/v2/reconcile_v2.go:272](internal/controller/v2/reconcile_v2.go:272) and passes it into the per-component flows. The vendor-specific spec builders live under `internal/controller/infra/managed/{component}/{vendor}/spec.go`: + +- [internal/controller/infra/managed/mysql/mysql/spec.go](internal/controller/infra/managed/mysql/mysql/spec.go) — `ToMysqlMySQLVendorSpec` +- [internal/controller/infra/managed/redis/opstree/spec.go](internal/controller/infra/managed/redis/opstree/spec.go) — `ToRedis…VendorSpec` +- [internal/controller/infra/managed/kafka/strimzi/spec.go](internal/controller/infra/managed/kafka/strimzi/spec.go) — `ToKafkaVendorSpec` / `ToKafkaNodePoolVendorSpec` +- [internal/controller/infra/managed/clickhouse/altinity/spec.go](internal/controller/infra/managed/clickhouse/altinity/spec.go) — `ToClickHouseVendorSpec` +- [internal/controller/infra/managed/minio/tenant/spec.go](internal/controller/infra/managed/minio/tenant/spec.go) — MinIO Tenant builder + +For each managed component: + +1. Plumb the manifest's per-component `TargetVersion` from the v2 reconciler dispatch layer ([internal/controller/v2/{mysql,redis,kafka,clickhouse,objectstore}.go](internal/controller/v2)) into the corresponding `To*VendorSpec` builder. The reconciler already has the parsed `Manifest` in scope at the dispatch site; thread the relevant `ComponentVersions` through the existing `managed*WriteState` functions (`managedMysqlWriteState` at [internal/controller/v2/mysql.go:100](internal/controller/v2/mysql.go:100), and the equivalents in the other component files). +2. The builder translates `TargetVersion` into the appropriate field on the underlying vendor CR — e.g. `InnoDBCluster.Spec.Version` (mysql-operator), `Kafka.Spec.Kafka.Version` (Strimzi), the Altinity ClickHouseInstallation image tag, the Opstree Redis image tag, the MinIO Tenant image tag. +3. **Managed-side version is operator-owned.** The manifest's `TargetVersion` is always used; user overrides on the CR are not honored for managed components. The existing `ManagedClickHouseSpec.Version` field is removed (see §2). +4. Write the resulting deployed version into `wandb.Status.{Component}Status.Version` during the existing `*InferStatus()` call. + +For components without an existing CRD `Version` field (everything except ClickHouse), no spec change is needed — the builder pins via the underlying vendor CR's image/version field. If the manifest leaves `TargetVersion` empty (older manifests), preserve today's behavior of letting the vendor operator pick a default. + +### 3b. External: probe the live component for its version + +Add a small per-component probe helper at `internal/controller/infra/external/{component}/probe.go`. Each helper: + +1. Resolves the connection secret already written by `WriteState` (reuse `external.ReadConnectionSecret` and the existing per-component decoders that build the typed connection struct in `*ReadState`). +2. Opens a short-lived client, queries the version, closes. +3. Returns `(version string, err error)`. + +| Component | Library | Query | +|---|---|---| +| MySQL | `database/sql` + `github.com/go-sql-driver/mysql` (add) | `SELECT VERSION()` | +| ClickHouse | `github.com/ClickHouse/clickhouse-go/v2` (add) | `SELECT version()` | +| Redis | `github.com/redis/go-redis/v9` (add) | `INFO server` -> parse `redis_version` | +| Kafka | `github.com/twmb/franz-go` (add) | `ApiVersions` request -> derived broker version, or `DescribeClusterRequest` | +| Object store | `github.com/minio/minio-go/v7` (already vendored) | `HEAD /` (S3 list-buckets); parse `Server` header for MinIO; for AWS/GCS, version isn't applicable — record `"s3"` | + +Wire each probe into the existing `*ReadState` -> `*InferStatus` flow under the external branch. The probe runs only when the connection secret is present (otherwise we have nothing to dial). Probe failures degrade gracefully: + +- Set `WBInfraStatus.Conditions` with type `VersionProbeReady` = False and the error. +- Leave `WBInfraStatus.Version` empty (or last-known — preserve through the existing condition merge if helpful). +- Don't fail the whole reconcile — the rest of the loop continues, and the webhook treats empty as "unknown" (which is itself a rejection signal on update — see §4). + +Probe timeouts must be tight (e.g. 5s) so a flaky external infra doesn't stall the reconcile loop. + +Files most affected: + +- [internal/controller/v2/mysql.go](internal/controller/v2/mysql.go) — call probe in `externalMysqlInferStatus` +- [internal/controller/v2/redis.go](internal/controller/v2/redis.go) — same +- [internal/controller/v2/kafka.go](internal/controller/v2/kafka.go) +- [internal/controller/v2/clickhouse.go](internal/controller/v2/clickhouse.go) +- [internal/controller/v2/objectstore.go](internal/controller/v2/objectstore.go) +- New: `internal/controller/infra/external/{mysql,redis,kafka,clickhouse,objectstore}/probe.go` +- Optionally extend [internal/controller/infra/external/common.go](internal/controller/infra/external/common.go) with a shared `InferExternalStatusWithVersion` helper that wraps the existing `InferExternalStatus` and folds in the probed version. + +--- + +## 4. Webhook changes — `internal/webhook/v2/weightsandbiases_webhook.go` + +The validating webhook today is empty-struct only. To enforce minimums on update: + +1. **Validator dependencies** — `WeightsAndBiasesCustomValidator` only needs additional fields if other validations require injected state (e.g. logger, optional `client.Reader`). The version-check uses `manifest.GetServerManifest`, which is package-level. Today's call sites at [cmd/main.go:359](cmd/main.go:359), [internal/webhook/v2/webhook_suite_test.go:115](internal/webhook/v2/webhook_suite_test.go:115), and [internal/controller/suite_test.go:159](internal/controller/suite_test.go:159) stay unchanged unless dependencies are added. + +2. **New validation step** invoked from `ValidateUpdate` only — Create has no status to compare, and the reconciler will surface a not-ready condition on first deploy: + - If `oldWandb.Spec.Wandb.Version == newWandb.Spec.Wandb.Version`, skip — only enforce on actual upgrades. + - Fetch the new manifest: `manifest.GetServerManifest(ctx, newWandb.Spec.Wandb.ManifestRepository, newWandb.Spec.Wandb.Version)`. `GetServerManifest` already caches OCI pulls to a local volume at `/tmp/server-manifest` and resolves locally before going remote (see [pkg/wandb/manifest/manifest.go:540](pkg/wandb/manifest/manifest.go:540)), so the webhook does not need its own cache layer — once the controller has reconciled a version, the webhook's lookup for that same version is a fast local read. + - For each external component (i.e., where `Spec.{Component}.External{Component} != nil`), look up the manifest's per-component `MinVersion`: + - `Bucket["default"].MinVersion` (object store) + - `Mysql["default"].MinVersion` + - `Redis["default"].MinVersion` + - `Clickhouse["default"].MinVersion` + - `Kafka.MinVersion` + - Compare against `newWandb.Status.{Component}Status.Version`: + - If `MinVersion` is empty -> no requirement, skip. + - If status version is empty -> reject: *"the version of external X has not been detected yet; wait for the operator to populate status.{x}Status.version before upgrading"*. + - If status version is below `MinVersion` -> reject: *"external X is at version 8.0.20; W&B 0.79.0 requires at least 8.0.36"*. + - Otherwise -> accept (no upper bound on external infra). + - Use `github.com/Masterminds/semver/v3` for parsing; comparison is a direct `<` against parsed `*semver.Version`. A small helper in `pkg/wandb/manifest/version.go` (`(ComponentVersions).MeetsMinimum(v string) (ok bool, reason string)`) keeps the webhook code clean. + - Return errors as `field.Invalid(field.NewPath("spec","wandb","version"), newVersion, msg)` collected into `field.ErrorList`, then wrapped with `apierrors.NewInvalid` — matching the existing pattern in this file. + +3. **Managed components are not validated by the webhook.** The reconciler is the sole owner of managed-component versions: it pins to the manifest's `TargetVersion`, no user override, no second source of truth. + +4. **No new defaulter logic is required.** Defaults are unchanged. + +5. The fetched manifest, if it can't be loaded (network error, malformed), should fail-closed: reject the update with *"could not load manifest for version X: …"*. This is a hard guard against silently allowing upgrades the operator can't verify. + +--- + +## 5. Files to change (summary) + +**Modified** + +- [pkg/wandb/manifest/manifest.go](pkg/wandb/manifest/manifest.go) — add `ComponentVersions` (with `MinVersion`, `TargetVersion`) inlined into `InfraConfig` and `KafkaConfig`; merge support. +- [internal/controller/v2/{mysql,redis,kafka,clickhouse,objectstore}.go](internal/controller/v2) — call probes (external), thread `ComponentVersions` into `managed*WriteState`, write versions into status (both branches). +- `internal/controller/infra/managed/{mysql/mysql,redis/opstree,kafka/strimzi,clickhouse/altinity,minio/tenant}/spec.go` — extend the `To*VendorSpec` builders to accept and apply the manifest `TargetVersion` to the underlying vendor CR's image/version field. +- [internal/controller/infra/external/common.go](internal/controller/infra/external/common.go) — optional shared `InferExternalStatusWithVersion` helper. +- [internal/webhook/v2/weightsandbiases_webhook.go](internal/webhook/v2/weightsandbiases_webhook.go) — add `validateInfraVersionRequirements` step in `ValidateUpdate`. +- [api/v2/weightsandbiases_types.go](api/v2/weightsandbiases_types.go) — add `Version` to `WBInfraStatus`; remove `Version` from `ManagedClickHouseSpec`. +- `go.mod` — add `go-sql-driver/mysql`, `clickhouse-go/v2`, `redis/go-redis/v9`, `twmb/franz-go`. Verify `minio-go/v7` is sufficient for object store. +- Regenerated: `config/crd/bases/apps.wandb.com_weightsandbiases.yaml`, `api/v2/zz_generated.deepcopy.go`. + +**New** + +- `internal/controller/infra/external/{mysql,redis,kafka,clickhouse,objectstore}/probe.go` — one per component. +- `pkg/wandb/manifest/version.go` — defines `ComponentVersions.MeetsMinimum(v string) (ok bool, reason string)` and a `Versions(component, instance string) ComponentVersions` lookup over a `Manifest`. +- `internal/webhook/v2/weightsandbiases_webhook_version_test.go` — table-driven tests covering: at/above min (accept), below min (reject), missing status version (reject), empty min (accept), no-op when wandb version unchanged, manifest fetch error (reject). + +--- + +## Verification + +Run from the worktree root. + +1. **Code generation & lint** + ```sh + make generate && make manifests + make lint + ``` + +2. **Unit & integration tests** — the existing webhook suite uses `envtest`; the new version-check tests will plug in there. + ```sh + make test + ``` + Confirm the new webhook tests cover both accept and reject paths, and that probe code has unit tests with mocked clients. + +3. **Local end-to-end with Tilt** (per `Tiltfile` / `DEVELOPMENT.md`): + - Apply a `WeightsAndBiases` with `spec.wandb.version = 0.78.0` and an *external* MySQL pointing at an 8.0.20 instance. + - Wait for `status.mysqlStatus.version = "8.0.20"` to appear. + - `kubectl patch` the CR to bump `spec.wandb.version = 0.79.0` (assuming the manifest declares `mysql.default.minVersion: "8.0.36"` for that release). + - Expect the API server to reject the patch with the *below-min* error. + - Upgrade the external MySQL to 8.0.36, wait for status to refresh, retry the patch — expect success. + - Upgrade the external MySQL to 9.0.0, retry the patch — expect success (no upper bound on external infra). + +4. **Managed-side check**: + - With managed MySQL on 0.78.0, bump `spec.wandb.version = 0.79.0`. + - The reconciler should re-translate `InnoDBCluster.Spec.Version` to the manifest's `targetVersion` and `status.mysqlStatus.version` should reflect the upgraded version once mysql-operator finishes the rollout. + - Confirm that the removed `spec.clickhouse.managedClickhouse.version` field is rejected by the API server (CRD schema no longer permits it) — the deployed image stays pinned to manifest `targetVersion`. + +5. **Failure-mode checks**: + - Point an external probe at unreachable host -> confirm `VersionProbeReady` condition is False and reconcile doesn't loop-crash. + - Bump version while status version is still empty -> confirm webhook rejects with the "not yet detected" message. + - Manifest fetch fails on update -> confirm webhook fails closed.