From 4658bfb0a85daaf28713b583d22989279088c566 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Thu, 11 Aug 2022 18:54:10 +0100
Subject: [PATCH] Improve error handling when spinning up homeservers (#434)

* Stop starting homeservers on first fail
* waitForContainer: remove unused `err` variable
* waitForContainer: skip /versions check on failed server
---
 internal/docker/builder.go  |  2 ++
 internal/docker/deployer.go | 11 ++++-------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/internal/docker/builder.go b/internal/docker/builder.go
index bb0c93c2..98d9b755 100644
--- a/internal/docker/builder.go
+++ b/internal/docker/builder.go
@@ -260,6 +260,8 @@ func (d *Builder) construct(bprint b.Blueprint) (errs []error) {
 			}); delErr != nil {
 				d.log("%s: failed to remove container which failed to deploy: %s", res.contextStr, delErr)
 			}
+			// there is little point continuing to set up the remaining homeservers at this point
+			return
 		}
 		// kill the container
 		defer func(r result) {
diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go
index d242d134..3cc66deb 100644
--- a/internal/docker/deployer.go
+++ b/internal/docker/deployer.go
@@ -227,7 +227,7 @@ func deployImage(
 		// Ensure that the homeservers under test can contact the host, so they can
 		// interact with a complement-controlled test server.
 		// Note: this feature of docker landed in Docker 20.10,
-		// see https://github.com/moby/moby/pull/40007 
+		// see https://github.com/moby/moby/pull/40007
 		extraHosts = []string{"host.docker.internal:host-gateway"}
 	}
 
@@ -423,9 +423,7 @@ func waitForPorts(ctx context.Context, docker *client.Client, containerID string
 }
 
 // Waits until a homeserver deployment is ready to serve requests.
-func waitForContainer(ctx context.Context, docker *client.Client, hsDep *HomeserverDeployment, stopTime time.Time) (iterCount int, err error) {
-	var lastErr error = nil
-
+func waitForContainer(ctx context.Context, docker *client.Client, hsDep *HomeserverDeployment, stopTime time.Time) (iterCount int, lastErr error) {
 	iterCount = 0
 
 	// If the container has a healthcheck, wait for it first
@@ -433,7 +431,7 @@ func waitForContainer(ctx context.Context, docker *client.Client, hsDep *Homeser
 		iterCount += 1
 		if time.Now().After(stopTime) {
 			lastErr = fmt.Errorf("timed out checking for homeserver to be up: %s", lastErr)
-			break
+			return
 		}
 		inspect, err := docker.ContainerInspect(ctx, hsDep.ContainerID)
 		if err != nil {
@@ -477,8 +475,7 @@ func waitForContainer(ctx context.Context, docker *client.Client, hsDep *Homeser
 		lastErr = nil
 		break
 	}
-
-	return iterCount, lastErr
+	return
 }
 
 // RoundTripper is a round tripper that maps https://hs1 to the federation port of the container