feat: add foundational control plane support for Spock 6 clusters #457
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| //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: nightly CI | ||
| // 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) and the verify-replicating | ||
| // step (VerifySubscriptionReplicatingResource) added in this same ticket. | ||
| 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.4"), | ||
|
|
||
| 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) | ||
| }) | ||
|
Comment on lines
+29
to
+99
There was a problem hiding this comment. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Preserve failed Spock 6 E2E environments. Add the standard 📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,6 +85,68 @@ func PopulateNodes(existing, new []*NodeResources) (*resource.State, error) { | |
| return merged, nil | ||
| } | ||
|
|
||
| // EnablePeerSubscriptions returns a diff that enables the peer subscriptions | ||
|
Member
There was a problem hiding this comment. This comment is inaccurate. Every create and update database process finishes with a desired end state, which we compute in control-plane/server/internal/database/operations/end.go Lines 47 to 51 in 4d64d7f So in an add-node operation, we enable subscriptions near the end. If you look at the golden_test diff in this PR, you'll see that we were already enabling the subscription on line 206. This change just enables the subscription earlier in the flow, which I don't think is necessary. Could you please explain more about what problem you saw that led you to make this change?
Contributor
Author
There was a problem hiding this comment. You're right, my comment was wrong. I turned this phase off and added a node to a live 3 node Spock 6 cluster, and every subscription, including the peer ones, went to replicating on its own, just like you said, driven by end.go. There's no bug here. I missed that end.go already re enables these later in the same operation, that's what led to the wrong "permanently disabled" comment. I kept the enable call though, because the verify step right after it needs the subscription to already be enabled to check it. By the time end.go runs, this one's just a no op. Comment's fixed to explain that now. If you'd rather I move the verify step to run after end.go so this whole phase can go away, happy to do that too, just didn't want to touch shared code without checking with you first.
Member
There was a problem hiding this comment. This check is a good idea, but I think we could implement it in a way that works everywhere. I mentioned in your other PR that it could make sense to raise an error whenever a subscription goes from healthy to unhealthy: #456 (comment) We could extend that to raise an error whenever a subscription goes from healthy, nonexistent, or disabled to unhealthy, which would apply here too. Could you please remove that verification step/resource and the new enable subscription step from this PR? We can create a dedicated ticket to add the replication status check, and I'll fill in some implementation suggestions. |
||
| // addPeerResources creates disabled. It must be applied as a separate, later | ||
| // phase than PopulateNodes' own state. | ||
| // | ||
| // addPeerResources creates each peer->new-node subscription disabled so the | ||
| // peer-catchup chain (SyncEvent -> WaitForSyncEvent -> PeerCatchup -> | ||
| // LagTracker -> ReplicationSlotAdvanceFromCTS -> ReplicationOriginAdvance) | ||
| // can run without a live subscriber racing that setup. But a single | ||
| // resource.State can only express one desired value per identifier, so that | ||
| // same state can never also declare "now enable it" — nothing else in the | ||
| // codebase ever does, which left these subscriptions permanently disabled. | ||
| // This returns a second state that re-declares the same SubscriptionResource | ||
| // identifiers with Disabled: false; applied after the populate phase is | ||
| // fully persisted, its diff sees disabled->enabled and calls Update, which | ||
| // is what actually flips sub_enabled in spock.subscription. | ||
| func EnablePeerSubscriptions(existing, new []*NodeResources) (*resource.State, error) { | ||
| existingNodeNames := make([]string, len(existing)) | ||
| for i, n := range existing { | ||
| existingNodeNames[i] = n.NodeName | ||
| } | ||
|
|
||
| enable := resource.NewState() | ||
| for _, node := range new { | ||
| if node.SourceNode == "" { | ||
| continue | ||
| } | ||
| dbName := node.DatabaseName | ||
| for _, peer := range existingNodeNames { | ||
| if peer == node.NodeName || peer == node.SourceNode { | ||
| continue | ||
| } | ||
| err := enable.AddResource( | ||
| &database.SubscriptionResource{ | ||
| DatabaseName: dbName, | ||
| SubscriberNode: node.NodeName, | ||
| ProviderNode: peer, | ||
| Disabled: false, | ||
| ExtraDependencies: []resource.Identifier{ | ||
| database.ReplicationOriginAdvanceResourceIdentifier(peer, node.NodeName, dbName), | ||
| }, | ||
| }, | ||
| // Verify the enable actually took effect. Same phase, not a | ||
| // separate one: this is a new resource type/identifier, not | ||
| // a re-declaration of an existing one, so it can safely | ||
| // depend on the SubscriptionResource declared just above | ||
| // within this same state and run after it in the same | ||
| // apply pass. | ||
| &database.VerifySubscriptionReplicatingResource{ | ||
| DatabaseName: dbName, | ||
| SubscriberNode: node.NodeName, | ||
| ProviderNode: peer, | ||
| }, | ||
| ) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to add peer-enable resource to 'enable' state: %w", err) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return enable, nil | ||
| } | ||
|
|
||
| func addPeerResources( | ||
| state *resource.State, | ||
| dbName string, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a nightly job for this? It looks like we've only been publishing new images once every few weeks.