diff --git a/.circleci/config.yml b/.circleci/config.yml index f12d5144..111f00e7 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -61,13 +61,21 @@ commands: docker-compose-plugin sudo systemctl start docker jobs: - test: + # basic_check: unit tests + lint + license check (make ci) + basic_check: executor: common steps: - common_setup - run: name: Run CI checks command: make ci TEST_RERUN_FAILS=2 + - store_test_results: + path: . + + test_cluster: + executor: common + steps: + - common_setup - run: name: Run cluster tests command: | @@ -75,11 +83,44 @@ jobs: make start-local-registry make buildx-init make test-cluster CONTROL_PLANE_IMAGE_REPO=172.17.0.1:5000/control-plane TEST_RERUN_FAILS=2 + - store_test_results: + path: . + + test_e2e: + executor: common + parallelism: 3 + steps: + - common_setup + - run: + name: Select this container's e2e tests + command: | + # Split e2e test files (excluding main_test.go, whose only test + # is the special TestMain entrypoint and can't be filtered via + # -run) across the 3 parallel containers, falling back to an + # even file-count split until enough --junitfile timing history + # (test-e2e-results.xml) has accumulated across runs. + TEST_FILES=$(grep -l '^func Test' e2e/*_test.go | grep -v 'e2e/main_test.go' \ + | circleci tests split --split-by=timings --timings-type=filename) + if [ -z "$TEST_FILES" ]; then + TEST_NAMES="" + else + TEST_NAMES=$(grep -h '^func Test' $TEST_FILES \ + | sed -E 's/^func (Test[A-Za-z0-9_]*).*/\1/' | paste -sd '|') + fi + # $$ (not $) so that Make's own recursive variable expansion of + # $(E2E_RUN) in the Makefile collapses it back to a single + # literal $ instead of silently dropping a lone trailing $. + echo "export E2E_RUN='^(${TEST_NAMES})\$\$'" >> "$BASH_ENV" + echo "Running tests: ${TEST_NAMES}" - run: name: Run e2e tests with Docker Compose command: | - make ci-compose-detached - make test-e2e E2E_PARALLEL=4 E2E_DEBUG=1 E2E_FIXTURE=ci TEST_RERUN_FAILS=2 + if [ -z "$E2E_RUN" ] || [ "$E2E_RUN" = '^()$' ]; then + echo "No tests assigned to this container, skipping" + else + make ci-compose-detached + make test-e2e E2E_PARALLEL=4 E2E_DEBUG=1 E2E_FIXTURE=ci TEST_RERUN_FAILS=2 + fi - run: name: Archive debug output command: | @@ -172,7 +213,9 @@ workflows: test: unless: << pipeline.parameters.build_ref >> jobs: - - test + - basic_check + - test_cluster + - test_e2e release: jobs: diff --git a/Makefile b/Makefile index 9fd51b14..33ce6123 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ docker_compose_dev=WORKSPACE_DIR=$(shell pwd) \ docker_compose_ci=docker compose -f ./docker/control-plane-ci/docker-compose.yaml e2e_args=-tags=e2e_test -count=1 -timeout=45m \ $(if $(E2E_PARALLEL),-parallel $(E2E_PARALLEL)) \ - $(if $(E2E_RUN),-run $(E2E_RUN)) \ + $(if $(E2E_RUN),-run "$(E2E_RUN)") \ -args \ $(if $(E2E_FIXTURE),-fixture $(E2E_FIXTURE)) \ $(if $(filter 1,$(E2E_SKIP_CLEANUP)),-skip-cleanup) \ diff --git a/clustertest/add_remove_host_test.go b/clustertest/add_remove_host_test.go index e786e9bf..70d9da1e 100644 --- a/clustertest/add_remove_host_test.go +++ b/clustertest/add_remove_host_test.go @@ -154,7 +154,7 @@ func TestForcedHostRemovalWithDatabase(t *testing.T) { cluster.Host("host-3").Stop(t) // Allow cluster some time to detect the container termination - time.Sleep(5 * time.Second) + time.Sleep(15 * time.Second) tLog(t, "removing host-3 from cluster") resp, err := cluster.Client().RemoveHost(ctx, &api.RemoveHostPayload{ @@ -175,7 +175,7 @@ func TestForcedHostRemovalWithDatabase(t *testing.T) { require.NoError(t, err, "database not healthy with 2 nodes") tLog(t, "verifying cluster has 2 hosts") - err = waitForHostCount(ctx, cluster.Client(), 2, 30*time.Second) + err = waitForHostCount(ctx, cluster.Client(), 2, 60*time.Second) require.NoError(t, err, "cluster should have 2 hosts") // Ensure host-3 is not in the list diff --git a/clustertest/etcd_mode_change_test.go b/clustertest/etcd_mode_change_test.go index 26d05de6..9a872944 100644 --- a/clustertest/etcd_mode_change_test.go +++ b/clustertest/etcd_mode_change_test.go @@ -11,6 +11,7 @@ import ( ) func TestPromoteClientToServer(t *testing.T) { + t.Parallel() tLog(t, "initializing cluster with 3 etcd servers and 1 etcd client") @@ -55,6 +56,7 @@ func TestPromoteClientToServer(t *testing.T) { } func TestDemoteServerToClient(t *testing.T) { + t.Parallel() tLog(t, "initializing cluster with 4 etcd servers") diff --git a/docs/development/supported-services.md b/docs/development/supported-services.md index 7083781d..cff4073c 100644 --- a/docs/development/supported-services.md +++ b/docs/development/supported-services.md @@ -807,8 +807,8 @@ for state transitions. Key patterns to replicate for a new service type: |---------|-------------|-------------------| | Single-host provision | `TestProvisionMCPService` | Service reaches `"running"` state | | Multi-host provision | `TestProvisionMultiHostMCPService` | One instance per host, all reach `"running"` | -| Add to existing DB | `TestUpdateDatabaseAddService` | Service added without affecting database | -| Remove from DB | `TestUpdateDatabaseRemoveService` | Empty `Services` array removes the service | +| Add to existing DB | `TestUpdateDatabaseService/Add` | Service added without affecting database | +| Remove from DB | `TestUpdateDatabaseService/Remove` | Empty `Services` array removes the service | | Stability | `TestUpdateDatabaseServiceStable` | Unrelated DB update doesn't recreate service (checks `created_at` and `container_id` unchanged) | | Bad version | `TestProvisionMCPServiceUnsupportedVersion` | Unregistered version fails task, DB goes to `"failed"` | | Recovery | `TestProvisionMCPServiceRecovery` | Failed DB recovered by updating with valid version | diff --git a/e2e/add_replica_test.go b/e2e/add_replica_test.go index 3ed59cb3..e621d440 100644 --- a/e2e/add_replica_test.go +++ b/e2e/add_replica_test.go @@ -38,7 +38,11 @@ func TestAddReplica(t *testing.T) { Port: pointerTo(0), PatroniPort: pointerTo(0), Nodes: []*api.DatabaseNodeSpec{ - {Name: "n1", HostIds: []api.Identifier{api.Identifier(host1)}}, + { + Name: "n1", + HostIds: []api.Identifier{api.Identifier(host1)}, + PostgresqlConf: fastCheckpointConf, + }, {Name: "n2", HostIds: []api.Identifier{api.Identifier(host2)}}, }, }, @@ -78,6 +82,7 @@ func TestAddReplica(t *testing.T) { api.Identifier(host1), api.Identifier(host3), }, + PostgresqlConf: fastCheckpointConf, }, {Name: "n2", HostIds: []api.Identifier{api.Identifier(host2)}}, }, diff --git a/e2e/database_test.go b/e2e/database_test.go index bce282e2..d4a6ed1e 100644 --- a/e2e/database_test.go +++ b/e2e/database_test.go @@ -405,7 +405,7 @@ func (f *DatabaseFixture) WaitForReplication(ctx context.Context, t testing.TB, f.WithConnection(ctx, primaryOpts, t, func(conn *pgx.Conn) { var synced bool - row := conn.QueryRow(ctx, "CALL spock.wait_for_sync_event(true, $1, $2::pg_lsn, 30);", peerNode, lsn) + row := conn.QueryRow(ctx, "CALL spock.wait_for_sync_event(true, $1, $2::pg_lsn, 60);", peerNode, lsn) require.NoError(t, row.Scan(&synced)) assert.True(t, synced) }) diff --git a/e2e/db_create_test.go b/e2e/db_create_test.go index 590f3429..4605d3e7 100644 --- a/e2e/db_create_test.go +++ b/e2e/db_create_test.go @@ -57,10 +57,14 @@ func testCreateDB(t *testing.T, nodeCount int, deployReplicas bool) { if deployReplicas { hosts = append(hosts, controlplane.Identifier(host3)) } - nodes = append(nodes, &controlplane.DatabaseNodeSpec{ + node := &controlplane.DatabaseNodeSpec{ Name: fmt.Sprintf("n%d", i), HostIds: hosts, - }) + } + if deployReplicas { + node.PostgresqlConf = fastCheckpointConf + } + nodes = append(nodes, node) } db := fixture.NewDatabaseFixture(ctx, t, &controlplane.CreateDatabaseRequest{ @@ -141,12 +145,12 @@ func testCreateDB(t *testing.T, nodeCount int, deployReplicas bool) { row := conn.QueryRow(ctx, "SELECT pg_current_wal_lsn() AS commit_lsn") require.NoError(t, row.Scan(&commitLSN)) - // Wait for up to 10 seconds and verify commit_lsn has replayed + // Wait for up to 20 seconds and verify commit_lsn has replayed // This should prevent flaky tests var replayLSN string var hasReplayed bool - for i := 0; i < 10; i++ { + for i := 0; i < 20; i++ { rows, err := conn.Query(ctx, "SELECT replay_lsn, (replay_lsn >= $1::pg_lsn) AS has_replayed FROM pg_stat_replication", commitLSN) require.NoError(t, err) defer rows.Close() diff --git a/e2e/db_update_add_node_test.go b/e2e/db_update_add_node_test.go index 8ecdb36f..cba41441 100644 --- a/e2e/db_update_add_node_test.go +++ b/e2e/db_update_add_node_test.go @@ -175,7 +175,7 @@ func verifyWALReplay(ctx context.Context, t *testing.T, conn *pgx.Conn) { var commitLSN string require.NoError(t, conn.QueryRow(ctx, "SELECT pg_current_wal_lsn()").Scan(&commitLSN)) - for i := 0; i < 10; i++ { + for i := 0; i < 20; i++ { rows, err := conn.Query(ctx, "SELECT (replay_lsn >= $1::pg_lsn) AS has_replayed FROM pg_stat_replication", commitLSN) require.NoError(t, err) defer rows.Close() diff --git a/e2e/main_test.go b/e2e/main_test.go index bd8c89ec..b836294f 100644 --- a/e2e/main_test.go +++ b/e2e/main_test.go @@ -71,3 +71,11 @@ func TestMain(m *testing.M) { func pointerTo[T any](v T) *T { return &v } + +// fastCheckpointConf shortens checkpoint_timeout/checkpoint_completion_target +// so that replica bootstrap (pg_basebackup) on the primary isn't delayed by +// the production checkpoint defaults in short-lived test databases. +var fastCheckpointConf = map[string]any{ + "checkpoint_timeout": "30s", + "checkpoint_completion_target": "0", +} diff --git a/e2e/service_provisioning_test.go b/e2e/service_provisioning_test.go index adfeacf3..3b28b80e 100644 --- a/e2e/service_provisioning_test.go +++ b/e2e/service_provisioning_test.go @@ -229,8 +229,12 @@ func TestProvisionMultiHostMCPService(t *testing.T) { t.Log("Multi-host MCP service provisioning test completed successfully") } -// TestUpdateDatabaseAddService tests adding a service to an existing database. -func TestUpdateDatabaseAddService(t *testing.T) { +// TestUpdateDatabaseService verifies that services can be declaratively +// added to and removed from an existing database via update, sharing a +// single database across subtests since the update behavior (not the +// database creation) is under test. Add runs before Remove since Remove +// depends on a service already being present. +func TestUpdateDatabaseService(t *testing.T) { t.Parallel() fixture.SkipIfServicesUnsupported(t) @@ -246,7 +250,7 @@ func TestUpdateDatabaseAddService(t *testing.T) { // Create database without services db := fixture.NewDatabaseFixture(ctx, t, &controlplane.CreateDatabaseRequest{ Spec: &controlplane.DatabaseSpec{ - DatabaseName: "test_add_service", + DatabaseName: "test_update_database_service", DatabaseUsers: []*controlplane.DatabaseUserSpec{ { Username: "admin", @@ -269,61 +273,102 @@ func TestUpdateDatabaseAddService(t *testing.T) { // Verify no service instances initially assert.Empty(t, db.ServiceInstances, "Should have no service instances initially") - t.Log("Adding MCP service to existing database") + addOK := t.Run("Add", func(t *testing.T) { + t.Log("Adding MCP service to existing database") - // Update database to add service - err := db.Update(ctx, UpdateOptions{ - Spec: &controlplane.DatabaseSpec{ - DatabaseName: "test_add_service", - DatabaseUsers: []*controlplane.DatabaseUserSpec{ - { - Username: "admin", - Password: pointerTo("testpassword"), - DbOwner: pointerTo(true), - Attributes: []string{"LOGIN", "SUPERUSER"}, + // Update database to add service + err := db.Update(ctx, UpdateOptions{ + Spec: &controlplane.DatabaseSpec{ + DatabaseName: "test_update_database_service", + DatabaseUsers: []*controlplane.DatabaseUserSpec{ + { + Username: "admin", + Password: pointerTo("testpassword"), + DbOwner: pointerTo(true), + Attributes: []string{"LOGIN", "SUPERUSER"}, + }, }, - }, - Port: pointerTo(0), - PatroniPort: pointerTo(0), - Nodes: []*controlplane.DatabaseNodeSpec{ - { - Name: "n1", - HostIds: []controlplane.Identifier{controlplane.Identifier(host1)}, + Port: pointerTo(0), + PatroniPort: pointerTo(0), + Nodes: []*controlplane.DatabaseNodeSpec{ + { + Name: "n1", + HostIds: []controlplane.Identifier{controlplane.Identifier(host1)}, + }, }, - }, - Services: []*controlplane.ServiceSpec{ - { - ServiceID: "mcp-server", - ServiceType: "mcp", - ConnectAs: "admin", - //Version: "1.0.0", - Version: "latest", - HostIds: []controlplane.Identifier{controlplane.Identifier(host2)}, - Config: map[string]any{ - "llm_enabled": true, - "llm_provider": "ollama", - "llm_model": "llama2", - "ollama_url": "http://localhost:11434", + Services: []*controlplane.ServiceSpec{ + { + ServiceID: "mcp-server", + ServiceType: "mcp", + ConnectAs: "admin", + //Version: "1.0.0", + Version: "latest", + HostIds: []controlplane.Identifier{controlplane.Identifier(host2)}, + Config: map[string]any{ + "llm_enabled": true, + "llm_provider": "ollama", + "llm_model": "llama2", + "ollama_url": "http://localhost:11434", + }, }, }, }, - }, + }) + require.NoError(t, err, "Failed to update database") + + t.Log("Database updated, verifying service instance was added") + + // Verify service instance was created + require.NotNil(t, db.ServiceInstances, "ServiceInstances should not be nil") + require.Len(t, db.ServiceInstances, 1, "Expected 1 service instance after update") + + serviceInstance := db.ServiceInstances[0] + assert.Equal(t, "mcp-server", serviceInstance.ServiceID, "Service ID should match") + assert.Equal(t, string(host2), serviceInstance.HostID, "Host ID should match") + + t.Logf("Service instance added: %s (state: %s)", serviceInstance.ServiceInstanceID, serviceInstance.State) + + t.Log("Add service to existing database test completed successfully") }) - require.NoError(t, err, "Failed to update database") + if !addOK { + t.Fatal("Add subtest failed; skipping Remove since it depends on the service being present") + } - t.Log("Database updated, verifying service instance was added") + t.Run("Remove", func(t *testing.T) { + t.Log("Removing service from database") - // Verify service instance was created - require.NotNil(t, db.ServiceInstances, "ServiceInstances should not be nil") - require.Len(t, db.ServiceInstances, 1, "Expected 1 service instance after update") + // Update database to remove service (empty services array) + err := db.Update(ctx, UpdateOptions{ + Spec: &controlplane.DatabaseSpec{ + DatabaseName: "test_update_database_service", + DatabaseUsers: []*controlplane.DatabaseUserSpec{ + { + Username: "admin", + Password: pointerTo("testpassword"), + DbOwner: pointerTo(true), + Attributes: []string{"LOGIN", "SUPERUSER"}, + }, + }, + Port: pointerTo(0), + PatroniPort: pointerTo(0), + Nodes: []*controlplane.DatabaseNodeSpec{ + { + Name: "n1", + HostIds: []controlplane.Identifier{controlplane.Identifier(host1)}, + }, + }, + Services: []*controlplane.ServiceSpec{}, // Empty services array + }, + }) + require.NoError(t, err, "Failed to update database") - serviceInstance := db.ServiceInstances[0] - assert.Equal(t, "mcp-server", serviceInstance.ServiceID, "Service ID should match") - assert.Equal(t, string(host2), serviceInstance.HostID, "Host ID should match") + t.Log("Database updated, verifying service instance was removed") - t.Logf("Service instance added: %s (state: %s)", serviceInstance.ServiceInstanceID, serviceInstance.State) + // Verify service instance was removed (declarative deletion) + assert.Empty(t, db.ServiceInstances, "Service instances should be empty after removal") - t.Log("Add service to existing database test completed successfully") + t.Log("Remove service test completed successfully") + }) } // TestProvisionMCPServiceUnsupportedVersion tests that database creation fails @@ -600,6 +645,22 @@ func TestProvisionMCPServiceRecovery(t *testing.T) { require.NoError(t, err, "GetDatabase should succeed after update") assert.Equal(t, "available", db.State, "Database should be available after recovery update") + // Service instance creation can lag briefly behind the database + // reaching "available", so poll instead of asserting immediately. + if len(db.ServiceInstances) == 0 { + t.Log("Service instance not yet present, waiting for it to appear...") + + instanceDeadline := time.Now().Add(1 * time.Minute) + for time.Now().Before(instanceDeadline) && len(db.ServiceInstances) == 0 { + time.Sleep(2 * time.Second) + + db, err = fixture.Client.GetDatabase(ctx, &controlplane.GetDatabasePayload{ + DatabaseID: dbID, + }) + require.NoError(t, err, "Failed to refresh database") + } + } + // Service instance should now exist require.NotNil(t, db.ServiceInstances, "ServiceInstances should not be nil after recovery") require.Len(t, db.ServiceInstances, 1, "Expected 1 service instance after recovery") @@ -935,93 +996,3 @@ func TestUpdateMCPServiceConfig(t *testing.T) { t.Logf("Service instance %s updated in-place after config change", serviceInstanceID) t.Log("MCP service config update test completed successfully") } - -// TestUpdateDatabaseRemoveService tests removing a service from a database. -func TestUpdateDatabaseRemoveService(t *testing.T) { - t.Parallel() - - fixture.SkipIfServicesUnsupported(t) - - host1 := fixture.HostIDs()[0] - - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) - defer cancel() - - t.Log("Creating database with MCP service") - - // Create database with service - db := fixture.NewDatabaseFixture(ctx, t, &controlplane.CreateDatabaseRequest{ - Spec: &controlplane.DatabaseSpec{ - DatabaseName: "test_remove_service", - DatabaseUsers: []*controlplane.DatabaseUserSpec{ - { - Username: "admin", - Password: pointerTo("testpassword"), - DbOwner: pointerTo(true), - Attributes: []string{"LOGIN", "SUPERUSER"}, - }, - }, - Port: pointerTo(0), - PatroniPort: pointerTo(0), - Nodes: []*controlplane.DatabaseNodeSpec{ - { - Name: "n1", - HostIds: []controlplane.Identifier{controlplane.Identifier(host1)}, - }, - }, - Services: []*controlplane.ServiceSpec{ - { - ServiceID: "mcp-server", - ServiceType: "mcp", - ConnectAs: "admin", - //Version: "1.0.0", - Version: "latest", - HostIds: []controlplane.Identifier{controlplane.Identifier(host1)}, - Config: map[string]any{ - "llm_enabled": true, - "llm_provider": "anthropic", - "llm_model": "claude-sonnet-4-5", - "anthropic_api_key": "sk-ant-test", - }, - }, - }, - }, - }) - - // Verify service instance exists - require.Len(t, db.ServiceInstances, 1, "Expected 1 service instance initially") - - t.Log("Removing service from database") - - // Update database to remove service (empty services array) - err := db.Update(ctx, UpdateOptions{ - Spec: &controlplane.DatabaseSpec{ - DatabaseName: "test_remove_service", - DatabaseUsers: []*controlplane.DatabaseUserSpec{ - { - Username: "admin", - Password: pointerTo("testpassword"), - DbOwner: pointerTo(true), - Attributes: []string{"LOGIN", "SUPERUSER"}, - }, - }, - Port: pointerTo(0), - PatroniPort: pointerTo(0), - Nodes: []*controlplane.DatabaseNodeSpec{ - { - Name: "n1", - HostIds: []controlplane.Identifier{controlplane.Identifier(host1)}, - }, - }, - Services: []*controlplane.ServiceSpec{}, // Empty services array - }, - }) - require.NoError(t, err, "Failed to update database") - - t.Log("Database updated, verifying service instance was removed") - - // Verify service instance was removed (declarative deletion) - assert.Empty(t, db.ServiceInstances, "Service instances should be empty after removal") - - t.Log("Remove service test completed successfully") -}