From 521d41d172c18f47afc1ca6712cd209e82acb8d8 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Mon, 7 Oct 2019 10:00:07 +0100 Subject: [PATCH 01/43] Revert "Mute docker packaging tests (DockerTests)" This reverts commit b9584672a460e6b2fcff47ecdd72e856bafa7dc2. --- .../groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java | 3 +-- .../java/org/elasticsearch/packaging/test/DockerTests.java | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 2f254b30466a3..6203d0c9b12e5 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -319,8 +319,7 @@ private List configureDistributions(Project project, List currentDistros = new ArrayList<>(); List upgradeDistros = new ArrayList<>(); - // Docker disabled for https://github.com/elastic/elasticsearch/issues/47639 - for (Type type : Arrays.asList(Type.DEB, Type.RPM /*,Type.DOCKER*/)) { + for (Type type : Arrays.asList(Type.DEB, Type.RPM, Type.DOCKER)) { for (Flavor flavor : Flavor.values()) { for (boolean bundledJdk : Arrays.asList(true, false)) { // We should never add a Docker distro with bundledJdk == false diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index daad67f7fb111..52205263d3ed3 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -28,7 +28,6 @@ import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; -import org.junit.Ignore; import java.nio.file.Files; import java.nio.file.Path; @@ -55,7 +54,6 @@ import static org.hamcrest.Matchers.emptyString; import static org.junit.Assume.assumeTrue; -@Ignore("https://github.com/elastic/elasticsearch/issues/47639") public class DockerTests extends PackagingTestCase { protected DockerShell sh; From c01dd7b939b067b3fbea187ac4014911d856f77e Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Mon, 7 Oct 2019 16:08:07 +0100 Subject: [PATCH 02/43] Only define Docker pkg tests where appropriate The Docker packaging tests were being defined irrespective of whether Docker was actually available in the current environment. Change this to check that Docker is available and vaguely functional before defining the test tasks. As part of this, define a seperate utility class for checking Docker, and call that instead of defining checks in-line in BuildPlugin.groovy --- .../elasticsearch/gradle/BuildPlugin.groovy | 85 ++++------ .../gradle/test/DistroTestPlugin.java | 12 +- .../java/org/elasticsearch/gradle/Docker.java | 160 ++++++++++++++++++ .../gradle/BuildPluginTests.java | 11 -- .../org/elasticsearch/gradle/DockerTests.java | 32 ++++ 5 files changed, 235 insertions(+), 65 deletions(-) create mode 100644 buildSrc/src/main/java/org/elasticsearch/gradle/Docker.java create mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/DockerTests.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index c5335648e65af..a466adc242ee8 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -78,16 +78,17 @@ import org.gradle.external.javadoc.CoreJavadocOptions import org.gradle.internal.jvm.Jvm import org.gradle.language.base.plugins.LifecycleBasePlugin import org.gradle.process.CommandLineArgumentProvider -import org.gradle.process.ExecResult -import org.gradle.process.ExecSpec import org.gradle.util.GradleVersion import java.nio.charset.StandardCharsets import java.nio.file.Files -import java.util.regex.Matcher +import static org.elasticsearch.gradle.Docker.DOCKER_BINARIES +import static org.elasticsearch.gradle.Docker.getDockerAvailability +import static org.elasticsearch.gradle.Docker.getDockerPath import static org.elasticsearch.gradle.tool.Boilerplate.findByName import static org.elasticsearch.gradle.tool.Boilerplate.maybeConfigure + /** * Encapsulates build configuration for elasticsearch projects. */ @@ -189,8 +190,7 @@ class BuildPlugin implements Plugin { */ // check if the Docker binary exists and record its path - final List maybeDockerBinaries = ['/usr/bin/docker', '/usr/local/bin/docker'] - final String dockerBinary = maybeDockerBinaries.find { it -> new File(it).exists() } + final String dockerBinary = getDockerPath() final boolean buildDocker final String buildDockerProperty = System.getProperty("build.docker") @@ -209,55 +209,49 @@ class BuildPlugin implements Plugin { ext.set('requiresDocker', []) rootProject.gradle.taskGraph.whenReady { TaskExecutionGraph taskGraph -> final List tasks = taskGraph.allTasks.intersect(ext.get('requiresDocker') as List).collect { " ${it.path}".toString()} + if (tasks.isEmpty() == false) { + final Docker.DockerAvailability dockerAvailability = getDockerAvailability() + + if (dockerAvailability.isAvailable() == true) { + return + } + /* - * There are tasks in the task graph that require Docker. Now we are failing because either the Docker binary does not - * exist or because execution of a privileged Docker command failed. + * There are tasks in the task graph that require Docker. + * Now we are failing because either the Docker binary does + * not exist or because execution of a privileged Docker + * command failed. */ - if (dockerBinary == null) { + if (dockerAvailability.getPath() == null) { final String message = String.format( Locale.ROOT, "Docker (checked [%s]) is required to run the following task%s: \n%s", - maybeDockerBinaries.join(","), + DOCKER_BINARIES.join(","), tasks.size() > 1 ? "s" : "", tasks.join('\n')) throwDockerRequiredException(message) } - // we use a multi-stage Docker build, check the Docker version since 17.05 - final ByteArrayOutputStream dockerVersionOutput = new ByteArrayOutputStream() - LoggedExec.exec( - rootProject, - { ExecSpec it -> - it.commandLine = [dockerBinary, '--version'] - it.standardOutput = dockerVersionOutput - }) - final String dockerVersion = dockerVersionOutput.toString().trim() - checkDockerVersionRecent(dockerVersion) - - final ByteArrayOutputStream dockerImagesErrorOutput = new ByteArrayOutputStream() - // the Docker binary executes, check that we can execute a privileged command - final ExecResult dockerImagesResult = LoggedExec.exec( - rootProject, - { ExecSpec it -> - it.commandLine = [dockerBinary, "images"] - it.errorOutput = dockerImagesErrorOutput - it.ignoreExitValue = true - }) - - if (dockerImagesResult.exitValue != 0) { + if (dockerAvailability.isVersionHighEnough() == false) { final String message = String.format( Locale.ROOT, - "a problem occurred running Docker from [%s] yet it is required to run the following task%s: \n%s\n" + - "the problem is that Docker exited with exit code [%d] with standard error output [%s]", - dockerBinary, - tasks.size() > 1 ? "s" : "", - tasks.join('\n'), - dockerImagesResult.exitValue, - dockerImagesErrorOutput.toString().trim()) + "building Docker images requires Docker version 17.05+ due to use of multi-stage builds yet was [%s]", + dockerAvailability.getVersion()) throwDockerRequiredException(message) } + // Some other problem, print the error + final String message = String.format( + Locale.ROOT, + "a problem occurred running Docker from [%s] yet it is required to run the following task%s: \n%s\n" + + "the problem is that Docker exited with exit code [%d] with standard error output [%s]", + dockerBinary, + tasks.size() > 1 ? "s" : "", + tasks.join('\n'), + dockerAvailability.getLastCommand().exitCode, + dockerAvailability.getLastCommand().stderr.trim()) + throwDockerRequiredException(message) } } } @@ -265,21 +259,6 @@ class BuildPlugin implements Plugin { (ext.get('requiresDocker') as List).add(task) } - protected static void checkDockerVersionRecent(String dockerVersion) { - final Matcher matcher = dockerVersion =~ /Docker version (\d+\.\d+)\.\d+(?:-[a-zA-Z0-9]+)?, build [0-9a-f]{7,40}/ - assert matcher.matches(): dockerVersion - final dockerMajorMinorVersion = matcher.group(1) - final String[] majorMinor = dockerMajorMinorVersion.split("\\.") - if (Integer.parseInt(majorMinor[0]) < 17 - || (Integer.parseInt(majorMinor[0]) == 17 && Integer.parseInt(majorMinor[1]) < 5)) { - final String message = String.format( - Locale.ROOT, - "building Docker images requires Docker version 17.05+ due to use of multi-stage builds yet was [%s]", - dockerVersion) - throwDockerRequiredException(message) - } - } - private static void throwDockerRequiredException(final String message) { throw new GradleException( message + "\nyou can address this by attending to the reported issue, " diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 6203d0c9b12e5..913dcf069c7a9 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -60,6 +60,7 @@ import java.util.Random; import java.util.stream.Collectors; +import static org.elasticsearch.gradle.Docker.getDockerAvailability; import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertLinuxPath; import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertWindowsPath; @@ -319,7 +320,16 @@ private List configureDistributions(Project project, List currentDistros = new ArrayList<>(); List upgradeDistros = new ArrayList<>(); - for (Type type : Arrays.asList(Type.DEB, Type.RPM, Type.DOCKER)) { + final List applicableTypes = new ArrayList<>(); + applicableTypes.add(Type.DEB); + applicableTypes.add(Type.RPM); + + final String buildDockerProperty = System.getProperty("build.docker"); + if ((buildDockerProperty == null || "true".equals(buildDockerProperty)) && getDockerAvailability().isAvailable()) { + applicableTypes.add(Type.DOCKER); + } + + for (Type type : applicableTypes) { for (Flavor flavor : Flavor.values()) { for (boolean bundledJdk : Arrays.asList(true, false)) { // We should never add a Docker distro with bundledJdk == false diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/Docker.java b/buildSrc/src/main/java/org/elasticsearch/gradle/Docker.java new file mode 100644 index 0000000000000..9f78e329e4808 --- /dev/null +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/Docker.java @@ -0,0 +1,160 @@ +package org.elasticsearch.gradle; + +import org.apache.commons.io.IOUtils; + +import java.io.File; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.TimeUnit; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class Docker { + public static String[] DOCKER_BINARIES = { "/usr/bin/docker", "/usr/local/bin/docker" }; + + public static class DockerAvailability { + private boolean isAvailable = false; + private boolean isVersionHighEnough = false; + private String path = null; + private String version = null; + private Result lastCommand = null; + + public boolean isAvailable() { + return isAvailable; + } + + public boolean isVersionHighEnough() { + return isVersionHighEnough; + } + + public String getPath() { + return path; + } + + public String getVersion() { + return version; + } + + public Result getLastCommand() { + return lastCommand; + } + } + + public static Optional getDockerPath() { + // Check if the Docker binary exists + return List.of(DOCKER_BINARIES) + .stream() + .filter(path -> new File(path).exists()) + .findFirst(); + } + + public static DockerAvailability getDockerAvailability() { + final DockerAvailability dockerAvailability = new DockerAvailability(); + + // Check if the Docker binary exists + final Optional dockerBinary = getDockerPath(); + + if (dockerBinary.isEmpty()) { + return dockerAvailability; + } + + dockerAvailability.path = dockerBinary.get(); + + // Since we use a multi-stage Docker build, check the Docker version since 17.05 + final Result versionResult = runCommand(dockerAvailability.path, "--version"); + + if (versionResult.isSuccess() == false) { + dockerAvailability.lastCommand = versionResult; + return dockerAvailability; + } + + final String version = extractVersion(versionResult.stdout.trim()); + dockerAvailability.version = version; + + if (checkVersion(version) == false) { + return dockerAvailability; + } + + dockerAvailability.isVersionHighEnough = true; + + // The Docker binary executes, so check that we can execute a privileged command + final Result imagesResult = runCommand(dockerAvailability.path, "images"); + + if (imagesResult.isSuccess() == false) { + dockerAvailability.lastCommand = versionResult; + return dockerAvailability; + } + + dockerAvailability.isAvailable = true; + return dockerAvailability; + } + + // package-private for testing + static String extractVersion(String dockerVersion) { + final Pattern dockerVersionPattern = Pattern.compile("Docker version (\\d+\\.\\d+\\.\\d+(?:-[a-zA-Z0-9]+)?), build [0-9a-f]{7,40}"); + + final Matcher matcher = dockerVersionPattern.matcher(dockerVersion); + if (!matcher.matches()) { + throw new IllegalArgumentException("Can't extract version from: [" + dockerVersion + "]"); + } + + return matcher.group(1); + } + + // package-private for testing + static boolean checkVersion(String dockerVersion) { + final String[] majorMinor = dockerVersion.split("\\."); + + return (Integer.parseInt(majorMinor[0]) > 17 + || (Integer.parseInt(majorMinor[0]) == 17 && Integer.parseInt(majorMinor[1]) >= 5)); + } + + private static Result runCommand(String... args) { + final ProcessBuilder command = new ProcessBuilder().command(args); + try { + final Process process = command.start(); + + if (process.waitFor(10, TimeUnit.SECONDS) == false) { + if (process.isAlive()) { + process.destroyForcibly(); + } + + Result result = new Result(-1, + IOUtils.toString(process.getInputStream()), + IOUtils.toString(process.getErrorStream())); + + throw new IllegalStateException( + "Timed out running shell command: " + command + "\n" + + "Result:\n" + result + ); + } + + String stdout = IOUtils.toString(process.getInputStream()); + String stderr = IOUtils.toString(process.getErrorStream()); + + return new Result(process.exitValue(), stdout, stderr); + } catch (Exception e) { + return new Result(-1, "", e.getMessage()); + } + } + + public static class Result { + public final int exitCode; + public final String stdout; + public final String stderr; + + public Result(int exitCode, String stdout, String stderr) { + this.exitCode = exitCode; + this.stdout = stdout; + this.stderr = stderr; + } + + public boolean isSuccess() { + return exitCode == 0; + } + + public String toString() { + return "exitCode = [" + exitCode + "] " + "stdout = [" + stdout.trim() + "] " + "stderr = [" + stderr.trim() + "]"; + } + } +} diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/BuildPluginTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/BuildPluginTests.java index c61a0a3935898..8d6d0be00dbe8 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/BuildPluginTests.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/BuildPluginTests.java @@ -28,17 +28,6 @@ public class BuildPluginTests extends GradleUnitTestCase { - public void testPassingDockerVersions() { - BuildPlugin.checkDockerVersionRecent("Docker version 18.06.1-ce, build e68fc7a215d7"); - BuildPlugin.checkDockerVersionRecent("Docker version 17.05.0, build e68fc7a"); - BuildPlugin.checkDockerVersionRecent("Docker version 17.05.1, build e68fc7a"); - } - - @Test(expected = GradleException.class) - public void testFailingDockerVersions() { - BuildPlugin.checkDockerVersionRecent("Docker version 17.04.0, build e68fc7a"); - } - @Test(expected = GradleException.class) public void testRepositoryURIThatUsesHttpScheme() throws URISyntaxException { final URI uri = new URI("http://s3.amazonaws.com/artifacts.elastic.co/maven"); diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/DockerTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/DockerTests.java new file mode 100644 index 0000000000000..9b3c24f5c61a0 --- /dev/null +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/DockerTests.java @@ -0,0 +1,32 @@ +package org.elasticsearch.gradle; + +import org.junit.Test; + +import static org.elasticsearch.gradle.Docker.checkVersion; +import static org.elasticsearch.gradle.Docker.extractVersion; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +public class DockerTests { + + @Test + public void testExtractVersion() { + assertThat(extractVersion("Docker version 18.06.1-ce, build e68fc7a215d7"), equalTo("18.06.1-ce")); + assertThat(extractVersion("Docker version 17.05.0, build e68fc7a"), equalTo("17.05.0")); + assertThat(extractVersion("Docker version 17.05.1, build e68fc7a"), equalTo("17.05.1")); + } + + @Test + public void testCheckAcceptableVersions() { + assertTrue(checkVersion("18.06.1-ce")); + assertTrue(checkVersion("17.05.1-ce")); + } + + @Test + public void testCheckUnacceptableVersions() { + assertFalse(checkVersion("17.04.0")); + assertFalse(checkVersion("16.99.0")); + } +} From d2dccce4beccf57c2b2b3e187d49aa6163098a6c Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Mon, 7 Oct 2019 16:20:13 +0100 Subject: [PATCH 03/43] Fix lint error --- .../src/test/java/org/elasticsearch/gradle/DockerTests.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/DockerTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/DockerTests.java index 9b3c24f5c61a0..73e8200896264 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/DockerTests.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/DockerTests.java @@ -1,15 +1,13 @@ package org.elasticsearch.gradle; +import org.elasticsearch.gradle.test.GradleUnitTestCase; import org.junit.Test; import static org.elasticsearch.gradle.Docker.checkVersion; import static org.elasticsearch.gradle.Docker.extractVersion; import static org.hamcrest.CoreMatchers.equalTo; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; -public class DockerTests { +public class DockerTests extends GradleUnitTestCase { @Test public void testExtractVersion() { From 5eb2a2809427aa88a987333dcfe5c5729f9a6349 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Tue, 8 Oct 2019 10:22:36 +0100 Subject: [PATCH 04/43] Docs and refactoring --- .../elasticsearch/gradle/BuildPlugin.groovy | 10 +- .../gradle/test/DistroTestPlugin.java | 18 +-- .../gradle/{Docker.java => DockerUtils.java} | 128 ++++++++++++++---- ...DockerTests.java => DockerUtilsTests.java} | 6 +- 4 files changed, 115 insertions(+), 47 deletions(-) rename buildSrc/src/main/java/org/elasticsearch/gradle/{Docker.java => DockerUtils.java} (64%) rename buildSrc/src/test/java/org/elasticsearch/gradle/{DockerTests.java => DockerUtilsTests.java} (81%) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index a466adc242ee8..8e92ea56b78ca 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -83,9 +83,9 @@ import org.gradle.util.GradleVersion import java.nio.charset.StandardCharsets import java.nio.file.Files -import static org.elasticsearch.gradle.Docker.DOCKER_BINARIES -import static org.elasticsearch.gradle.Docker.getDockerAvailability -import static org.elasticsearch.gradle.Docker.getDockerPath +import static DockerUtils.DOCKER_BINARIES +import static DockerUtils.getDockerAvailability +import static DockerUtils.getDockerPath import static org.elasticsearch.gradle.tool.Boilerplate.findByName import static org.elasticsearch.gradle.tool.Boilerplate.maybeConfigure @@ -190,7 +190,7 @@ class BuildPlugin implements Plugin { */ // check if the Docker binary exists and record its path - final String dockerBinary = getDockerPath() + final String dockerBinary = getDockerPath().orElse(null) final boolean buildDocker final String buildDockerProperty = System.getProperty("build.docker") @@ -211,7 +211,7 @@ class BuildPlugin implements Plugin { final List tasks = taskGraph.allTasks.intersect(ext.get('requiresDocker') as List).collect { " ${it.path}".toString()} if (tasks.isEmpty() == false) { - final Docker.DockerAvailability dockerAvailability = getDockerAvailability() + final DockerUtils.DockerAvailability dockerAvailability = getDockerAvailability() if (dockerAvailability.isAvailable() == true) { return diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 913dcf069c7a9..ec9caff76f2f4 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -60,7 +60,7 @@ import java.util.Random; import java.util.stream.Collectors; -import static org.elasticsearch.gradle.Docker.getDockerAvailability; +import static org.elasticsearch.gradle.DockerUtils.getDockerAvailability; import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertLinuxPath; import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertWindowsPath; @@ -320,16 +320,15 @@ private List configureDistributions(Project project, List currentDistros = new ArrayList<>(); List upgradeDistros = new ArrayList<>(); - final List applicableTypes = new ArrayList<>(); - applicableTypes.add(Type.DEB); - applicableTypes.add(Type.RPM); - final String buildDockerProperty = System.getProperty("build.docker"); - if ((buildDockerProperty == null || "true".equals(buildDockerProperty)) && getDockerAvailability().isAvailable()) { - applicableTypes.add(Type.DOCKER); - } + final boolean shouldAddDocker = (buildDockerProperty == null || "true".equals(buildDockerProperty)) + && getDockerAvailability().isAvailable(); - for (Type type : applicableTypes) { + final List applicablePackageTypes = shouldAddDocker + ? List.of(Type.DEB, Type.RPM, Type.DOCKER) + : List.of(Type.DEB, Type.RPM); + + for (Type type : applicablePackageTypes) { for (Flavor flavor : Flavor.values()) { for (boolean bundledJdk : Arrays.asList(true, false)) { // We should never add a Docker distro with bundledJdk == false @@ -347,6 +346,7 @@ private List configureDistributions(Project project, addDistro(distributions, type, null, Flavor.OSS, true, upgradeVersion.toString(), upgradeDistros); } } + for (Platform platform : Arrays.asList(Platform.LINUX, Platform.WINDOWS)) { for (Flavor flavor : Flavor.values()) { for (boolean bundledJdk : Arrays.asList(true, false)) { diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/Docker.java b/buildSrc/src/main/java/org/elasticsearch/gradle/DockerUtils.java similarity index 64% rename from buildSrc/src/main/java/org/elasticsearch/gradle/Docker.java rename to buildSrc/src/main/java/org/elasticsearch/gradle/DockerUtils.java index 9f78e329e4808..ab3b66628c943 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/Docker.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/DockerUtils.java @@ -9,37 +9,24 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -public class Docker { +/** + * Contains utilities for checking whether Docker is installed, is executable, + * has a recent enough version, and appears to be functional. The Elasticsearch build + * requires Docker >= 17.05 as it uses a multi-stage build. + */ +public class DockerUtils { + /** + * Defines the possible locations of the Docker CLI. These will be searched in order. + */ public static String[] DOCKER_BINARIES = { "/usr/bin/docker", "/usr/local/bin/docker" }; - public static class DockerAvailability { - private boolean isAvailable = false; - private boolean isVersionHighEnough = false; - private String path = null; - private String version = null; - private Result lastCommand = null; - - public boolean isAvailable() { - return isAvailable; - } - - public boolean isVersionHighEnough() { - return isVersionHighEnough; - } - - public String getPath() { - return path; - } - - public String getVersion() { - return version; - } - - public Result getLastCommand() { - return lastCommand; - } - } - + /** + * Searches the entries in {@link #DOCKER_BINARIES} for the Docker CLI. This method does + * not check whether the Docker installation appears usable, see {@link #getDockerAvailability()} + * instead. + * + * @return the path to a CLI, if available. + */ public static Optional getDockerPath() { // Check if the Docker binary exists return List.of(DOCKER_BINARIES) @@ -48,6 +35,10 @@ public static Optional getDockerPath() { .findFirst(); } + /** + * Searches for a functional Docker installation, and returns information about the search. + * @return the results of the search. + */ public static DockerAvailability getDockerAvailability() { final DockerAvailability dockerAvailability = new DockerAvailability(); @@ -89,7 +80,65 @@ public static DockerAvailability getDockerAvailability() { return dockerAvailability; } + /** + * This class represents the results of a Docker search from {@link #getDockerAvailability()}}. + */ + public static class DockerAvailability { + private boolean isAvailable = false; + private boolean isVersionHighEnough = false; + private String path = null; + private String version = null; + private Result lastCommand = null; + + /** + * Indicates whether Docker is available and meets the required criteria. + * @return true if, and only if, Docker is: + *
    + *
  • Installed
  • + *
  • Executable
  • + *
  • Is at least version 17.05
  • + *
  • Can execute a command that requires privileges
  • + *
+ */ + public boolean isAvailable() { + return isAvailable; + } + + /** + * @return true if the installed Docker version is >= 17.05 + */ + public boolean isVersionHighEnough() { + return isVersionHighEnough; + } + + /** + * @return the path to the Docker CLI, or null + */ + public String getPath() { + return path; + } + + /** + * @return The installed Docker version, or null + */ + public String getVersion() { + return version; + } + + /** + * @return Information about the last command executes while probing Docker, or null. + */ + public Result getLastCommand() { + return lastCommand; + } + } + // package-private for testing + /** + * Given the string that docker --version prints, extract the docker version from it. + * @param dockerVersion the version string to parse + * @return the Docker version + */ static String extractVersion(String dockerVersion) { final Pattern dockerVersionPattern = Pattern.compile("Docker version (\\d+\\.\\d+\\.\\d+(?:-[a-zA-Z0-9]+)?), build [0-9a-f]{7,40}"); @@ -102,6 +151,11 @@ static String extractVersion(String dockerVersion) { } // package-private for testing + /** + * Checks whether the supplied Docker version is >= 17.05 + * @param dockerVersion the version to check + * @return true if the version is high enough + */ static boolean checkVersion(String dockerVersion) { final String[] majorMinor = dockerVersion.split("\\."); @@ -109,7 +163,18 @@ static boolean checkVersion(String dockerVersion) { || (Integer.parseInt(majorMinor[0]) == 17 && Integer.parseInt(majorMinor[1]) >= 5)); } + /** + * Runs a command and captures the exit code, standard output and standard error. + * @param args the command and any arguments to execute + * @return a object that captures the result of running the command. If an exception occurring + * while running the command, or the process was killed after reaching the 10s timeout, + * then the exit code will be -1. + */ private static Result runCommand(String... args) { + if (args.length == 0) { + throw new IllegalArgumentException("Cannot execute with no command"); + } + final ProcessBuilder command = new ProcessBuilder().command(args); try { final Process process = command.start(); @@ -138,12 +203,15 @@ private static Result runCommand(String... args) { } } + /** + * This class models the result of running a command. It captures the exit code, standard output and standard error. + */ public static class Result { public final int exitCode; public final String stdout; public final String stderr; - public Result(int exitCode, String stdout, String stderr) { + Result(int exitCode, String stdout, String stderr) { this.exitCode = exitCode; this.stdout = stdout; this.stderr = stderr; diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/DockerTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/DockerUtilsTests.java similarity index 81% rename from buildSrc/src/test/java/org/elasticsearch/gradle/DockerTests.java rename to buildSrc/src/test/java/org/elasticsearch/gradle/DockerUtilsTests.java index 73e8200896264..754a70a12f888 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/DockerTests.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/DockerUtilsTests.java @@ -3,11 +3,11 @@ import org.elasticsearch.gradle.test.GradleUnitTestCase; import org.junit.Test; -import static org.elasticsearch.gradle.Docker.checkVersion; -import static org.elasticsearch.gradle.Docker.extractVersion; +import static org.elasticsearch.gradle.DockerUtils.checkVersion; +import static org.elasticsearch.gradle.DockerUtils.extractVersion; import static org.hamcrest.CoreMatchers.equalTo; -public class DockerTests extends GradleUnitTestCase { +public class DockerUtilsTests extends GradleUnitTestCase { @Test public void testExtractVersion() { From ec8b0852f25ff70be9b84d42e0be2fb544dc02ac Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Tue, 8 Oct 2019 12:42:58 +0100 Subject: [PATCH 05/43] Enable IPv4 forwarding on ubuntu in 16.04 --- Vagrantfile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Vagrantfile b/Vagrantfile index 93b60b46872fd..1f249f5648feb 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -216,6 +216,10 @@ def ubuntu_docker(config) # Add vagrant to the Docker group, so that it can run commands usermod -aG docker vagrant + + # Enable IPv4 forwarding + sed -i '/net.ipv4.ip_forward/s/^#//' /etc/sysctl.conf + systemctl restart networking SHELL end From 1f48f446a0f71d317c3aefcb6f590a47859bd355 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Tue, 8 Oct 2019 14:16:09 +0100 Subject: [PATCH 06/43] Use host networking when building docker images --- distribution/docker/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/docker/build.gradle b/distribution/docker/build.gradle index 0905bde750d00..0fb93e997c709 100644 --- a/distribution/docker/build.gradle +++ b/distribution/docker/build.gradle @@ -162,7 +162,7 @@ void addBuildDockerImage(final boolean oss) { ] } executable 'docker' - final List dockerArgs = ['build', files(oss), '--pull', '--no-cache'] + final List dockerArgs = ['build', files(oss), '--network', 'host', '--pull', '--no-cache'] for (final String tag : tags) { dockerArgs.add('--tag') dockerArgs.add(tag) From 560adb644a31552a848577dd0a910ef1ec18fa8a Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 9 Oct 2019 12:35:02 +0100 Subject: [PATCH 07/43] Address review feedback --- .../elasticsearch/gradle/BuildPlugin.groovy | 54 +--- .../gradle/test/DistroTestPlugin.java | 17 +- .../org/elasticsearch/gradle/DockerUtils.java | 228 ----------------- .../gradle/tool/DockerUtils.java | 233 ++++++++++++++++++ .../gradle/DockerUtilsTests.java | 30 --- distribution/docker/build.gradle | 2 +- 6 files changed, 250 insertions(+), 314 deletions(-) delete mode 100644 buildSrc/src/main/java/org/elasticsearch/gradle/DockerUtils.java create mode 100644 buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java delete mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/DockerUtilsTests.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index 8e92ea56b78ca..8dada7cf95b05 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -83,11 +83,10 @@ import org.gradle.util.GradleVersion import java.nio.charset.StandardCharsets import java.nio.file.Files -import static DockerUtils.DOCKER_BINARIES -import static DockerUtils.getDockerAvailability -import static DockerUtils.getDockerPath import static org.elasticsearch.gradle.tool.Boilerplate.findByName import static org.elasticsearch.gradle.tool.Boilerplate.maybeConfigure +import static org.elasticsearch.gradle.tool.DockerUtils.assertDockerIsAvailable +import static org.elasticsearch.gradle.tool.DockerUtils.getDockerPath /** * Encapsulates build configuration for elasticsearch projects. @@ -211,47 +210,7 @@ class BuildPlugin implements Plugin { final List tasks = taskGraph.allTasks.intersect(ext.get('requiresDocker') as List).collect { " ${it.path}".toString()} if (tasks.isEmpty() == false) { - final DockerUtils.DockerAvailability dockerAvailability = getDockerAvailability() - - if (dockerAvailability.isAvailable() == true) { - return - } - - /* - * There are tasks in the task graph that require Docker. - * Now we are failing because either the Docker binary does - * not exist or because execution of a privileged Docker - * command failed. - */ - if (dockerAvailability.getPath() == null) { - final String message = String.format( - Locale.ROOT, - "Docker (checked [%s]) is required to run the following task%s: \n%s", - DOCKER_BINARIES.join(","), - tasks.size() > 1 ? "s" : "", - tasks.join('\n')) - throwDockerRequiredException(message) - } - - if (dockerAvailability.isVersionHighEnough() == false) { - final String message = String.format( - Locale.ROOT, - "building Docker images requires Docker version 17.05+ due to use of multi-stage builds yet was [%s]", - dockerAvailability.getVersion()) - throwDockerRequiredException(message) - } - - // Some other problem, print the error - final String message = String.format( - Locale.ROOT, - "a problem occurred running Docker from [%s] yet it is required to run the following task%s: \n%s\n" + - "the problem is that Docker exited with exit code [%d] with standard error output [%s]", - dockerBinary, - tasks.size() > 1 ? "s" : "", - tasks.join('\n'), - dockerAvailability.getLastCommand().exitCode, - dockerAvailability.getLastCommand().stderr.trim()) - throwDockerRequiredException(message) + assertDockerIsAvailable(tasks) } } } @@ -259,13 +218,6 @@ class BuildPlugin implements Plugin { (ext.get('requiresDocker') as List).add(task) } - private static void throwDockerRequiredException(final String message) { - throw new GradleException( - message + "\nyou can address this by attending to the reported issue, " - + "removing the offending tasks from being executed, " - + "or by passing -Dbuild.docker=false") - } - /** Add a check before gradle execution phase which ensures java home for the given java version is set. */ static void requireJavaHome(Task task, int version) { // use root project for global accounting diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index ec9caff76f2f4..d79103c56e2c7 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -39,6 +39,7 @@ import org.gradle.api.Task; import org.gradle.api.artifacts.Configuration; import org.gradle.api.file.Directory; +import org.gradle.api.logging.Logging; import org.gradle.api.plugins.ExtraPropertiesExtension; import org.gradle.api.plugins.JavaBasePlugin; import org.gradle.api.provider.Provider; @@ -46,6 +47,7 @@ import org.gradle.api.tasks.TaskInputs; import org.gradle.api.tasks.TaskProvider; import org.gradle.api.tasks.testing.Test; +import org.slf4j.Logger; import java.io.IOException; import java.io.UncheckedIOException; @@ -60,11 +62,12 @@ import java.util.Random; import java.util.stream.Collectors; -import static org.elasticsearch.gradle.DockerUtils.getDockerAvailability; +import static org.elasticsearch.gradle.tool.DockerUtils.getDockerAvailability; import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertLinuxPath; import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertWindowsPath; public class DistroTestPlugin implements Plugin { + private final Logger logger = Logging.getLogger(getClass()); private static final String SYSTEM_JDK_VERSION = "11.0.2+9"; private static final String SYSTEM_JDK_VENDOR = "openjdk"; @@ -320,9 +323,15 @@ private List configureDistributions(Project project, List currentDistros = new ArrayList<>(); List upgradeDistros = new ArrayList<>(); - final String buildDockerProperty = System.getProperty("build.docker"); - final boolean shouldAddDocker = (buildDockerProperty == null || "true".equals(buildDockerProperty)) - && getDockerAvailability().isAvailable(); + boolean shouldAddDocker = false; + + try { + final String buildDockerProperty = System.getProperty("build.docker"); + shouldAddDocker = (buildDockerProperty == null || "true".equals(buildDockerProperty)) + && getDockerAvailability().isAvailable; + } catch (Exception e) { + logger.warn("Caught exception while checking for Docker", e); + } final List applicablePackageTypes = shouldAddDocker ? List.of(Type.DEB, Type.RPM, Type.DOCKER) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/DockerUtils.java b/buildSrc/src/main/java/org/elasticsearch/gradle/DockerUtils.java deleted file mode 100644 index ab3b66628c943..0000000000000 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/DockerUtils.java +++ /dev/null @@ -1,228 +0,0 @@ -package org.elasticsearch.gradle; - -import org.apache.commons.io.IOUtils; - -import java.io.File; -import java.util.List; -import java.util.Optional; -import java.util.concurrent.TimeUnit; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -/** - * Contains utilities for checking whether Docker is installed, is executable, - * has a recent enough version, and appears to be functional. The Elasticsearch build - * requires Docker >= 17.05 as it uses a multi-stage build. - */ -public class DockerUtils { - /** - * Defines the possible locations of the Docker CLI. These will be searched in order. - */ - public static String[] DOCKER_BINARIES = { "/usr/bin/docker", "/usr/local/bin/docker" }; - - /** - * Searches the entries in {@link #DOCKER_BINARIES} for the Docker CLI. This method does - * not check whether the Docker installation appears usable, see {@link #getDockerAvailability()} - * instead. - * - * @return the path to a CLI, if available. - */ - public static Optional getDockerPath() { - // Check if the Docker binary exists - return List.of(DOCKER_BINARIES) - .stream() - .filter(path -> new File(path).exists()) - .findFirst(); - } - - /** - * Searches for a functional Docker installation, and returns information about the search. - * @return the results of the search. - */ - public static DockerAvailability getDockerAvailability() { - final DockerAvailability dockerAvailability = new DockerAvailability(); - - // Check if the Docker binary exists - final Optional dockerBinary = getDockerPath(); - - if (dockerBinary.isEmpty()) { - return dockerAvailability; - } - - dockerAvailability.path = dockerBinary.get(); - - // Since we use a multi-stage Docker build, check the Docker version since 17.05 - final Result versionResult = runCommand(dockerAvailability.path, "--version"); - - if (versionResult.isSuccess() == false) { - dockerAvailability.lastCommand = versionResult; - return dockerAvailability; - } - - final String version = extractVersion(versionResult.stdout.trim()); - dockerAvailability.version = version; - - if (checkVersion(version) == false) { - return dockerAvailability; - } - - dockerAvailability.isVersionHighEnough = true; - - // The Docker binary executes, so check that we can execute a privileged command - final Result imagesResult = runCommand(dockerAvailability.path, "images"); - - if (imagesResult.isSuccess() == false) { - dockerAvailability.lastCommand = versionResult; - return dockerAvailability; - } - - dockerAvailability.isAvailable = true; - return dockerAvailability; - } - - /** - * This class represents the results of a Docker search from {@link #getDockerAvailability()}}. - */ - public static class DockerAvailability { - private boolean isAvailable = false; - private boolean isVersionHighEnough = false; - private String path = null; - private String version = null; - private Result lastCommand = null; - - /** - * Indicates whether Docker is available and meets the required criteria. - * @return true if, and only if, Docker is: - *
    - *
  • Installed
  • - *
  • Executable
  • - *
  • Is at least version 17.05
  • - *
  • Can execute a command that requires privileges
  • - *
