Fix problem if topology isn't applied correctly

Author: ArtDu

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+## [Unreleased]
+- Fix problem if topology isn't applied correctly
+
 ## [0.5.4] - 2023-03-31
 - Use tarantool image as base instead of centos in cartridge container
 

src/main/java/org/testcontainers/containers/TarantoolCartridgeContainer.java

@@ -2,6 +2,8 @@
 
 import com.github.dockerjava.api.command.InspectContainerResponse;
 import io.tarantool.driver.exceptions.TarantoolConnectionException;
+
+import org.testcontainers.containers.exceptions.CartridgeTopologyException;
 import org.testcontainers.images.builder.ImageFromDockerfile;
 
 import java.net.URL;
@@ -478,16 +480,21 @@ private boolean setupTopology() {
                         "--run-dir=" + TARANTOOL_RUN_DIR,
                         "--file=" + replicasetsFileName, "setup", "--bootstrap-vshard");
                 if (result.getExitCode() != 0) {
-                    throw new RuntimeException("Failed to change the app topology via cartridge CLI: "
-                            + result.getStdout());
+                    throw new CartridgeTopologyException("Failed to change the app topology via cartridge CLI: "
+                                                             + result.getStdout());
                 }
             } catch (Exception e) {
-                throw new RuntimeException("Failed to change the app topology: " + e.getMessage());
+                throw new CartridgeTopologyException(e);
             }
 
         } else {
             try {
-                executeScript(topologyConfigurationFile).get();
+                List<?> res = executeScript(topologyConfigurationFile).get();
+                if (res.size() >= 2 && res.get(1) != null && res.get(1) instanceof Map) {
+                    HashMap<?, ?> error = ((HashMap<?, ?>) res.get(1));
+                    // that means topology already exists
+                    return error.get("str").toString().contains("collision with another server");
+                }
                 // The client connection will be closed after that command
             } catch (Exception e) {
                 if (e instanceof ExecutionException) {
@@ -500,7 +507,7 @@ private boolean setupTopology() {
                         return false;
                     }
                 } else {
-                    throw new RuntimeException("Failed to change the app topology", e);
+                    throw new CartridgeTopologyException(e);
                 }
             }
         }
@@ -516,7 +523,7 @@ private void retryingSetupTopology() {
                 throw new RuntimeException(e);
             }
             if (!setupTopology()) {
-                throw new RuntimeException("Failed to change the app topology after retry");
+                throw new CartridgeTopologyException("Failed to change the app topology after retry");
             }
         }
     }

src/main/java/org/testcontainers/containers/TarantoolContainerClientHelper.java

