Bind container ports to host ports by default in product tests

trinodb · Aug 2, 2021 · c3cd438 · c3cd438
1 parent 4bb71c5
commit c3cd438
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 12 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -409,4 +409,4 @@ jobs:
       - name: Product Tests
         run: |
           bin/ptl suite run \
-            --suite ${{ matrix.suite }} --config config-${{ matrix.config }} --logs-dir logs/ --timeout 2h
+            --suite ${{ matrix.suite }} --config config-${{ matrix.config }} --bind=off --logs-dir logs/ --timeout 2h
diff --git a/...t-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/EnvironmentModule.java b/...t-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/EnvironmentModule.java
@@ -31,7 +31,6 @@
 
 import java.io.File;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.inject.Scopes.SINGLETON;
 import static com.google.inject.multibindings.MapBinder.newMapBinder;
 import static io.trino.tests.product.launcher.env.Environments.nameForConfigClass;
@@ -90,15 +89,15 @@ public EnvironmentConfig provideEnvironmentConfig(EnvironmentOptions options, En
     @Singleton
     public PortBinder providePortBinder(EnvironmentOptions options)
     {
-        switch (options.bindPorts) {
-            case -1:
-                return new PortBinder.DefaultPortBinder();
-            case 0:
-                return new PortBinder.FixedPortBinder();
-            default:
-                checkArgument(options.bindPorts > 0, "Ports base must be greater than 0");
-                return new PortBinder.ShiftingPortBinder(new PortBinder.FixedPortBinder(), options.bindPorts);
+        if (options.bindPorts) {
+            if (options.bindPortsBase > 0) {
+                return new PortBinder.ShiftingPortBinder(new PortBinder.FixedPortBinder(), options.bindPortsBase);
+            }
+
+            return new PortBinder.FixedPortBinder();
         }
+
+        return new PortBinder.DefaultPortBinder();
     }
 
     @Provides

diff --git a/...-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/EnvironmentOptions.java b/...-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/EnvironmentOptions.java
@@ -13,14 +13,25 @@
  */
 package io.trino.tests.product.launcher.env;
 
+import picocli.CommandLine;
+import picocli.CommandLine.Model.CommandSpec;
+import picocli.CommandLine.Spec;
+
 import java.io.File;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static io.trino.tests.product.launcher.env.EnvironmentContainers.COORDINATOR;
+import static java.util.Locale.ENGLISH;
 import static picocli.CommandLine.Option;
 
 public final class EnvironmentOptions
 {
     private static final String DEFAULT_VALUE = "(default: ${DEFAULT-VALUE})";
+    public static final String BIND_ON_HOST = "on";
+    public static final String DO_NOT_BIND = "off";
+
+    @Spec
+    private CommandSpec spec;
 
     @Option(names = "--config", paramLabel = "<config>", description = "Environment config to use")
     public String config = "config-default";
@@ -31,8 +42,8 @@ public final class EnvironmentOptions
     @Option(names = "--without-trino", description = "Do not start " + COORDINATOR)
     public boolean withoutPrestoMaster;
 
-    @Option(names = "--bind", description = "Bind ports on localhost starting from a given base port " + DEFAULT_VALUE, defaultValue = "-1", fallbackValue = "0", arity = "0..1")
-    public int bindPorts = -1;
+    public boolean bindPorts = true;
+    int bindPortsBase;
 
     @Option(names = "--debug", description = "Open Java debug ports")
     public boolean debug;
@@ -43,6 +54,28 @@ public final class EnvironmentOptions
     @Option(names = "--launcher-bin", paramLabel = "<launcher bin>", description = "Launcher bin path (used to display run commands)", defaultValue = "${launcher.bin}", hidden = true)
     public String launcherBin;
 
+    @Option(names = "--bind", description = "Bind exposed container ports to host ports, possible values: " + BIND_ON_HOST + ", " + DO_NOT_BIND + ", [port base number] " + DEFAULT_VALUE, defaultValue = BIND_ON_HOST, arity = "0..1", fallbackValue = BIND_ON_HOST)
+    public void setBindOnHost(String value)
+    {
+        switch (value.toLowerCase(ENGLISH)) {
+            case BIND_ON_HOST:
+                this.bindPorts = true;
+                break;
+            case DO_NOT_BIND:
+                this.bindPorts = false;
+                break;
+            default:
+                try {
+                    this.bindPortsBase = Integer.parseInt(value);
+                    this.bindPorts = true;
+                    checkArgument(this.bindPortsBase > 0, "Port bind base must be a positive integer");
+                }
+                catch (Exception e) {
+                    throw new CommandLine.ParameterException(spec.commandLine(), "Port bind base is invalid", e);
+                }
+        }
+    }
+
     public static EnvironmentOptions empty()
     {
         return new EnvironmentOptions();