diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d0e54b..4b1b857 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/main/java/org/testcontainers/containers/TarantoolCartridgeContainer.java b/src/main/java/org/testcontainers/containers/TarantoolCartridgeContainer.java index 12e4470..2382ca3 100644 --- a/src/main/java/org/testcontainers/containers/TarantoolCartridgeContainer.java +++ b/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"); } } } diff --git a/src/main/java/org/testcontainers/containers/TarantoolContainerClientHelper.java b/src/main/java/org/testcontainers/containers/TarantoolContainerClientHelper.java index 7446504..cfaa559 100644 --- a/src/main/java/org/testcontainers/containers/TarantoolContainerClientHelper.java +++ b/src/main/java/org/testcontainers/containers/TarantoolContainerClientHelper.java @@ -75,7 +75,7 @@ public CompletableFuture> 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> executeCommand(String command, Object... arguments) { diff --git a/src/main/java/org/testcontainers/containers/exceptions/CartridgeContainerException.java b/src/main/java/org/testcontainers/containers/exceptions/CartridgeContainerException.java new file mode 100644 index 0000000..521ea9d --- /dev/null +++ b/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 { +} diff --git a/src/main/java/org/testcontainers/containers/exceptions/CartridgeTopologyException.java b/src/main/java/org/testcontainers/containers/exceptions/CartridgeTopologyException.java new file mode 100644 index 0000000..06fd37f --- /dev/null +++ b/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); + } +} diff --git a/src/main/java/org/testcontainers/containers/exceptions/TarantoolContainerException.java b/src/main/java/org/testcontainers/containers/exceptions/TarantoolContainerException.java new file mode 100644 index 0000000..2cbccc3 --- /dev/null +++ b/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); + } +} diff --git a/src/main/java/org/testcontainers/containers/exceptions/package-info.java b/src/main/java/org/testcontainers/containers/exceptions/package-info.java new file mode 100644 index 0000000..b328999 --- /dev/null +++ b/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; diff --git a/src/test/java/org/testcontainers/containers/TarantoolCartridgeBootstrapFromLuaWithFixedPortsTest.java b/src/test/java/org/testcontainers/containers/TarantoolCartridgeBootstrapFromLuaWithFixedPortsTest.java index cacd8b0..0a020b1 100644 --- a/src/test/java/org/testcontainers/containers/TarantoolCartridgeBootstrapFromLuaWithFixedPortsTest.java +++ b/src/test/java/org/testcontainers/containers/TarantoolCartridgeBootstrapFromLuaWithFixedPortsTest.java @@ -1,7 +1,9 @@ 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; @@ -9,6 +11,9 @@ 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()); + } + } } diff --git a/src/test/resources/cartridge/incorrect_topology.lua b/src/test/resources/cartridge/incorrect_topology.lua new file mode 100644 index 0000000..c9dfeed --- /dev/null +++ b/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' } } -- non-existent server + } } +return cartridge.admin_edit_topology({ replicasets = replicasets })