diff --git a/docs/using/image-management.md b/docs/using/image-management.md index 6b2c09b6..b4194387 100644 --- a/docs/using/image-management.md +++ b/docs/using/image-management.md @@ -320,9 +320,8 @@ pinning a specific build. See This preview supports only creating a new database on Spock 6. It does not support upgrading an existing database from Spock 5.x to - Spock 6. The Control Plane does not validate or block a - `spock_version` change on an existing database's spec, but doing so - is unsupported and can break replication, since a Spock 6 + Spock 6. The Control Plane rejects a spec update that changes the + Spock major version on an existing database, since a Spock 6 subscription cannot sync from a Spock 5.x peer. !!! warning diff --git a/server/internal/database/service.go b/server/internal/database/service.go index 29d10951..90494535 100644 --- a/server/internal/database/service.go +++ b/server/internal/database/service.go @@ -1027,6 +1027,16 @@ func ValidateChangedSpec(current, updated *Spec) error { errs = append(errs, errors.New("database name cannot be changed")) } + // Spock version is a database-wide property (every node inherits + // Spec.SpockVersion; there's no per-node override), so it's checked once + // here rather than per-instance. Checking it per-instance would miss a + // major version change submitted alongside a HostIDs change, since that + // changes the instance ID and would no longer match between current and + // updated. + if err := spockMajorVersionChanged(current.SpockVersion, updated.SpockVersion); err != nil { + errs = append(errs, err) + } + currentInstances, err := instancesByID(current) if err != nil { errs = append(errs, fmt.Errorf("failed to compute instances from current spec: %w", err)) @@ -1072,20 +1082,55 @@ func instancesByID(spec *Spec) (map[string]*InstanceSpec, error) { return byID, nil } +// majorVersionChanged rejects a Postgres major version change for a matched +// instance. Unlike Spock, Postgres supports a per-node major-version override +// (Node.PostgresVersion), so this intentionally only checks instances that +// persist across the update: replacing an instance via a HostIDs change +// (which produces a new instance ID) is exempt, since that's the supported +// path for moving a node to a new Postgres major version during a rolling +// upgrade. func majorVersionChanged(old, new *ds.PgEdgeVersion) error { if old == nil || new == nil { return errors.New("expected both current and updated versions to be defined") } - oldPgMajor, ok := old.PostgresVersion.Major() + return checkMajorUnchanged("postgres", old.PostgresVersion, new.PostgresVersion, "") +} + +// spockMajorVersionChanged rejects a Spock major version change on an +// existing database. A Spock N subscription cannot sync from a Spock N-1 +// peer, so this must be caught here rather than left to fail at replication +// time. It does not apply to a dedicated Spock upgrade workflow (PLAT-720), +// which is expected to have its own validation. +func spockMajorVersionChanged(current, updated string) error { + oldVersion, err := ds.ParseVersion(current) + if err != nil { + return fmt.Errorf("failed to parse current spock version: %w", err) + } + newVersion, err := ds.ParseVersion(updated) + if err != nil { + return fmt.Errorf("failed to parse updated spock version: %w", err) + } + return checkMajorUnchanged( + "spock", oldVersion, newVersion, + ": changing the spock major version on an existing database is not supported", + ) +} + +// checkMajorUnchanged returns an error if old and new have different major +// version components. label identifies the version kind (e.g. "postgres", +// "spock") in every error message, and detail is appended verbatim to the +// mismatch error so callers can add version-kind-specific context. +func checkMajorUnchanged(label string, old, new *ds.Version, detail string) error { + oldMajor, ok := old.Major() if !ok { - return errors.New("current postgres version is missing its major component") + return fmt.Errorf("current %s version is missing its major component", label) } - newPgMajor, ok := new.PostgresVersion.Major() + newMajor, ok := new.Major() if !ok { - return errors.New("updated postgres version is missing its major component") + return fmt.Errorf("updated %s version is missing its major component", label) } - if oldPgMajor != newPgMajor { - return fmt.Errorf("major version changed from %d to %d", oldPgMajor, newPgMajor) + if oldMajor != newMajor { + return fmt.Errorf("%s major version changed from %d to %d%s", label, oldMajor, newMajor, detail) } return nil } diff --git a/server/internal/database/service_test.go b/server/internal/database/service_test.go index 3904a98a..640663e3 100644 --- a/server/internal/database/service_test.go +++ b/server/internal/database/service_test.go @@ -192,6 +192,99 @@ func TestValidateChangedSpec(t *testing.T) { }, expectedErr: "major version changed from 17 to 18", }, + { + name: "valid postgres major version change alongside a host reassignment", + current: &database.Spec{ + TenantID: utils.PointerTo("tenant-id"), + DatabaseName: "test", + PostgresVersion: "17.6", + SpockVersion: "5", + Nodes: []*database.Node{ + {Name: "n1", HostIDs: []string{"host-1"}}, + }, + }, + updated: &database.Spec{ + TenantID: utils.PointerTo("tenant-id"), + DatabaseName: "test", + PostgresVersion: "18.0", + SpockVersion: "5", + Nodes: []*database.Node{ + // Unlike Spock, moving a node to a new host is the + // supported way to bump its Postgres major version + // during a rolling upgrade, so this is intentionally + // exempt from the major-version check. + {Name: "n1", HostIDs: []string{"host-2"}}, + }, + }, + }, + { + name: "valid spock minor version change", + current: &database.Spec{ + TenantID: utils.PointerTo("tenant-id"), + DatabaseName: "test", + PostgresVersion: "18.0", + SpockVersion: "5.0.6", + Nodes: []*database.Node{ + {Name: "n1", HostIDs: []string{"host-1"}}, + }, + }, + updated: &database.Spec{ + TenantID: utils.PointerTo("tenant-id"), + DatabaseName: "test", + PostgresVersion: "18.0", + SpockVersion: "5.0.9", + Nodes: []*database.Node{ + {Name: "n1", HostIDs: []string{"host-1"}}, + }, + }, + }, + { + name: "invalid spock major version change", + current: &database.Spec{ + TenantID: utils.PointerTo("tenant-id"), + DatabaseName: "test", + PostgresVersion: "18.0", + SpockVersion: "5", + Nodes: []*database.Node{ + {Name: "n1", HostIDs: []string{"host-1"}}, + }, + }, + updated: &database.Spec{ + TenantID: utils.PointerTo("tenant-id"), + DatabaseName: "test", + PostgresVersion: "18.0", + SpockVersion: "6", + Nodes: []*database.Node{ + {Name: "n1", HostIDs: []string{"host-1"}}, + }, + }, + expectedErr: "spock major version changed from 5 to 6", + }, + { + name: "invalid spock major version change alongside a host reassignment", + current: &database.Spec{ + TenantID: utils.PointerTo("tenant-id"), + DatabaseName: "test", + PostgresVersion: "18.0", + SpockVersion: "5", + Nodes: []*database.Node{ + {Name: "n1", HostIDs: []string{"host-1"}}, + }, + }, + updated: &database.Spec{ + TenantID: utils.PointerTo("tenant-id"), + DatabaseName: "test", + PostgresVersion: "18.0", + SpockVersion: "6", + Nodes: []*database.Node{ + // Reassigning the node to a new host changes its + // instance ID, so this must still be caught even though + // no instance ID matches between current and updated. + {Name: "n1", HostIDs: []string{"host-2"}}, + }, + }, + expectedErr: "spock major version changed from 5 to 6", + }, } { t.Run(tc.name, func(t *testing.T) { err := database.ValidateChangedSpec(tc.current, tc.updated)