From 94a8e6117c71c765f500af86d48f056224ec2bd8 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 12 May 2025 12:05:13 -0500 Subject: [PATCH 1/8] Add args to custom deployment/cleanup --- test_main.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/test_main.go b/test_main.go index c04641a9..6c010757 100644 --- a/test_main.go +++ b/test_main.go @@ -6,17 +6,18 @@ import ( "testing" "github.com/matrix-org/complement/b" + "github.com/matrix-org/complement/config" "github.com/matrix-org/complement/ct" ) var ( testPackage *TestPackage - customDeployer func(numServers int) Deployment + customDeployer func(t ct.TestLike, numServers int, config *config.Complement) Deployment ) type complementOpts struct { - cleanup func() - customDeployment func(numServers int) Deployment + cleanup func(config *config.Complement) + customDeployment func(t ct.TestLike, numServers int, config *config.Complement) Deployment } type opt func(*complementOpts) @@ -24,14 +25,14 @@ type opt func(*complementOpts) // It is called BEFORE Complement containers are destroyed. // This function should be used for per-suite cleanup operations e.g tearing down containers, killing // child processes, etc. -func WithCleanup(fn func()) opt { +func WithCleanup(fn func(config *config.Complement)) opt { return func(co *complementOpts) { co.cleanup = fn } } // WithDeployment adds a custom mechanism to deploy homeservers. -func WithDeployment(fn func(numServers int) Deployment) opt { +func WithDeployment(fn func(t ct.TestLike, numServers int, config *config.Complement) Deployment) opt { return func(co *complementOpts) { co.customDeployment = fn } @@ -64,7 +65,8 @@ func TestMain(m *testing.M, namespace string, customOpts ...opt) { } exitCode := m.Run() if opts.cleanup != nil { - opts.cleanup() + // TODO: Need access to `testPackage.Config` to see if `PackageNamespace` etc. + opts.cleanup(testPackage.Config) } testPackage.Cleanup() os.Exit(exitCode) @@ -91,7 +93,9 @@ func Deploy(t ct.TestLike, numServers int) Deployment { ct.Fatalf(t, "Deploy: testPackage not set, did you forget to call complement.TestMain?") } if customDeployer != nil { - return customDeployer(numServers) + // TODO: Need a way to return an error or pass in `t` + // TODO: Need access to `testPackage.Config` to see if `DebugLoggingEnabled`, `SpawnHSTimeout`, `PackageNamespace` etc. + return customDeployer(t, numServers, testPackage.Config) } return testPackage.Deploy(t, numServers) } From 896ab4f1efd303555bab2fa327aa15b037cdda68 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 12 May 2025 21:04:10 -0500 Subject: [PATCH 2/8] Remove TODO docs --- test_main.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test_main.go b/test_main.go index 6c010757..94ceda48 100644 --- a/test_main.go +++ b/test_main.go @@ -16,7 +16,14 @@ var ( ) type complementOpts struct { - cleanup func(config *config.Complement) + // Args: + // - We pass in the Complement config (`testPackage.Config`) so the deployer can inspect + // `DebugLoggingEnabled`, `SpawnHSTimeout`, `PackageNamespace` etc. + cleanup func(config *config.Complement) + // Args: + // - We pass in `t` as there needs to be a way to handle an error in the custom deployer. + // - We pass in the Complement config (`testPackage.Config`) so the deployer can inspect + // `DebugLoggingEnabled`, `SpawnHSTimeout`, `PackageNamespace`, etc. customDeployment func(t ct.TestLike, numServers int, config *config.Complement) Deployment } type opt func(*complementOpts) @@ -65,7 +72,6 @@ func TestMain(m *testing.M, namespace string, customOpts ...opt) { } exitCode := m.Run() if opts.cleanup != nil { - // TODO: Need access to `testPackage.Config` to see if `PackageNamespace` etc. opts.cleanup(testPackage.Config) } testPackage.Cleanup() @@ -93,8 +99,6 @@ func Deploy(t ct.TestLike, numServers int) Deployment { ct.Fatalf(t, "Deploy: testPackage not set, did you forget to call complement.TestMain?") } if customDeployer != nil { - // TODO: Need a way to return an error or pass in `t` - // TODO: Need access to `testPackage.Config` to see if `DebugLoggingEnabled`, `SpawnHSTimeout`, `PackageNamespace` etc. return customDeployer(t, numServers, testPackage.Config) } return testPackage.Deploy(t, numServers) From 9035d88a02ad52b3ccdacf7ac7b162b1ff3a8281 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 13 May 2025 15:45:46 -0500 Subject: [PATCH 3/8] Introduce `GetFullyQualifiedHomeserverName(...)` --- internal/docker/deployment.go | 10 ++++++++++ test_package.go | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/internal/docker/deployment.go b/internal/docker/deployment.go index 4322a5ed..7b2b04f8 100644 --- a/internal/docker/deployment.go +++ b/internal/docker/deployment.go @@ -57,6 +57,16 @@ func (hsDep *HomeserverDeployment) SetEndpoints(baseURL string, fedBaseURL strin } } +func (d *Deployment) GetFullyQualifiedHomeserverName(hsName string) (string, error) { + _, ok := d.HS[hsName] + if !ok { + return "", fmt.Errorf("Deployment.GetFullyQualifiedHomeserverName - HS name '%s' not found", hsName) + } + // We have network aliases for each Docker container that will resolve the `hsName` to + // the container. + return hsName, nil +} + // DestroyAtCleanup destroys the entire deployment. It should be called at cleanup time for dirty // deployments only. Handles configuration options for things which should run at container destroy // time, like post-run scripts and printing logs. diff --git a/test_package.go b/test_package.go index 720b57d2..999d64a9 100644 --- a/test_package.go +++ b/test_package.go @@ -20,6 +20,12 @@ import ( // Deployment provides a way for tests to interact with a set of homeservers. type Deployment interface { + // Returns the resolvable host name of a homeserver given its short alias (e.g., + // "hs1", "hs2"). + // + // In the case of the standard Docker deployment, this will be the same `hs1`, `hs2` + // but may be different for other custom deployments. + GetFullyQualifiedHomeserverName(hsName string) (string, error) // UnauthenticatedClient returns a blank CSAPI client. UnauthenticatedClient(t ct.TestLike, serverName string) *client.CSAPI // Register a new user on the given server. From 10578f6a88e5f4014b0d2f63c7ec2f8c2819ddae Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 13 May 2025 15:51:12 -0500 Subject: [PATCH 4/8] Make `GetFullyQualifiedHomeserverName` easier to use in tests Example: ``` alice.MustJoinRoom(t, bobRoomID, []string{ shardDeployment2.GetFullyQualifiedHomeserverName(t, "hs1"), }) ``` --- internal/docker/deployment.go | 6 +++--- test_package.go | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/internal/docker/deployment.go b/internal/docker/deployment.go index 7b2b04f8..12566886 100644 --- a/internal/docker/deployment.go +++ b/internal/docker/deployment.go @@ -57,14 +57,14 @@ func (hsDep *HomeserverDeployment) SetEndpoints(baseURL string, fedBaseURL strin } } -func (d *Deployment) GetFullyQualifiedHomeserverName(hsName string) (string, error) { +func (d *Deployment) GetFullyQualifiedHomeserverName(t ct.TestLike, hsName string) string { _, ok := d.HS[hsName] if !ok { - return "", fmt.Errorf("Deployment.GetFullyQualifiedHomeserverName - HS name '%s' not found", hsName) + ct.Fatalf(t, "Deployment.GetFullyQualifiedHomeserverName - HS name '%s' not found", hsName) } // We have network aliases for each Docker container that will resolve the `hsName` to // the container. - return hsName, nil + return hsName } // DestroyAtCleanup destroys the entire deployment. It should be called at cleanup time for dirty diff --git a/test_package.go b/test_package.go index 999d64a9..a6126d1d 100644 --- a/test_package.go +++ b/test_package.go @@ -20,12 +20,13 @@ import ( // Deployment provides a way for tests to interact with a set of homeservers. type Deployment interface { - // Returns the resolvable host name of a homeserver given its short alias (e.g., - // "hs1", "hs2"). + // Returns the resolvable server name (host) of a homeserver given its short alias + // (e.g., "hs1", "hs2"). // // In the case of the standard Docker deployment, this will be the same `hs1`, `hs2` - // but may be different for other custom deployments. - GetFullyQualifiedHomeserverName(hsName string) (string, error) + // but may be different for other custom deployments (ex. + // `shardDeployment1.GetFullyQualifiedHomeserverName(t, "hs1")` -> `hs1.shard1:8081`). + GetFullyQualifiedHomeserverName(t ct.TestLike, hsName string) string // UnauthenticatedClient returns a blank CSAPI client. UnauthenticatedClient(t ct.TestLike, serverName string) *client.CSAPI // Register a new user on the given server. From 8fb49da217f12e013a4a6100fd950d4cc8771836 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 14 May 2025 18:36:22 -0500 Subject: [PATCH 5/8] Try using `WithAPIVersionNegotiation` to avoid Docker API version mismatch issues See https://github.com/matrix-org/complement/pull/778#discussion_r2089901327 --- internal/docker/deployer.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go index 310af59e..a72d9bbf 100644 --- a/internal/docker/deployer.go +++ b/internal/docker/deployer.go @@ -56,7 +56,10 @@ type Deployer struct { } func NewDeployer(deployNamespace string, cfg *config.Complement) (*Deployer, error) { - cli, err := client.NewEnvClient() + cli, err := client.NewClientWithOpts( + client.FromEnv, + client.WithAPIVersionNegotiation(), + ) if err != nil { return nil, err } From d591b4d448b48fc76154049b8f5123a48dc47c1e Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 14 May 2025 18:39:00 -0500 Subject: [PATCH 6/8] Use `NewClientWithOpts` in both places --- internal/docker/builder.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/docker/builder.go b/internal/docker/builder.go index ab9abccb..721fdd9b 100644 --- a/internal/docker/builder.go +++ b/internal/docker/builder.go @@ -47,7 +47,10 @@ type Builder struct { } func NewBuilder(cfg *config.Complement) (*Builder, error) { - cli, err := client.NewEnvClient() + cli, err := client.NewClientWithOpts( + client.FromEnv, + client.WithAPIVersionNegotiation(), + ) if err != nil { return nil, err } From c443481cbb0d4a1f7abec098809e16838c18a58d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 16 May 2025 10:44:37 -0500 Subject: [PATCH 7/8] Revert "Make `GetFullyQualifiedHomeserverName` easier to use in tests" This reverts commit 10578f6a88e5f4014b0d2f63c7ec2f8c2819ddae. --- internal/docker/deployment.go | 6 +++--- test_package.go | 9 ++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/internal/docker/deployment.go b/internal/docker/deployment.go index 12566886..7b2b04f8 100644 --- a/internal/docker/deployment.go +++ b/internal/docker/deployment.go @@ -57,14 +57,14 @@ func (hsDep *HomeserverDeployment) SetEndpoints(baseURL string, fedBaseURL strin } } -func (d *Deployment) GetFullyQualifiedHomeserverName(t ct.TestLike, hsName string) string { +func (d *Deployment) GetFullyQualifiedHomeserverName(hsName string) (string, error) { _, ok := d.HS[hsName] if !ok { - ct.Fatalf(t, "Deployment.GetFullyQualifiedHomeserverName - HS name '%s' not found", hsName) + return "", fmt.Errorf("Deployment.GetFullyQualifiedHomeserverName - HS name '%s' not found", hsName) } // We have network aliases for each Docker container that will resolve the `hsName` to // the container. - return hsName + return hsName, nil } // DestroyAtCleanup destroys the entire deployment. It should be called at cleanup time for dirty diff --git a/test_package.go b/test_package.go index a6126d1d..999d64a9 100644 --- a/test_package.go +++ b/test_package.go @@ -20,13 +20,12 @@ import ( // Deployment provides a way for tests to interact with a set of homeservers. type Deployment interface { - // Returns the resolvable server name (host) of a homeserver given its short alias - // (e.g., "hs1", "hs2"). + // Returns the resolvable host name of a homeserver given its short alias (e.g., + // "hs1", "hs2"). // // In the case of the standard Docker deployment, this will be the same `hs1`, `hs2` - // but may be different for other custom deployments (ex. - // `shardDeployment1.GetFullyQualifiedHomeserverName(t, "hs1")` -> `hs1.shard1:8081`). - GetFullyQualifiedHomeserverName(t ct.TestLike, hsName string) string + // but may be different for other custom deployments. + GetFullyQualifiedHomeserverName(hsName string) (string, error) // UnauthenticatedClient returns a blank CSAPI client. UnauthenticatedClient(t ct.TestLike, serverName string) *client.CSAPI // Register a new user on the given server. From ae8cf4240bc3a216b5cfef470e0ccf9ce25d1535 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 16 May 2025 10:44:45 -0500 Subject: [PATCH 8/8] Revert "Introduce `GetFullyQualifiedHomeserverName(...)`" This reverts commit 9035d88a02ad52b3ccdacf7ac7b162b1ff3a8281. --- internal/docker/deployment.go | 10 ---------- test_package.go | 6 ------ 2 files changed, 16 deletions(-) diff --git a/internal/docker/deployment.go b/internal/docker/deployment.go index 7b2b04f8..4322a5ed 100644 --- a/internal/docker/deployment.go +++ b/internal/docker/deployment.go @@ -57,16 +57,6 @@ func (hsDep *HomeserverDeployment) SetEndpoints(baseURL string, fedBaseURL strin } } -func (d *Deployment) GetFullyQualifiedHomeserverName(hsName string) (string, error) { - _, ok := d.HS[hsName] - if !ok { - return "", fmt.Errorf("Deployment.GetFullyQualifiedHomeserverName - HS name '%s' not found", hsName) - } - // We have network aliases for each Docker container that will resolve the `hsName` to - // the container. - return hsName, nil -} - // DestroyAtCleanup destroys the entire deployment. It should be called at cleanup time for dirty // deployments only. Handles configuration options for things which should run at container destroy // time, like post-run scripts and printing logs. diff --git a/test_package.go b/test_package.go index 999d64a9..720b57d2 100644 --- a/test_package.go +++ b/test_package.go @@ -20,12 +20,6 @@ import ( // Deployment provides a way for tests to interact with a set of homeservers. type Deployment interface { - // Returns the resolvable host name of a homeserver given its short alias (e.g., - // "hs1", "hs2"). - // - // In the case of the standard Docker deployment, this will be the same `hs1`, `hs2` - // but may be different for other custom deployments. - GetFullyQualifiedHomeserverName(hsName string) (string, error) // UnauthenticatedClient returns a blank CSAPI client. UnauthenticatedClient(t ct.TestLike, serverName string) *client.CSAPI // Register a new user on the given server.