diff --git a/e2e/add_replica_test.go b/e2e/add_replica_test.go new file mode 100644 index 00000000..3ed59cb3 --- /dev/null +++ b/e2e/add_replica_test.go @@ -0,0 +1,107 @@ +//go:build e2e_test + +package e2e + +import ( + "testing" + + "github.com/jackc/pgx/v5" + "github.com/stretchr/testify/require" + + api "github.com/pgEdge/control-plane/api/apiv1/gen/control_plane" +) + +func TestAddReplica(t *testing.T) { + t.Parallel() + + host1 := fixture.HostIDs()[0] + host2 := fixture.HostIDs()[1] + host3 := fixture.HostIDs()[2] + + username := "admin" + password := "password" + + tLog(t, "creating database") + + ctx := t.Context() + db := fixture.NewDatabaseFixture(ctx, t, &api.CreateDatabaseRequest{ + Spec: &api.DatabaseSpec{ + DatabaseName: "add_replica", + DatabaseUsers: []*api.DatabaseUserSpec{ + { + Username: username, + Password: pointerTo(password), + DbOwner: pointerTo(true), + Attributes: []string{"LOGIN", "SUPERUSER"}, + }, + }, + Port: pointerTo(0), + PatroniPort: pointerTo(0), + Nodes: []*api.DatabaseNodeSpec{ + {Name: "n1", HostIds: []api.Identifier{api.Identifier(host1)}}, + {Name: "n2", HostIds: []api.Identifier{api.Identifier(host2)}}, + }, + }, + }) + + tLog(t, "creating test data") + + opts := ConnectionOptions{ + Username: username, + Password: password, + } + db.WithConnection(ctx, opts, t, func(conn *pgx.Conn) { + _, err := conn.Exec(ctx, "CREATE TABLE foo (id int primary key, val text)") + require.NoError(t, err) + _, err = conn.Exec(ctx, "INSERT INTO foo (id, val) VALUES (1, 'foo')") + require.NoError(t, err) + }) + + tLog(t, "adding a replica") + + err := db.Update(ctx, UpdateOptions{ + Spec: &api.DatabaseSpec{ + DatabaseName: "add_replica", + DatabaseUsers: []*api.DatabaseUserSpec{ + { + Username: username, + DbOwner: pointerTo(true), + Attributes: []string{"LOGIN", "SUPERUSER"}, + }, + }, + Port: pointerTo(0), + PatroniPort: pointerTo(0), + Nodes: []*api.DatabaseNodeSpec{ + { + Name: "n1", + HostIds: []api.Identifier{ + api.Identifier(host1), + api.Identifier(host3), + }, + }, + {Name: "n2", HostIds: []api.Identifier{api.Identifier(host2)}}, + }, + }, + }) + require.NoError(t, err) + + tLog(t, "validating that replica exists and is populated") + + opts = ConnectionOptions{ + Username: username, + Password: password, + Matcher: And(WithNode("n1"), WithRole("replica")), + } + db.WithConnection(ctx, opts, t, func(conn *pgx.Conn) { + const isInRecoveryQuery = "SELECT pg_is_in_recovery()" + const valQuery = "SELECT val FROM foo WHERE id = 1" + + var isInRecovery bool + require.NoError(t, conn.QueryRow(ctx, isInRecoveryQuery).Scan(&isInRecovery)) + require.True(t, isInRecovery) + + var val string + require.NoError(t, conn.QueryRow(ctx, valQuery).Scan(&val)) + require.Equal(t, "foo", val) + }) +} diff --git a/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_a_replica.json b/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_a_replica.json index 1749aca7..c16a14f7 100644 --- a/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_a_replica.json +++ b/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_a_replica.json @@ -15,21 +15,13 @@ "reason": "does_not_exist", "diff": null } - ] - ], - [ + ], [ { "type": "update", "resource_id": "database.node::n1", - "reason": "has_diff", - "diff": [ - { - "value": "n1-instance-2-id", - "op": "add", - "path": "/instance_ids/-" - } - ] + "reason": "dependency_updated", + "diff": null } ], [ @@ -61,21 +53,21 @@ "reason": "dependency_updated", "diff": null } - ], + ] + ], + [ [ { "type": "create", - "resource_id": "database.switchover::n1-instance-1-id", + "resource_id": "monitor.instance::n1-instance-2-id", "reason": "does_not_exist", "diff": null } - ] - ], - [ + ], [ { "type": "create", - "resource_id": "monitor.instance::n1-instance-2-id", + "resource_id": "database.switchover::n1-instance-1-id", "reason": "does_not_exist", "diff": null } diff --git a/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_a_replica_with_primary_update.json b/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_a_replica_with_primary_update.json new file mode 100644 index 00000000..a1ad69e6 --- /dev/null +++ b/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_a_replica_with_primary_update.json @@ -0,0 +1,138 @@ +[ + [ + [ + { + "type": "create", + "resource_id": "orchestrator.resource::n1-instance-1-dep-2-id", + "reason": "does_not_exist", + "diff": null + } + ], + [ + { + "type": "update", + "resource_id": "database.instance::n1-instance-1-id", + "reason": "dependency_updated", + "diff": null + } + ], + [ + { + "type": "update", + "resource_id": "database.node::n1", + "reason": "dependency_updated", + "diff": null + }, + { + "type": "update", + "resource_id": "monitor.instance::n1-instance-1-id", + "reason": "dependency_updated", + "diff": null + } + ], + [ + { + "type": "update", + "resource_id": "database.postgres_database::n1:test", + "reason": "dependency_updated", + "diff": null + } + ], + [ + { + "type": "update", + "resource_id": "database.replication_slot::n1:n2:test", + "reason": "dependency_updated", + "diff": null + } + ], + [ + { + "type": "update", + "resource_id": "database.subscription::n1:n2:test", + "reason": "dependency_updated", + "diff": null + }, + { + "type": "update", + "resource_id": "database.subscription::n2:n1:test", + "reason": "dependency_updated", + "diff": null + } + ] + ], + [ + [ + { + "type": "create", + "resource_id": "orchestrator.resource::n1-instance-2-dep-1-id", + "reason": "does_not_exist", + "diff": null + } + ], + [ + { + "type": "create", + "resource_id": "database.instance::n1-instance-2-id", + "reason": "does_not_exist", + "diff": null + } + ], + [ + { + "type": "update", + "resource_id": "database.node::n1", + "reason": "dependency_updated", + "diff": null + } + ], + [ + { + "type": "update", + "resource_id": "database.postgres_database::n1:test", + "reason": "dependency_updated", + "diff": null + } + ], + [ + { + "type": "update", + "resource_id": "database.replication_slot::n1:n2:test", + "reason": "dependency_updated", + "diff": null + } + ], + [ + { + "type": "update", + "resource_id": "database.subscription::n1:n2:test", + "reason": "dependency_updated", + "diff": null + }, + { + "type": "update", + "resource_id": "database.subscription::n2:n1:test", + "reason": "dependency_updated", + "diff": null + } + ] + ], + [ + [ + { + "type": "create", + "resource_id": "monitor.instance::n1-instance-2-id", + "reason": "does_not_exist", + "diff": null + } + ], + [ + { + "type": "create", + "resource_id": "database.switchover::n1-instance-1-id", + "reason": "does_not_exist", + "diff": null + } + ] + ] +] \ No newline at end of file diff --git a/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_multiple_replicas_concurrent.json b/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_multiple_replicas_concurrent.json index 4a2f78c6..5590eb51 100644 --- a/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_multiple_replicas_concurrent.json +++ b/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_multiple_replicas_concurrent.json @@ -27,33 +27,19 @@ "reason": "does_not_exist", "diff": null } - ] - ], - [ + ], [ { "type": "update", "resource_id": "database.node::n1", - "reason": "has_diff", - "diff": [ - { - "value": "n1-instance-2-id", - "op": "add", - "path": "/instance_ids/-" - } - ] + "reason": "dependency_updated", + "diff": null }, { "type": "update", "resource_id": "database.node::n2", - "reason": "has_diff", - "diff": [ - { - "value": "n2-instance-2-id", - "op": "add", - "path": "/instance_ids/-" - } - ] + "reason": "dependency_updated", + "diff": null } ], [ @@ -97,33 +83,33 @@ "reason": "dependency_updated", "diff": null } - ], + ] + ], + [ [ { "type": "create", - "resource_id": "database.switchover::n1-instance-1-id", + "resource_id": "monitor.instance::n1-instance-2-id", "reason": "does_not_exist", "diff": null }, { "type": "create", - "resource_id": "database.switchover::n2-instance-1-id", + "resource_id": "monitor.instance::n2-instance-2-id", "reason": "does_not_exist", "diff": null } - ] - ], - [ + ], [ { "type": "create", - "resource_id": "monitor.instance::n1-instance-2-id", + "resource_id": "database.switchover::n1-instance-1-id", "reason": "does_not_exist", "diff": null }, { "type": "create", - "resource_id": "monitor.instance::n2-instance-2-id", + "resource_id": "database.switchover::n2-instance-1-id", "reason": "does_not_exist", "diff": null } diff --git a/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_multiple_replicas_rolling.json b/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_multiple_replicas_rolling.json index 8b88961a..022d3516 100644 --- a/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_multiple_replicas_rolling.json +++ b/server/internal/database/operations/golden_test/TestUpdateDatabase/adding_multiple_replicas_rolling.json @@ -15,21 +15,13 @@ "reason": "does_not_exist", "diff": null } - ] - ], - [ + ], [ { "type": "update", "resource_id": "database.node::n1", - "reason": "has_diff", - "diff": [ - { - "value": "n1-instance-2-id", - "op": "add", - "path": "/instance_ids/-" - } - ] + "reason": "dependency_updated", + "diff": null } ], [ @@ -61,14 +53,6 @@ "reason": "dependency_updated", "diff": null } - ], - [ - { - "type": "create", - "resource_id": "database.switchover::n1-instance-1-id", - "reason": "does_not_exist", - "diff": null - } ] ], [ @@ -87,21 +71,13 @@ "reason": "does_not_exist", "diff": null } - ] - ], - [ + ], [ { "type": "update", "resource_id": "database.node::n2", - "reason": "has_diff", - "diff": [ - { - "value": "n2-instance-2-id", - "op": "add", - "path": "/instance_ids/-" - } - ] + "reason": "dependency_updated", + "diff": null } ], [ @@ -133,33 +109,33 @@ "reason": "dependency_updated", "diff": null } - ], + ] + ], + [ [ { - "type": "update", - "resource_id": "database.switchover::n1-instance-1-id", - "reason": "dependency_updated", + "type": "create", + "resource_id": "monitor.instance::n1-instance-2-id", + "reason": "does_not_exist", "diff": null }, { "type": "create", - "resource_id": "database.switchover::n2-instance-1-id", + "resource_id": "monitor.instance::n2-instance-2-id", "reason": "does_not_exist", "diff": null } - ] - ], - [ + ], [ { "type": "create", - "resource_id": "monitor.instance::n1-instance-2-id", + "resource_id": "database.switchover::n1-instance-1-id", "reason": "does_not_exist", "diff": null }, { "type": "create", - "resource_id": "monitor.instance::n2-instance-2-id", + "resource_id": "database.switchover::n2-instance-1-id", "reason": "does_not_exist", "diff": null } diff --git a/server/internal/database/operations/update_database.go b/server/internal/database/operations/update_database.go index 5cac9cbb..69b019cb 100644 --- a/server/internal/database/operations/update_database.go +++ b/server/internal/database/operations/update_database.go @@ -60,7 +60,7 @@ func UpdateDatabase( // is available to be a source node. var states []*resource.State if len(updates) > 0 { - u, err := update(updates) + u, err := update(start, updates) if err != nil { return nil, err } @@ -126,7 +126,7 @@ func partitionNodes(start *resource.State, nodes []*NodeResources) ([]*NodeResou return updates, adds, nil } -func updateFunc(options UpdateDatabaseOptions) (func([]*NodeResources) ([]*resource.State, error), error) { +func updateFunc(options UpdateDatabaseOptions) (func(*resource.State, []*NodeResources) ([]*resource.State, error), error) { switch options.NodeUpdateStrategy { case "", NodeUpdateStrategyRolling: return RollingUpdateNodes, nil diff --git a/server/internal/database/operations/update_database_test.go b/server/internal/database/operations/update_database_test.go index c1ab528c..fbcd178c 100644 --- a/server/internal/database/operations/update_database_test.go +++ b/server/internal/database/operations/update_database_test.go @@ -341,6 +341,28 @@ func TestUpdateDatabase(t *testing.T) { }, }, }, + { + name: "adding a replica with primary update", + options: operations.UpdateDatabaseOptions{}, + start: twoNodeState, + // The primary instance should get updated before the replica is + // added. + nodes: []*operations.NodeResources{ + { + NodeName: "n1", + InstanceResources: []*database.InstanceResources{ + n1Instance1WithNewDependency, + n1Instance2, + }, + DatabaseName: "test", + }, + { + NodeName: "n2", + InstanceResources: []*database.InstanceResources{n2Instance1}, + DatabaseName: "test", + }, + }, + }, { name: "add an instance dependency with forced update", options: operations.UpdateDatabaseOptions{ diff --git a/server/internal/database/operations/update_nodes.go b/server/internal/database/operations/update_nodes.go index 03328267..951b4647 100644 --- a/server/internal/database/operations/update_nodes.go +++ b/server/internal/database/operations/update_nodes.go @@ -2,6 +2,7 @@ package operations import ( "fmt" + "slices" "github.com/pgEdge/control-plane/server/internal/database" "github.com/pgEdge/control-plane/server/internal/patroni" @@ -12,28 +13,32 @@ import ( // state only contains changed resources from the previous state. These states // should be cumulatively merged with previous states to produce the sequence of // desired states. This function always updates the node's replica instances. -func UpdateNode(node *NodeResources) ([]*resource.State, error) { - // Updates are performed on replica instances first +func UpdateNode(start *resource.State, node *NodeResources) ([]*resource.State, error) { var primary *resource.State - + var replicaUpdates, replicaAdds []*resource.State var primaryHostID string - states := make([]*resource.State, 0, len(node.InstanceResources)) - for _, inst := range node.InstanceResources { - instanceID := inst.InstanceID() + for _, inst := range node.InstanceResources { state, err := inst.InstanceState() if err != nil { return nil, err } - if instanceID == node.PrimaryInstanceID { + + switch { + case inst.InstanceID() == node.PrimaryInstanceID: + if !start.HasResources(inst.Instance.Identifier()) { + return nil, fmt.Errorf("invalid state: node %s exists, but its primary instance '%s' hasn't been created yet", node.NodeName, node.PrimaryInstanceID) + } primary = state primaryHostID = inst.HostID() - } else { - states = append(states, state) + case start.HasResources(inst.Instance.Identifier()): + replicaUpdates = append(replicaUpdates, state) + default: + replicaAdds = append(replicaAdds, state) } } if primary == nil { - // TODO(PLAT-240): This is another place where we assume that a node has + // TODO(PLAT-582): This is another place where we assume that a node has // a primary instance. We're returning an error here so that we don't // break downstream components that make the same assumption. We'll need // to change this if we want workflows to handle nodes without a primary @@ -41,8 +46,8 @@ func UpdateNode(node *NodeResources) ([]*resource.State, error) { return nil, fmt.Errorf("node %s has no primary instance", node.NodeName) } - // This condition is true when we have replicas - if len(states) != 0 { + // This condition is true when we have existing replicas + if len(replicaUpdates) != 0 { // Ensure that we always switch back to the original primary err := primary.AddResource(&database.SwitchoverResource{ HostID: primaryHostID, @@ -54,7 +59,10 @@ func UpdateNode(node *NodeResources) ([]*resource.State, error) { } } - states = append(states, primary) + // Existing replicas should be updated first and new replicas should be + // added last. + states := slices.Concat(replicaUpdates, []*resource.State{primary}, replicaAdds) + nodeState, err := node.nodeResourceState() if err != nil { return nil, err @@ -67,11 +75,11 @@ func UpdateNode(node *NodeResources) ([]*resource.State, error) { // RollingUpdateNodes returns a sequence of state diffs where each of the given // nodes is updated in-order sequentially. This function retains the // replica-first order from UpdateNode. -func RollingUpdateNodes(nodes []*NodeResources) ([]*resource.State, error) { +func RollingUpdateNodes(start *resource.State, nodes []*NodeResources) ([]*resource.State, error) { // Updates each node sequentially var states []*resource.State for _, node := range nodes { - update, err := UpdateNode(node) + update, err := UpdateNode(start, node) if err != nil { return nil, err } @@ -83,11 +91,11 @@ func RollingUpdateNodes(nodes []*NodeResources) ([]*resource.State, error) { // ConcurrentUpdateNodes returns a sequence of state diffs where each of the // given nodes is updated simultaneously. This function retains the // replica-first order from UpdateNode. -func ConcurrentUpdateNodes(nodes []*NodeResources) ([]*resource.State, error) { +func ConcurrentUpdateNodes(start *resource.State, nodes []*NodeResources) ([]*resource.State, error) { // Updates each node simultaneously states := make([][]*resource.State, len(nodes)) for i, node := range nodes { - update, err := UpdateNode(node) + update, err := UpdateNode(start, node) if err != nil { return nil, err } diff --git a/server/internal/database/operations/update_nodes_test.go b/server/internal/database/operations/update_nodes_test.go index 84109f0d..697652b8 100644 --- a/server/internal/database/operations/update_nodes_test.go +++ b/server/internal/database/operations/update_nodes_test.go @@ -19,7 +19,8 @@ func TestUpdateNode(t *testing.T) { for _, tc := range []struct { name string - input *operations.NodeResources + start *resource.State + node *operations.NodeResources expected []*resource.State expectedErr string }{ @@ -27,7 +28,17 @@ func TestUpdateNode(t *testing.T) { // When there's one instance, we should produce one state with the // instance and the node resource. name: "one instance", - input: &operations.NodeResources{ + start: makeState(t, + []resource.Resource{ + instance1.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{instance1.InstanceID()}, + }, + }, + instance1.InstanceDependencies, + ), + node: &operations.NodeResources{ DatabaseName: "test", NodeName: "n1", PrimaryInstanceID: instance1.InstanceID(), @@ -51,7 +62,24 @@ func TestUpdateNode(t *testing.T) { // instance and a second state with the primary instance and the // node resource. name: "two instances", - input: &operations.NodeResources{ + start: makeState(t, + []resource.Resource{ + instance1.Instance, + instance2.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{ + instance1.InstanceID(), + instance2.InstanceID(), + }, + }, + }, + slices.Concat( + instance1.InstanceDependencies, + instance2.InstanceDependencies, + ), + ), + node: &operations.NodeResources{ DatabaseName: "test", NodeName: "n1", PrimaryInstanceID: instance1.InstanceID(), @@ -91,7 +119,27 @@ func TestUpdateNode(t *testing.T) { // With 3 instances, we should produce three states, where the last // state contains the primary instance and the node resource. name: "three instances", - input: &operations.NodeResources{ + start: makeState(t, + []resource.Resource{ + instance1.Instance, + instance2.Instance, + instance3.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{ + instance1.InstanceID(), + instance2.InstanceID(), + instance3.InstanceID(), + }, + }, + }, + slices.Concat( + instance1.InstanceDependencies, + instance2.InstanceDependencies, + instance3.InstanceDependencies, + ), + ), + node: &operations.NodeResources{ DatabaseName: "test", NodeName: "n1", PrimaryInstanceID: instance1.InstanceID(), @@ -136,11 +184,58 @@ func TestUpdateNode(t *testing.T) { }, }, { - // TODO(PLAT-240): we need to decide how to handle this case. For + // New instances are processed after existing ones. + name: "adding a replica", + start: makeState(t, + []resource.Resource{ + instance1.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{ + instance1.InstanceID(), + }, + }, + }, + instance1.InstanceDependencies, + ), + node: &operations.NodeResources{ + DatabaseName: "test", + NodeName: "n1", + PrimaryInstanceID: instance1.InstanceID(), + InstanceResources: []*database.InstanceResources{ + instance1, + instance2, + }, + }, + expected: []*resource.State{ + makeState(t, + []resource.Resource{ + instance1.Instance, + }, + instance1.InstanceDependencies, + ), + makeState(t, + []resource.Resource{ + instance2.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{ + instance1.InstanceID(), + instance2.InstanceID(), + }, + }, + }, + instance2.InstanceDependencies, + ), + }, + }, + { + // TODO(PLAT-582): we need to decide how to handle this case. For // now, this produces an error to avoid breaking downstream // components. - name: "no primary", - input: &operations.NodeResources{ + name: "no primary", + start: resource.NewState(), + node: &operations.NodeResources{ DatabaseName: "test", NodeName: "n1", InstanceResources: []*database.InstanceResources{ @@ -149,9 +244,22 @@ func TestUpdateNode(t *testing.T) { }, expectedErr: "node n1 has no primary instance", }, + { + name: "primary not created", + start: resource.NewState(), + node: &operations.NodeResources{ + DatabaseName: "test", + NodeName: "n1", + PrimaryInstanceID: instance1.InstanceID(), + InstanceResources: []*database.InstanceResources{ + instance1, + }, + }, + expectedErr: "invalid state: node n1 exists, but its primary instance 'n1-instance-1-id' hasn't been created yet", + }, } { t.Run(tc.name, func(t *testing.T) { - out, err := operations.UpdateNode(tc.input) + out, err := operations.UpdateNode(tc.start, tc.node) if tc.expectedErr != "" { assert.Nil(t, out) assert.ErrorContains(t, err, tc.expectedErr) @@ -171,14 +279,25 @@ func TestRollingUpdateNodes(t *testing.T) { for _, tc := range []struct { name string - input []*operations.NodeResources + start *resource.State + nodes []*operations.NodeResources expected []*resource.State expectedErr string }{ { // This should look identical to the UpdateNode output. name: "one node with one instance", - input: []*operations.NodeResources{ + start: makeState(t, + []resource.Resource{ + n1Instance1.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{n1Instance1.InstanceID()}, + }, + }, + n1Instance1.InstanceDependencies, + ), + nodes: []*operations.NodeResources{ { DatabaseName: "test", NodeName: "n1", @@ -202,7 +321,25 @@ func TestRollingUpdateNodes(t *testing.T) { { // This should produce two states with one instance in each. name: "two nodes with one instance each", - input: []*operations.NodeResources{ + start: makeState(t, + []resource.Resource{ + n1Instance1.Instance, + n2Instance1.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{n1Instance1.InstanceID()}, + }, + &database.NodeResource{ + Name: "n2", + InstanceIDs: []string{n2Instance1.InstanceID()}, + }, + }, + slices.Concat( + n1Instance1.InstanceDependencies, + n2Instance1.InstanceDependencies, + ), + ), + nodes: []*operations.NodeResources{ { DatabaseName: "test", NodeName: "n1", @@ -244,7 +381,30 @@ func TestRollingUpdateNodes(t *testing.T) { // primary instance and node resource, n2's instance and node // resource. name: "two nodes with one replica", - input: []*operations.NodeResources{ + start: makeState(t, + []resource.Resource{ + n1Instance1.Instance, + n1Instance2.Instance, + n2Instance1.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{ + n1Instance1.InstanceID(), + n1Instance2.InstanceID(), + }, + }, + &database.NodeResource{ + Name: "n2", + InstanceIDs: []string{n2Instance1.InstanceID()}, + }, + }, + slices.Concat( + n1Instance1.InstanceDependencies, + n1Instance2.InstanceDependencies, + n2Instance1.InstanceDependencies, + ), + ), + nodes: []*operations.NodeResources{ { DatabaseName: "test", NodeName: "n1", @@ -302,7 +462,35 @@ func TestRollingUpdateNodes(t *testing.T) { // This should produce four states: n1's replica, n1's primary + // node, n2's replica, n2's primary + node. name: "two nodes with two replicas", - input: []*operations.NodeResources{ + start: makeState(t, + []resource.Resource{ + n1Instance1.Instance, + n1Instance2.Instance, + n2Instance1.Instance, + n2Instance2.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{ + n1Instance1.InstanceID(), + n1Instance2.InstanceID(), + }, + }, + &database.NodeResource{ + Name: "n2", + InstanceIDs: []string{ + n2Instance1.InstanceID(), + n2Instance2.InstanceID(), + }, + }, + }, + slices.Concat( + n1Instance1.InstanceDependencies, + n1Instance2.InstanceDependencies, + n2Instance1.InstanceDependencies, + n2Instance2.InstanceDependencies, + ), + ), + nodes: []*operations.NodeResources{ { DatabaseName: "test", NodeName: "n1", @@ -373,9 +561,81 @@ func TestRollingUpdateNodes(t *testing.T) { ), }, }, + { + // This should produce three states: n1's primary instance, n1's + // replica instance and node resource, n2's instance and node + // resource. + name: "two nodes with one new replica", + start: makeState(t, + []resource.Resource{ + n1Instance1.Instance, + n2Instance1.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{n1Instance1.InstanceID()}, + }, + &database.NodeResource{ + Name: "n2", + InstanceIDs: []string{n2Instance1.InstanceID()}, + }, + }, + slices.Concat( + n1Instance1.InstanceDependencies, + n2Instance1.InstanceDependencies, + ), + ), + nodes: []*operations.NodeResources{ + { + DatabaseName: "test", + NodeName: "n1", + PrimaryInstanceID: n1Instance1.InstanceID(), + InstanceResources: []*database.InstanceResources{ + n1Instance1, + n1Instance2, + }, + }, + { + DatabaseName: "test", + NodeName: "n2", + PrimaryInstanceID: n2Instance1.InstanceID(), + InstanceResources: []*database.InstanceResources{n2Instance1}, + }, + }, + expected: []*resource.State{ + makeState(t, + []resource.Resource{ + n1Instance1.Instance, + }, + n1Instance1.InstanceDependencies, + ), + makeState(t, + []resource.Resource{ + n1Instance2.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{ + n1Instance1.InstanceID(), + n1Instance2.InstanceID(), + }, + }, + }, + n1Instance2.InstanceDependencies, + ), + makeState(t, + []resource.Resource{ + n2Instance1.Instance, + &database.NodeResource{ + Name: "n2", + InstanceIDs: []string{n2Instance1.InstanceID()}, + }, + }, + n2Instance1.InstanceDependencies, + ), + }, + }, } { t.Run(tc.name, func(t *testing.T) { - out, err := operations.RollingUpdateNodes(tc.input) + out, err := operations.RollingUpdateNodes(tc.start, tc.nodes) if tc.expectedErr != "" { assert.Nil(t, out) assert.ErrorContains(t, err, tc.expectedErr) @@ -395,14 +655,25 @@ func TestConcurrentUpdateNodes(t *testing.T) { for _, tc := range []struct { name string - input []*operations.NodeResources + start *resource.State + nodes []*operations.NodeResources expected []*resource.State expectedErr string }{ { // This should look identical to the UpdateNode output. name: "one node with one instance", - input: []*operations.NodeResources{ + start: makeState(t, + []resource.Resource{ + n1Instance1.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{n1Instance1.InstanceID()}, + }, + }, + n1Instance1.InstanceDependencies, + ), + nodes: []*operations.NodeResources{ { DatabaseName: "test", NodeName: "n1", @@ -426,7 +697,25 @@ func TestConcurrentUpdateNodes(t *testing.T) { { // This should produce one state with both instances/nodes. name: "two nodes with one instance each", - input: []*operations.NodeResources{ + start: makeState(t, + []resource.Resource{ + n1Instance1.Instance, + n2Instance1.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{n1Instance1.InstanceID()}, + }, + &database.NodeResource{ + Name: "n2", + InstanceIDs: []string{n2Instance1.InstanceID()}, + }, + }, + slices.Concat( + n1Instance1.InstanceDependencies, + n2Instance1.InstanceDependencies, + ), + ), + nodes: []*operations.NodeResources{ { DatabaseName: "test", NodeName: "n1", @@ -466,7 +755,30 @@ func TestConcurrentUpdateNodes(t *testing.T) { // primary instance + node, followed by n1's primary instance and // node resource. name: "two nodes with one replica", - input: []*operations.NodeResources{ + start: makeState(t, + []resource.Resource{ + n1Instance1.Instance, + n1Instance2.Instance, + n2Instance1.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{ + n1Instance1.InstanceID(), + n1Instance2.InstanceID(), + }, + }, + &database.NodeResource{ + Name: "n2", + InstanceIDs: []string{n2Instance1.InstanceID()}, + }, + }, + slices.Concat( + n1Instance1.InstanceDependencies, + n1Instance2.InstanceDependencies, + n2Instance1.InstanceDependencies, + ), + ), + nodes: []*operations.NodeResources{ { DatabaseName: "test", NodeName: "n1", @@ -522,7 +834,35 @@ func TestConcurrentUpdateNodes(t *testing.T) { // This should produce two states: n1's replica and n2's replica, // followed by n1's primary + node and n2's primary + node. name: "two nodes with two replicas", - input: []*operations.NodeResources{ + start: makeState(t, + []resource.Resource{ + n1Instance1.Instance, + n1Instance2.Instance, + n2Instance1.Instance, + n2Instance2.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{ + n1Instance1.InstanceID(), + n1Instance2.InstanceID(), + }, + }, + &database.NodeResource{ + Name: "n2", + InstanceIDs: []string{ + n2Instance1.InstanceID(), + n2Instance2.InstanceID(), + }, + }, + }, + slices.Concat( + n1Instance1.InstanceDependencies, + n1Instance2.InstanceDependencies, + n2Instance1.InstanceDependencies, + n2Instance2.InstanceDependencies, + ), + ), + nodes: []*operations.NodeResources{ { DatabaseName: "test", NodeName: "n1", @@ -589,9 +929,158 @@ func TestConcurrentUpdateNodes(t *testing.T) { ), }, }, + { + // This should produce two states: n1's primary instance + n2's + // instance and node resource, n1's replica instance and node + // resource. + name: "two nodes with one new replica", + start: makeState(t, + []resource.Resource{ + n1Instance1.Instance, + n2Instance1.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{n1Instance1.InstanceID()}, + }, + &database.NodeResource{ + Name: "n2", + InstanceIDs: []string{n2Instance1.InstanceID()}, + }, + }, + slices.Concat( + n1Instance1.InstanceDependencies, + n2Instance1.InstanceDependencies, + ), + ), + nodes: []*operations.NodeResources{ + { + DatabaseName: "test", + NodeName: "n1", + PrimaryInstanceID: n1Instance1.InstanceID(), + InstanceResources: []*database.InstanceResources{ + n1Instance1, + n1Instance2, + }, + }, + { + DatabaseName: "test", + NodeName: "n2", + PrimaryInstanceID: n2Instance1.InstanceID(), + InstanceResources: []*database.InstanceResources{n2Instance1}, + }, + }, + expected: []*resource.State{ + makeState(t, + []resource.Resource{ + n1Instance1.Instance, + n2Instance1.Instance, + &database.NodeResource{ + Name: "n2", + InstanceIDs: []string{n2Instance1.InstanceID()}, + }, + }, + slices.Concat( + n1Instance1.InstanceDependencies, + n2Instance1.InstanceDependencies, + ), + ), + makeState(t, + []resource.Resource{ + n1Instance2.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{ + n1Instance1.InstanceID(), + n1Instance2.InstanceID(), + }, + }, + }, + n1Instance2.InstanceDependencies, + ), + }, + }, + { + // This should produce two states: n1 and n2's primary instances, n1 + // and n2's replica instances and node resources. + name: "two nodes with two new replicas", + start: makeState(t, + []resource.Resource{ + n1Instance1.Instance, + n2Instance1.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{n1Instance1.InstanceID()}, + }, + &database.NodeResource{ + Name: "n2", + InstanceIDs: []string{n2Instance1.InstanceID()}, + }, + }, + slices.Concat( + n1Instance1.InstanceDependencies, + n2Instance1.InstanceDependencies, + ), + ), + nodes: []*operations.NodeResources{ + { + DatabaseName: "test", + NodeName: "n1", + PrimaryInstanceID: n1Instance1.InstanceID(), + InstanceResources: []*database.InstanceResources{ + n1Instance1, + n1Instance2, + }, + }, + { + DatabaseName: "test", + NodeName: "n2", + PrimaryInstanceID: n2Instance1.InstanceID(), + InstanceResources: []*database.InstanceResources{ + n2Instance1, + n2Instance2, + }, + }, + }, + expected: []*resource.State{ + makeState(t, + []resource.Resource{ + n1Instance1.Instance, + n2Instance1.Instance, + }, + slices.Concat( + n1Instance1.InstanceDependencies, + n2Instance1.InstanceDependencies, + ), + ), + makeState(t, + []resource.Resource{ + n1Instance2.Instance, + n2Instance2.Instance, + &database.NodeResource{ + Name: "n1", + InstanceIDs: []string{ + n1Instance1.InstanceID(), + n1Instance2.InstanceID(), + }, + }, + &database.NodeResource{ + Name: "n2", + InstanceIDs: []string{ + n2Instance1.InstanceID(), + n2Instance2.InstanceID(), + }, + }, + }, + slices.Concat( + n1Instance2.InstanceDependencies, + n2Instance2.InstanceDependencies, + ), + ), + }, + }, } { t.Run(tc.name, func(t *testing.T) { - out, err := operations.ConcurrentUpdateNodes(tc.input) + out, err := operations.ConcurrentUpdateNodes(tc.start, tc.nodes) if tc.expectedErr != "" { assert.Nil(t, out) assert.ErrorContains(t, err, tc.expectedErr)