- */ - public boolean isAvailable() { - return isAvailable; - } - - /** - * @return true if the installed Docker version is >= 17.05 - */ - public boolean isVersionHighEnough() { - return isVersionHighEnough; - } - - /** - * @return the path to the Docker CLI, or null - */ - public String getPath() { - return path; - } - - /** - * @return The installed Docker version, or null - */ - public String getVersion() { - return version; - } - - /** - * @return Information about the last command executes while probing Docker, or null. - */ - public Result getLastCommand() { - return lastCommand; - } - } - - // package-private for testing - /** - * Given the string that docker --version prints, extract the docker version from it. - * @param dockerVersion the version string to parse - * @return the Docker version - */ - static String extractVersion(String dockerVersion) { - final Pattern dockerVersionPattern = Pattern.compile("Docker version (\\d+\\.\\d+\\.\\d+(?:-[a-zA-Z0-9]+)?), build [0-9a-f]{7,40}"); - - final Matcher matcher = dockerVersionPattern.matcher(dockerVersion); - if (!matcher.matches()) { - throw new IllegalArgumentException("Can't extract version from: [" + dockerVersion + "]"); - } - - return matcher.group(1); - } - - // package-private for testing - /** - * Checks whether the supplied Docker version is >= 17.05 - * @param dockerVersion the version to check - * @return true if the version is high enough - */ - static boolean checkVersion(String dockerVersion) { - final String[] majorMinor = dockerVersion.split("\\."); - - return (Integer.parseInt(majorMinor[0]) > 17 - || (Integer.parseInt(majorMinor[0]) == 17 && Integer.parseInt(majorMinor[1]) >= 5)); - } - - /** - * Runs a command and captures the exit code, standard output and standard error. - * @param args the command and any arguments to execute - * @return a object that captures the result of running the command. If an exception occurring - * while running the command, or the process was killed after reaching the 10s timeout, - * then the exit code will be -1. - */ - private static Result runCommand(String... args) { - if (args.length == 0) { - throw new IllegalArgumentException("Cannot execute with no command"); - } - - final ProcessBuilder command = new ProcessBuilder().command(args); - try { - final Process process = command.start(); - - if (process.waitFor(10, TimeUnit.SECONDS) == false) { - if (process.isAlive()) { - process.destroyForcibly(); - } - - Result result = new Result(-1, - IOUtils.toString(process.getInputStream()), - IOUtils.toString(process.getErrorStream())); - - throw new IllegalStateException( - "Timed out running shell command: " + command + "\n" + - "Result:\n" + result - ); - } - - String stdout = IOUtils.toString(process.getInputStream()); - String stderr = IOUtils.toString(process.getErrorStream()); - - return new Result(process.exitValue(), stdout, stderr); - } catch (Exception e) { - return new Result(-1, "", e.getMessage()); - } - } - - /** - * This class models the result of running a command. It captures the exit code, standard output and standard error. - */ - public static class Result { - public final int exitCode; - public final String stdout; - public final String stderr; - - Result(int exitCode, String stdout, String stderr) { - this.exitCode = exitCode; - this.stdout = stdout; - this.stderr = stderr; - } - - public boolean isSuccess() { - return exitCode == 0; - } - - public String toString() { - return "exitCode = [" + exitCode + "] " + "stdout = [" + stdout.trim() + "] " + "stderr = [" + stderr.trim() + "]"; - } - } -} diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java new file mode 100644 index 0000000000000..400dca1df1d0a --- /dev/null +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java @@ -0,0 +1,233 @@ +package org.elasticsearch.gradle.tool; + +import org.apache.commons.io.IOUtils; +import org.elasticsearch.gradle.Version; +import org.gradle.api.GradleException; + +import java.io.File; +import java.util.List; +import java.util.Locale; +import java.util.Optional; + +/** + * Contains utilities for checking whether Docker is installed, is executable, + * has a recent enough version, and appears to be functional. The Elasticsearch build + * requires Docker >= 17.05 as it uses a multi-stage build. + */ +public class DockerUtils { + /** + * Defines the possible locations of the Docker CLI. These will be searched in order. + */ + private static String[] DOCKER_BINARIES = { "/usr/bin/docker", "/usr/local/bin/docker" }; + + /** + * Searches the entries in {@link #DOCKER_BINARIES} for the Docker CLI. This method does + * not check whether the Docker installation appears usable, see {@link #getDockerAvailability()} + * instead. + * + * @return the path to a CLI, if available. + */ + public static Optional getDockerPath() { + // Check if the Docker binary exists + return List.of(DOCKER_BINARIES) + .stream() + .filter(path -> new File(path).exists()) + .findFirst(); + } + + /** + * Searches for a functional Docker installation, and returns information about the search. + * @return the results of the search. + */ + public static DockerAvailability getDockerAvailability() throws Exception { + String dockerPath = null; + Result lastResult = null; + Version version = null; + boolean isVersionHighEnough = false; + + // Check if the Docker binary exists + final Optional dockerBinary = getDockerPath(); + + if (dockerBinary.isPresent()) { + dockerPath = dockerBinary.get(); + + // Since we use a multi-stage Docker build, check the Docker version since 17.05 + lastResult = runCommand(dockerPath, "version", "--format", "{{.Server.Version}}"); + + if (lastResult.isSuccess() == true) { + version = Version.fromString(lastResult.stdout.trim()); + + isVersionHighEnough = version.onOrAfter("17.05.0"); + + if (isVersionHighEnough == true) { + // Check that we can execute a privileged command + lastResult = runCommand(dockerPath, "images"); + } + } + } + + boolean isAvailable = isVersionHighEnough && lastResult.isSuccess() == true; + + return new DockerAvailability(isAvailable, isVersionHighEnough, dockerPath, version, lastResult); + } + + /** + * An immutable class that represents the results of a Docker search from {@link #getDockerAvailability()}}. + */ + public static class DockerAvailability { + /** + * Indicates whether Docker is available and meets the required criteria. + * True if, and only if, Docker is: + *
    + *
  • Installed
  • + *
  • Executable
  • + *
  • Is at least version 17.05
  • + *
  • Can execute a command that requires privileges
  • + *
+ */ + public final boolean isAvailable; + + /** + * True if the installed Docker version is >= 17.05 + */ + public final boolean isVersionHighEnough; + + /** + * The path to the Docker CLI, or null + */ + public final String path; + + /** + * The installed Docker version, or null + */ + public final Version version; + + /** + * Information about the last command executes while probing Docker, or null. + */ + public final Result lastCommand; + + public DockerAvailability(boolean isAvailable, boolean isVersionHighEnough, String path, Version version, Result lastCommand) { + this.isAvailable = isAvailable; + this.isVersionHighEnough = isVersionHighEnough; + this.path = path; + this.version = version; + this.lastCommand = lastCommand; + } + } + + /** + * Given a list of tasks that requires Docker, check whether Docker is available, otherwise + * throw an exception. + * @param tasks the tasks that require Docker + * @throws GradleException if Docker is not available. The exception message gives the reason. + */ + public static void assertDockerIsAvailable(List tasks) { + DockerAvailability availability = null; + + try { + availability = getDockerAvailability(); + } catch (Exception e) { + throwDockerRequiredException("Something went wrong while checking Docker's availability", e); + } + + if (availability.isAvailable == true) { + return; + } + + /* + * There are tasks in the task graph that require Docker. + * Now we are failing because either the Docker binary does + * not exist or because execution of a privileged Docker + * command failed. + */ + if (availability.path == null) { + final String message = String.format( + Locale.ROOT, + "Docker (checked [%s]) is required to run the following task%s: \n%s", + String.join(", ", DOCKER_BINARIES), + tasks.size() > 1 ? "s" : "", + String.join("\n", tasks)); + throwDockerRequiredException(message); + } + + if (availability.isVersionHighEnough == false) { + final String message = String.format( + Locale.ROOT, + "building Docker images requires Docker version 17.05+ due to use of multi-stage builds yet was [%s]", + availability.version); + throwDockerRequiredException(message); + } + + // Some other problem, print the error + final String message = String.format( + Locale.ROOT, + "a problem occurred running Docker from [%s] yet it is required to run the following task%s: \n%s\n" + + "the problem is that Docker exited with exit code [%d] with standard error output [%s]", + availability.path, + tasks.size() > 1 ? "s" : "", + String.join("\n", tasks), + availability.lastCommand.exitCode, + availability.lastCommand.stderr.trim()); + throwDockerRequiredException(message); + } + + private static void throwDockerRequiredException(final String message) { + throwDockerRequiredException(message, null); + } + + private static void throwDockerRequiredException(final String message, Exception e) { + throw new GradleException( + message + "\nyou can address this by attending to the reported issue, " + + "removing the offending tasks from being executed, " + + "or by passing -Dbuild.docker=false", e); + } + + /** + * Runs a command and captures the exit code, standard output and standard error. + * @param args the command and any arguments to execute + * @return a object that captures the result of running the command. If an exception occurring + * while running the command, or the process was killed after reaching the 10s timeout, + * then the exit code will be -1. + */ + private static Result runCommand(String... args) throws Exception { + if (args.length == 0) { + throw new IllegalArgumentException("Cannot execute with no command"); + } + + final ProcessBuilder command = new ProcessBuilder().command(args); + final Process process = command.start(); + + process.waitFor(); + + String stdout = IOUtils.toString(process.getInputStream()); + String stderr = IOUtils.toString(process.getErrorStream()); + + process.destroy(); + + return new Result(process.exitValue(), stdout, stderr); + } + + /** + * This class models the result of running a command. It captures the exit code, standard output and standard error. + */ + private static class Result { + public final int exitCode; + public final String stdout; + public final String stderr; + + Result(int exitCode, String stdout, String stderr) { + this.exitCode = exitCode; + this.stdout = stdout; + this.stderr = stderr; + } + + public boolean isSuccess() { + return exitCode == 0; + } + + public String toString() { + return "exitCode = [" + exitCode + "] " + "stdout = [" + stdout.trim() + "] " + "stderr = [" + stderr.trim() + "]"; + } + } +} diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/DockerUtilsTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/DockerUtilsTests.java deleted file mode 100644 index 754a70a12f888..0000000000000 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/DockerUtilsTests.java +++ /dev/null @@ -1,30 +0,0 @@ -package org.elasticsearch.gradle; - -import org.elasticsearch.gradle.test.GradleUnitTestCase; -import org.junit.Test; - -import static org.elasticsearch.gradle.DockerUtils.checkVersion; -import static org.elasticsearch.gradle.DockerUtils.extractVersion; -import static org.hamcrest.CoreMatchers.equalTo; - -public class DockerUtilsTests extends GradleUnitTestCase { - - @Test - public void testExtractVersion() { - assertThat(extractVersion("Docker version 18.06.1-ce, build e68fc7a215d7"), equalTo("18.06.1-ce")); - assertThat(extractVersion("Docker version 17.05.0, build e68fc7a"), equalTo("17.05.0")); - assertThat(extractVersion("Docker version 17.05.1, build e68fc7a"), equalTo("17.05.1")); - } - - @Test - public void testCheckAcceptableVersions() { - assertTrue(checkVersion("18.06.1-ce")); - assertTrue(checkVersion("17.05.1-ce")); - } - - @Test - public void testCheckUnacceptableVersions() { - assertFalse(checkVersion("17.04.0")); - assertFalse(checkVersion("16.99.0")); - } -} diff --git a/distribution/docker/build.gradle b/distribution/docker/build.gradle index 0fb93e997c709..0905bde750d00 100644 --- a/distribution/docker/build.gradle +++ b/distribution/docker/build.gradle @@ -162,7 +162,7 @@ void addBuildDockerImage(final boolean oss) { ] } executable 'docker' - final List dockerArgs = ['build', files(oss), '--network', 'host', '--pull', '--no-cache'] + final List dockerArgs = ['build', files(oss), '--pull', '--no-cache'] for (final String tag : tags) { dockerArgs.add('--tag') dockerArgs.add(tag) From 7212a9cdb9937ecd8eaab357d97ed1a100058733 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 9 Oct 2019 15:39:31 +0100 Subject: [PATCH 08/43] Exclude Docker distro tests per VM Introduce a VM exclude list, so that the docker packaging tests are run on all VMs other than those listed. Previously the code checked that whether Docker was installed, but that wasn't sufficient becasue the check would happen on the CI host, instead of in the VM. --- .../gradle/test/DistroTestPlugin.java | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index d79103c56e2c7..8eca33391cfab 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -62,13 +62,27 @@ import java.util.Random; import java.util.stream.Collectors; -import static org.elasticsearch.gradle.tool.DockerUtils.getDockerAvailability; import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertLinuxPath; import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertWindowsPath; public class DistroTestPlugin implements Plugin { private final Logger logger = Logging.getLogger(getClass()); + /** + * The Docker distribution is tested on all OS platforms, apart + * from those listed here. + */ + private static final List DOCKER_EXCLUDE_LIST = List.of( + "centos-6", + "debian-8", + "oel-6", + "oel-7", + "opensuse-42", + "sles-12", + "windows-2012r2", + "windows-2016" + ); + private static final String SYSTEM_JDK_VERSION = "11.0.2+9"; private static final String SYSTEM_JDK_VENDOR = "openjdk"; private static final String GRADLE_JDK_VERSION = "12.0.1+12@69cfe15208a647278a19ef0990eea691"; @@ -124,6 +138,10 @@ public void apply(Project project) { TaskProvider distroTest = vmProject.getTasks().register("distroTest"); for (ElasticsearchDistribution distribution : distributions) { + if (distribution.getType() == Type.DOCKER && DOCKER_EXCLUDE_LIST.contains(vmProject.getName())) { + continue; + } + String destructiveTaskName = destructiveDistroTestTaskName(distribution); Platform platform = distribution.getPlatform(); // this condition ensures windows boxes get windows distributions, and linux boxes get linux distributions @@ -323,21 +341,7 @@ private List configureDistributions(Project project, List currentDistros = new ArrayList<>(); List upgradeDistros = new ArrayList<>(); - boolean shouldAddDocker = false; - - try { - final String buildDockerProperty = System.getProperty("build.docker"); - shouldAddDocker = (buildDockerProperty == null || "true".equals(buildDockerProperty)) - && getDockerAvailability().isAvailable; - } catch (Exception e) { - logger.warn("Caught exception while checking for Docker", e); - } - - final List applicablePackageTypes = shouldAddDocker - ? List.of(Type.DEB, Type.RPM, Type.DOCKER) - : List.of(Type.DEB, Type.RPM); - - for (Type type : applicablePackageTypes) { + for (Type type : List.of(Type.DEB, Type.RPM, Type.DOCKER)) { for (Flavor flavor : Flavor.values()) { for (boolean bundledJdk : Arrays.asList(true, false)) { // We should never add a Docker distro with bundledJdk == false From de9bb330163f3c4916f7555a336df5f4336c90b0 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 10 Oct 2019 10:24:46 +0100 Subject: [PATCH 09/43] Handle Docker version numbers The Version class is very strict about what suffices it allows after the patch number, which meant it rejected Docker versions such as `19.03.2-ce`. Introduce a "relaxed" parsing mode that allow any style of suffix. --- .../gradle/tool/DockerUtils.java | 2 +- .../org/elasticsearch/gradle/Version.java | 30 +++++++++++++++++-- .../elasticsearch/gradle/VersionTests.java | 14 ++++++++- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java index 400dca1df1d0a..f41efbaab0ffd 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java @@ -55,7 +55,7 @@ public static DockerAvailability getDockerAvailability() throws Exception { lastResult = runCommand(dockerPath, "version", "--format", "{{.Server.Version}}"); if (lastResult.isSuccess() == true) { - version = Version.fromString(lastResult.stdout.trim()); + version = Version.fromString(lastResult.stdout.trim(), Version.Mode.RELAXED); isVersionHighEnough = version.onOrAfter("17.05.0"); diff --git a/buildSrc/src/minimumRuntime/java/org/elasticsearch/gradle/Version.java b/buildSrc/src/minimumRuntime/java/org/elasticsearch/gradle/Version.java index 31738f140878d..d31e15b842b2c 100644 --- a/buildSrc/src/minimumRuntime/java/org/elasticsearch/gradle/Version.java +++ b/buildSrc/src/minimumRuntime/java/org/elasticsearch/gradle/Version.java @@ -13,9 +13,28 @@ public final class Version implements Comparable { private final int revision; private final int id; + /** + * Specifies how a version string should be parsed. + */ + public enum Mode { + /** + * Strict parsing only allows known suffixes after the patch number: "-alpha", "-beta" or "-rc". The + * suffix "-SNAPSHOT" is also allowed, either after the patch number, or after the other suffices. + */ + STRICT, + + /** + * Relaxed parsing allows any alphanumeric suffix after the patch number. + */ + RELAXED + } + private static final Pattern pattern = Pattern.compile("(\\d)+\\.(\\d+)\\.(\\d+)(-alpha\\d+|-beta\\d+|-rc\\d+)?(-SNAPSHOT)?"); + private static final Pattern relaxedPattern = + Pattern.compile("(\\d)+\\.(\\d+)\\.(\\d+)(-[a-zA-Z0-9_]+)*?"); + public Version(int major, int minor, int revision) { Objects.requireNonNull(major, "major version can't be null"); Objects.requireNonNull(minor, "minor version can't be null"); @@ -36,11 +55,18 @@ private static int parseSuffixNumber(String substring) { } public static Version fromString(final String s) { + return fromString(s, Mode.STRICT); + } + + public static Version fromString(final String s, final Mode mode) { Objects.requireNonNull(s); - Matcher matcher = pattern.matcher(s); + Matcher matcher = mode == Mode.STRICT ? pattern.matcher(s) : relaxedPattern.matcher(s); if (matcher.matches() == false) { + String expected = mode == Mode.STRICT == true + ? "major.minor.revision[-(alpha|beta|rc)Number][-SNAPSHOT]" + : "major.minor.revision[-extra]"; throw new IllegalArgumentException( - "Invalid version format: '" + s + "'. Should be major.minor.revision[-(alpha|beta|rc)Number][-SNAPSHOT]" + "Invalid version format: '" + s + "'. Should be " + expected ); } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/VersionTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/VersionTests.java index 3394285157e17..ae2fb0e6215db 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/VersionTests.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/VersionTests.java @@ -40,6 +40,14 @@ public void testVersionParsing() { assertVersionEquals("6.1.2-beta1-SNAPSHOT", 6, 1, 2); } + public void testRelaxedVersionParsing() { + assertVersionEquals("6.1.2", 6, 1, 2, Version.Mode.RELAXED); + assertVersionEquals("6.1.2-SNAPSHOT", 6, 1, 2, Version.Mode.RELAXED); + assertVersionEquals("6.1.2-beta1-SNAPSHOT", 6, 1, 2, Version.Mode.RELAXED); + assertVersionEquals("6.1.2-foo", 6, 1, 2, Version.Mode.RELAXED); + assertVersionEquals("6.1.2-foo-bar", 6, 1, 2, Version.Mode.RELAXED); + } + public void testCompareWithStringVersions() { assertTrue("1.10.20 is not interpreted as before 2.0.0", Version.fromString("1.10.20").before("2.0.0") @@ -100,7 +108,11 @@ private void assertOrder(Version smaller, Version bigger) { } private void assertVersionEquals(String stringVersion, int major, int minor, int revision) { - Version version = Version.fromString(stringVersion); + assertVersionEquals(stringVersion, major, minor, revision, Version.Mode.STRICT); + } + + private void assertVersionEquals(String stringVersion, int major, int minor, int revision, Version.Mode mode) { + Version version = Version.fromString(stringVersion, mode); assertEquals(major, version.getMajor()); assertEquals(minor, version.getMinor()); assertEquals(revision, version.getRevision()); From e8a920a3d0bf97e25c79c7b246a23dab12ee83c1 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 10 Oct 2019 15:39:08 +0100 Subject: [PATCH 10/43] Only run Docker tests on some host OSs Separately to running packaging tests in Vagrant, we also run packaging tests through CI on a variety of operating systems. Change how the test tasks are generated so that the local Docker packaging tests are only defined if the host OS is supported. --- Vagrantfile | 2 +- .../gradle/test/DistroTestPlugin.java | 83 +++++++++++++++++-- 2 files changed, 75 insertions(+), 10 deletions(-) diff --git a/Vagrantfile b/Vagrantfile index 1f249f5648feb..c86b0a910c239 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -126,7 +126,7 @@ Vagrant.configure(2) do |config| end 'fedora-29'.tap do |box| config.vm.define box, define_opts do |config| - config.vm.box = 'elastic/fedora-28-x86_64' + config.vm.box = 'elastic/fedora-29-x86_64' dnf_common config, box dnf_docker config end diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 8eca33391cfab..f1a03995821be 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -28,18 +28,19 @@ import org.elasticsearch.gradle.ElasticsearchDistribution.Type; import org.elasticsearch.gradle.Jdk; import org.elasticsearch.gradle.JdkDownloadPlugin; +import org.elasticsearch.gradle.OS; import org.elasticsearch.gradle.Version; import org.elasticsearch.gradle.VersionProperties; import org.elasticsearch.gradle.vagrant.BatsProgressLogger; import org.elasticsearch.gradle.vagrant.VagrantBasePlugin; import org.elasticsearch.gradle.vagrant.VagrantExtension; +import org.gradle.api.GradleException; import org.gradle.api.NamedDomainObjectContainer; import org.gradle.api.Plugin; import org.gradle.api.Project; import org.gradle.api.Task; import org.gradle.api.artifacts.Configuration; import org.gradle.api.file.Directory; -import org.gradle.api.logging.Logging; import org.gradle.api.plugins.ExtraPropertiesExtension; import org.gradle.api.plugins.JavaBasePlugin; import org.gradle.api.provider.Provider; @@ -47,12 +48,12 @@ import org.gradle.api.tasks.TaskInputs; import org.gradle.api.tasks.TaskProvider; import org.gradle.api.tasks.testing.Test; -import org.slf4j.Logger; import java.io.IOException; import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -66,13 +67,13 @@ import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertWindowsPath; public class DistroTestPlugin implements Plugin { - private final Logger logger = Logging.getLogger(getClass()); - /** - * The Docker distribution is tested on all OS platforms, apart - * from those listed here. + * This plugin generates test tasks for the various VMs in :qa:os. The + * Docker packaging tests are run on all of them, apart from those + * listed here. These values correspond to the sub-project names, which + * correspond to the directory names. */ - private static final List DOCKER_EXCLUDE_LIST = List.of( + private static final List DOCKER_VM_EXCLUDE_LIST = List.of( "centos-6", "debian-8", "oel-6", @@ -83,6 +84,23 @@ public class DistroTestPlugin implements Plugin { "windows-2016" ); + /** + * This plugin generates a number of test tasks, some of which are + * Docker packaging tests. When running on the host OS i.e. not in a + * VM, only certain operating systems are supported. This list + * specifies the Linux OS variants that are tested. The actual question + * of whether to run the Docker tests is answered by {@link + * #shouldRunDockerTests()}. + */ + private static final List DOCKER_LINUX_INCLUDE_LIST = List.of( + "centos-7", + "debian-9", + "fedora-28", + "fedora-29", + "ubuntu-16.04", + "ubuntu-18.04" + ); + private static final String SYSTEM_JDK_VERSION = "11.0.2+9"; private static final String SYSTEM_JDK_VENDOR = "openjdk"; private static final String GRADLE_JDK_VERSION = "12.0.1+12@69cfe15208a647278a19ef0990eea691"; @@ -119,7 +137,10 @@ public void apply(Project project) { TaskProvider destructiveDistroTest = project.getTasks().register("destructiveDistroTest"); for (ElasticsearchDistribution distribution : distributions) { TaskProvider destructiveTask = configureDistroTest(project, distribution); - destructiveDistroTest.configure(t -> t.dependsOn(destructiveTask)); + // Docker tests are not currently supported on Windows + if (shouldRunDockerTests()) { + destructiveDistroTest.configure(t -> t.dependsOn(destructiveTask)); + } } Map> batsTests = new HashMap<>(); batsTests.put("bats oss", configureBatsTest(project, "oss", distributionsDir, copyDistributionsTask)); @@ -138,7 +159,7 @@ public void apply(Project project) { TaskProvider distroTest = vmProject.getTasks().register("distroTest"); for (ElasticsearchDistribution distribution : distributions) { - if (distribution.getType() == Type.DOCKER && DOCKER_EXCLUDE_LIST.contains(vmProject.getName())) { + if (distribution.getType() == Type.DOCKER && DOCKER_VM_EXCLUDE_LIST.contains(vmProject.getName())) { continue; } @@ -420,4 +441,48 @@ private static String destructiveDistroTestTaskName(ElasticsearchDistribution di distro.getFlavor(), distro.getBundledJdk()); } + + /** + * The {@link DistroTestPlugin} generates a number of test tasks, some + * of which are Docker packaging tests. When running on the host OS + * i.e. not in a VM, only certain operating systems are supported. This + * method determines whether the Docker tests should be run on the host + * OS. + */ + private static boolean shouldRunDockerTests() { + switch (OS.current()) { + case WINDOWS: + // Not currently supported. + return false; + + case MAC: + // Assume that Docker for Mac is installed + return true; + + case LINUX: + final Path osRelease = Paths.get("/etc/os-release"); + + if (Files.exists(osRelease)) { + Map values = new HashMap<>(); + + try { + for (String line : Files.readAllLines(osRelease)) { + final String[] parts = line.split("=", 2); + values.put(parts[0], parts[1]); + } + } catch (IOException e) { + throw new GradleException("Failed to read /etc/os-release", e); + } + + final String id = values.get("ID") + "-" + values.get("VERSION"); + + return DOCKER_LINUX_INCLUDE_LIST.contains(id); + } + + return false; + + default: + return false; + } + } } From 2d8ca75a0ae0e54ef487b66b05e005bfed0943b7 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 10 Oct 2019 16:13:16 +0100 Subject: [PATCH 11/43] Make /etc/os-release parsing better --- .../gradle/test/DistroTestPlugin.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index f1a03995821be..219c777ed9bd0 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -467,8 +467,20 @@ private static boolean shouldRunDockerTests() { try { for (String line : Files.readAllLines(osRelease)) { + final String trimmed = line.trim(); + + // ignore empty and comment lines + if (trimmed.isEmpty() || trimmed.startsWith("#")) { + continue; + } + final String[] parts = line.split("=", 2); - values.put(parts[0], parts[1]); + final String key = parts[0]; + // remove optional leading and trailing quotes + final String value = parts[1] + .replaceAll("^['\"]", "") + .replaceAll("['\"]$", ""); + values.put(key, value); } } catch (IOException e) { throw new GradleException("Failed to read /etc/os-release", e); From 1684857e9c5c4a1015d59748d96d5d836457fc87 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 10 Oct 2019 19:24:27 +0100 Subject: [PATCH 12/43] Try to fix Docker task generation --- .../org/elasticsearch/gradle/test/DistroTestPlugin.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 219c777ed9bd0..52b460fd7b556 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -134,11 +134,12 @@ public void apply(Project project) { TaskProvider copyUpgradeTask = configureCopyUpgradeTask(project, upgradeVersion, upgradeDir); TaskProvider copyPluginsTask = configureCopyPluginsTask(project, pluginsDir); + final boolean dockerTestEnabled = shouldRunDockerTests(); + TaskProvider destructiveDistroTest = project.getTasks().register("destructiveDistroTest"); for (ElasticsearchDistribution distribution : distributions) { - TaskProvider destructiveTask = configureDistroTest(project, distribution); - // Docker tests are not currently supported on Windows - if (shouldRunDockerTests()) { + if (distribution.getType() != Type.DOCKER || dockerTestEnabled == true) { + TaskProvider destructiveTask = configureDistroTest(project, distribution); destructiveDistroTest.configure(t -> t.dependsOn(destructiveTask)); } } From 1641b382992d3bafdd56ee82994997e7ae3e6414 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Fri, 11 Oct 2019 15:38:14 +0100 Subject: [PATCH 13/43] Add a debug stmt, figure out debian-8 issue --- .../elasticsearch/gradle/test/DistroTestPlugin.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 52b460fd7b556..db0a590105ce7 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -41,6 +41,8 @@ import org.gradle.api.Task; import org.gradle.api.artifacts.Configuration; import org.gradle.api.file.Directory; +import org.gradle.api.logging.Logger; +import org.gradle.api.logging.Logging; import org.gradle.api.plugins.ExtraPropertiesExtension; import org.gradle.api.plugins.JavaBasePlugin; import org.gradle.api.provider.Provider; @@ -67,6 +69,7 @@ import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertWindowsPath; public class DistroTestPlugin implements Plugin { + private static final Logger logger = Logging.getLogger(DistroTestPlugin.class); /** * This plugin generates test tasks for the various VMs in :qa:os. The * Docker packaging tests are run on all of them, apart from those @@ -477,10 +480,10 @@ private static boolean shouldRunDockerTests() { final String[] parts = line.split("=", 2); final String key = parts[0]; - // remove optional leading and trailing quotes + // remove optional leading and trailing quotes and whitespace final String value = parts[1] - .replaceAll("^['\"]", "") - .replaceAll("['\"]$", ""); + .replaceAll("^['\"]\\s*", "") + .replaceAll("\\s*['\"]$", ""); values.put(key, value); } } catch (IOException e) { @@ -489,6 +492,8 @@ private static boolean shouldRunDockerTests() { final String id = values.get("ID") + "-" + values.get("VERSION"); + logger.info("Linux OS id is [" + id + "], is it in the list? -> " + DOCKER_LINUX_INCLUDE_LIST.contains(id)); + return DOCKER_LINUX_INCLUDE_LIST.contains(id); } From 2d96534e6583cfa760b80c89418236d1abbe03ee Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 23 Oct 2019 11:42:13 +0100 Subject: [PATCH 14/43] Fix how we use data from /etc/os-release --- .../gradle/test/DistroTestPlugin.java | 14 ++++++++++---- .../org/elasticsearch/gradle/tool/DockerUtils.java | 9 +++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index db0a590105ce7..4f851e39cb77a 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -163,11 +163,13 @@ public void apply(Project project) { TaskProvider distroTest = vmProject.getTasks().register("distroTest"); for (ElasticsearchDistribution distribution : distributions) { + String destructiveTaskName = destructiveDistroTestTaskName(distribution); + if (distribution.getType() == Type.DOCKER && DOCKER_VM_EXCLUDE_LIST.contains(vmProject.getName())) { + logger.info("Not generating task [" + destructiveTaskName + "] as [" + vmProject.getName() + "] is in the exclude list"); continue; } - String destructiveTaskName = destructiveDistroTestTaskName(distribution); Platform platform = distribution.getPlatform(); // this condition ensures windows boxes get windows distributions, and linux boxes get linux distributions if (isWindows(vmProject) == (platform == Platform.WINDOWS)) { @@ -490,11 +492,15 @@ private static boolean shouldRunDockerTests() { throw new GradleException("Failed to read /etc/os-release", e); } - final String id = values.get("ID") + "-" + values.get("VERSION"); + final String id = values.get("ID") + "-" + values.get("VERSION_ID"); - logger.info("Linux OS id is [" + id + "], is it in the list? -> " + DOCKER_LINUX_INCLUDE_LIST.contains(id)); + final boolean contains = DOCKER_LINUX_INCLUDE_LIST.contains(id); + + if (contains == false) { + logger.warn("Linux OS id [" + id + "] is not present in the include list"); + } - return DOCKER_LINUX_INCLUDE_LIST.contains(id); + return contains; } return false; diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java index f41efbaab0ffd..e276376c5f8fc 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java @@ -151,6 +151,15 @@ public static void assertDockerIsAvailable(List tasks) { throwDockerRequiredException(message); } + if (availability.version == null) { + final String message = String.format( + Locale.ROOT, + "Docker is required to run the following task%s, but it doesn't appear to be running: \n%s", + tasks.size() > 1 ? "s" : "", + String.join("\n", tasks)); + throwDockerRequiredException(message); + } + if (availability.isVersionHighEnough == false) { final String message = String.format( Locale.ROOT, From 64051eddd6c6715d65692f020c199c12f06011e7 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 23 Oct 2019 11:49:57 +0100 Subject: [PATCH 15/43] Fix checkstyle --- .../org/elasticsearch/gradle/test/DistroTestPlugin.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 4f851e39cb77a..27fb9f5db0a3b 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -166,7 +166,9 @@ public void apply(Project project) { String destructiveTaskName = destructiveDistroTestTaskName(distribution); if (distribution.getType() == Type.DOCKER && DOCKER_VM_EXCLUDE_LIST.contains(vmProject.getName())) { - logger.info("Not generating task [" + destructiveTaskName + "] as [" + vmProject.getName() + "] is in the exclude list"); + logger.info( + "Not generating task [" + destructiveTaskName + "] as [" + vmProject.getName() + "] is in the exclude list" + ); continue; } From 6a640657f4972938cc517b7a0ab700f19afa3e1f Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 23 Oct 2019 14:46:56 +0100 Subject: [PATCH 16/43] Trying to diagnose CI failures --- .../org/elasticsearch/gradle/test/DistroTestPlugin.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 27fb9f5db0a3b..3e62656be8252 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -474,7 +474,9 @@ private static boolean shouldRunDockerTests() { Map values = new HashMap<>(); try { - for (String line : Files.readAllLines(osRelease)) { + final List osReleaseLines = Files.readAllLines(osRelease); + logger.warn(String.join("\n", osReleaseLines)); + for (String line : osReleaseLines) { final String trimmed = line.trim(); // ignore empty and comment lines @@ -498,9 +500,7 @@ private static boolean shouldRunDockerTests() { final boolean contains = DOCKER_LINUX_INCLUDE_LIST.contains(id); - if (contains == false) { - logger.warn("Linux OS id [" + id + "] is not present in the include list"); - } + logger.warn("Linux OS id [" + id + "] is " + (contains ? "" : "not ") + "present in the include list"); return contains; } From 8cb138914802d3d8be7fe85cd3460df67210cf95 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 23 Oct 2019 15:26:13 +0100 Subject: [PATCH 17/43] Still trying to diagnose failures on CI --- .../org/elasticsearch/gradle/test/DistroTestPlugin.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 3e62656be8252..ceb6f77ab4b3d 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -169,6 +169,9 @@ public void apply(Project project) { logger.info( "Not generating task [" + destructiveTaskName + "] as [" + vmProject.getName() + "] is in the exclude list" ); + System.err.println( + "[DEBUG] Not generating task [" + destructiveTaskName + "] as [" + vmProject.getName() + "] is in the exclude list" + ); continue; } @@ -475,7 +478,7 @@ private static boolean shouldRunDockerTests() { try { final List osReleaseLines = Files.readAllLines(osRelease); - logger.warn(String.join("\n", osReleaseLines)); + System.err.println(String.join("\n", osReleaseLines)); for (String line : osReleaseLines) { final String trimmed = line.trim(); @@ -501,6 +504,7 @@ private static boolean shouldRunDockerTests() { final boolean contains = DOCKER_LINUX_INCLUDE_LIST.contains(id); logger.warn("Linux OS id [" + id + "] is " + (contains ? "" : "not ") + "present in the include list"); + System.err.println("[DEBUG] Linux OS id [" + id + "] is " + (contains ? "" : "not ") + "present in the include list"); return contains; } From b0e3a9eba2db6ce815f3d1ccf85b77f5c91832a2 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 23 Oct 2019 16:26:44 +0100 Subject: [PATCH 18/43] Please, just print my debug info --- .../elasticsearch/gradle/test/DistroTestPlugin.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index ceb6f77ab4b3d..57f8ebb0eb7c3 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -166,12 +166,9 @@ public void apply(Project project) { String destructiveTaskName = destructiveDistroTestTaskName(distribution); if (distribution.getType() == Type.DOCKER && DOCKER_VM_EXCLUDE_LIST.contains(vmProject.getName())) { - logger.info( + logger.error( "Not generating task [" + destructiveTaskName + "] as [" + vmProject.getName() + "] is in the exclude list" ); - System.err.println( - "[DEBUG] Not generating task [" + destructiveTaskName + "] as [" + vmProject.getName() + "] is in the exclude list" - ); continue; } @@ -478,7 +475,7 @@ private static boolean shouldRunDockerTests() { try { final List osReleaseLines = Files.readAllLines(osRelease); - System.err.println(String.join("\n", osReleaseLines)); + logger.error(String.join("\n", osReleaseLines)); for (String line : osReleaseLines) { final String trimmed = line.trim(); @@ -503,8 +500,7 @@ private static boolean shouldRunDockerTests() { final boolean contains = DOCKER_LINUX_INCLUDE_LIST.contains(id); - logger.warn("Linux OS id [" + id + "] is " + (contains ? "" : "not ") + "present in the include list"); - System.err.println("[DEBUG] Linux OS id [" + id + "] is " + (contains ? "" : "not ") + "present in the include list"); + logger.error("Linux OS id [" + id + "] is " + (contains ? "" : "not ") + "present in the include list"); return contains; } From a20b3d453c2196b4e7cf9575a9c5ce9498f72bdb Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 24 Oct 2019 10:00:18 +0100 Subject: [PATCH 19/43] Please, just print my debug info --- .../org/elasticsearch/gradle/test/DistroTestPlugin.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 57f8ebb0eb7c3..bf53559a08a05 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -139,6 +139,8 @@ public void apply(Project project) { final boolean dockerTestEnabled = shouldRunDockerTests(); + logger.error("[DEBUG] dockerTestEnabled = " + dockerTestEnabled); + TaskProvider destructiveDistroTest = project.getTasks().register("destructiveDistroTest"); for (ElasticsearchDistribution distribution : distributions) { if (distribution.getType() != Type.DOCKER || dockerTestEnabled == true) { @@ -458,6 +460,8 @@ private static String destructiveDistroTestTaskName(ElasticsearchDistribution di * OS. */ private static boolean shouldRunDockerTests() { + logger.error("[DEBUG] shouldRunDockerTests - OS.current() == " + OS.current()); + switch (OS.current()) { case WINDOWS: // Not currently supported. @@ -503,6 +507,8 @@ private static boolean shouldRunDockerTests() { logger.error("Linux OS id [" + id + "] is " + (contains ? "" : "not ") + "present in the include list"); return contains; + } else { + logger.error("[DEBUG] /etc/os-release does not exist!"); } return false; From daec2ad22c15ac16c943b6b9452b4c590caff212 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 24 Oct 2019 12:50:53 +0100 Subject: [PATCH 20/43] Still trying to debug --- .../gradle/test/DistroTestPlugin.java | 60 +++++++++++-------- .../gradle/test/DistroTestPluginTests.java | 59 ++++++++++++++++++ 2 files changed, 95 insertions(+), 24 deletions(-) create mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index bf53559a08a05..b70b4ebded745 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -139,7 +139,7 @@ public void apply(Project project) { final boolean dockerTestEnabled = shouldRunDockerTests(); - logger.error("[DEBUG] dockerTestEnabled = " + dockerTestEnabled); + logger.error("[BADGER] dockerTestEnabled = " + dockerTestEnabled); TaskProvider destructiveDistroTest = project.getTasks().register("destructiveDistroTest"); for (ElasticsearchDistribution distribution : distributions) { @@ -169,7 +169,7 @@ public void apply(Project project) { if (distribution.getType() == Type.DOCKER && DOCKER_VM_EXCLUDE_LIST.contains(vmProject.getName())) { logger.error( - "Not generating task [" + destructiveTaskName + "] as [" + vmProject.getName() + "] is in the exclude list" + "[BADGER] Not generating task [" + destructiveTaskName + "] as [" + vmProject.getName() + "] is in the exclude list" ); continue; } @@ -452,6 +452,31 @@ private static String destructiveDistroTestTaskName(ElasticsearchDistribution di distro.getBundledJdk()); } + static Map parseOsRelease(final List osReleaseLines) { + final Map values = new HashMap<>(); + + for (String line : osReleaseLines) { + final String trimmed = line.trim(); + + // ignore empty and comment lines + if (trimmed.isEmpty() || trimmed.startsWith("#")) { + continue; + } + + final String[] parts = line.split("=", 2); + final String key = parts[0]; + // remove optional leading and trailing quotes and whitespace + final String value = parts[1].replaceAll("^['\"]?\\s*", "").replaceAll("\\s*['\"]?$", ""); + values.put(key, value); + } + + return values; + } + + static String deriveId(final Map osRelease) { + return osRelease.get("ID") + "-" + osRelease.get("VERSION_ID"); + } + /** * The {@link DistroTestPlugin} generates a number of test tasks, some * of which are Docker packaging tests. When running on the host OS @@ -460,60 +485,47 @@ private static String destructiveDistroTestTaskName(ElasticsearchDistribution di * OS. */ private static boolean shouldRunDockerTests() { - logger.error("[DEBUG] shouldRunDockerTests - OS.current() == " + OS.current()); + logger.error("[BADGER] shouldRunDockerTests - OS.current() == " + OS.current()); switch (OS.current()) { case WINDOWS: // Not currently supported. + logger.error("[BADGER] shouldRunDockerTests = false on " + OS.WINDOWS); return false; case MAC: // Assume that Docker for Mac is installed + logger.error("[BADGER] shouldRunDockerTests = true on " + OS.MAC); return true; case LINUX: final Path osRelease = Paths.get("/etc/os-release"); if (Files.exists(osRelease)) { - Map values = new HashMap<>(); + Map values; try { final List osReleaseLines = Files.readAllLines(osRelease); - logger.error(String.join("\n", osReleaseLines)); - for (String line : osReleaseLines) { - final String trimmed = line.trim(); - - // ignore empty and comment lines - if (trimmed.isEmpty() || trimmed.startsWith("#")) { - continue; - } - - final String[] parts = line.split("=", 2); - final String key = parts[0]; - // remove optional leading and trailing quotes and whitespace - final String value = parts[1] - .replaceAll("^['\"]\\s*", "") - .replaceAll("\\s*['\"]$", ""); - values.put(key, value); - } + values = parseOsRelease(osReleaseLines); } catch (IOException e) { throw new GradleException("Failed to read /etc/os-release", e); } - final String id = values.get("ID") + "-" + values.get("VERSION_ID"); + final String id = deriveId(values); final boolean contains = DOCKER_LINUX_INCLUDE_LIST.contains(id); - logger.error("Linux OS id [" + id + "] is " + (contains ? "" : "not ") + "present in the include list"); + logger.error("[BADGER] Linux OS id [" + id + "] is " + (contains ? "" : "not ") + "present in the include list"); return contains; } else { - logger.error("[DEBUG] /etc/os-release does not exist!"); + logger.error("[BADGER] /etc/os-release does not exist!"); } return false; default: + logger.error("[BADGER] shouldRunDockerTests = false on " + OS.current()); return false; } } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java new file mode 100644 index 0000000000000..dea95e6d6bc51 --- /dev/null +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java @@ -0,0 +1,59 @@ +package org.elasticsearch.gradle.test; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.elasticsearch.gradle.test.DistroTestPlugin.deriveId; +import static org.elasticsearch.gradle.test.DistroTestPlugin.parseOsRelease; +import static org.hamcrest.CoreMatchers.equalTo; + +public class DistroTestPluginTests extends GradleIntegrationTestCase { + + public void testParseOsReleaseOnOracle() { + final List lines = List + .of( + "NAME=\"Oracle Linux Server\"", + "VERSION=\"6.10\"", + "ID=\"ol\"", + "VERSION_ID=\"6.10\"", + "PRETTY_NAME=\"Oracle Linux Server 6.10\"", + "ANSI_COLOR=\"0;31\"", + "CPE_NAME=\"cpe:/o:oracle:linux:6:10:server\"", + "HOME_URL" + "=\"https://linux.oracle.com/\"", + "BUG_REPORT_URL=\"https://bugzilla.oracle.com/\"", + "", + "ORACLE_BUGZILLA_PRODUCT" + "=\"Oracle Linux 6\"", + "ORACLE_BUGZILLA_PRODUCT_VERSION=6.10", + "ORACLE_SUPPORT_PRODUCT=\"Oracle Linux\"", + "ORACLE_SUPPORT_PRODUCT_VERSION=6.10" + ); + + final Map results = parseOsRelease(lines); + + final Map expected = new HashMap<>(); + expected.put("ANSI_COLOR", "0;31"); + expected.put("BUG_REPORT_URL", "https://bugzilla.oracle.com/"); + expected.put("CPE_NAME", "cpe:/o:oracle:linux:6:10:server"); + expected.put("HOME_URL" + "", "https://linux.oracle.com/"); + expected.put("ID", "ol"); + expected.put("NAME", "Oracle Linux Server"); + expected.put("ORACLE_BUGZILLA_PRODUCT" + "", "Oracle Linux 6"); + expected.put("ORACLE_BUGZILLA_PRODUCT_VERSION", "6.10"); + expected.put("ORACLE_SUPPORT_PRODUCT", "Oracle Linux"); + expected.put("ORACLE_SUPPORT_PRODUCT_VERSION", "6.10"); + expected.put("PRETTY_NAME", "Oracle Linux Server 6.10"); + expected.put("VERSION", "6.10"); + expected.put("VERSION_ID", "6.10"); + + assertThat(expected, equalTo(results)); + } + + public void testDeriveIdOnOracle() { + final Map osRelease = new HashMap<>(); + osRelease.put("ID", "ol"); + osRelease.put("VERSION_ID", "6.10"); + + assertThat("ol-6.10", equalTo(deriveId(osRelease))); + } +} From be25db41e5da688a60ed7b8f610785df1756c141 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 24 Oct 2019 16:37:17 +0100 Subject: [PATCH 21/43] Only define docker distros when Docker is available --- .../gradle/test/DistroTestPlugin.java | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index b70b4ebded745..0a609704b7d32 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -119,6 +119,8 @@ public class DistroTestPlugin implements Plugin { private static final String IN_VM_SYSPROP = "tests.inVM"; private static final String DISTRIBUTION_SYSPROP = "tests.distribution"; + private static final boolean dockerTestEnabled = shouldRunDockerTests(); + @Override public void apply(Project project) { project.getPluginManager().apply(DistributionDownloadPlugin.class); @@ -137,10 +139,6 @@ public void apply(Project project) { TaskProvider copyUpgradeTask = configureCopyUpgradeTask(project, upgradeVersion, upgradeDir); TaskProvider copyPluginsTask = configureCopyPluginsTask(project, pluginsDir); - final boolean dockerTestEnabled = shouldRunDockerTests(); - - logger.error("[BADGER] dockerTestEnabled = " + dockerTestEnabled); - TaskProvider destructiveDistroTest = project.getTasks().register("destructiveDistroTest"); for (ElasticsearchDistribution distribution : distributions) { if (distribution.getType() != Type.DOCKER || dockerTestEnabled == true) { @@ -168,8 +166,8 @@ public void apply(Project project) { String destructiveTaskName = destructiveDistroTestTaskName(distribution); if (distribution.getType() == Type.DOCKER && DOCKER_VM_EXCLUDE_LIST.contains(vmProject.getName())) { - logger.error( - "[BADGER] Not generating task [" + destructiveTaskName + "] as [" + vmProject.getName() + "] is in the exclude list" + logger.debug( + "Not generating task [" + destructiveTaskName + "] as [" + vmProject.getName() + "] is in the exclude list" ); continue; } @@ -375,8 +373,17 @@ private List configureDistributions(Project project, for (Type type : List.of(Type.DEB, Type.RPM, Type.DOCKER)) { for (Flavor flavor : Flavor.values()) { for (boolean bundledJdk : Arrays.asList(true, false)) { - // We should never add a Docker distro with bundledJdk == false - boolean skip = type == Type.DOCKER && bundledJdk == false; + boolean skip = false; + + if (type == Type.DOCKER) { + if (dockerTestEnabled == false) { + skip = true; + } else if (bundledJdk == false) { + // We should never add a Docker distro with bundledJdk == false + skip = true; + } + } + if (skip == false) { addDistro(distributions, type, null, flavor, bundledJdk, VersionProperties.getElasticsearch(), currentDistros); } @@ -485,17 +492,15 @@ static String deriveId(final Map osRelease) { * OS. */ private static boolean shouldRunDockerTests() { - logger.error("[BADGER] shouldRunDockerTests - OS.current() == " + OS.current()); - switch (OS.current()) { case WINDOWS: // Not currently supported. - logger.error("[BADGER] shouldRunDockerTests = false on " + OS.WINDOWS); + logger.debug("shouldRunDockerTests = false on " + OS.WINDOWS); return false; case MAC: // Assume that Docker for Mac is installed - logger.error("[BADGER] shouldRunDockerTests = true on " + OS.MAC); + logger.debug("shouldRunDockerTests = true on " + OS.MAC); return true; case LINUX: @@ -515,17 +520,17 @@ private static boolean shouldRunDockerTests() { final boolean contains = DOCKER_LINUX_INCLUDE_LIST.contains(id); - logger.error("[BADGER] Linux OS id [" + id + "] is " + (contains ? "" : "not ") + "present in the include list"); + logger.debug("Linux OS id [" + id + "] is " + (contains ? "" : "not ") + "present in the include list"); return contains; } else { - logger.error("[BADGER] /etc/os-release does not exist!"); + logger.debug("/etc/os-release does not exist!"); } return false; default: - logger.error("[BADGER] shouldRunDockerTests = false on " + OS.current()); + logger.debug("shouldRunDockerTests = false on " + OS.current()); return false; } } From 31fec0b45c13d98561245d7e619a9ee03cefa370 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Sat, 26 Oct 2019 16:47:42 +0100 Subject: [PATCH 22/43] Use host networking when building docker images --- distribution/docker/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/docker/build.gradle b/distribution/docker/build.gradle index 0905bde750d00..267a8072730a7 100644 --- a/distribution/docker/build.gradle +++ b/distribution/docker/build.gradle @@ -162,7 +162,7 @@ void addBuildDockerImage(final boolean oss) { ] } executable 'docker' - final List dockerArgs = ['build', files(oss), '--pull', '--no-cache'] + final List dockerArgs = ['build', files(oss), '--network=host', '--pull', '--no-cache'] for (final String tag : tags) { dockerArgs.add('--tag') dockerArgs.add(tag) From 229301721d662a45d9fbfc8736c1ad5018d7c76b Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Mon, 28 Oct 2019 14:51:17 +0000 Subject: [PATCH 23/43] Revert docker network change --- distribution/docker/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/docker/build.gradle b/distribution/docker/build.gradle index 267a8072730a7..0905bde750d00 100644 --- a/distribution/docker/build.gradle +++ b/distribution/docker/build.gradle @@ -162,7 +162,7 @@ void addBuildDockerImage(final boolean oss) { ] } executable 'docker' - final List dockerArgs = ['build', files(oss), '--network=host', '--pull', '--no-cache'] + final List dockerArgs = ['build', files(oss), '--pull', '--no-cache'] for (final String tag : tags) { dockerArgs.add('--tag') dockerArgs.add(tag) From 8c819fb1f19e68f7c99d066cfe3fc6f7781fc423 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Tue, 29 Oct 2019 09:07:08 +0000 Subject: [PATCH 24/43] Workaround infra issue 15654 --- .ci/os.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.ci/os.sh b/.ci/os.sh index 59a8a9a03a7b9..356796d040de5 100755 --- a/.ci/os.sh +++ b/.ci/os.sh @@ -6,6 +6,14 @@ if which zypper > /dev/null ; then sudo zypper install -y insserv-compat fi +if [ -e /etc/sysctl.d/99-gce.conf ]; then + # The GCE defaults disable IPv4 forwarding, which breaks the Docker + # build. Workaround this by renaming the file so that it is executed + # earlier than our own overrides. This ultimately needs to be fixed at + # the image level - see infra issue 15654. + sudo mv /etc/sysctl.d/99-gce.conf /etc/sysctl.d/98-gce.conf +fi + # Required by bats sudo touch /etc/is_vagrant_vm sudo useradd vagrant From b8d44c77aecf42e2814d1f13ed886bf86a56f0ef Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Tue, 29 Oct 2019 10:10:47 +0000 Subject: [PATCH 25/43] Increase docker pkg test timeouts Slowness in CI can completely ruin a Docker test run, since the current timeouts are a bit tight. --- .../java/org/elasticsearch/packaging/util/Docker.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index d78b60236bc4a..2b9bf1d6c0b2b 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -146,7 +146,7 @@ private static void waitForElasticsearchToStart() throws InterruptedException { } Thread.sleep(1000); - } while (attempt++ < 5); + } while (attempt++ < 20); if (!isElasticsearchRunning) { final String logs = sh.run("docker logs " + containerId).stdout; @@ -240,7 +240,7 @@ public static void assertPermissionsAndOwnership(Path path, Set Date: Tue, 29 Oct 2019 14:55:59 +0000 Subject: [PATCH 26/43] Tiny change to try and get CI to work --- .ci/os.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.ci/os.sh b/.ci/os.sh index 15db4c10b04a5..7cb94ab9fa93f 100755 --- a/.ci/os.sh +++ b/.ci/os.sh @@ -9,8 +9,10 @@ fi if [ -e /etc/sysctl.d/99-gce.conf ]; then # The GCE defaults disable IPv4 forwarding, which breaks the Docker # build. Workaround this by renaming the file so that it is executed - # earlier than our own overrides. This ultimately needs to be fixed at - # the image level - see infra issue 15654. + # earlier than our own overrides. + # + # This ultimately needs to be fixed at the image level - see infra + # issue 15654. sudo mv /etc/sysctl.d/99-gce.conf /etc/sysctl.d/98-gce.conf fi From 0831884d3203f95da51d2d05140fe727295fb57c Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Tue, 29 Oct 2019 16:13:26 +0000 Subject: [PATCH 27/43] Docker test fixes following JVM options work --- .../org/elasticsearch/packaging/util/Docker.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index 2b9bf1d6c0b2b..8e6bc4a46e13c 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -137,20 +137,24 @@ private static void waitForElasticsearchToStart() throws InterruptedException { boolean isElasticsearchRunning = false; int attempt = 0; + String psOutput; + do { - String psOutput = dockerShell.run("ps ax").stdout; + // Give the container a chance to crash out + Thread.sleep(1000); + + psOutput = dockerShell.run("ps ax").stdout; - if (psOutput.contains("/usr/share/elasticsearch/jdk/bin/java -X")) { + if (psOutput.contains("/usr/share/elasticsearch/jdk/bin/java")) { isElasticsearchRunning = true; break; } - Thread.sleep(1000); - } while (attempt++ < 20); + } while (attempt++ < 5); if (!isElasticsearchRunning) { - final String logs = sh.run("docker logs " + containerId).stdout; - fail("Elasticsearch container did start successfully.\n\n" + logs); + final String dockerLogs = sh.run("docker logs " + containerId).stdout; + fail("Elasticsearch container did start successfully.\n\n" + psOutput + "\n\n" + dockerLogs); } } From 0a29d8d31cba8dd9d72fb4c3c867122f3149f229 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 30 Oct 2019 10:44:18 +0000 Subject: [PATCH 28/43] Simplify how Docker test tasks are defined Rather than using some complicate /etc/os-release parsing to determine whether to define docker packaging test tasks, re-use the methods in DockerUtils and just check if Docker can be used. Also, when defining the VM tasks, rely on properties in the qa/os sub-projects to control whether to run Docker tests inside the VMs. --- .../gradle/test/DistroTestPlugin.java | 154 +++--------------- .../gradle/test/DistroTestPluginTests.java | 59 ------- qa/os/centos-7/build.gradle | 1 + qa/os/debian-9/build.gradle | 1 + qa/os/fedora-28/build.gradle | 1 + qa/os/fedora-29/build.gradle | 1 + qa/os/ubuntu-1604/build.gradle | 1 + qa/os/ubuntu-1804/build.gradle | 1 + 8 files changed, 31 insertions(+), 188 deletions(-) delete mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 3a193fb6a8181..5ad03e74e120a 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -28,13 +28,11 @@ import org.elasticsearch.gradle.ElasticsearchDistribution.Type; import org.elasticsearch.gradle.Jdk; import org.elasticsearch.gradle.JdkDownloadPlugin; -import org.elasticsearch.gradle.OS; import org.elasticsearch.gradle.Version; import org.elasticsearch.gradle.VersionProperties; import org.elasticsearch.gradle.vagrant.BatsProgressLogger; import org.elasticsearch.gradle.vagrant.VagrantBasePlugin; import org.elasticsearch.gradle.vagrant.VagrantExtension; -import org.gradle.api.GradleException; import org.gradle.api.NamedDomainObjectContainer; import org.gradle.api.Plugin; import org.gradle.api.Project; @@ -55,7 +53,6 @@ import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -66,44 +63,12 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static org.elasticsearch.gradle.tool.DockerUtils.getDockerAvailability; import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertLinuxPath; import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertWindowsPath; public class DistroTestPlugin implements Plugin { private static final Logger logger = Logging.getLogger(DistroTestPlugin.class); - /** - * This plugin generates test tasks for the various VMs in :qa:os. The - * Docker packaging tests are run on all of them, apart from those - * listed here. These values correspond to the sub-project names, which - * correspond to the directory names. - */ - private static final List DOCKER_VM_EXCLUDE_LIST = List.of( - "centos-6", - "debian-8", - "oel-6", - "oel-7", - "opensuse-42", - "sles-12", - "windows-2012r2", - "windows-2016" - ); - - /** - * This plugin generates a number of test tasks, some of which are - * Docker packaging tests. When running on the host OS i.e. not in a - * VM, only certain operating systems are supported. This list - * specifies the Linux OS variants that are tested. The actual question - * of whether to run the Docker tests is answered by {@link - * #shouldRunDockerTests()}. - */ - private static final List DOCKER_LINUX_INCLUDE_LIST = List.of( - "centos-7", - "debian-9", - "fedora-28", - "fedora-29", - "ubuntu-16.04", - "ubuntu-18.04" - ); private static final String SYSTEM_JDK_VERSION = "11.0.2+9"; private static final String SYSTEM_JDK_VENDOR = "openjdk"; @@ -120,8 +85,6 @@ public class DistroTestPlugin implements Plugin { private static final String IN_VM_SYSPROP = "tests.inVM"; private static final String DISTRIBUTION_SYSPROP = "tests.distribution"; - private static final boolean dockerTestEnabled = shouldRunDockerTests(); - @Override public void apply(Project project) { project.getPluginManager().apply(DistributionDownloadPlugin.class); @@ -140,9 +103,11 @@ public void apply(Project project) { TaskProvider copyUpgradeTask = configureCopyUpgradeTask(project, upgradeVersion, upgradeDir); TaskProvider copyPluginsTask = configureCopyPluginsTask(project, pluginsDir); + final boolean dockerAvailable = isDockerAvailable(); + TaskProvider destructiveDistroTest = project.getTasks().register("destructiveDistroTest"); for (ElasticsearchDistribution distribution : distributions) { - if (distribution.getType() != Type.DOCKER || dockerTestEnabled == true) { + if (distribution.getType() != Type.DOCKER || dockerAvailable == true) { TaskProvider destructiveTask = configureDistroTest(project, distribution); destructiveDistroTest.configure(t -> t.dependsOn(destructiveTask)); } @@ -165,21 +130,24 @@ public void apply(Project project) { TaskProvider distroTest = vmProject.getTasks().register("distroTest"); for (ElasticsearchDistribution distribution : distributions) { String destructiveTaskName = destructiveDistroTestTaskName(distribution); - - if (distribution.getType() == Type.DOCKER && DOCKER_VM_EXCLUDE_LIST.contains(vmProject.getName())) { - logger.debug( - "Not generating task [" + destructiveTaskName + "] as [" + vmProject.getName() + "] is in the exclude list" - ); - continue; - } - Platform platform = distribution.getPlatform(); // this condition ensures windows boxes get windows distributions, and linux boxes get linux distributions if (isWindows(vmProject) == (platform == Platform.WINDOWS)) { TaskProvider vmTask = configureVMWrapperTask(vmProject, distribution.getName() + " distribution", destructiveTaskName, vmDependencies); vmTask.configure(t -> t.dependsOn(distribution)); - distroTest.configure(t -> t.dependsOn(vmTask)); + + distroTest.configure(t -> { + // Only VM sub-projects that are specifically opted-in to testing Docker should + // have the Docker task added as a dependency. The shouldTestDocker property could + // be null, hence we use Boolean.TRUE.equals() + boolean shouldExecute = distribution.getType() != Type.DOCKER + || Boolean.TRUE.equals(vmProject.findProperty("shouldTestDocker")) == true; + + if (shouldExecute) { + t.dependsOn(vmTask); + } + }); } } @@ -380,16 +348,8 @@ private List configureDistributions(Project project, for (Type type : List.of(Type.DEB, Type.RPM, Type.DOCKER)) { for (Flavor flavor : Flavor.values()) { for (boolean bundledJdk : Arrays.asList(true, false)) { - boolean skip = false; - - if (type == Type.DOCKER) { - if (dockerTestEnabled == false) { - skip = true; - } else if (bundledJdk == false) { - // We should never add a Docker distro with bundledJdk == false - skip = true; - } - } + // All our Docker images include a bundled JDK so it doesn't make sense to test without one + boolean skip = type == Type.DOCKER && bundledJdk == false; if (skip == false) { addDistro(distributions, type, null, flavor, bundledJdk, VersionProperties.getElasticsearch(), currentDistros); @@ -466,79 +426,15 @@ private static String destructiveDistroTestTaskName(ElasticsearchDistribution di distro.getBundledJdk()); } - static Map parseOsRelease(final List osReleaseLines) { - final Map values = new HashMap<>(); - - for (String line : osReleaseLines) { - final String trimmed = line.trim(); - - // ignore empty and comment lines - if (trimmed.isEmpty() || trimmed.startsWith("#")) { - continue; - } - - final String[] parts = line.split("=", 2); - final String key = parts[0]; - // remove optional leading and trailing quotes and whitespace - final String value = parts[1].replaceAll("^['\"]?\\s*", "").replaceAll("\\s*['\"]?$", ""); - values.put(key, value); - } - - return values; - } - - static String deriveId(final Map osRelease) { - return osRelease.get("ID") + "-" + osRelease.get("VERSION_ID"); - } - /** - * The {@link DistroTestPlugin} generates a number of test tasks, some - * of which are Docker packaging tests. When running on the host OS - * i.e. not in a VM, only certain operating systems are supported. This - * method determines whether the Docker tests should be run on the host - * OS. + * Determines whether Docker is available, for the purpose of defining packaging test tasks. */ - private static boolean shouldRunDockerTests() { - switch (OS.current()) { - case WINDOWS: - // Not currently supported. - logger.debug("shouldRunDockerTests = false on " + OS.WINDOWS); - return false; - - case MAC: - // Assume that Docker for Mac is installed - logger.debug("shouldRunDockerTests = true on " + OS.MAC); - return true; - - case LINUX: - final Path osRelease = Paths.get("/etc/os-release"); - - if (Files.exists(osRelease)) { - Map values; - - try { - final List osReleaseLines = Files.readAllLines(osRelease); - values = parseOsRelease(osReleaseLines); - } catch (IOException e) { - throw new GradleException("Failed to read /etc/os-release", e); - } - - final String id = deriveId(values); - - final boolean contains = DOCKER_LINUX_INCLUDE_LIST.contains(id); - - logger.debug("Linux OS id [" + id + "] is " + (contains ? "" : "not ") + "present in the include list"); - - return contains; - } else { - logger.debug("/etc/os-release does not exist!"); - } - - return false; - - default: - logger.debug("shouldRunDockerTests = false on " + OS.current()); - return false; + private static boolean isDockerAvailable() { + try { + return getDockerAvailability().isAvailable; + } catch (Exception e) { + logger.warn("Failed to determine whether Docker is available while configuring distro test tasks", e); + return false; } } } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java deleted file mode 100644 index dea95e6d6bc51..0000000000000 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java +++ /dev/null @@ -1,59 +0,0 @@ -package org.elasticsearch.gradle.test; - -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import static org.elasticsearch.gradle.test.DistroTestPlugin.deriveId; -import static org.elasticsearch.gradle.test.DistroTestPlugin.parseOsRelease; -import static org.hamcrest.CoreMatchers.equalTo; - -public class DistroTestPluginTests extends GradleIntegrationTestCase { - - public void testParseOsReleaseOnOracle() { - final List lines = List - .of( - "NAME=\"Oracle Linux Server\"", - "VERSION=\"6.10\"", - "ID=\"ol\"", - "VERSION_ID=\"6.10\"", - "PRETTY_NAME=\"Oracle Linux Server 6.10\"", - "ANSI_COLOR=\"0;31\"", - "CPE_NAME=\"cpe:/o:oracle:linux:6:10:server\"", - "HOME_URL" + "=\"https://linux.oracle.com/\"", - "BUG_REPORT_URL=\"https://bugzilla.oracle.com/\"", - "", - "ORACLE_BUGZILLA_PRODUCT" + "=\"Oracle Linux 6\"", - "ORACLE_BUGZILLA_PRODUCT_VERSION=6.10", - "ORACLE_SUPPORT_PRODUCT=\"Oracle Linux\"", - "ORACLE_SUPPORT_PRODUCT_VERSION=6.10" - ); - - final Map results = parseOsRelease(lines); - - final Map expected = new HashMap<>(); - expected.put("ANSI_COLOR", "0;31"); - expected.put("BUG_REPORT_URL", "https://bugzilla.oracle.com/"); - expected.put("CPE_NAME", "cpe:/o:oracle:linux:6:10:server"); - expected.put("HOME_URL" + "", "https://linux.oracle.com/"); - expected.put("ID", "ol"); - expected.put("NAME", "Oracle Linux Server"); - expected.put("ORACLE_BUGZILLA_PRODUCT" + "", "Oracle Linux 6"); - expected.put("ORACLE_BUGZILLA_PRODUCT_VERSION", "6.10"); - expected.put("ORACLE_SUPPORT_PRODUCT", "Oracle Linux"); - expected.put("ORACLE_SUPPORT_PRODUCT_VERSION", "6.10"); - expected.put("PRETTY_NAME", "Oracle Linux Server 6.10"); - expected.put("VERSION", "6.10"); - expected.put("VERSION_ID", "6.10"); - - assertThat(expected, equalTo(results)); - } - - public void testDeriveIdOnOracle() { - final Map osRelease = new HashMap<>(); - osRelease.put("ID", "ol"); - osRelease.put("VERSION_ID", "6.10"); - - assertThat("ol-6.10", equalTo(deriveId(osRelease))); - } -} diff --git a/qa/os/centos-7/build.gradle b/qa/os/centos-7/build.gradle index e69de29bb2d1d..814b04d4aec5f 100644 --- a/qa/os/centos-7/build.gradle +++ b/qa/os/centos-7/build.gradle @@ -0,0 +1 @@ +project.ext.shouldTestDocker = true diff --git a/qa/os/debian-9/build.gradle b/qa/os/debian-9/build.gradle index e69de29bb2d1d..814b04d4aec5f 100644 --- a/qa/os/debian-9/build.gradle +++ b/qa/os/debian-9/build.gradle @@ -0,0 +1 @@ +project.ext.shouldTestDocker = true diff --git a/qa/os/fedora-28/build.gradle b/qa/os/fedora-28/build.gradle index e69de29bb2d1d..814b04d4aec5f 100644 --- a/qa/os/fedora-28/build.gradle +++ b/qa/os/fedora-28/build.gradle @@ -0,0 +1 @@ +project.ext.shouldTestDocker = true diff --git a/qa/os/fedora-29/build.gradle b/qa/os/fedora-29/build.gradle index e69de29bb2d1d..814b04d4aec5f 100644 --- a/qa/os/fedora-29/build.gradle +++ b/qa/os/fedora-29/build.gradle @@ -0,0 +1 @@ +project.ext.shouldTestDocker = true diff --git a/qa/os/ubuntu-1604/build.gradle b/qa/os/ubuntu-1604/build.gradle index e69de29bb2d1d..814b04d4aec5f 100644 --- a/qa/os/ubuntu-1604/build.gradle +++ b/qa/os/ubuntu-1604/build.gradle @@ -0,0 +1 @@ +project.ext.shouldTestDocker = true diff --git a/qa/os/ubuntu-1804/build.gradle b/qa/os/ubuntu-1804/build.gradle index e69de29bb2d1d..814b04d4aec5f 100644 --- a/qa/os/ubuntu-1804/build.gradle +++ b/qa/os/ubuntu-1804/build.gradle @@ -0,0 +1 @@ +project.ext.shouldTestDocker = true From 048832a5533b6eb70e67bec82e7add1354ac40f4 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 30 Oct 2019 16:24:07 +0000 Subject: [PATCH 29/43] Reimplement Docker tests exclude list Relying on whether Docker is available on a host to drive running the Docker tests is not sufficient, because we expect and requrie Docker to be available on some OS versions. Revert to maintaining a list, and externalise it to `.ci/dockerOnLinuxExclusions` in the repository root. --- .ci/dockerOnLinuxExclusions | 14 +++ .../gradle/test/DistroTestPlugin.java | 105 ++++++++++++++++-- .../gradle/test/DistroTestPluginTests.java | 59 ++++++++++ 3 files changed, 168 insertions(+), 10 deletions(-) create mode 100644 .ci/dockerOnLinuxExclusions create mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java diff --git a/.ci/dockerOnLinuxExclusions b/.ci/dockerOnLinuxExclusions new file mode 100644 index 0000000000000..01585db789d1b --- /dev/null +++ b/.ci/dockerOnLinuxExclusions @@ -0,0 +1,14 @@ +# This file specifies the Linux OS versions on which we can't build and +# test Docker images for some reason. These values correspond to ID and +# VERSION_ID from /etc/os-release, and a matching value will cause the +# Docker tests to be skipped on that OS. If /etc/os-release doesn't exist +# (as is the case on centos-6, for example) then that OS will again be +# excluded. + +centos-6 +debian-8 +opensuse-15-1 +oraclelinux-6 +oraclelinux-7 +sles-12 +sles-15 diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 5ad03e74e120a..5f74fcddf066f 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -28,11 +28,13 @@ import org.elasticsearch.gradle.ElasticsearchDistribution.Type; import org.elasticsearch.gradle.Jdk; import org.elasticsearch.gradle.JdkDownloadPlugin; +import org.elasticsearch.gradle.OS; import org.elasticsearch.gradle.Version; import org.elasticsearch.gradle.VersionProperties; import org.elasticsearch.gradle.vagrant.BatsProgressLogger; import org.elasticsearch.gradle.vagrant.VagrantBasePlugin; import org.elasticsearch.gradle.vagrant.VagrantExtension; +import org.gradle.api.GradleException; import org.gradle.api.NamedDomainObjectContainer; import org.gradle.api.Plugin; import org.gradle.api.Project; @@ -53,6 +55,7 @@ import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -63,7 +66,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import static org.elasticsearch.gradle.tool.DockerUtils.getDockerAvailability; import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertLinuxPath; import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertWindowsPath; @@ -103,11 +105,11 @@ public void apply(Project project) { TaskProvider copyUpgradeTask = configureCopyUpgradeTask(project, upgradeVersion, upgradeDir); TaskProvider copyPluginsTask = configureCopyPluginsTask(project, pluginsDir); - final boolean dockerAvailable = isDockerAvailable(); + final boolean runDockerTests = shouldRunDockerTests(project); TaskProvider destructiveDistroTest = project.getTasks().register("destructiveDistroTest"); for (ElasticsearchDistribution distribution : distributions) { - if (distribution.getType() != Type.DOCKER || dockerAvailable == true) { + if (distribution.getType() != Type.DOCKER || runDockerTests == true) { TaskProvider destructiveTask = configureDistroTest(project, distribution); destructiveDistroTest.configure(t -> t.dependsOn(destructiveTask)); } @@ -426,15 +428,98 @@ private static String destructiveDistroTestTaskName(ElasticsearchDistribution di distro.getBundledJdk()); } + static Map parseOsRelease(final List osReleaseLines) { + final Map values = new HashMap<>(); + + for (String line : osReleaseLines) { + final String trimmed = line.trim(); + + // ignore empty and comment lines + if (trimmed.isEmpty() || trimmed.startsWith("#")) { + continue; + } + + final String[] parts = line.split("=", 2); + final String key = parts[0]; + // remove optional leading and trailing quotes and whitespace + final String value = parts[1].replaceAll("^['\"]?\\s*", "").replaceAll("\\s*['\"]?$", ""); + values.put(key, value); + } + + return values; + } + + static String deriveId(final Map osRelease) { + return osRelease.get("ID") + "-" + osRelease.get("VERSION_ID"); + } + + private static List getLinuxExclusionList(Project project) { + final String exclusionsFilename = "dockerOnLinuxExclusions"; + final Path exclusionsPath = project.getRootDir().toPath().resolve(Path.of(".ci", exclusionsFilename)); + + try { + return Files.readAllLines(exclusionsPath) + .stream() + .map(String::trim) + .filter(line -> line.startsWith("#")) + .collect(Collectors.toList()); + } catch (IOException e) { + throw new GradleException("Failed to read .ci/" + exclusionsFilename, e); + } + } + /** - * Determines whether Docker is available, for the purpose of defining packaging test tasks. + * The {@link DistroTestPlugin} generates a number of test tasks, some + * of which are Docker packaging tests. When running on the host OS or in CI + * i.e. not in a Vagrant VM, only certain operating systems are supported. This + * method determines whether the Docker tests should be run on the host + * OS. Essentially, unless an OS and version is specifically excluded, we expect + * to be able to run Docker and test the Docker images. + * @param project */ - private static boolean isDockerAvailable() { - try { - return getDockerAvailability().isAvailable; - } catch (Exception e) { - logger.warn("Failed to determine whether Docker is available while configuring distro test tasks", e); - return false; + private static boolean shouldRunDockerTests(Project project) { + switch (OS.current()) { + case WINDOWS: + // Not yet supported. + return false; + + case MAC: + // Assume that Docker for Mac is installed, since Docker is part of the dev workflow. + return true; + + case LINUX: + // Only some hosts in CI are configured with Docker. We attempt to work out the OS + // and version, so that we know whether to expect to find Docker. We don't attempt + // to probe for whether Docker is available, because that doesn't tell us whether + // Docker is unavailable when it should be. + final Path osRelease = Paths.get("/etc/os-release"); + + if (Files.exists(osRelease)) { + Map values; + + try { + final List osReleaseLines = Files.readAllLines(osRelease); + values = parseOsRelease(osReleaseLines); + } catch (IOException e) { + throw new GradleException("Failed to read /etc/os-release", e); + } + + final String id = deriveId(values); + + final boolean shouldExclude = getLinuxExclusionList(project).contains(id); + + logger.info("Linux OS id [" + id + "] is " + (shouldExclude ? "" : "not ") + "present in the exclude list"); + + return shouldExclude == false; + } else { + logger.warn("/etc/os-release does not exist!"); + } + + return false; + + default: + logger.warn("Unknown OS [" + OS.current() + "], answering false to shouldRunDockerTests()"); + return false; } } } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java new file mode 100644 index 0000000000000..dea95e6d6bc51 --- /dev/null +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java @@ -0,0 +1,59 @@ +package org.elasticsearch.gradle.test; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.elasticsearch.gradle.test.DistroTestPlugin.deriveId; +import static org.elasticsearch.gradle.test.DistroTestPlugin.parseOsRelease; +import static org.hamcrest.CoreMatchers.equalTo; + +public class DistroTestPluginTests extends GradleIntegrationTestCase { + + public void testParseOsReleaseOnOracle() { + final List lines = List + .of( + "NAME=\"Oracle Linux Server\"", + "VERSION=\"6.10\"", + "ID=\"ol\"", + "VERSION_ID=\"6.10\"", + "PRETTY_NAME=\"Oracle Linux Server 6.10\"", + "ANSI_COLOR=\"0;31\"", + "CPE_NAME=\"cpe:/o:oracle:linux:6:10:server\"", + "HOME_URL" + "=\"https://linux.oracle.com/\"", + "BUG_REPORT_URL=\"https://bugzilla.oracle.com/\"", + "", + "ORACLE_BUGZILLA_PRODUCT" + "=\"Oracle Linux 6\"", + "ORACLE_BUGZILLA_PRODUCT_VERSION=6.10", + "ORACLE_SUPPORT_PRODUCT=\"Oracle Linux\"", + "ORACLE_SUPPORT_PRODUCT_VERSION=6.10" + ); + + final Map results = parseOsRelease(lines); + + final Map expected = new HashMap<>(); + expected.put("ANSI_COLOR", "0;31"); + expected.put("BUG_REPORT_URL", "https://bugzilla.oracle.com/"); + expected.put("CPE_NAME", "cpe:/o:oracle:linux:6:10:server"); + expected.put("HOME_URL" + "", "https://linux.oracle.com/"); + expected.put("ID", "ol"); + expected.put("NAME", "Oracle Linux Server"); + expected.put("ORACLE_BUGZILLA_PRODUCT" + "", "Oracle Linux 6"); + expected.put("ORACLE_BUGZILLA_PRODUCT_VERSION", "6.10"); + expected.put("ORACLE_SUPPORT_PRODUCT", "Oracle Linux"); + expected.put("ORACLE_SUPPORT_PRODUCT_VERSION", "6.10"); + expected.put("PRETTY_NAME", "Oracle Linux Server 6.10"); + expected.put("VERSION", "6.10"); + expected.put("VERSION_ID", "6.10"); + + assertThat(expected, equalTo(results)); + } + + public void testDeriveIdOnOracle() { + final Map osRelease = new HashMap<>(); + osRelease.put("ID", "ol"); + osRelease.put("VERSION_ID", "6.10"); + + assertThat("ol-6.10", equalTo(deriveId(osRelease))); + } +} From 99ded1ce65ce7855cf611902e2a64751bb18b4d6 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Wed, 30 Oct 2019 16:47:53 +0000 Subject: [PATCH 30/43] Remove sles-15 from .ci/dockerOnLinuxExclusions --- .ci/dockerOnLinuxExclusions | 1 - 1 file changed, 1 deletion(-) diff --git a/.ci/dockerOnLinuxExclusions b/.ci/dockerOnLinuxExclusions index 01585db789d1b..dfb4096ae28ae 100644 --- a/.ci/dockerOnLinuxExclusions +++ b/.ci/dockerOnLinuxExclusions @@ -11,4 +11,3 @@ opensuse-15-1 oraclelinux-6 oraclelinux-7 sles-12 -sles-15 From 12bd9fb360eb9b830ad8c42df90612a76db450dc Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 31 Oct 2019 11:40:07 +0000 Subject: [PATCH 31/43] Fix access modifiers in DockerUtils --- .../gradle/tool/DockerUtils.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java index e276376c5f8fc..ea4e94333d976 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java @@ -39,7 +39,7 @@ public static Optional getDockerPath() { * Searches for a functional Docker installation, and returns information about the search. * @return the results of the search. */ - public static DockerAvailability getDockerAvailability() throws Exception { + private static DockerAvailability getDockerAvailability() throws Exception { String dockerPath = null; Result lastResult = null; Version version = null; @@ -74,7 +74,7 @@ public static DockerAvailability getDockerAvailability() throws Exception { /** * An immutable class that represents the results of a Docker search from {@link #getDockerAvailability()}}. */ - public static class DockerAvailability { + private static class DockerAvailability { /** * Indicates whether Docker is available and meets the required criteria. * True if, and only if, Docker is: @@ -85,12 +85,12 @@ public static class DockerAvailability { *
  • Can execute a command that requires privileges
  • * */ - public final boolean isAvailable; + final boolean isAvailable; /** * True if the installed Docker version is >= 17.05 */ - public final boolean isVersionHighEnough; + final boolean isVersionHighEnough; /** * The path to the Docker CLI, or null @@ -105,9 +105,9 @@ public static class DockerAvailability { /** * Information about the last command executes while probing Docker, or null. */ - public final Result lastCommand; + final Result lastCommand; - public DockerAvailability(boolean isAvailable, boolean isVersionHighEnough, String path, Version version, Result lastCommand) { + DockerAvailability(boolean isAvailable, boolean isVersionHighEnough, String path, Version version, Result lastCommand) { this.isAvailable = isAvailable; this.isVersionHighEnough = isVersionHighEnough; this.path = path; @@ -221,9 +221,9 @@ private static Result runCommand(String... args) throws Exception { * This class models the result of running a command. It captures the exit code, standard output and standard error. */ private static class Result { - public final int exitCode; - public final String stdout; - public final String stderr; + final int exitCode; + final String stdout; + final String stderr; Result(int exitCode, String stdout, String stderr) { this.exitCode = exitCode; @@ -231,7 +231,7 @@ private static class Result { this.stderr = stderr; } - public boolean isSuccess() { + boolean isSuccess() { return exitCode == 0; } From 97fca15d72ff84be56b117758521a20332202031 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 31 Oct 2019 14:26:40 +0000 Subject: [PATCH 32/43] Use project.exec to run commands in DockerUtils --- .../elasticsearch/gradle/BuildPlugin.groovy | 2 +- .../gradle/tool/DockerUtils.java | 44 +++++++++---------- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index ba08d65418a08..cde0f3326f1c6 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -208,7 +208,7 @@ class BuildPlugin implements Plugin { final List tasks = taskGraph.allTasks.intersect(ext.get('requiresDocker') as List).collect { " ${it.path}".toString()} if (tasks.isEmpty() == false) { - assertDockerIsAvailable(tasks) + assertDockerIsAvailable(task.project, tasks) } } } diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java index ea4e94333d976..03fccf4e76313 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java @@ -1,9 +1,11 @@ package org.elasticsearch.gradle.tool; -import org.apache.commons.io.IOUtils; import org.elasticsearch.gradle.Version; import org.gradle.api.GradleException; +import org.gradle.api.Project; +import org.gradle.process.ExecResult; +import java.io.ByteArrayOutputStream; import java.io.File; import java.util.List; import java.util.Locale; @@ -39,7 +41,7 @@ public static Optional getDockerPath() { * Searches for a functional Docker installation, and returns information about the search. * @return the results of the search. */ - private static DockerAvailability getDockerAvailability() throws Exception { + private static DockerAvailability getDockerAvailability(Project project) { String dockerPath = null; Result lastResult = null; Version version = null; @@ -52,7 +54,7 @@ private static DockerAvailability getDockerAvailability() throws Exception { dockerPath = dockerBinary.get(); // Since we use a multi-stage Docker build, check the Docker version since 17.05 - lastResult = runCommand(dockerPath, "version", "--format", "{{.Server.Version}}"); + lastResult = runCommand(project, dockerPath, "version", "--format", "{{.Server.Version}}"); if (lastResult.isSuccess() == true) { version = Version.fromString(lastResult.stdout.trim(), Version.Mode.RELAXED); @@ -61,7 +63,7 @@ private static DockerAvailability getDockerAvailability() throws Exception { if (isVersionHighEnough == true) { // Check that we can execute a privileged command - lastResult = runCommand(dockerPath, "images"); + lastResult = runCommand(project, dockerPath, "images"); } } } @@ -72,7 +74,7 @@ private static DockerAvailability getDockerAvailability() throws Exception { } /** - * An immutable class that represents the results of a Docker search from {@link #getDockerAvailability()}}. + * An immutable class that represents the results of a Docker search from {@link #getDockerAvailability(Project)}}. */ private static class DockerAvailability { /** @@ -119,17 +121,12 @@ private static class DockerAvailability { /** * Given a list of tasks that requires Docker, check whether Docker is available, otherwise * throw an exception. + * @param project a Gradle project * @param tasks the tasks that require Docker * @throws GradleException if Docker is not available. The exception message gives the reason. */ - public static void assertDockerIsAvailable(List tasks) { - DockerAvailability availability = null; - - try { - availability = getDockerAvailability(); - } catch (Exception e) { - throwDockerRequiredException("Something went wrong while checking Docker's availability", e); - } + public static void assertDockerIsAvailable(Project project, List tasks) { + DockerAvailability availability = getDockerAvailability(project); if (availability.isAvailable == true) { return; @@ -199,22 +196,23 @@ private static void throwDockerRequiredException(final String message, Exception * while running the command, or the process was killed after reaching the 10s timeout, * then the exit code will be -1. */ - private static Result runCommand(String... args) throws Exception { + private static Result runCommand(Project project, String... args) { if (args.length == 0) { throw new IllegalArgumentException("Cannot execute with no command"); } - final ProcessBuilder command = new ProcessBuilder().command(args); - final Process process = command.start(); - - process.waitFor(); - - String stdout = IOUtils.toString(process.getInputStream()); - String stderr = IOUtils.toString(process.getErrorStream()); + ByteArrayOutputStream stdout = new ByteArrayOutputStream(); + ByteArrayOutputStream stderr = new ByteArrayOutputStream(); - process.destroy(); + final ExecResult execResult = project.exec(spec -> { + // javac advises casting to Object to specify the varargs method, + // and silences a warning. + spec.setCommandLine((Object) args); + spec.setStandardOutput(stdout); + spec.setErrorOutput(stderr); + }); - return new Result(process.exitValue(), stdout, stderr); + return new Result(execResult.getExitValue(), stdout.toString(), stderr.toString()); } /** From 617f49453fc5f43dda520acfd46a9613098d1fe2 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 31 Oct 2019 14:38:50 +0000 Subject: [PATCH 33/43] Fix --- .../main/java/org/elasticsearch/gradle/tool/DockerUtils.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java index 03fccf4e76313..dcc1c6845a225 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java @@ -205,9 +205,8 @@ private static Result runCommand(Project project, String... args) { ByteArrayOutputStream stderr = new ByteArrayOutputStream(); final ExecResult execResult = project.exec(spec -> { - // javac advises casting to Object to specify the varargs method, - // and silences a warning. - spec.setCommandLine((Object) args); + // The redundant cast is to silence a compiler warning. + spec.setCommandLine((Object[]) args); spec.setStandardOutput(stdout); spec.setErrorOutput(stderr); }); From ffcf939dda00100ce588eb09e987fc2ee002f461 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 31 Oct 2019 14:51:11 +0000 Subject: [PATCH 34/43] Fix JavaDoc ref --- .../main/java/org/elasticsearch/gradle/tool/DockerUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java index dcc1c6845a225..2442ffce427c8 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/tool/DockerUtils.java @@ -24,7 +24,7 @@ public class DockerUtils { /** * Searches the entries in {@link #DOCKER_BINARIES} for the Docker CLI. This method does - * not check whether the Docker installation appears usable, see {@link #getDockerAvailability()} + * not check whether the Docker installation appears usable, see {@link #getDockerAvailability(Project)} * instead. * * @return the path to a CLI, if available. From 0b49a7a053f0c3075f1774438758cc277f222291 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 31 Oct 2019 16:06:53 +0000 Subject: [PATCH 35/43] Don't always create a docker distribution Restore the logic that avoided creating a Docker distribution in DistroTestPlugin, because that has the effect of building the Docker image even when we don't want to test it. --- .../elasticsearch/gradle/test/DistroTestPlugin.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 5f74fcddf066f..609a8155f1e5d 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -89,6 +89,8 @@ public class DistroTestPlugin implements Plugin { @Override public void apply(Project project) { + final boolean runDockerTests = shouldRunDockerTests(project); + project.getPluginManager().apply(DistributionDownloadPlugin.class); project.getPluginManager().apply(BuildPlugin.class); @@ -100,13 +102,11 @@ public void apply(Project project) { Provider upgradeDir = project.getLayout().getBuildDirectory().dir("packaging/upgrade"); Provider pluginsDir = project.getLayout().getBuildDirectory().dir("packaging/plugins"); - List distributions = configureDistributions(project, upgradeVersion); + List distributions = configureDistributions(project, upgradeVersion, runDockerTests); TaskProvider copyDistributionsTask = configureCopyDistributionsTask(project, distributionsDir); TaskProvider copyUpgradeTask = configureCopyUpgradeTask(project, upgradeVersion, upgradeDir); TaskProvider copyPluginsTask = configureCopyPluginsTask(project, pluginsDir); - final boolean runDockerTests = shouldRunDockerTests(project); - TaskProvider destructiveDistroTest = project.getTasks().register("destructiveDistroTest"); for (ElasticsearchDistribution distribution : distributions) { if (distribution.getType() != Type.DOCKER || runDockerTests == true) { @@ -342,7 +342,7 @@ private static TaskProvider configureBatsTest(Project project, Str }); } - private List configureDistributions(Project project, Version upgradeVersion) { + private List configureDistributions(Project project, Version upgradeVersion, boolean runDockerTests) { NamedDomainObjectContainer distributions = DistributionDownloadPlugin.getContainer(project); List currentDistros = new ArrayList<>(); List upgradeDistros = new ArrayList<>(); @@ -351,7 +351,7 @@ private List configureDistributions(Project project, for (Flavor flavor : Flavor.values()) { for (boolean bundledJdk : Arrays.asList(true, false)) { // All our Docker images include a bundled JDK so it doesn't make sense to test without one - boolean skip = type == Type.DOCKER && bundledJdk == false; + boolean skip = type == Type.DOCKER && runDockerTests == false && bundledJdk == false; if (skip == false) { addDistro(distributions, type, null, flavor, bundledJdk, VersionProperties.getElasticsearch(), currentDistros); @@ -511,10 +511,9 @@ private static boolean shouldRunDockerTests(Project project) { logger.info("Linux OS id [" + id + "] is " + (shouldExclude ? "" : "not ") + "present in the exclude list"); return shouldExclude == false; - } else { - logger.warn("/etc/os-release does not exist!"); } + logger.warn("/etc/os-release does not exist!"); return false; default: From 611b914e842b5c77693e5bea199df4b1420cf71d Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 31 Oct 2019 16:14:40 +0000 Subject: [PATCH 36/43] Fix logic --- .../groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 609a8155f1e5d..0f378204267ed 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -351,7 +351,7 @@ private List configureDistributions(Project project, for (Flavor flavor : Flavor.values()) { for (boolean bundledJdk : Arrays.asList(true, false)) { // All our Docker images include a bundled JDK so it doesn't make sense to test without one - boolean skip = type == Type.DOCKER && runDockerTests == false && bundledJdk == false; + boolean skip = type == Type.DOCKER && (runDockerTests == false || bundledJdk == false); if (skip == false) { addDistro(distributions, type, null, flavor, bundledJdk, VersionProperties.getElasticsearch(), currentDistros); From aac11a470103e1601ce9455e91505983da85ad40 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 31 Oct 2019 22:02:22 +0000 Subject: [PATCH 37/43] Update .ci/dockerOnLinuxExclusions --- .ci/dockerOnLinuxExclusions | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.ci/dockerOnLinuxExclusions b/.ci/dockerOnLinuxExclusions index dfb4096ae28ae..1ee1058e187ee 100644 --- a/.ci/dockerOnLinuxExclusions +++ b/.ci/dockerOnLinuxExclusions @@ -8,6 +8,6 @@ centos-6 debian-8 opensuse-15-1 -oraclelinux-6 -oraclelinux-7 +ol-6.10 +ol-7.7 sles-12 From e0010a6b286174e28fc17a46a52533d7c367abaf Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Mon, 4 Nov 2019 09:00:34 +0000 Subject: [PATCH 38/43] Expand comment re: configuring Docker tasks for Vagrant --- .../org/elasticsearch/gradle/test/DistroTestPlugin.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 0f378204267ed..823c94dd0e0bc 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -141,8 +141,13 @@ public void apply(Project project) { distroTest.configure(t -> { // Only VM sub-projects that are specifically opted-in to testing Docker should - // have the Docker task added as a dependency. The shouldTestDocker property could - // be null, hence we use Boolean.TRUE.equals() + // have the Docker task added as a dependency. Although we control whether Docker + // is installed in the VM via `Vagrantfile` and we could auto-detect its presence + // in the VM, the test tasks e.g. `destructiveDistroTest.default-docker` are defined + // on the host during Gradle's configuration phase and not in the VM, so + // auto-detection doesn't work. + // + // The shouldTestDocker property could be null, hence we use Boolean.TRUE.equals() boolean shouldExecute = distribution.getType() != Type.DOCKER || Boolean.TRUE.equals(vmProject.findProperty("shouldTestDocker")) == true; From 63a758d86d310876fc92760e2512bacd70200ed3 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Mon, 4 Nov 2019 12:37:28 +0000 Subject: [PATCH 39/43] Tweak logging --- .../groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 87ec3d0150a14..d950c90178e95 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -513,7 +513,7 @@ private static boolean shouldRunDockerTests(Project project) { final boolean shouldExclude = getLinuxExclusionList(project).contains(id); - logger.info("Linux OS id [" + id + "] is " + (shouldExclude ? "" : "not ") + "present in the exclude list"); + logger.warn("Linux OS id [" + id + "] is " + (shouldExclude ? "" : "not ") + "present in the Docker exclude list"); return shouldExclude == false; } From 3e8d10d1b764313ce2d7a957d682b6869a2d2ca0 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Mon, 4 Nov 2019 14:20:39 +0000 Subject: [PATCH 40/43] Add debugging statements --- .../org/elasticsearch/gradle/test/DistroTestPlugin.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index d950c90178e95..3664805dab3ac 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -455,6 +455,7 @@ static Map parseOsRelease(final List osReleaseLines) { } static String deriveId(final Map osRelease) { + logger.warn(osRelease.toString()); return osRelease.get("ID") + "-" + osRelease.get("VERSION_ID"); } @@ -463,11 +464,15 @@ private static List getLinuxExclusionList(Project project) { final Path exclusionsPath = project.getRootDir().toPath().resolve(Path.of(".ci", exclusionsFilename)); try { - return Files.readAllLines(exclusionsPath) + final List exclusionsList = Files.readAllLines(exclusionsPath) .stream() .map(String::trim) - .filter(line -> line.startsWith("#")) + .filter(line -> line.isEmpty() || line.startsWith("#")) .collect(Collectors.toList()); + + logger.warn("Exclusion list: " + exclusionsList); + + return exclusionsList; } catch (IOException e) { throw new GradleException("Failed to read .ci/" + exclusionsFilename, e); } From cedacee73edb428cf3d9f81e9b73baaf9408f44a Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Mon, 4 Nov 2019 14:37:19 +0000 Subject: [PATCH 41/43] Pointless change to get CI to run the right code --- .ci/dockerOnLinuxExclusions | 1 + 1 file changed, 1 insertion(+) diff --git a/.ci/dockerOnLinuxExclusions b/.ci/dockerOnLinuxExclusions index 1ee1058e187ee..1b15128f86d92 100644 --- a/.ci/dockerOnLinuxExclusions +++ b/.ci/dockerOnLinuxExclusions @@ -11,3 +11,4 @@ opensuse-15-1 ol-6.10 ol-7.7 sles-12 + From ef924fcdd8e9e177f4196ef26c1d120c99d5c16b Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Mon, 4 Nov 2019 15:09:37 +0000 Subject: [PATCH 42/43] Various fixes to docker packaging test logic --- .../gradle/test/DistroTestPlugin.java | 21 ++++----------- .../gradle/test/DistroTestPluginTests.java | 26 +++++++++++++++++++ 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java index 3664805dab3ac..417e13400c114 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java @@ -436,26 +436,19 @@ private static String destructiveDistroTestTaskName(ElasticsearchDistribution di static Map parseOsRelease(final List osReleaseLines) { final Map values = new HashMap<>(); - for (String line : osReleaseLines) { - final String trimmed = line.trim(); - - // ignore empty and comment lines - if (trimmed.isEmpty() || trimmed.startsWith("#")) { - continue; - } - + osReleaseLines.stream().map(String::trim).filter(line -> (line.isEmpty() || line.startsWith("#")) == false).forEach(line -> { final String[] parts = line.split("=", 2); final String key = parts[0]; // remove optional leading and trailing quotes and whitespace final String value = parts[1].replaceAll("^['\"]?\\s*", "").replaceAll("\\s*['\"]?$", ""); + values.put(key, value); - } + }); return values; } static String deriveId(final Map osRelease) { - logger.warn(osRelease.toString()); return osRelease.get("ID") + "-" + osRelease.get("VERSION_ID"); } @@ -464,15 +457,11 @@ private static List getLinuxExclusionList(Project project) { final Path exclusionsPath = project.getRootDir().toPath().resolve(Path.of(".ci", exclusionsFilename)); try { - final List exclusionsList = Files.readAllLines(exclusionsPath) + return Files.readAllLines(exclusionsPath) .stream() .map(String::trim) - .filter(line -> line.isEmpty() || line.startsWith("#")) + .filter(line -> (line.isEmpty() || line.startsWith("#")) == false) .collect(Collectors.toList()); - - logger.warn("Exclusion list: " + exclusionsList); - - return exclusionsList; } catch (IOException e) { throw new GradleException("Failed to read .ci/" + exclusionsFilename, e); } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java index dea95e6d6bc51..f88a4c11415bc 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/DistroTestPluginTests.java @@ -49,6 +49,32 @@ public void testParseOsReleaseOnOracle() { assertThat(expected, equalTo(results)); } + /** + * Trailing whitespace should be removed + */ + public void testRemoveTrailingWhitespace() { + final List lines = List.of("NAME=\"Oracle Linux Server\" "); + + final Map results = parseOsRelease(lines); + + final Map expected = Map.of("NAME", "Oracle Linux Server"); + + assertThat(expected, equalTo(results)); + } + + /** + * Comments should be removed + */ + public void testRemoveComments() { + final List lines = List.of("# A comment", "NAME=\"Oracle Linux Server\""); + + final Map results = parseOsRelease(lines); + + final Map expected = Map.of("NAME", "Oracle Linux Server"); + + assertThat(expected, equalTo(results)); + } + public void testDeriveIdOnOracle() { final Map osRelease = new HashMap<>(); osRelease.put("ID", "ol"); From 9d50aaa1453405a1e0b66ff21bdc76cd35e3de40 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Mon, 4 Nov 2019 15:40:44 +0000 Subject: [PATCH 43/43] Pointless change to try and get CI to run the right code --- .ci/dockerOnLinuxExclusions | 2 -- 1 file changed, 2 deletions(-) diff --git a/.ci/dockerOnLinuxExclusions b/.ci/dockerOnLinuxExclusions index 1b15128f86d92..ab344641baff9 100644 --- a/.ci/dockerOnLinuxExclusions +++ b/.ci/dockerOnLinuxExclusions @@ -4,11 +4,9 @@ # Docker tests to be skipped on that OS. If /etc/os-release doesn't exist # (as is the case on centos-6, for example) then that OS will again be # excluded. - centos-6 debian-8 opensuse-15-1 ol-6.10 ol-7.7 sles-12 -