diff --git a/.circleci/config.yml b/.circleci/config.yml index 3fcd8dc4..6170c60d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -137,6 +137,31 @@ jobs: when: always - store_test_results: path: . + test_e2e_spock6: + executor: common + steps: + - common_setup + - run: + name: Run Spock 6 add-node e2e test against the latest dev image + command: | + make ci-compose-detached + make test-e2e E2E_DEBUG=1 E2E_FIXTURE=ci E2E_RUN='^TestSpock6AddNode$$' TEST_RERUN_FAILS=2 + - run: + name: Archive debug output + command: | + if [[ -d ./e2e/debug ]]; then + sudo journalctl -u docker.service > ./e2e/debug/docker-service.log + tar -czf e2e-debug.tar.gz -C e2e debug + fi + when: on_fail + - store_artifacts: + path: e2e-debug.tar.gz + - run: + name: Ensure Docker Compose is stopped + command: make ci-compose-down + when: always + - store_test_results: + path: . release: executor: common steps: @@ -313,3 +338,22 @@ workflows: jobs: - build_image: context: control-plane-release + + weekly_spock6: + # Runs independent of any commit activity, so drift introduced by an + # upstream Spock 6 build gets caught even if nobody touches this repo + # that week. Weekly rather than nightly: the upstream dev image only + # gets a new build every few weeks in practice, so nightly would just + # be ~20+ no-op runs for every one that actually catches something. + # The e2e test itself points at the floating spock6DevImage tag (see + # e2e/spock6_add_node_test.go), so this job needs no extra plumbing + # to track "latest" - only the schedule. + triggers: + - schedule: + cron: "0 6 * * 1" + filters: + branches: + only: + - main + jobs: + - test_e2e_spock6 diff --git a/e2e/custom_db_create_test.go b/e2e/custom_db_create_test.go index be5d0009..cf6dd39a 100644 --- a/e2e/custom_db_create_test.go +++ b/e2e/custom_db_create_test.go @@ -5,8 +5,8 @@ package e2e import ( "context" "fmt" - "log" "slices" + "strconv" "strings" "testing" "time" @@ -98,7 +98,7 @@ func TestCreateDbWithVersions(t *testing.T) { Password: password, } - verifyPgVersion(ctx, db, primaryOpts, version.PostgresVersion, t) + verifyPgVersion(ctx, db, primaryOpts, version.PostgresVersion, version.SpockVersion, t) verifyPrimaryNodes(ctx, db, primaryOpts, t) } @@ -110,7 +110,7 @@ func TestCreateDbWithVersions(t *testing.T) { Username: username, Password: password, } - verifyPgVersion(ctx, db, connOpts, version.PostgresVersion, t) + verifyPgVersion(ctx, db, connOpts, version.PostgresVersion, version.SpockVersion, t) verifyReplicasNodes(ctx, db, connOpts, t) } @@ -206,19 +206,30 @@ func verifyReplicasNodes(ctx context.Context, db *DatabaseFixture, }) } -// Validate postgresql version +// Validate postgresql version. Spock 6 manifest entries point at a +// floating/mutable dev image tag (see version-manifest.json), so their +// resolved Postgres minor can drift past the declared version at any +// time - only the major version is checked for those. Pinned versions +// (Spock <= 5) are still checked for an exact match. func verifyPgVersion(ctx context.Context, db *DatabaseFixture, - primaryOpts ConnectionOptions, expectedVersion string, t testing.TB) { + primaryOpts ConnectionOptions, expectedVersion string, spockVersion string, t testing.TB) { db.WithConnection(ctx, primaryOpts, t, func(conn *pgx.Conn) { var versionStr string err := conn.QueryRow(ctx, "SELECT version()").Scan(&versionStr) if err != nil { - log.Fatalf("Failed to fetch PostgreSQL version: %v", err) + t.Fatalf("Failed to fetch PostgreSQL version: %v", err) } - if !strings.Contains(versionStr, expectedVersion) { - log.Fatalf("Expected PostgreSQL version %s, but got: %s", expectedVersion, versionStr) + + versionToMatch := expectedVersion + spockMajorStr, _, _ := strings.Cut(spockVersion, ".") + if spockMajor, err := strconv.Atoi(spockMajorStr); err == nil && spockMajor >= 6 { + versionToMatch, _, _ = strings.Cut(expectedVersion, ".") + } + + if !strings.Contains(versionStr, versionToMatch) { + t.Fatalf("Expected PostgreSQL version %s, but got: %s", versionToMatch, versionStr) } - tLogf(t, "PostgreSQL version validation passed (found %s)\n", expectedVersion) + tLogf(t, "PostgreSQL version validation passed (found %s)\n", versionStr) }) } diff --git a/e2e/spock6_add_node_test.go b/e2e/spock6_add_node_test.go new file mode 100644 index 00000000..03940a3c --- /dev/null +++ b/e2e/spock6_add_node_test.go @@ -0,0 +1,100 @@ +//go:build e2e_test + +package e2e + +import ( + "context" + "testing" + "time" + + "github.com/jackc/pgx/v5" + controlplane "github.com/pgEdge/control-plane/api/apiv1/gen/control_plane" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// spock6DevImage is a floating/mutable tag tracking the latest Spock 6 +// development build. Not pinned to a specific build number: the scheduled +// CI job re-running this test picks up whatever the tag currently resolves +// to, with no extra plumbing needed to point CI at "latest." +const spock6DevImage = "ghcr.io/pgedge/pgedge-postgres:18-spock6-standard" + +// TestSpock6AddNode validates the add-node workflow end-to-end against a +// real Spock 6 cluster: creates a 2-node database pinned to a Spock 6 dev +// image via orchestrator_opts.swarm.image (bypassing manifest version +// constraints, since spock6 manifest entries are deliberately "dev" +// stability and never auto-selected), adds a 3rd node, and confirms the +// full mesh reaches "replicating" — exercising the Spock-major-gated +// spock.progress query (PeerCatchupResource). +func TestSpock6AddNode(t *testing.T) { + t.Parallel() + + const ( + username = "admin" + password = "password" + dbName = "spock6_add_node_db" + ) + + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Minute) + defer cancel() + + hostIDs := fixture.HostIDs() + + nodeSpec := func(name, hostID string) *controlplane.DatabaseNodeSpec { + return &controlplane.DatabaseNodeSpec{ + Name: name, + HostIds: []controlplane.Identifier{controlplane.Identifier(hostID)}, + OrchestratorOpts: &controlplane.OrchestratorOpts{ + Swarm: &controlplane.SwarmOpts{Image: pointerTo(spock6DevImage)}, + }, + } + } + + t.Log("Step 1: Creating 2-node Spock 6 database fixture") + db := fixture.NewDatabaseFixture(ctx, t, &controlplane.CreateDatabaseRequest{ + Spec: &controlplane.DatabaseSpec{ + DatabaseName: dbName, + PostgresVersion: pointerTo("18.6"), + SpockVersion: pointerTo("6"), + Port: pointerTo(0), + PatroniPort: pointerTo(0), + DatabaseUsers: []*controlplane.DatabaseUserSpec{{ + Username: username, + Password: pointerTo(password), + DbOwner: pointerTo(true), + Attributes: []string{"LOGIN", "SUPERUSER"}, + }}, + Nodes: []*controlplane.DatabaseNodeSpec{ + nodeSpec("n1", hostIDs[0]), + nodeSpec("n2", hostIDs[1]), + }, + }, + }) + t.Logf("Database created: %s", db.ID) + + t.Log("Step 2: Adding n3 node with n1 as source") + db.Spec.Nodes = append(db.Spec.Nodes, func() *controlplane.DatabaseNodeSpec { + n := nodeSpec("n3", hostIDs[2]) + n.SourceNode = pointerTo("n1") + return n + }()) + require.NoError(t, db.Update(ctx, UpdateOptions{Spec: db.Spec})) + t.Log("Add-node completed successfully against Spock 6") + + t.Log("Step 3: Waiting for full mesh replication") + db.WaitForReplication(ctx, t, username, password) + t.Log("Replication complete") + + t.Log("Step 4: Verifying spock.spock_version() reports major 6 on the new node") + n3Opts := ConnectionOptions{ + Matcher: And(WithNode("n3"), WithRole("primary")), + Username: username, + Password: password, + } + db.WithConnection(ctx, n3Opts, t, func(conn *pgx.Conn) { + var version string + err := conn.QueryRow(ctx, "SELECT spock.spock_version();").Scan(&version) + require.NoError(t, err) + assert.Regexp(t, `^6\.`, version, "expected node n3 to be running Spock 6, got %q", version) + }) +} diff --git a/server/internal/database/peer_catchup_resource.go b/server/internal/database/peer_catchup_resource.go index 597b0e7f..117a1919 100644 --- a/server/internal/database/peer_catchup_resource.go +++ b/server/internal/database/peer_catchup_resource.go @@ -77,6 +77,12 @@ func (r *PeerCatchupResource) Refresh(ctx context.Context, rc *resource.Context) } defer conn.Close(ctx) + spockVersion, err := getLiveSpockVersion(ctx, conn) + if err != nil { + return fmt.Errorf("failed to check spock version on source node %q: %w", r.SourceNode, err) + } + spockMajor, _ := spockVersion.Major() + const pollInterval = 500 * time.Millisecond for { @@ -84,7 +90,7 @@ func (r *PeerCatchupResource) Refresh(ctx context.Context, rc *resource.Context) return ctx.Err() } - reached, err := postgres.SpockProgressReachedLSN(r.PeerNode, syncEvent.SyncEventLsn). + reached, err := postgres.SpockProgressReachedLSN(spockMajor, r.PeerNode, syncEvent.SyncEventLsn). Scalar(ctx, conn) if err != nil { return fmt.Errorf("failed to query spock progress for peer %q: %w", r.PeerNode, err) diff --git a/server/internal/database/reconcile_versions_test.go b/server/internal/database/reconcile_versions_test.go index 92ffb774..3493a1e8 100644 --- a/server/internal/database/reconcile_versions_test.go +++ b/server/internal/database/reconcile_versions_test.go @@ -122,6 +122,59 @@ func TestReconcileVersions(t *testing.T) { }, }, }, + { + // Before ds.ParseVersion accepted pre-release suffixes, this + // instance would be silently skipped at reconcile_versions.go's + // ds.ParsePgEdgeVersion call (line 141) and never appear in + // updatedInstances. It should now reconcile normally, with the + // pre-release suffix dropped by Normalize(). + name: "spock beta version is not silently skipped", + spec: &database.StoredSpec{ + Spec: &database.Spec{ + PostgresVersion: "17.4", + SpockVersion: "5", + Nodes: []*database.Node{ + {Name: "n1", HostIDs: []string{"host-1"}}, + }, + }, + }, + instances: []*database.StoredInstance{ + { + InstanceID: "n1-host-1", + NodeName: "n1", + HostID: "host-1", + PgEdgeVersion: ds.MustParsePgEdgeVersion("17.4", "5"), + }, + }, + statuses: []*database.StoredInstanceStatus{ + { + InstanceID: "n1-host-1", + Status: &database.InstanceStatus{ + StatusUpdatedAt: utils.PointerTo(time.Now()), + Role: utils.PointerTo(patroni.InstanceRolePrimary), + PostgresVersion: utils.PointerTo("17.5"), + SpockVersion: utils.PointerTo("6.0.0-beta.1"), + }, + }, + }, + expectedSpec: &database.StoredSpec{ + Spec: &database.Spec{ + PostgresVersion: "17.5", + SpockVersion: "6", + Nodes: []*database.Node{ + {Name: "n1", HostIDs: []string{"host-1"}}, + }, + }, + }, + expectedInstances: []*database.StoredInstance{ + { + InstanceID: "n1-host-1", + NodeName: "n1", + HostID: "host-1", + PgEdgeVersion: ds.MustParsePgEdgeVersion("17.5", "6"), + }, + }, + }, { name: "all nodes updated spock only", spec: &database.StoredSpec{ diff --git a/server/internal/database/sync_event_resource.go b/server/internal/database/sync_event_resource.go index 9a6377bb..c9989888 100644 --- a/server/internal/database/sync_event_resource.go +++ b/server/internal/database/sync_event_resource.go @@ -13,18 +13,31 @@ import ( var minSpockVersionForSyncEventArgs = ds.MustParseVersion(postgres.MinSpockVersionForSyncEventArgs) +// getLiveSpockVersion queries conn directly for the Spock version actually +// running on this connection, rather than trusting spec/stored state — spec +// state can lag behind what's really deployed (e.g. mid add-node, mid +// upgrade), and every SQL-shape decision gated on Spock version needs to +// match what's really there. +func getLiveSpockVersion(ctx context.Context, conn *pgx.Conn) (*ds.Version, error) { + versionStr, err := postgres.GetSpockVersion().Scalar(ctx, conn) + if err != nil { + return nil, fmt.Errorf("failed to get spock version: %w", err) + } + version, err := ds.ParseVersion(versionStr) + if err != nil { + return nil, fmt.Errorf("failed to parse spock version %q: %w", versionStr, err) + } + return version, nil +} + // spockSupportsSyncEventArgs reports whether conn's Spock version is new // enough for spock.sync_event(boolean) and the 5-arg // spock.wait_for_sync_event(..., wait_if_disabled) — see // postgres.MinSpockVersionForSyncEventArgs. func spockSupportsSyncEventArgs(ctx context.Context, conn *pgx.Conn) (bool, error) { - versionStr, err := postgres.GetSpockVersion().Scalar(ctx, conn) - if err != nil { - return false, fmt.Errorf("failed to get spock version: %w", err) - } - version, err := ds.ParseVersion(versionStr) + version, err := getLiveSpockVersion(ctx, conn) if err != nil { - return false, fmt.Errorf("failed to parse spock version %q: %w", versionStr, err) + return false, err } return version.Compare(minSpockVersionForSyncEventArgs) >= 0, nil } diff --git a/server/internal/ds/versions.go b/server/internal/ds/versions.go index 5128dd3f..597c1b10 100644 --- a/server/internal/ds/versions.go +++ b/server/internal/ds/versions.go @@ -47,6 +47,11 @@ var _ encoding.TextUnmarshaler = (*Version)(nil) type Version struct { Components []uint64 `json:"components"` + // PreRelease is the optional suffix after a "-" (e.g. "beta.1" in + // "6.0.0-beta.1"). It is an opaque string, not decomposed into SemVer + // precedence rules, and is intentionally dropped by MajorVersion and + // MajorMinorVersion since a major/major-minor bucket never carries one. + PreRelease string `json:"pre_release,omitempty"` } func (v *Version) Major() (uint64, bool) { @@ -64,6 +69,8 @@ func (v *Version) MajorString() (string, bool) { return strconv.FormatUint(major, 10), true } +// MajorVersion returns just the major component. PreRelease is intentionally +// dropped: a major-version bucket never carries a pre-release suffix. func (v *Version) MajorVersion() *Version { if len(v.Components) == 0 { return &Version{} @@ -73,6 +80,9 @@ func (v *Version) MajorVersion() *Version { } } +// MajorMinorVersion returns the major.minor components. PreRelease is +// intentionally dropped: a major.minor bucket never carries a pre-release +// suffix. func (v *Version) MajorMinorVersion() *Version { components := slices.Clone(v.Components) if len(components) > 2 { @@ -88,12 +98,17 @@ func (v *Version) String() string { for i, c := range v.Components { components[i] = strconv.FormatUint(c, 10) } - return strings.Join(components, ".") + s := strings.Join(components, ".") + if v.PreRelease != "" { + s += "-" + v.PreRelease + } + return s } func (v *Version) Clone() *Version { return &Version{ Components: slices.Clone(v.Components), + PreRelease: v.PreRelease, } } @@ -107,6 +122,7 @@ func (v *Version) UnmarshalText(data []byte) error { return err } v.Components = parsed.Components + v.PreRelease = parsed.PreRelease return nil } @@ -138,11 +154,32 @@ func (v *Version) UnmarshalJSON(data []byte) error { } } +// Compare orders by numeric components first. If those are equal, a +// pre-release never compares equal to its release counterpart (a release +// sorts after any pre-release of the same numeric version), and two +// different pre-releases of the same numeric version fall back to a plain +// string comparison. This is NOT full SemVer pre-release precedence — it's +// just enough to avoid falsely claiming two different versions are equal. func (v *Version) Compare(other *Version) int { - return slices.Compare(v.Components, other.Components) + if c := slices.Compare(v.Components, other.Components); c != 0 { + return c + } + switch { + case v.PreRelease == other.PreRelease: + return 0 + case v.PreRelease == "": + return 1 + case other.PreRelease == "": + return -1 + default: + return strings.Compare(v.PreRelease, other.PreRelease) + } } -var semverRegexp = regexp.MustCompile(`^\d+(\.\d+){0,2}$`) +// semverRegexp matches "[.[.]][-]". The +// pre-release group accepts dot/hyphen-separated alphanumeric identifiers +// (e.g. "beta", "beta.1", "rc.2") but not SemVer build metadata ("+..."). +var semverRegexp = regexp.MustCompile(`^(\d+(?:\.\d+){0,2})(?:-([0-9A-Za-z]+(?:[.-][0-9A-Za-z]+)*))?$`) func MustParseVersion(s string) *Version { v, err := ParseVersion(s) @@ -153,10 +190,11 @@ func MustParseVersion(s string) *Version { } func ParseVersion(s string) (*Version, error) { - if !semverRegexp.MatchString(s) { + m := semverRegexp.FindStringSubmatch(s) + if m == nil { return nil, fmt.Errorf("invalid version format: %q", s) } - parts := strings.Split(s, ".") + parts := strings.Split(m[1], ".") components := make([]uint64, len(parts)) for i, p := range parts { c, err := strconv.ParseUint(p, 10, 64) @@ -165,7 +203,7 @@ func ParseVersion(s string) (*Version, error) { } components[i] = c } - return &Version{Components: components}, nil + return &Version{Components: components, PreRelease: m[2]}, nil } type PgEdgeVersion struct { diff --git a/server/internal/ds/versions_test.go b/server/internal/ds/versions_test.go index 7b19dd90..a6996031 100644 --- a/server/internal/ds/versions_test.go +++ b/server/internal/ds/versions_test.go @@ -61,9 +61,25 @@ func TestParseVersion(t *testing.T) { expectedErr: "invalid version format", }, { - // Intentionally not supporting pre-release identifiers because they - // are not comparable. - input: "5.0.0-beta", + // Pre-release identifiers are accepted so we can tolerate a + // live-observed beta version (e.g. from spock_version()) without + // silently dropping the instance from reconciliation. This is not + // full SemVer precedence — see Version.Compare. + input: "5.0.0-beta", + expected: &ds.Version{Components: []uint64{5, 0, 0}, PreRelease: "beta"}, + }, + { + input: "6.0.0-beta.1", + expected: &ds.Version{Components: []uint64{6, 0, 0}, PreRelease: "beta.1"}, + }, + { + // Still rejected: empty pre-release suffix. + input: "5.0.0-", + expectedErr: "invalid version format", + }, + { + // Still rejected: SemVer build metadata is out of scope. + input: "5.0.0-beta+build1", expectedErr: "invalid version format", }, } { @@ -86,6 +102,7 @@ func TestVersion(t *testing.T) { "17", "17.6", "5.0.0", + "6.0.0-beta.1", } { t.Run(tc, func(t *testing.T) { out, err := ds.ParseVersion(tc) @@ -96,6 +113,14 @@ func TestVersion(t *testing.T) { } }) + t.Run("MajorVersion and MajorMinorVersion drop PreRelease", func(t *testing.T) { + v, err := ds.ParseVersion("6.0.0-beta.1") + require.NoError(t, err) + + assert.Equal(t, &ds.Version{Components: []uint64{6}}, v.MajorVersion()) + assert.Equal(t, &ds.Version{Components: []uint64{6, 0}}, v.MajorMinorVersion()) + }) + t.Run("Compare", func(t *testing.T) { for _, tc := range []struct { a *ds.Version @@ -166,6 +191,30 @@ func TestVersion(t *testing.T) { b: &ds.Version{Components: []uint64{1, 0, 0}}, expected: -1, }, + { + // A pre-release must never compare equal to its release + // counterpart, even though the numeric Components match. + a: &ds.Version{Components: []uint64{6, 0, 0}, PreRelease: "beta.1"}, + b: &ds.Version{Components: []uint64{6, 0, 0}}, + expected: -1, + }, + { + a: &ds.Version{Components: []uint64{6, 0, 0}}, + b: &ds.Version{Components: []uint64{6, 0, 0}, PreRelease: "beta.1"}, + expected: 1, + }, + { + a: &ds.Version{Components: []uint64{6, 0, 0}, PreRelease: "beta.1"}, + b: &ds.Version{Components: []uint64{6, 0, 0}, PreRelease: "beta.1"}, + expected: 0, + }, + { + // Two different pre-releases of the same numeric version: not + // full SemVer precedence, just guaranteed non-equal. + a: &ds.Version{Components: []uint64{6, 0, 0}, PreRelease: "beta.1"}, + b: &ds.Version{Components: []uint64{6, 0, 0}, PreRelease: "beta.2"}, + expected: -1, + }, } { t.Run(fmt.Sprintf("%s and %s", tc.a.String(), tc.b.String()), func(t *testing.T) { result := tc.a.Compare(tc.b) @@ -228,6 +277,14 @@ func TestNewPgEdgeVersion(t *testing.T) { spockVersion: "invalid", expectedErr: "invalid spock version", }, + { + postgresVersion: "17.6", + spockVersion: "6.0.0-beta.1", + expected: &ds.PgEdgeVersion{ + PostgresVersion: &ds.Version{Components: []uint64{17, 6}}, + SpockVersion: &ds.Version{Components: []uint64{6, 0, 0}, PreRelease: "beta.1"}, + }, + }, } { t.Run(tc.postgresVersion+"_"+tc.spockVersion, func(t *testing.T) { result, err := ds.ParsePgEdgeVersion(tc.postgresVersion, tc.spockVersion) diff --git a/server/internal/orchestrator/swarm/manifest_loader.go b/server/internal/orchestrator/swarm/manifest_loader.go index 0f1a0adf..01417e47 100644 --- a/server/internal/orchestrator/swarm/manifest_loader.go +++ b/server/internal/orchestrator/swarm/manifest_loader.go @@ -382,16 +382,36 @@ func buildVersions(cfg config.Config, mf *versionManifest) (*Versions, error) { } img := &Images{ PgEdgeImage: serviceImageTag(cfg, e.Image), + Stability: e.Stability, } versions.addImage(pv, img) if e.Default { + if e.Stability != "" && e.Stability != "stable" { + return nil, fmt.Errorf("invalid version entry {postgres:%s spock:%s}: a %q-stability entry cannot be marked default", + e.PostgresVersion, e.SpockVersion, e.Stability) + } defaultVer = pv } } if defaultVer == nil { - // Fall back to the last entry if no default is marked. - defaultVer = versions.supportedVersions[len(versions.supportedVersions)-1] + // Fall back to the last stable entry if no default is marked. A + // non-stable (e.g. "dev") entry must never become the default just + // because it happens to be last in the manifest. + for i := len(entries) - 1; i >= 0 && defaultVer == nil; i-- { + if entries[i].Stability != "" && entries[i].Stability != "stable" { + continue + } + pv, err := ds.ParsePgEdgeVersion(entries[i].PostgresVersion, entries[i].SpockVersion) + if err != nil { + return nil, fmt.Errorf("invalid version entry {postgres:%s spock:%s}: %w", + entries[i].PostgresVersion, entries[i].SpockVersion, err) + } + defaultVer = pv + } + if defaultVer == nil { + return nil, fmt.Errorf("manifest has no stable entry to use as a default") + } } versions.defaultVersion = defaultVer diff --git a/server/internal/orchestrator/swarm/manifest_loader_test.go b/server/internal/orchestrator/swarm/manifest_loader_test.go index 963bfa1e..3d6db868 100644 --- a/server/internal/orchestrator/swarm/manifest_loader_test.go +++ b/server/internal/orchestrator/swarm/manifest_loader_test.go @@ -12,6 +12,7 @@ import ( "time" "github.com/pgEdge/control-plane/server/internal/config" + "github.com/pgEdge/control-plane/server/internal/ds" "github.com/pgEdge/control-plane/server/internal/testutils" ) @@ -553,6 +554,154 @@ func TestValidateManifest(t *testing.T) { } } +// TestBuildVersions_StabilityWired verifies that a "stability" value on a +// manifest entry is actually carried through to the runtime Images.Stability +// field. Regression test: buildVersions previously parsed Stability off the +// JSON entry but never copied it onto the constructed *Images, so every +// entry loaded from a real manifest silently ended up with Stability == "", +// which the filtering logic in AvailableUpgrades/FindUpgrade treats as +// "stable" — a "dev" entry would have had zero effect. +func TestBuildVersions_StabilityWired(t *testing.T) { + m := &ManifestLoader{logger: testutils.Logger(t), cfg: config.Config{ + DockerSwarm: config.DockerSwarm{ImageRepositoryHost: "ghcr.io/pgedge"}, + }} + data, err := json.Marshal(map[string]any{ + "schema_version": 1, + "images": map[string]any{ + "postgres": []map[string]any{ + { + "postgres_version": "18.4", + "spock_version": "5", + "image": "pgedge-postgres:18.4-spock5.0.10-standard-1", + "stability": "stable", + "default": true, + }, + { + "postgres_version": "18.4", + "spock_version": "6", + "image": "pgedge-postgres:18-spock6-standard", + "stability": "dev", + }, + }, + }, + }) + if err != nil { + t.Fatal(err) + } + + v, _, err := m.parseManifestData(data) + if err != nil { + t.Fatalf("parseManifestData: %v", err) + } + + spock6 := ds.MustParsePgEdgeVersion("18.4", "6") + imgs, err := v.GetImages(spock6) + if err != nil { + t.Fatalf("GetImages(spock6): %v", err) + } + if imgs.Stability != "dev" { + t.Errorf("Stability = %q, want %q", imgs.Stability, "dev") + } + + if v.Default().SpockVersion.String() != "5" { + t.Errorf("default spock version = %s, want 5 (dev entry must never be default)", v.Default().SpockVersion) + } +} + +// TestBuildVersions_RejectsDevDefault verifies that manifest loading fails +// outright if a non-stable entry is marked default, rather than silently +// allowing a dev image to become the default for new database creation. +func TestBuildVersions_RejectsDevDefault(t *testing.T) { + m := &ManifestLoader{logger: testutils.Logger(t), cfg: config.Config{ + DockerSwarm: config.DockerSwarm{ImageRepositoryHost: "ghcr.io/pgedge"}, + }} + data, err := json.Marshal(map[string]any{ + "schema_version": 1, + "images": map[string]any{ + "postgres": []map[string]any{ + { + "postgres_version": "18.4", + "spock_version": "6", + "image": "pgedge-postgres:18-spock6-standard", + "stability": "dev", + "default": true, + }, + }, + }, + }) + if err != nil { + t.Fatal(err) + } + + if _, _, err := m.parseManifestData(data); err == nil { + t.Fatal("expected error when a dev-stability entry is marked default") + } +} + +// TestBuildVersions_FallbackDefaultSkipsNonStable verifies that when no entry +// is explicitly marked default, the implicit "last entry" fallback still +// never selects a non-stable entry. +func TestBuildVersions_FallbackDefaultSkipsNonStable(t *testing.T) { + m := &ManifestLoader{logger: testutils.Logger(t), cfg: config.Config{ + DockerSwarm: config.DockerSwarm{ImageRepositoryHost: "ghcr.io/pgedge"}, + }} + data, err := json.Marshal(map[string]any{ + "schema_version": 1, + "images": map[string]any{ + "postgres": []map[string]any{ + { + "postgres_version": "18.4", + "spock_version": "5", + "image": "pgedge-postgres:18.4-spock5.0.10-standard-1", + "stability": "stable", + }, + { + "postgres_version": "18.4", + "spock_version": "6", + "image": "pgedge-postgres:18-spock6-standard", + "stability": "dev", + }, + }, + }, + }) + if err != nil { + t.Fatal(err) + } + + v, _, err := m.parseManifestData(data) + if err != nil { + t.Fatalf("parseManifestData: %v", err) + } + if v.Default().SpockVersion.String() != "5" { + t.Errorf("default spock version = %s, want 5 (fallback must skip the trailing dev entry)", v.Default().SpockVersion) + } +} + +// TestEmbeddedManifestValid_Spock6DevEntryNotDefault verifies the real, +// shipped version-manifest.json's Spock 6 dev entry is loaded (so it's +// reachable via an explicit postgres_version/spock_version request or an +// orchestrator_opts image override) but never selected as the default. +func TestEmbeddedManifestValid_Spock6DevEntryNotDefault(t *testing.T) { + m := &ManifestLoader{logger: testutils.Logger(t)} + v, _, err := m.parseManifestData(embeddedManifest) + if err != nil { + t.Fatalf("embedded manifest cannot be parsed: %v", err) + } + + spock6 := ds.MustParsePgEdgeVersion("18.6", "6") + imgs, err := v.GetImages(spock6) + if err != nil { + t.Fatalf("expected embedded manifest to have a spock6 entry: %v", err) + } + if imgs.Stability != "dev" { + t.Errorf("spock6 entry Stability = %q, want %q", imgs.Stability, "dev") + } + + if major, _ := v.Default().SpockVersion.Major(); major != 5 { + t.Errorf("default spock major = %d, want 5 (spock6 dev entry must never be default)", major) + } +} + // TestManifestLoader_ImageTagsHaveRegistryPrefix verifies that all image tags // returned by Versions and ServiceVersions include the configured registry // host. diff --git a/server/internal/orchestrator/swarm/version-manifest.json b/server/internal/orchestrator/swarm/version-manifest.json index 05f7ef36..55bf6c01 100644 --- a/server/internal/orchestrator/swarm/version-manifest.json +++ b/server/internal/orchestrator/swarm/version-manifest.json @@ -92,6 +92,12 @@ "image": "pgedge-postgres:18.4-spock5.0.10-standard-1", "stability": "stable", "default": true + }, + { + "postgres_version": "18.6", + "spock_version": "6", + "image": "pgedge-postgres:18-spock6-standard", + "stability": "dev" } ], "postgrest": [ diff --git a/server/internal/postgres/create_db.go b/server/internal/postgres/create_db.go index 2fa61933..917ef979 100644 --- a/server/internal/postgres/create_db.go +++ b/server/internal/postgres/create_db.go @@ -491,23 +491,29 @@ func AdvanceReplicationOrigin(slotName, lsn string) Statement { // SpockProgressReachedLSN reports whether the local node's apply progress // from the named peer has reached targetLSN. Uses remote_lsn (the LSN of the -// last applied commit in Spock 5.x) rather than received_lsn, which can -// advance on keepalive messages before any commits have been applied. -func SpockProgressReachedLSN(peerNodeName, targetLSN string) Query[bool] { +// last applied commit) on Spock < 6, or remote_commit_lsn on Spock >= 6 — +// spock.progress became a view over apply_group_progress() in Spock 6 and +// the column was renamed. Neither uses received_lsn, which can advance on +// keepalive messages before any commits have been applied. +func SpockProgressReachedLSN(spockMajor uint64, peerNodeName, targetLSN string) Query[bool] { + column := "remote_lsn" + if spockMajor >= 6 { + column = "remote_commit_lsn" + } return Query[bool]{ - SQL: ` + SQL: fmt.Sprintf(` SELECT COALESCE( - (SELECT p.remote_lsn >= @target_lsn::pg_lsn + (SELECT p.%s >= @target_lsn::pg_lsn FROM spock.progress p JOIN spock.node n ON n.node_id = p.remote_node_id WHERE p.node_id = (SELECT node_id FROM spock.node_info()) AND n.node_name = @peer_node_name), false ) - `, + `, column), Args: pgx.NamedArgs{ "peer_node_name": peerNodeName, - "target_lsn": targetLSN, + "target_lsn": targetLSN, }, } } diff --git a/server/internal/postgres/create_db_test.go b/server/internal/postgres/create_db_test.go index 31a64ef5..d0616805 100644 --- a/server/internal/postgres/create_db_test.go +++ b/server/internal/postgres/create_db_test.go @@ -33,6 +33,39 @@ func TestSyncEvent(t *testing.T) { } } +func TestSpockProgressReachedLSN(t *testing.T) { + for _, tc := range []struct { + name string + spockMajor uint64 + expectedColumn string + }{ + { + name: "spock 5", + spockMajor: 5, + expectedColumn: "p.remote_lsn", + }, + { + name: "spock 6", + spockMajor: 6, + expectedColumn: "p.remote_commit_lsn", + }, + { + name: "spock 7 (future major, treated like 6)", + spockMajor: 7, + expectedColumn: "p.remote_commit_lsn", + }, + } { + t.Run(tc.name, func(t *testing.T) { + query := postgres.SpockProgressReachedLSN(tc.spockMajor, "n1", "0/0") + assert.Contains(t, query.SQL, tc.expectedColumn) + assert.Equal(t, pgx.NamedArgs{ + "peer_node_name": "n1", + "target_lsn": "0/0", + }, query.Args) + }) + } +} + func TestWaitForSyncEvent(t *testing.T) { for _, tc := range []struct { name string