Add deployment.GetFullyQualifiedHomeserverName(t, hsName) to support custom Deployment
#780
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -260,7 +260,7 @@ func (r *ServerRoom) MustHaveMembershipForUser(t ct.TestLike, userID, wantMember | |
| } | ||
|
|
||
| // ServersInRoom gets all servers currently joined to the room | ||
| func (r *ServerRoom) ServersInRoom() (servers []spec.ServerName) { | ||
| serverSet := make(map[string]struct{}) | ||
|
|
||
| r.StateMutex.RLock() | ||
|
|
@@ -282,7 +282,7 @@ func (r *ServerRoom) ServersInRoom() (servers []string) { | |
| r.StateMutex.RUnlock() | ||
|
|
||
| for server := range serverSet { | ||
| servers = append(servers, spec.ServerName(server)) | ||
| } | ||
|
|
||
| return | ||
|
|
@@ -498,21 +498,27 @@ func (i *ServerRoomImplDefault) GenerateSendJoinResponse(room *ServerRoom, s *Se | |
| authEvents := room.AuthChainForEvents(stateEvents) | ||
|
|
||
| // get servers in room *before* the join event | ||
| serversInRoom := []spec.ServerName{s.serverName} | ||
| if !omitServersInRoom { | ||
| serversInRoom = room.ServersInRoom() | ||
| } | ||
|
|
||
| serversInRoomStrings := make([]string, len(serversInRoom)) | ||
| for i, serverName := range serversInRoom { | ||
| serversInRoomStrings[i] = string(serverName) | ||
| } | ||
|
|
||
| // insert the join event into the room state | ||
| room.AddEvent(joinEvent) | ||
| log.Printf("Received send-join of event %s", joinEvent.EventID()) | ||
|
|
||
| // return state and auth chain | ||
| return fclient.RespSendJoin{ | ||
| Origin: s.serverName, | ||
| AuthEvents: gomatrixserverlib.NewEventJSONsFromEvents(authEvents), | ||
| StateEvents: gomatrixserverlib.NewEventJSONsFromEvents(stateEvents), | ||
| MembersOmitted: expectPartialState, | ||
| // TODO: It feels like `ServersInRoom` should be `[]spec.ServerName` instead of `[]string` | ||
| ServersInRoom: serversInRoomStrings, | ||
|
Comment on lines
+521
to
+522
There was a problem hiding this comment. This feels like a (not going to fix in this PR) |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import ( | |
| "github.com/matrix-org/complement/ct" | ||
| "github.com/matrix-org/complement/helpers" | ||
| "github.com/matrix-org/gomatrixserverlib" | ||
| "github.com/matrix-org/gomatrixserverlib/spec" | ||
| ) | ||
|
|
||
| // Deployment is the complete instantiation of a Blueprint, with running containers | ||
|
|
@@ -57,6 +58,16 @@ func (hsDep *HomeserverDeployment) SetEndpoints(baseURL string, fedBaseURL strin | |
| } | ||
| } | ||
|
|
||
| func (d *Deployment) GetFullyQualifiedHomeserverName(t ct.TestLike, hsName string) spec.ServerName { | ||
| _, ok := d.HS[hsName] | ||
| if !ok { | ||
| 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 spec.ServerName(hsName) | ||
| } | ||
kegsay marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|
There was a problem hiding this comment. Currently, I don't think the built-in Complement I'd rather leave it as-is until we need it or at-least do this in a follow-up PR. See the PR description for more context on multiple There was a problem hiding this comment. The PR description doesn't give more context on why multiple
Why would you do this? The There was a problem hiding this comment. @kegsay In the current setup of our custom And we specifically want to test that homeservers between multiple shards can federate with each other as a sanity check (make sure our shards can deploy homeserver tenants correctly): func TestCrossShardFederation(t *testing.T) {
// Create two shards with their own homeserver tenants
shardDeployment1 := complement.Deploy(t, 1)
defer shardDeployment1.Destroy(t)
shardDeployment2 := complement.Deploy(t, 1)
defer shardDeployment2.Destroy(t)
alice := shardDeployment1.Register(t, "hs1", helpers.RegistrationOpts{})
bob := shardDeployment2.Register(t, "hs1", helpers.RegistrationOpts{})
aliceRoomID := alice.MustCreateRoom(t, map[string]any{
"preset": "public_chat",
})
bobRoomID := bob.MustCreateRoom(t, map[string]any{
"preset": "public_chat",
})
t.Run("parallel", func(t *testing.T) {
t.Run("shard1 -> shard2", func(t *testing.T) {
// Since these tests use the same config, they can be run in parallel
t.Parallel()
alice.MustJoinRoom(t, bobRoomID, []string{
shardDeployment2.GetFullyQualifiedHomeserverName(t, "hs1"),
})
bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(alice.UserID, bobRoomID))
})
t.Run("shard2 -> shard1", func(t *testing.T) {
// Since these tests use the same config, they can be run in parallel
t.Parallel()
bob.MustJoinRoom(t, aliceRoomID, []string{
shardDeployment1.GetFullyQualifiedHomeserverName(t, "hs1"),
})
alice.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, aliceRoomID))
})
})
}This does assume that each Better way to go about this? One alternative I can think of is to bake this information into the Another is to statically assign each homeserver to a shard like |
||
| // 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. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Basically, wherever we expect people to use
deployment.GetFullyQualifiedHomeserverName(t, hsName), I've updated these function signatures to acceptspec.ServerNameinstead of just plain strings.I also think this is more semantically correct for the places because this needs to be a resolvable homeserver address in the federation.