Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce Docker images build #36246

Merged
merged 18 commits into from
Dec 6, 2018
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,93 @@ class BuildPlugin implements Plugin<Project> {
project.ext.java9Home = project.rootProject.ext.java9Home
}

static void requireDocker(final Task task) {
final Project rootProject = task.project.rootProject
if (rootProject.hasProperty('requiresDocker') == false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using an essentially global property like this, can we have a property on the tasks using docker themselves? Then we can use a .all closure on the tasks container looking for the property, and check for docker in the middle of configuration instead of waiting until all projects are configured. There would still be a rootProject property to "remember" docker was already checked, but the tasks won't (directly) modify the root project.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on marking the task, but I would make it a custom task and use tasks.withType.
The checks could live in the same class as static methods.
We could at some time introduce an interface and have a generic "this task requires something that we need to check up-front" mechanism, we already have others: docker-compose, jdk versions tasks (tests) that require specific inputs to run.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these are very good suggestions but I’m going to ask one of you to pick this up on a follow-up please?

/*
* This is our first time encountering a task that requires Docker. We will add an extension that will let us track the tasks
* that register as requiring Docker. We will add a delayed execution that when the task graph is ready if any such tasks are
* in the task graph, then we check two things:
* - the Docker binary is available
* - we can execute a Docker command that requires privileges
*
* If either of these fail, we fail the build.
*/
final boolean buildDocker
final String buildDockerProperty = System.getProperty("build.docker")
if (buildDockerProperty == null || buildDockerProperty == "true") {
buildDocker = true
} else if (buildDockerProperty == "false") {
buildDocker = false
} else {
throw new IllegalArgumentException(
"expected build.docker to be unset or one of \"true\" or \"false\" but was [" + buildDockerProperty + "]")
}
rootProject.rootProject.ext.buildDocker = buildDocker
rootProject.rootProject.ext.requiresDocker = []
rootProject.gradle.taskGraph.whenReady { TaskExecutionGraph taskGraph ->
// check if the Docker binary exists and record its path
final String dockerBinary
if (new File('/usr/bin/docker').exists()) {
dockerBinary = '/usr/bin/docker'
} else if (new File('/usr/local/bin/docker').exists()) {
dockerBinary = '/usr/local/bin/docker'
}
final int exitCode
final String dockerErrorOutput
if (dockerBinary == null) {
exitCode = -1
dockerErrorOutput = null
} else {
// the Docker binary executes, check that we can execute a privileged command
final Process process = new ProcessBuilder(dockerBinary, "images").start()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: project.exec or LoggedExec.exec(project would be more idiomatic here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed some changes related to this.

exitCode = process.waitFor()
if (exitCode == 0) {
return
}
// capture the standard error output into a single token using the beginning of input delimiter
final Scanner scanner = new Scanner(process.errorStream).useDelimiter("\\A")
dockerErrorOutput = scanner.hasNext() ? scanner.next() : ""
process.closeStreams()
}
final List<String> tasks =
((List<Task>)rootProject.requiresDocker).findAll { taskGraph.hasTask(it) }.collect { " ${it.path}".toString()}
if (tasks.isEmpty() == false) {
/*
* 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) {
throw new GradleException(
String.format(
Locale.ROOT,
"Docker is required to run the following task%s: \n%s",
tasks.size() > 1 ? "s" : "",
tasks.join('\n')))
} else {
assert exitCode > 0 && dockerErrorOutput != null
throw new GradleException(
String.format(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be useful to clearly distinguish docker not being found in one of the two expected locations, vs an error when trying to run the binary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rjernst So I think I did distinguished the two cases, or am I misunderstanding what you mean?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, you are right. My eyes glossed over the first if block.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use a task property and a tasks.all as I suggested, I think this can all be moved together so that the if/elseif on the docker file check can end with an else that throws the error, so you don't need to fake out the exit code.

Locale.ROOT,
"a problem occurred running Docker 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]",
tasks.size() > 1 ? "s" : "",
tasks.join('\n'),
exitCode,
dockerErrorOutput.trim()))
}
}
}
}
if (rootProject.buildDocker) {
rootProject.requiresDocker.add(task)
} else {
// do not overwrite an existing onlyIf closure
final Closure onlyIf = task.onlyIf
task.onlyIf { onlyIf && false }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A task can have multiple onlyIfs but if any one of them return false the others don't really matter. This can be expressed more straight forward as task.enabled = false

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed ed4bf10.

}
}

private static String findCompilerJavaHome() {
final String compilerJavaHome = System.getenv('JAVA_HOME')
final String compilerJavaProperty = System.getProperty('compiler.java')
Expand Down
106 changes: 106 additions & 0 deletions distribution/docker/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
import org.elasticsearch.gradle.BuildPlugin
import org.elasticsearch.gradle.LoggedExec
import org.elasticsearch.gradle.MavenFilteringHack
import org.elasticsearch.gradle.VersionProperties

apply plugin: 'base'

configurations {
dockerPlugins
dockerSource
ossDockerSource
}

dependencies {
dockerPlugins project(path: ":plugins:ingest-geoip", configuration: 'zip')
dockerPlugins project(path: ":plugins:ingest-user-agent", configuration: 'zip')
dockerSource project(path: ":distribution:archives:tar")
ossDockerSource project(path: ":distribution:archives:oss-tar")
}

ext.expansions = { oss ->
return [
'elasticsearch' : oss ? "elasticsearch-oss-${VersionProperties.elasticsearch}.tar.gz" : "elasticsearch-${VersionProperties.elasticsearch}.tar.gz",
'jdkUrl' : 'https://download.java.net/java/GA/jdk11/13/GPL/openjdk-11.0.1_linux-x64_bin.tar.gz',
'jdkVersion' : '11.0.1',
'license': oss ? 'Apache-2.0' : 'Elastic License',
'ingest-geoip' : "ingest-geoip-${VersionProperties.elasticsearch}.zip",
'ingest-user-agent' : "ingest-user-agent-${VersionProperties.elasticsearch}.zip",
'version' : VersionProperties.elasticsearch
]
}

private static String files(final boolean oss) {
return "build/${ oss ? 'oss-' : ''}docker"
}

private static String taskName(final String prefix, final boolean oss, final String suffix) {
return "${prefix}${oss ? 'Oss' : ''}${suffix}"
}

void addCopyDockerContextTask(final boolean oss) {
task(taskName("copy", oss, "DockerContext"), type: Sync) {
into files(oss)

into('bin') {
from 'src/docker/bin'
}

into('config') {
from 'src/docker/config'
}

if (oss) {
from configurations.ossDockerSource
} else {
from configurations.dockerSource
}

from configurations.dockerPlugins
}
}

void addCopyDockerfileTask(final boolean oss) {
task(taskName("copy", oss, "Dockerfile"), type: Copy) {
mustRunAfter(taskName("copy", oss, "DockerContext"))
into files(oss)

from('src/docker/Dockerfile') {
MavenFilteringHack.filter(it, expansions(oss))
}
}
}

void addBuildDockerImage(final boolean oss) {
final Task buildDockerImageTask = task(taskName("build", oss, "DockerImage"), type: LoggedExec) {
dependsOn taskName("copy", oss, "DockerContext")
dependsOn taskName("copy", oss, "Dockerfile")
final List<String> tags
if (oss) {
tags = [ "docker.elastic.co/elasticsearch/elasticsearch-oss:${VersionProperties.elasticsearch}" ]
} else {
tags = [
"elasticsearch:${VersionProperties.elasticsearch}",
"docker.elastic.co/elasticsearch/elasticsearch:${VersionProperties.elasticsearch}",
"docker.elastic.co/elasticsearch/elasticsearch-full:${VersionProperties.elasticsearch}"
]
}
executable 'docker'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that we'll need docker installed even for ./gradlew assemble.
Are we ok with any version of docker for that?
It might make sense to add up-front checks like we have for the JDK to make sure it's there and it works.
Permissions are also something we are likely to run into as many tutorials online advertise the use of sudo docker people might not have permissions set up for the user running the build.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@atorok I pushed 684fcb5.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@atorok I pushed one more change, to check if we can even run Docker: 26fe606

This gives output like:

FAILURE: Build failed with an exception.

* What went wrong:
a problem occurred running Docker yet it is required to run the following tasks: 
  :distribution:docker:buildDockerImage
  :distribution:docker:buildOssDockerImage
the problem is that [docker images] exited with exit code [1] with error output [Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Get http://%2Fvar%2Frun%2Fdocker.sock/v1.39/images/json: dial unix /var/run/docker.sock: connect: permission denied]

Your input @nik9000 @rjernst would be appreciated here too!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems right. We talked about making a parameter that let you explicitly skip the tasks which would be a nice thing to have. Something like -Dbuild.docker=false.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nik9000 I pushed b20be22. It's okay?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be an ideal time to consider using img or Buildah as the build executor instead of the Docker daemon. I personally consider this to be the "next step" in improving the build, and it just seems like a great time to redefine the dependencies.

Getting away from the Docker daemon not only improves the local build experience for developers, especially if they don't run Linux on their workstations, it also makes a much better story for running builds in container environments, like the work that @mgreau is actively pursuing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion. Alas the Docker daemon will stay around since we are migrating some of our testing away from Vagrant fixtures to Docker Compose fixtures. That’s not to say we won’t take your suggestion up.

final List<String> dockerArgs = ['build', files(oss), '--pull']
for (final String tag : tags) {
dockerArgs.add('--tag')
dockerArgs.add(tag)
}
args dockerArgs.toArray()
}
BuildPlugin.requireDocker(buildDockerImageTask)
}

for (final boolean oss : [false, true]) {
addCopyDockerContextTask(oss)
addCopyDockerfileTask(oss)
addBuildDockerImage(oss)
}

assemble.dependsOn "buildOssDockerImage"
assemble.dependsOn "buildDockerImage"
92 changes: 92 additions & 0 deletions distribution/docker/src/docker/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
################################################################################
# This Dockerfile was generated from the template at distribution/src/docker/Dockerfile
#
# Beginning of multi stage Dockerfile
################################################################################

################################################################################
# Build stage 0 `builder`:
# Extract elasticsearch artifact
# Install required plugins
# Set gid=0 and make group perms==owner perms
################################################################################

FROM centos:7 AS builder

ENV PATH /usr/share/elasticsearch/bin:$PATH
ENV JAVA_HOME /opt/jdk-${jdkVersion}

RUN curl -s ${jdkUrl} | tar -C /opt -zxf -

# Replace OpenJDK's built-in CA certificate keystore with the one from the OS
# vendor. The latter is superior in several ways.
# REF: https://github.com/elastic/elasticsearch-docker/issues/171
RUN ln -sf /etc/pki/ca-trust/extracted/java/cacerts /opt/jdk-${jdkVersion}/lib/security/cacerts

RUN yum install -y unzip which

RUN groupadd -g 1000 elasticsearch && \
adduser -u 1000 -g 1000 -d /usr/share/elasticsearch elasticsearch

WORKDIR /usr/share/elasticsearch

COPY ${elasticsearch} ${ingest-geoip} ${ingest-user-agent} /opt/
RUN tar zxf /opt/${elasticsearch} --strip-components=1
RUN elasticsearch-plugin install --batch file:///opt/${ingest-geoip}
RUN elasticsearch-plugin install --batch file:///opt/${ingest-user-agent}
RUN mkdir -p config data logs
RUN chmod 0775 config data logs
COPY config/elasticsearch.yml config/log4j2.properties config/


################################################################################
# Build stage 1 (the actual elasticsearch image):
# Copy elasticsearch from stage 0
# Add entrypoint
################################################################################

FROM centos:7

ENV ELASTIC_CONTAINER true
ENV JAVA_HOME /opt/jdk-${jdkVersion}

COPY --from=builder /opt/jdk-${jdkVersion} /opt/jdk-${jdkVersion}

RUN yum update -y && \
yum install -y nc unzip wget which && \
yum clean all

RUN groupadd -g 1000 elasticsearch && \
adduser -u 1000 -g 1000 -G 0 -d /usr/share/elasticsearch elasticsearch && \
chmod 0775 /usr/share/elasticsearch && \
chgrp 0 /usr/share/elasticsearch

WORKDIR /usr/share/elasticsearch
COPY --from=builder --chown=1000:0 /usr/share/elasticsearch /usr/share/elasticsearch
ENV PATH /usr/share/elasticsearch/bin:$PATH

COPY --chown=1000:0 bin/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh

# Openshift overrides USER and uses ones with randomly uid>1024 and gid=0
# Allow ENTRYPOINT (and ES) to run even with a different user
RUN chgrp 0 /usr/local/bin/docker-entrypoint.sh && \
chmod g=u /etc/passwd && \
chmod 0775 /usr/local/bin/docker-entrypoint.sh

EXPOSE 9200 9300

LABEL org.label-schema.schema-version="1.0" \
org.label-schema.vendor="Elastic" \
org.label-schema.name="elasticsearch" \
org.label-schema.version="${version}" \
org.label-schema.url="https://www.elastic.co/products/elasticsearch" \
org.label-schema.vcs-url="https://github.com/elastic/elasticsearch" \
license="${license}"

ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]
# Dummy overridable parameter parsed by entrypoint
CMD ["eswrapper"]

################################################################################
# End of multi-stage Dockerfile
################################################################################
100 changes: 100 additions & 0 deletions distribution/docker/src/docker/bin/docker-entrypoint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
#!/bin/bash
set -e

# Files created by Elasticsearch should always be group writable too
umask 0002

run_as_other_user_if_needed() {
if [[ "$(id -u)" == "0" ]]; then
# If running as root, drop to specified UID and run command
exec chroot --userspec=1000 / "${@}"
else
# Either we are running in Openshift with random uid and are a member of the root group
# or with a custom --user
exec "${@}"
fi
}

# Allow user specify custom CMD, maybe bin/elasticsearch itself
# for example to directly specify `-E` style parameters for elasticsearch on k8s
# or simply to run /bin/bash to check the image
if [[ "$1" != "eswrapper" ]]; then
if [[ "$(id -u)" == "0" && $(basename "$1") == "elasticsearch" ]]; then
# centos:7 chroot doesn't have the `--skip-chdir` option and
# changes our CWD.
# Rewrite CMD args to replace $1 with `elasticsearch` explicitly,
# so that we are backwards compatible with the docs
# from the previous Elasticsearch versions<6
# and configuration option D:
# https://www.elastic.co/guide/en/elasticsearch/reference/5.6/docker.html#_d_override_the_image_8217_s_default_ulink_url_https_docs_docker_com_engine_reference_run_cmd_default_command_or_options_cmd_ulink
# Without this, user could specify `elasticsearch -E x.y=z` but
# `bin/elasticsearch -E x.y=z` would not work.
set -- "elasticsearch" "${@:2}"
# Use chroot to switch to UID 1000
exec chroot --userspec=1000 / "$@"
else
# User probably wants to run something else, like /bin/bash, with another uid forced (Openshift?)
exec "$@"
fi
fi

# Parse Docker env vars to customize Elasticsearch
#
# e.g. Setting the env var cluster.name=testcluster
#
# will cause Elasticsearch to be invoked with -Ecluster.name=testcluster
#
# see https://www.elastic.co/guide/en/elasticsearch/reference/current/settings.html#_setting_default_settings

declare -a es_opts

while IFS='=' read -r envvar_key envvar_value
do
# Elasticsearch settings need to have at least two dot separated lowercase
# words, e.g. `cluster.name`, except for `processors` which we handle
# specially
if [[ "$envvar_key" =~ ^[a-z0-9_]+\.[a-z0-9_]+ || "$envvar_key" == "processors" ]]; then
if [[ ! -z $envvar_value ]]; then
es_opt="-E${envvar_key}=${envvar_value}"
es_opts+=("${es_opt}")
fi
fi
done < <(env)

# The virtual file /proc/self/cgroup should list the current cgroup
# membership. For each hierarchy, you can follow the cgroup path from
# this file to the cgroup filesystem (usually /sys/fs/cgroup/) and
# introspect the statistics for the cgroup for the given
# hierarchy. Alas, Docker breaks this by mounting the container
# statistics at the root while leaving the cgroup paths as the actual
# paths. Therefore, Elasticsearch provides a mechanism to override
# reading the cgroup path from /proc/self/cgroup and instead uses the
# cgroup path defined the JVM system property
# es.cgroups.hierarchy.override. Therefore, we set this value here so
# that cgroup statistics are available for the container this process
# will run in.
export ES_JAVA_OPTS="-Des.cgroups.hierarchy.override=/ $ES_JAVA_OPTS"

if [[ -d bin/x-pack ]]; then
# Check for the ELASTIC_PASSWORD environment variable to set the
# bootstrap password for Security.
#
# This is only required for the first node in a cluster with Security
# enabled, but we have no way of knowing which node we are yet. We'll just
# honor the variable if it's present.
if [[ -n "$ELASTIC_PASSWORD" ]]; then
[[ -f /usr/share/elasticsearch/config/elasticsearch.keystore ]] || (run_as_other_user_if_needed elasticsearch-keystore create)
if ! (run_as_other_user_if_needed elasticsearch-keystore list | grep -q '^bootstrap.password$'); then
(run_as_other_user_if_needed echo "$ELASTIC_PASSWORD" | elasticsearch-keystore add -x 'bootstrap.password')
fi
fi
fi

if [[ "$(id -u)" == "0" ]]; then
# If requested and running as root, mutate the ownership of bind-mounts
if [[ -n "$TAKE_FILE_OWNERSHIP" ]]; then
chown -R 1000:0 /usr/share/elasticsearch/{data,logs}
fi
fi

run_as_other_user_if_needed /usr/share/elasticsearch/bin/elasticsearch "${es_opts[@]}"
2 changes: 2 additions & 0 deletions distribution/docker/src/docker/config/elasticsearch.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
cluster.name: "docker-cluster"
network.host: 0.0.0.0