simplify container start logic, taking advantage of idempotency

The start endpoint should be idempotent when trying to start a container
that is already started (see [Daemon.containerStart][1], so we can take
advantage of that.

Once we either created the container, or received a "conflict" (name
already in use), we can try starting the container. As there's a time
window between a name being reserved and the container to exist (which
may produce a 404 during that time window), we ignore "not found" errors,
and retry the start.

[1]: https://github.com/moby/moby/blob/5318877858c9fc94b5cccc3cf1a75d4ec46951b8/daemon/start.go#L76-L93

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Author: thaJeztah

pkg/standalone/containers.go

@@ -174,36 +174,26 @@ func determineBridgeGatewayIP(ctx context.Context, dockerClient client.NetworkAP
 
 // waitForContainerToStart waits for a container to start.
 func waitForContainerToStart(ctx context.Context, dockerClient client.ContainerAPIClient, containerID string) error {
-	// Unfortunately the Docker API's /containers/{id}/wait API (and the
-	// corresponding Client.ContainerWait method) don't allow waiting for
-	// container startup, so instead we'll take a polling approach.
 	for i := 10; i > 0; i-- {
-		if status, err := dockerClient.ContainerInspect(ctx, containerID); err != nil {
-			// There is a small gap between the time that a container ID and
-			// name are registered and the time that the container is actually
-			// created and shows up in container list and inspect requests:
-			//
-			// https://github.com/moby/moby/blob/de24c536b0ea208a09e0fff3fd896c453da6ef2e/daemon/container.go#L138-L156
-			//
-			// Given that multiple install operations tend to end up tightly
-			// synchronized by the preceeding pull operation and that this
-			// method is specifically designed to work around these race
-			// conditions, we'll allow 404 errors to pass silently (at least up
-			// until the polling time out - unfortunately we can't make the 404
-			// acceptance window any smaller than that because the CUDA-based
-			// containers are large and can take time to create).
-			if !errdefs.IsNotFound(err) {
-				return fmt.Errorf("unable to inspect container (%s): %w", containerID[:12], err)
-			}
-		} else {
-			switch status.State.Status {
-			case container.StateRunning:
-				return nil
-			case container.StateCreated, container.StateRestarting:
-				// wait for container to start
-			default:
-				return fmt.Errorf("container is in unexpected state %q", status.State.Status)
-			}
+		err := dockerClient.ContainerStart(ctx, containerID, container.StartOptions{})
+		if err == nil {
+			return nil
+		}
+		// There is a small gap between the time that a container ID and
+		// name are registered and the time that the container is actually
+		// created and shows up in container list and inspect requests:
+		//
+		// https://github.com/moby/moby/blob/de24c536b0ea208a09e0fff3fd896c453da6ef2e/daemon/container.go#L138-L156
+		//
+		// Given that multiple install operations tend to end up tightly
+		// synchronized by the preceeding pull operation and that this
+		// method is specifically designed to work around these race
+		// conditions, we'll allow 404 errors to pass silently (at least up
+		// until the polling time out - unfortunately we can't make the 404
+		// acceptance window any smaller than that because the CUDA-based
+		// containers are large and can take time to create).
+		if !errdefs.IsNotFound(err) {
+			return err
 		}
 		if i > 1 {
 			select {
@@ -278,19 +268,13 @@ func CreateControllerContainer(ctx context.Context, dockerClient *client.Client,
 	// progress, then we wait for whichever install process creates the
 	// container first and then wait for its container to be ready.
 	resp, err := dockerClient.ContainerCreate(ctx, config, hostConfig, nil, nil, controllerContainerName)
-	if err != nil {
-		if errdefs.IsConflict(err) {
-			if err := waitForContainerToStart(ctx, dockerClient, controllerContainerName); err != nil {
-				return fmt.Errorf("failed waiting for concurrent installation: %w", err)
-			}
-			return nil
-		}
+	if !errdefs.IsConflict(err) {
 		return fmt.Errorf("failed to create container %s: %w", controllerContainerName, err)
 	}
 
 	// Start the container.
 	printer.Printf("Starting model runner container %s...\n", controllerContainerName)
-	if err := dockerClient.ContainerStart(ctx, resp.ID, container.StartOptions{}); err != nil {
+	if err := waitForContainerToStart(ctx, dockerClient, controllerContainerName); err != nil {
 		_ = dockerClient.ContainerRemove(ctx, resp.ID, container.RemoveOptions{Force: true})
 		return fmt.Errorf("failed to start container %s: %w", controllerContainerName, err)
 	}