Don't require container to be down in dcc.Down

internal/pkg/daemon/daemon.go

@@ -472,6 +472,10 @@ func (d *Daemon) performUpgrade(
 		return errors.Wrapf(err, "failed to check if service is running")
 	}
 
+	// This check is prone to race conditions, the image could be up at this point
+	// but exits before dcc.Down is called. However, at this point we are certain
+	// that upgrade height has been hit, so, it should be safe to Down an exited
+	// container.
 	if isRunning {
 		logger.Info("Executing compose down").Notifyf(ctx, "Shutting down chain to perform upgrade. Current image: %s, new image: %s", currImage, newImage)
 		if err = d.dcc.Down(ctx, serviceName, compose.DownTimeout); err != nil {

internal/pkg/docker/compose.go

@@ -182,26 +182,18 @@ func (dcc *ComposeClient) upgradeImageInComposeFile(ctx context.Context, service
 }
 
 func (dcc *ComposeClient) Down(ctx context.Context, serviceName string, timeout time.Duration) error {
-	isImageContainerRunning, err := dcc.IsServiceRunning(ctx, serviceName, timeout)
-	if err != nil {
-		return errors.Wrapf(err, "check for container running failed")
-	}
-	if !isImageContainerRunning {
-		return errors.Wrapf(ErrContainerNotRunning, "expected the container to run before calling docker compose down")
-	}
-
 	// 5 seconds buffer to handle the case when docker timeout (-t)
 	// takes slightly longer than defined timeout (thus delay context cancellation)
 	deadline := timeout + 5*time.Second
 	timeoutSeconds := int(math.Round(timeout.Seconds()))
 
-	err = cmd.ExecuteWithDeadlineAndLog(ctx, deadline, []string{}, "docker", "compose", "-f", dcc.composeFile, "down", "--remove-orphans", "-t", strconv.Itoa(timeoutSeconds))
+	err := cmd.ExecuteWithDeadlineAndLog(ctx, deadline, []string{}, "docker", "compose", "-f", dcc.composeFile, "down", "--remove-orphans", "-t", strconv.Itoa(timeoutSeconds))
 	if err != nil {
 		return errors.Wrapf(err, "docker compose down failed")
 	}
 
 	// verify from docker api that the container is down
-	isImageContainerRunning, err = dcc.IsServiceRunning(ctx, serviceName, timeout)
+	isImageContainerRunning, err := dcc.IsServiceRunning(ctx, serviceName, timeout)
 	if err != nil {
 		return errors.Wrapf(err, "check for container running failed")
 	}
@@ -243,7 +235,20 @@ func (dcc *ComposeClient) Up(ctx context.Context, serviceName string, timeout ti
 }
 
 func (dcc *ComposeClient) RestartServiceWithHaltHeight(ctx context.Context, composeConfig *config.ComposeCli, serviceName string, upgradeHeight int64) error {
-	err := dcc.Down(ctx, serviceName, composeConfig.DownTimeout)
+	isImageContainerRunning, err := dcc.IsServiceRunning(ctx, serviceName, composeConfig.DownTimeout)
+	if err != nil {
+		return errors.Wrapf(err, "check for container running failed")
+	}
+	if !isImageContainerRunning {
+		return errors.Wrapf(ErrContainerNotRunning, "expected the container to run before restarting with halt height")
+	}
+	// The check above is prone to race conditions and the
+	// container can exit after the check. That should be super rare
+	// and it should be safe to Down it anyways, since, we are sure that we want
+	// to register halt height and restart.
+	// If the container crashed due to some issue, let it crash again after the restart
+	// blazar can't do anything about that
+	err = dcc.Down(ctx, serviceName, composeConfig.DownTimeout)
 	if err != nil && !errors.Is(err, ErrContainerNotRunning) {
 		return errors.Wrapf(err, "docker compose down failed")
 	}

internal/pkg/docker/compose_test.go

@@ -272,23 +272,6 @@ func TestUpDownCompose(t *testing.T) {
 				require.NoError(t, err)
 			},
 		},
-		{
-			name: "simulate case where docker compose up didn't start the container",
-			testFn: func(t *testing.T, _ string) {
-				// step 1: fake docker binary to simulate docker compose up timeout (container isn't running)
-				err = os.WriteFile(filepath.Join(tempDir, "docker"), []byte("#!/bin/sh\nexit 0\n"), 0600)
-				require.NoError(t, err)
-
-				oldPath := os.Getenv("PATH")
-				os.Setenv("PATH", tempDir+":"+oldPath)
-
-				// step 2: with docker compose client try to start container and expect it to fail
-				err = dcc.Down(ctx, "s1", 0)
-				require.ErrorIs(t, err, ErrContainerNotRunning)
-
-				os.Setenv("PATH", oldPath)
-			},
-		},
 	}
 
 	for _, test := range tests {