@@ -75,7 +75,7 @@ public CompletableFuture<List<?>> executeScript(String scriptResourcePath) {
         String scriptName = Paths.get(scriptResourcePath).getFileName().toString();
         String containerPath = normalizePath(Paths.get(TMP_DIR, scriptName));
         container.copyFileToContainer(MountableFile.forClasspathResource(scriptResourcePath), containerPath);
-        return executeCommand(String.format("dofile('%s')", containerPath));
+        return executeCommand(String.format("return dofile('%s')", containerPath));
     }
 
     public CompletableFuture<List<?>> executeCommand(String command, Object... arguments) {

src/main/java/org/testcontainers/containers/exceptions/CartridgeContainerException.java

@@ -0,0 +1,9 @@
+package org.testcontainers.containers.exceptions;
+
+/**
+ * Base class for Cartridge runtime exceptions
+ *
+ * @author Artyom Dubinin
+ */
+public abstract class CartridgeContainerException extends TarantoolContainerException {
+}

src/main/java/org/testcontainers/containers/exceptions/CartridgeTopologyException.java

@@ -0,0 +1,14 @@
+package org.testcontainers.containers.exceptions;
+
+public class CartridgeTopologyException extends TarantoolContainerException {
+
+    static final String errorMsg = "Failed to change the app topology";
+
+    public CartridgeTopologyException(String message) {
+        super(message);
+    }
+
+    public CartridgeTopologyException(Throwable cause) {
+        super(errorMsg, cause);
+    }
+}

src/main/java/org/testcontainers/containers/exceptions/TarantoolContainerException.java

@@ -0,0 +1,24 @@
+package org.testcontainers.containers.exceptions;
+
+/**
+ * Base class for Tarantool runtime exceptions
+ *
+ * @author Artyom Dubinin
+ */
+public abstract class TarantoolContainerException extends RuntimeException {
+    public TarantoolContainerException(String message) {
+        super(message);
+    }
+
+    public TarantoolContainerException() {
+        super();
+    }
+
+    public TarantoolContainerException(Throwable cause) {
+        super(cause);
+    }
+
+    public TarantoolContainerException(String message, Throwable cause) {
+        super(message, cause);
+    }
+}

src/main/java/org/testcontainers/containers/exceptions/package-info.java

@@ -0,0 +1,4 @@
+/**
+ * Exception classes for internal client errors to be translated to user
+ */
+package org.testcontainers.containers.exceptions;

src/test/java/org/testcontainers/containers/TarantoolCartridgeBootstrapFromLuaWithFixedPortsTest.java

@@ -1,14 +1,19 @@
 package org.testcontainers.containers;
 
 import org.junit.jupiter.api.Test;
+import org.rnorth.ducttape.RetryCountExceededException;
 import org.slf4j.LoggerFactory;
+import org.testcontainers.containers.exceptions.CartridgeTopologyException;
 import org.testcontainers.containers.output.Slf4jLogConsumer;
 import org.testcontainers.junit.jupiter.Container;
 import org.testcontainers.junit.jupiter.Testcontainers;
 import org.testcontainers.utility.MountableFile;
 
 import java.time.Duration;
 
+import static org.junit.Assert.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
 /**
  * @author Alexey Kuzin
  */
@@ -21,8 +26,8 @@ public class TarantoolCartridgeBootstrapFromLuaWithFixedPortsTest {
                     "Dockerfile",
                     "cartridge/instances_fixedport.yml",
                     "cartridge/topology_fixedport.lua")
-                    .withCopyFileToContainer(MountableFile.forClasspathResource("cartridge"), "/app")
-                    .withCopyFileToContainer(MountableFile.forClasspathResource("cartridge/instances_fixedport.yml"),"/app/instances.yml")
+                    .withDirectoryBinding("cartridge")
+                    .withEnv("TARANTOOL_INSTANCES_FILE", "instances_fixedport.yml")
                     .withStartupTimeout(Duration.ofSeconds(300))
                     .withUseFixedPorts(true)
                     .withAPIPort(18081)
@@ -34,4 +39,25 @@ public class TarantoolCartridgeBootstrapFromLuaWithFixedPortsTest {
     public void test_StaticClusterContainer_StartsSuccessfully_ifFilesAreCopied() throws Exception {
         CartridgeContainerTestUtils.executeProfileReplaceSmokeTest(container);
     }
+
+    @Test
+    public void test_retryingSetupTopology_shouldWork() {
+        try (TarantoolCartridgeContainer testContainer =
+                 new TarantoolCartridgeContainer(
+                     "Dockerfile",
+                     "cartridge/instances.yml",
+                     "cartridge/incorrect_topology.lua")
+                     .withDirectoryBinding("cartridge")
+                     .withLogConsumer(new Slf4jLogConsumer(
+                         LoggerFactory.getLogger(TarantoolCartridgeBootstrapFromLuaWithFixedPortsTest.class)))) {
+            ContainerLaunchException ex = assertThrows(ContainerLaunchException.class, testContainer::start);
+            Throwable cause = ex.getCause();
+            assertEquals(RetryCountExceededException.class, cause.getClass());
+            cause = cause.getCause();
+            assertEquals(ContainerLaunchException.class, cause.getClass());
+            cause = cause.getCause();
+            assertEquals(CartridgeTopologyException.class, cause.getClass());
+            assertEquals("Failed to change the app topology after retry", cause.getMessage());
+        }
+    }
 }

src/test/resources/cartridge/incorrect_topology.lua

@@ -0,0 +1,16 @@
+cartridge = require('cartridge')
+replicasets = { {
+                    alias = 'app-router',
+                    roles = { 'vshard-router', 'app.roles.custom', 'app.roles.api_router' },
+                    join_servers = { { uri = 'localhost:3301' } }
+                }, {
+                    alias = 's1-storage',
+                    roles = { 'vshard-storage', 'app.roles.api_storage' },
+                    join_servers = { { uri = 'localhost:3302' } }
+                },
+                {
+                    alias = 's2-storage',
+                    roles = { 'vshard-storage', 'app.roles.api_storage' },
+                    join_servers = { { uri = 'localhost:3303' } }
+                } }
+return cartridge.admin_edit_topology({ replicasets = replicasets })