Repository: kestra-io/kestra
Issue: Python task attempts run in the same container
Labels: bug
backend

### Expected Behavior

When a python task fails, I would like the other attempts to run in another container, as they are new tasks.

### Actual Behaviour

When a python task fails, the next attempts of this task run in the same container which causes an error saying that this container is already used.

Attempt 2
ERROR    2023-01-31 04:30:02.029 1Status 409: {"message":"Conflict. The container name \"/4k73cyafagoksdsancdrk8-coaching-security-01-python\" is already in use by container \"8256073bdb5da28a5003f0481aaeb239ae40d225e65752fc185acd19a89e3926\". You have to remove (or rename) that container to be able to reuse that name."}

### Steps To Reproduce

Run a python task which fails with auto retry

### Environment Information

- Kestra Version: 0.5.3
- Operating System (OS / Docker / Kubernetes): Kubernetes
- Java Version (If not docker): 

### Example flow

_No response_

Issue comments:
I think we should just avoid using a custom name and let docker generate one. There is too many edge cases where it can fail and it didn't add any advantage.

WDYT @tchiotludo I can provide a PR to remove the container name as it didn't seems to be used by anything.
2 solutions : 
- support resume like kubernetes one [here](https://kestra.io/plugins/plugin-kubernetes/tasks/io.kestra.plugin.kubernetes.PodCreate.html#resume) activated by default.
- randomize the name of the container 

The first is better I think  

forgot the previous comment, look at this old issue : [container](https://github.com/kestra-io/kestra/issues/488)

Go for the randomisation for now, it will avoid this current bug at first 

@tchiotludo OK, I also think there is possible path where we didn't kill the container, I'll update the code about this part also
But as I said, if you want to randomize the name of the container, just let Docker do it for us! We don't use the name anywhere.

Pull request review comments:

Patch that resolved the issue:
diff --git a/core/src/main/java/io/kestra/core/tasks/scripts/runners/DockerScriptRunner.java b/core/src/main/java/io/kestra/core/tasks/scripts/runners/DockerScriptRunner.java
index fdd5686210..5f6967ca23 100644
--- a/core/src/main/java/io/kestra/core/tasks/scripts/runners/DockerScriptRunner.java
+++ b/core/src/main/java/io/kestra/core/tasks/scripts/runners/DockerScriptRunner.java
@@ -19,11 +19,9 @@
 import io.kestra.core.tasks.scripts.AbstractLogThread;
 import io.kestra.core.tasks.scripts.RunResult;
 import io.kestra.core.utils.RetryUtils;
-import io.kestra.core.utils.Slugify;
 import io.micronaut.context.ApplicationContext;
 import io.micronaut.core.convert.format.ReadableBytesTypeConverter;
 import lombok.SneakyThrows;
-import org.apache.commons.lang3.StringUtils;
 import org.apache.hc.core5.http.ConnectionClosedException;
 import org.slf4j.Logger;
 
@@ -100,20 +98,6 @@ private static void metadata(RunContext runContext, CreateContainerCmd container
         Map<String, String> execution = (Map<String, String>) runContext.getVariables().get("execution");
         Map<String, String> taskrun = (Map<String, String>) runContext.getVariables().get("taskrun");
 
-        String name = Slugify.of(String.join(
-            "-",
-            taskrun.get("id"),
-            flow.get("id"),
-            task.get("id")
-        ));
-
-        if (name.length() > 63) {
-            name = name.substring(0, 63);
-        }
-
-        name = StringUtils.stripEnd(name, "-");
-
-        container.withName(name);
         container.withLabels(ImmutableMap.of(
             "flow.kestra.io/id", flow.get("id"),
             "flow.kestra.io/namespace", flow.get("namespace"),
@@ -133,8 +117,6 @@ public RunResult run(
         AbstractBash.LogSupplier logSupplier,
         Map<String, Object> additionalVars
     ) throws Exception {
-        DockerClient dockerClient = getDockerClient(abstractBash, runContext, workingDirectory);
-
         if (abstractBash.getDockerOptions() == null) {
             throw new IllegalArgumentException("Missing required dockerOptions properties");
         }
@@ -143,13 +125,13 @@ public RunResult run(
         NameParser.ReposTag imageParse = NameParser.parseRepositoryTag(image);
 
         try (
-            CreateContainerCmd container = dockerClient.createContainerCmd(image);
-            PullImageCmd pull = dockerClient.pullImageCmd(image);
+            DockerClient dockerClient = getDockerClient(abstractBash, runContext, workingDirectory);
             PipedInputStream stdOutInputStream = new PipedInputStream();
             PipedOutputStream stdOutOutputStream = new PipedOutputStream(stdOutInputStream);
             PipedInputStream stdErrInputStream = new PipedInputStream();
             PipedOutputStream stdErrOutputStream = new PipedOutputStream(stdErrInputStream);
         ) {
+            CreateContainerCmd container = dockerClient.createContainerCmd(image);
             // properties
             metadata(runContext, container);
             HostConfig hostConfig = new HostConfig();
@@ -263,28 +245,30 @@ public RunResult run(
 
             // pull image
             if (abstractBash.getDockerOptions().getPullImage()) {
-                retryUtils.<Boolean, InternalServerErrorException>of(
-                    Exponential.builder()
-                        .delayFactor(2.0)
-                        .interval(Duration.ofSeconds(5))
-                        .maxInterval(Duration.ofSeconds(120))
-                        .maxAttempt(5)
-                        .build()
-                ).run(
-                    (bool, throwable) -> throwable instanceof InternalServerErrorException ||
-                        throwable.getCause() instanceof ConnectionClosedException,
-                    () -> {
-                        String tag = !imageParse.tag.isEmpty() ? imageParse.tag : "latest";
-                        String repository = pull.getRepository().contains(":")
-                            ? pull.getRepository().split(":")[0] : pull.getRepository();
-                        pull
-                            .withTag(tag)
-                            .exec(new PullImageResultCallback())
-                            .awaitCompletion();
-                        logger.debug("Image pulled [{}:{}]", repository, tag);
-                        return true;
-                    }
-                );
+                try (PullImageCmd pull = dockerClient.pullImageCmd(image)) {
+                    retryUtils.<Boolean, InternalServerErrorException>of(
+                        Exponential.builder()
+                            .delayFactor(2.0)
+                            .interval(Duration.ofSeconds(5))
+                            .maxInterval(Duration.ofSeconds(120))
+                            .maxAttempt(5)
+                            .build()
+                    ).run(
+                        (bool, throwable) -> throwable instanceof InternalServerErrorException ||
+                            throwable.getCause() instanceof ConnectionClosedException,
+                        () -> {
+                            String tag = !imageParse.tag.isEmpty() ? imageParse.tag : "latest";
+                            String repository = pull.getRepository().contains(":")
+                                ? pull.getRepository().split(":")[0] : pull.getRepository();
+                            pull
+                                .withTag(tag)
+                                .exec(new PullImageResultCallback())
+                                .awaitCompletion();
+                            logger.debug("Image pulled [{}:{}]", repository, tag);
+                            return true;
+                        }
+                    );
+                }
             }
 
             // start container
@@ -325,8 +309,6 @@ public void onNext(Frame item) {
                 stdOut.join();
                 stdErr.join();
 
-                dockerClient.removeContainerCmd(exec.getId()).exec();
-
                 if (exitCode != 0) {
                     throw new AbstractBash.BashException(exitCode, stdOut.getLogsCount(), stdErr.getLogsCount());
                 } else {
@@ -337,9 +319,24 @@ public void onNext(Frame item) {
             } catch (InterruptedException e) {
                 logger.warn("Killing process {} for InterruptedException", exec.getId());
 
-                dockerClient.killContainerCmd(exec.getId()).exec();
-                dockerClient.removeContainerCmd(exec.getId()).exec();
                 throw e;
+            } finally {
+                try  {
+                    var inspect = dockerClient.inspectContainerCmd(exec.getId()).exec();
+                    if (Boolean.TRUE.equals(inspect.getState().getRunning())) {
+                        // kill container as it's still running, this means there was an exception and the container didn't
+                        // come to a normal end.
+                        try {
+                            dockerClient.killContainerCmd(exec.getId()).exec();
+                        } catch (Exception e) {
+                            logger.error("Unable to kill a running container", e);
+                        }
+                    }
+                    dockerClient.removeContainerCmd(exec.getId()).exec();
+                } catch (Exception ignored) {
+
+                }
+
             }
         }
     }


Test patch: