Skip to content

Commit 1f3d101

Browse files
authored
Only define Docker pkg tests if Docker is available (#47640)
Closes #47639, and unmutes tests that were muted in b958467. The Docker packaging tests were being defined irrespective of whether Docker was actually available in the current environment. Instead, implement exclude lists so that in environments where Docker is not available, no Docker packaging tests are defined. For CI hosts, the build checks `.ci/dockerOnLinuxExclusions`. The Vagrant VMs can defined the extension property `shouldTestDocker` property to opt-in to packaging tests. As part of this, define a seperate utility class for checking Docker, and call that instead of defining checks in-line in BuildPlugin.groovy
1 parent 3aa44e4 commit 1f3d101

File tree

18 files changed

+539
-109
lines changed

18 files changed

+539
-109
lines changed

.ci/dockerOnLinuxExclusions

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# This file specifies the Linux OS versions on which we can't build and
2+
# test Docker images for some reason. These values correspond to ID and
3+
# VERSION_ID from /etc/os-release, and a matching value will cause the
4+
# Docker tests to be skipped on that OS. If /etc/os-release doesn't exist
5+
# (as is the case on centos-6, for example) then that OS will again be
6+
# excluded.
7+
centos-6
8+
debian-8
9+
opensuse-15-1
10+
ol-6.10
11+
ol-7.7
12+
sles-12

.ci/os.sh

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@ if which zypper > /dev/null ; then
66
sudo zypper install -y insserv-compat
77
fi
88

9+
if [ -e /etc/sysctl.d/99-gce.conf ]; then
10+
# The GCE defaults disable IPv4 forwarding, which breaks the Docker
11+
# build. Workaround this by renaming the file so that it is executed
12+
# earlier than our own overrides.
13+
#
14+
# This ultimately needs to be fixed at the image level - see infra
15+
# issue 15654.
16+
sudo mv /etc/sysctl.d/99-gce.conf /etc/sysctl.d/98-gce.conf
17+
fi
18+
919
# Required by bats
1020
sudo touch /etc/is_vagrant_vm
1121
sudo useradd vagrant

Vagrantfile

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ Vagrant.configure(2) do |config|
126126
end
127127
'fedora-29'.tap do |box|
128128
config.vm.define box, define_opts do |config|
129-
config.vm.box = 'elastic/fedora-28-x86_64'
129+
config.vm.box = 'elastic/fedora-29-x86_64'
130130
dnf_common config, box
131131
dnf_docker config
132132
end
@@ -216,6 +216,10 @@ def ubuntu_docker(config)
216216
217217
# Add vagrant to the Docker group, so that it can run commands
218218
usermod -aG docker vagrant
219+
220+
# Enable IPv4 forwarding
221+
sed -i '/net.ipv4.ip_forward/s/^#//' /etc/sysctl.conf
222+
systemctl restart networking
219223
SHELL
220224
end
221225

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

Lines changed: 5 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,14 @@ import org.gradle.external.javadoc.CoreJavadocOptions
7676
import org.gradle.internal.jvm.Jvm
7777
import org.gradle.language.base.plugins.LifecycleBasePlugin
7878
import org.gradle.process.CommandLineArgumentProvider
79-
import org.gradle.process.ExecResult
80-
import org.gradle.process.ExecSpec
8179
import org.gradle.util.GradleVersion
8280

8381
import java.nio.charset.StandardCharsets
8482
import java.nio.file.Files
85-
import java.util.regex.Matcher
8683

8784
import static org.elasticsearch.gradle.tool.Boilerplate.maybeConfigure
85+
import static org.elasticsearch.gradle.tool.DockerUtils.assertDockerIsAvailable
86+
import static org.elasticsearch.gradle.tool.DockerUtils.getDockerPath
8887

8988
/**
9089
* Encapsulates build configuration for elasticsearch projects.
@@ -183,8 +182,7 @@ class BuildPlugin implements Plugin<Project> {
183182
*/
184183

185184
// check if the Docker binary exists and record its path
186-
final List<String> maybeDockerBinaries = ['/usr/bin/docker', '/usr/local/bin/docker']
187-
final String dockerBinary = maybeDockerBinaries.find { it -> new File(it).exists() }
185+
final String dockerBinary = getDockerPath().orElse(null)
188186

189187
final boolean buildDocker
190188
final String buildDockerProperty = System.getProperty("build.docker")
@@ -203,84 +201,16 @@ class BuildPlugin implements Plugin<Project> {
203201
ext.set('requiresDocker', [])
204202
rootProject.gradle.taskGraph.whenReady { TaskExecutionGraph taskGraph ->
205203
final List<String> tasks = taskGraph.allTasks.intersect(ext.get('requiresDocker') as List<Task>).collect { " ${it.path}".toString()}
206-
if (tasks.isEmpty() == false) {
207-
/*
208-
* There are tasks in the task graph that require Docker. Now we are failing because either the Docker binary does not
209-
* exist or because execution of a privileged Docker command failed.
210-
*/
211-
if (dockerBinary == null) {
212-
final String message = String.format(
213-
Locale.ROOT,
214-
"Docker (checked [%s]) is required to run the following task%s: \n%s",
215-
maybeDockerBinaries.join(","),
216-
tasks.size() > 1 ? "s" : "",
217-
tasks.join('\n'))
218-
throwDockerRequiredException(message)
219-
}
220-
221-
// we use a multi-stage Docker build, check the Docker version since 17.05
222-
final ByteArrayOutputStream dockerVersionOutput = new ByteArrayOutputStream()
223-
LoggedExec.exec(
224-
rootProject,
225-
{ ExecSpec it ->
226-
it.commandLine = [dockerBinary, '--version']
227-
it.standardOutput = dockerVersionOutput
228-
})
229-
final String dockerVersion = dockerVersionOutput.toString().trim()
230-
checkDockerVersionRecent(dockerVersion)
231-
232-
final ByteArrayOutputStream dockerImagesErrorOutput = new ByteArrayOutputStream()
233-
// the Docker binary executes, check that we can execute a privileged command
234-
final ExecResult dockerImagesResult = LoggedExec.exec(
235-
rootProject,
236-
{ ExecSpec it ->
237-
it.commandLine = [dockerBinary, "images"]
238-
it.errorOutput = dockerImagesErrorOutput
239-
it.ignoreExitValue = true
240-
})
241-
242-
if (dockerImagesResult.exitValue != 0) {
243-
final String message = String.format(
244-
Locale.ROOT,
245-
"a problem occurred running Docker from [%s] yet it is required to run the following task%s: \n%s\n" +
246-
"the problem is that Docker exited with exit code [%d] with standard error output [%s]",
247-
dockerBinary,
248-
tasks.size() > 1 ? "s" : "",
249-
tasks.join('\n'),
250-
dockerImagesResult.exitValue,
251-
dockerImagesErrorOutput.toString().trim())
252-
throwDockerRequiredException(message)
253-
}
254204

205+
if (tasks.isEmpty() == false) {
206+
assertDockerIsAvailable(task.project, tasks)
255207
}
256208
}
257209
}
258210

259211
(ext.get('requiresDocker') as List<Task>).add(task)
260212
}
261213

262-
protected static void checkDockerVersionRecent(String dockerVersion) {
263-
final Matcher matcher = dockerVersion =~ /Docker version (\d+\.\d+)\.\d+(?:-[a-zA-Z0-9]+)?, build [0-9a-f]{7,40}/
264-
assert matcher.matches(): dockerVersion
265-
final dockerMajorMinorVersion = matcher.group(1)
266-
final String[] majorMinor = dockerMajorMinorVersion.split("\\.")
267-
if (Integer.parseInt(majorMinor[0]) < 17
268-
|| (Integer.parseInt(majorMinor[0]) == 17 && Integer.parseInt(majorMinor[1]) < 5)) {
269-
final String message = String.format(
270-
Locale.ROOT,
271-
"building Docker images requires Docker version 17.05+ due to use of multi-stage builds yet was [%s]",
272-
dockerVersion)
273-
throwDockerRequiredException(message)
274-
}
275-
}
276-
277-
private static void throwDockerRequiredException(final String message) {
278-
throw new GradleException(
279-
message + "\nyou can address this by attending to the reported issue, "
280-
+ "removing the offending tasks from being executed, "
281-
+ "or by passing -Dbuild.docker=false")
282-
}
283-
284214
/** Add a check before gradle execution phase which ensures java home for the given java version is set. */
285215
static void requireJavaHome(Task task, int version) {
286216
// use root project for global accounting

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/DistroTestPlugin.java

Lines changed: 124 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,22 @@
2828
import org.elasticsearch.gradle.ElasticsearchDistribution.Type;
2929
import org.elasticsearch.gradle.Jdk;
3030
import org.elasticsearch.gradle.JdkDownloadPlugin;
31+
import org.elasticsearch.gradle.OS;
3132
import org.elasticsearch.gradle.Version;
3233
import org.elasticsearch.gradle.VersionProperties;
3334
import org.elasticsearch.gradle.info.BuildParams;
3435
import org.elasticsearch.gradle.vagrant.BatsProgressLogger;
3536
import org.elasticsearch.gradle.vagrant.VagrantBasePlugin;
3637
import org.elasticsearch.gradle.vagrant.VagrantExtension;
38+
import org.gradle.api.GradleException;
3739
import org.gradle.api.NamedDomainObjectContainer;
3840
import org.gradle.api.Plugin;
3941
import org.gradle.api.Project;
4042
import org.gradle.api.Task;
4143
import org.gradle.api.artifacts.Configuration;
4244
import org.gradle.api.file.Directory;
45+
import org.gradle.api.logging.Logger;
46+
import org.gradle.api.logging.Logging;
4347
import org.gradle.api.plugins.ExtraPropertiesExtension;
4448
import org.gradle.api.plugins.JavaBasePlugin;
4549
import org.gradle.api.provider.Provider;
@@ -52,6 +56,7 @@
5256
import java.io.UncheckedIOException;
5357
import java.nio.file.Files;
5458
import java.nio.file.Path;
59+
import java.nio.file.Paths;
5560
import java.util.ArrayList;
5661
import java.util.Arrays;
5762
import java.util.HashMap;
@@ -66,6 +71,7 @@
6671
import static org.elasticsearch.gradle.vagrant.VagrantMachine.convertWindowsPath;
6772

6873
public class DistroTestPlugin implements Plugin<Project> {
74+
private static final Logger logger = Logging.getLogger(DistroTestPlugin.class);
6975

7076
private static final String SYSTEM_JDK_VERSION = "11.0.2+9";
7177
private static final String SYSTEM_JDK_VENDOR = "openjdk";
@@ -84,6 +90,8 @@ public class DistroTestPlugin implements Plugin<Project> {
8490

8591
@Override
8692
public void apply(Project project) {
93+
final boolean runDockerTests = shouldRunDockerTests(project);
94+
8795
project.getPluginManager().apply(DistributionDownloadPlugin.class);
8896
project.getPluginManager().apply(BuildPlugin.class);
8997

@@ -95,15 +103,17 @@ public void apply(Project project) {
95103
Provider<Directory> upgradeDir = project.getLayout().getBuildDirectory().dir("packaging/upgrade");
96104
Provider<Directory> pluginsDir = project.getLayout().getBuildDirectory().dir("packaging/plugins");
97105

98-
List<ElasticsearchDistribution> distributions = configureDistributions(project, upgradeVersion);
106+
List<ElasticsearchDistribution> distributions = configureDistributions(project, upgradeVersion, runDockerTests);
99107
TaskProvider<Copy> copyDistributionsTask = configureCopyDistributionsTask(project, distributionsDir);
100108
TaskProvider<Copy> copyUpgradeTask = configureCopyUpgradeTask(project, upgradeVersion, upgradeDir);
101109
TaskProvider<Copy> copyPluginsTask = configureCopyPluginsTask(project, pluginsDir);
102110

103111
TaskProvider<Task> destructiveDistroTest = project.getTasks().register("destructiveDistroTest");
104112
for (ElasticsearchDistribution distribution : distributions) {
105-
TaskProvider<?> destructiveTask = configureDistroTest(project, distribution);
106-
destructiveDistroTest.configure(t -> t.dependsOn(destructiveTask));
113+
if (distribution.getType() != Type.DOCKER || runDockerTests == true) {
114+
TaskProvider<?> destructiveTask = configureDistroTest(project, distribution);
115+
destructiveDistroTest.configure(t -> t.dependsOn(destructiveTask));
116+
}
107117
}
108118
Map<String, TaskProvider<?>> batsTests = new HashMap<>();
109119
batsTests.put("bats oss", configureBatsTest(project, "oss", distributionsDir, copyDistributionsTask));
@@ -129,7 +139,23 @@ public void apply(Project project) {
129139
TaskProvider<GradleDistroTestTask> vmTask =
130140
configureVMWrapperTask(vmProject, distribution.getName() + " distribution", destructiveTaskName, vmDependencies);
131141
vmTask.configure(t -> t.dependsOn(distribution));
132-
distroTest.configure(t -> t.dependsOn(vmTask));
142+
143+
distroTest.configure(t -> {
144+
// Only VM sub-projects that are specifically opted-in to testing Docker should
145+
// have the Docker task added as a dependency. Although we control whether Docker
146+
// is installed in the VM via `Vagrantfile` and we could auto-detect its presence
147+
// in the VM, the test tasks e.g. `destructiveDistroTest.default-docker` are defined
148+
// on the host during Gradle's configuration phase and not in the VM, so
149+
// auto-detection doesn't work.
150+
//
151+
// The shouldTestDocker property could be null, hence we use Boolean.TRUE.equals()
152+
boolean shouldExecute = distribution.getType() != Type.DOCKER
153+
|| Boolean.TRUE.equals(vmProject.findProperty("shouldTestDocker")) == true;
154+
155+
if (shouldExecute) {
156+
t.dependsOn(vmTask);
157+
}
158+
});
133159
}
134160
}
135161

@@ -321,17 +347,17 @@ private static TaskProvider<BatsTestTask> configureBatsTest(Project project, Str
321347
});
322348
}
323349

324-
private List<ElasticsearchDistribution> configureDistributions(Project project, Version upgradeVersion) {
350+
private List<ElasticsearchDistribution> configureDistributions(Project project, Version upgradeVersion, boolean runDockerTests) {
325351
NamedDomainObjectContainer<ElasticsearchDistribution> distributions = DistributionDownloadPlugin.getContainer(project);
326352
List<ElasticsearchDistribution> currentDistros = new ArrayList<>();
327353
List<ElasticsearchDistribution> upgradeDistros = new ArrayList<>();
328354

329-
// Docker disabled for https://github.com/elastic/elasticsearch/issues/47639
330-
for (Type type : Arrays.asList(Type.DEB, Type.RPM /*,Type.DOCKER*/)) {
355+
for (Type type : List.of(Type.DEB, Type.RPM, Type.DOCKER)) {
331356
for (Flavor flavor : Flavor.values()) {
332357
for (boolean bundledJdk : Arrays.asList(true, false)) {
333-
// We should never add a Docker distro with bundledJdk == false
334-
boolean skip = type == Type.DOCKER && bundledJdk == false;
358+
// All our Docker images include a bundled JDK so it doesn't make sense to test without one
359+
boolean skip = type == Type.DOCKER && (runDockerTests == false || bundledJdk == false);
360+
335361
if (skip == false) {
336362
addDistro(distributions, type, null, flavor, bundledJdk, VersionProperties.getElasticsearch(), currentDistros);
337363
}
@@ -345,6 +371,7 @@ private List<ElasticsearchDistribution> configureDistributions(Project project,
345371
addDistro(distributions, type, null, Flavor.OSS, true, upgradeVersion.toString(), upgradeDistros);
346372
}
347373
}
374+
348375
for (Platform platform : Arrays.asList(Platform.LINUX, Platform.WINDOWS)) {
349376
for (Flavor flavor : Flavor.values()) {
350377
for (boolean bundledJdk : Arrays.asList(true, false)) {
@@ -405,4 +432,92 @@ private static String destructiveDistroTestTaskName(ElasticsearchDistribution di
405432
distro.getFlavor(),
406433
distro.getBundledJdk());
407434
}
435+
436+
static Map<String, String> parseOsRelease(final List<String> osReleaseLines) {
437+
final Map<String, String> values = new HashMap<>();
438+
439+
osReleaseLines.stream().map(String::trim).filter(line -> (line.isEmpty() || line.startsWith("#")) == false).forEach(line -> {
440+
final String[] parts = line.split("=", 2);
441+
final String key = parts[0];
442+
// remove optional leading and trailing quotes and whitespace
443+
final String value = parts[1].replaceAll("^['\"]?\\s*", "").replaceAll("\\s*['\"]?$", "");
444+
445+
values.put(key, value);
446+
});
447+
448+
return values;
449+
}
450+
451+
static String deriveId(final Map<String, String> osRelease) {
452+
return osRelease.get("ID") + "-" + osRelease.get("VERSION_ID");
453+
}
454+
455+
private static List<String> getLinuxExclusionList(Project project) {
456+
final String exclusionsFilename = "dockerOnLinuxExclusions";
457+
final Path exclusionsPath = project.getRootDir().toPath().resolve(Path.of(".ci", exclusionsFilename));
458+
459+
try {
460+
return Files.readAllLines(exclusionsPath)
461+
.stream()
462+
.map(String::trim)
463+
.filter(line -> (line.isEmpty() || line.startsWith("#")) == false)
464+
.collect(Collectors.toList());
465+
} catch (IOException e) {
466+
throw new GradleException("Failed to read .ci/" + exclusionsFilename, e);
467+
}
468+
}
469+
470+
/**
471+
* The {@link DistroTestPlugin} generates a number of test tasks, some
472+
* of which are Docker packaging tests. When running on the host OS or in CI
473+
* i.e. not in a Vagrant VM, only certain operating systems are supported. This
474+
* method determines whether the Docker tests should be run on the host
475+
* OS. Essentially, unless an OS and version is specifically excluded, we expect
476+
* to be able to run Docker and test the Docker images.
477+
* @param project
478+
*/
479+
private static boolean shouldRunDockerTests(Project project) {
480+
switch (OS.current()) {
481+
case WINDOWS:
482+
// Not yet supported.
483+
return false;
484+
485+
case MAC:
486+
// Assume that Docker for Mac is installed, since Docker is part of the dev workflow.
487+
return true;
488+
489+
case LINUX:
490+
// Only some hosts in CI are configured with Docker. We attempt to work out the OS
491+
// and version, so that we know whether to expect to find Docker. We don't attempt
492+
// to probe for whether Docker is available, because that doesn't tell us whether
493+
// Docker is unavailable when it should be.
494+
final Path osRelease = Paths.get("/etc/os-release");
495+
496+
if (Files.exists(osRelease)) {
497+
Map<String, String> values;
498+
499+
try {
500+
final List<String> osReleaseLines = Files.readAllLines(osRelease);
501+
values = parseOsRelease(osReleaseLines);
502+
} catch (IOException e) {
503+
throw new GradleException("Failed to read /etc/os-release", e);
504+
}
505+
506+
final String id = deriveId(values);
507+
508+
final boolean shouldExclude = getLinuxExclusionList(project).contains(id);
509+
510+
logger.warn("Linux OS id [" + id + "] is " + (shouldExclude ? "" : "not ") + "present in the Docker exclude list");
511+
512+
return shouldExclude == false;
513+
}
514+
515+
logger.warn("/etc/os-release does not exist!");
516+
return false;
517+
518+
default:
519+
logger.warn("Unknown OS [" + OS.current() + "], answering false to shouldRunDockerTests()");
520+
return false;
521+
}
522+
}
408523
}

0 commit comments

Comments
 (0)