Skip to content

Use Docker for Elasticsearch source builds#1570

Merged
b-deam merged 4 commits intoelastic:masterfrom
b-deam:rally-docker-build
Sep 12, 2022
Merged

Use Docker for Elasticsearch source builds#1570
b-deam merged 4 commits intoelastic:masterfrom
b-deam:rally-docker-build

Conversation

@b-deam
Copy link
Copy Markdown
Member

@b-deam b-deam commented Aug 31, 2022 •
edited
Loading

With this commit we introduce a new a way to build Elasticsearch and
core plugins from source using Docker. This new method is configured via
the new CLI argument (--source-build-method), which allows users to
specify the desired method. Currently, there are only two options,
'default', and 'docker'.

Relates #1462

Sample invocations

  • esrally race --revision="@2022-07-07" --track=geonames --test-mode --target-hosts=127.0.0.1:29200 --challenge=append-no-conflicts --car=4gheap,basic-license --elasticsearch-plugins= --runtime-jdk=bundled --kill-running-processes --source-build-method=docker
  • esrally install --revision="@2022-07-15" --node-name="rally-node-0" --network-host="127.0.0.1" --http-port=39200 --master-nodes="rally-node-0" --seed-hosts="127.0.0.1:29300" --kill-running-processes --source-build-method=docker

Note for reviewers

Apple Silicon equipped Macs can't run ITs:

FYI: The actor system is bugged on MacOS and will hang when running esrally race --revision=..., you can work around this by commenting out line 1342 in .venv/lib/python3.8/site-packages/thespian/system/transport/TCPTransport.py.

#lsock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)

@b-deam b-deam force-pushed the rally-docker-build branch 2 times, most recently from b59ce3b to 098f006 Compare September 5, 2022 09:03
@b-deam b-deam requested review from dliappis and pquentin September 6, 2022 00:31
@b-deam b-deam self-assigned this Sep 6, 2022
@b-deam b-deam added :Config Config file format changes, new properties, ... :internal Changes for internal, undocumented features: e.g. experimental, release scripts enhancement Improves the status quo labels Sep 6, 2022
@b-deam b-deam marked this pull request as ready for review September 6, 2022 00:32
@b-deam
Copy link
Copy Markdown
Member Author

b-deam commented Sep 6, 2022

@elasticmachine run rally/it-python310

@b-deam
Copy link
Copy Markdown
Member Author

b-deam commented Sep 6, 2022

So ITs pass, but the archive function fails:

11:01:41 ___________________________________ summary ____________________________________
11:01:41   py310-it: commands succeeded
11:01:41   congratulations :)
11:01:41 + bash .ci/build.sh archive
11:01:41 find: ‘/var/lib/jenkins/workspace/elastic+rally+pull-request+it-python310/.rally/benchmarks/src/elasticsearch/distribution/archives/build/jvm-options-hack/jvm.options.d’: Permission denied
11:01:41 find: ‘/var/lib/jenkins/workspace/elastic+rally+pull-request+it-python310/.rally/benchmarks/src/elasticsearch/distribution/archives/linux-tar/build/install/elasticsearch-8.4.0-SNAPSHOT/jdk/jmods’: Permission denied
11:01:41 find: ‘/var/lib/jenkins/workspace/elastic+rally+pull-request+it-python310/.rally/benchmarks/src/elasticsearch/distribution/archives/linux-tar/build/install/elasticsearch-8.4.0-SNAPSHOT/jdk/lib’: Permission denied
11:01:41 find: ‘/var/lib/jenkins/workspace/elastic+rally+pull-request+it-python310/.rally/benchmarks/src/elasticsearch/distribution/archives/linux-tar/build/install/elasticsearch-8.4.0-SNAPSHOT/jdk/legal’: Permission denied
11:01:41 find: ‘/var/lib/jenkins/workspace/elastic+rally+pull-request+it-python310/.rally/benchmarks/src/elasticsearch/distribution/archives/linux-tar/build/install/elasticsearch-8.4.0-SNAPSHOT/jdk/conf’: Permission denied
11:01:41 find: ‘/var/lib/jenkins/workspace/elastic+rally+pull-request+it-python310/.rally/benchmarks/src/elasticsearch/distribution/archives/linux-tar/build/install/elasticsearch-8.4.0-SNAPSHOT/jdk/bin’: Permission denied
11:01:41 find: ‘/var/lib/jenkins/workspace/elastic+rally+pull-request+it-python310/.rally/benchmarks/src/elasticsearch/distribution/archives/linux-tar/build/install/elasticsearch-8.4.0-SNAPSHOT/jdk/include’: Permission denied
11:01:41 find: ‘/var/lib/jenkins/workspace/elastic+rally+pull-request+it-python310/.rally/benchmarks/src/elasticsearch/distribution/archives/linux-tar/build/install/elasticsearch-8.4.0-SNAPSHOT/config/jvm.options.d’: Permission denied
11:01:41 benchmarks/races/79cd2fbb-066f-45ba-bced-a8a5e2ab5133/rally-node-0/logs/server/rally-benchmark.log

@b-deam
Copy link
Copy Markdown
Member Author

b-deam commented Sep 6, 2022 •
edited
Loading

The docker build method also hangs on my Intel equipped Mac when running either ITs or esrally race --revision ... --source-build-method docker

@b-deam
Copy link
Copy Markdown
Member Author

b-deam commented Sep 6, 2022

@elasticmachine run rally/it-python310

dliappis
dliappis reviewed Sep 6, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@dliappis dliappis left a comment

Choose a reason for hiding this comment

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

Did a first pass and left some less-important comments.

Will test it out / dig further tomorrow.

@b-deam b-deam force-pushed the rally-docker-build branch from 0523244 to f201b51 Compare September 7, 2022 04:32
@b-deam
Copy link
Copy Markdown
Member Author

b-deam commented Sep 7, 2022

@pquentin / @dliappis - there were enough changes that the commit history was messy so I've force pushed the branch to start 'fresh'. I had to introduce a docker commit step in the process that adds the the invoker's user and group to the image, along with installing git. This was needed to fix the bind mount permissions issues on Linux (Docker Desktop for Mac uses NFS for bind mounts AFAIK).

@b-deam b-deam mentioned this pull request Sep 7, 2022
Merged
dliappis
dliappis approved these changes Sep 8, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@dliappis dliappis left a comment

Choose a reason for hiding this comment

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

I left a minor comment (and we discussed offline several other small improvements that @b-deam 's been pushing).

Tested locally (on Linux) works like a charm. One gotcha is that if you already have an ES checkout in ~./rally/benchmarks/src/elasticsearch containing files/dirs with ownership different to local euid/gid, gradlew will fail within the builder container. The easiest way to troubleshoot it is to inspect ~/.rally/logs/build.log.

esrally/mechanic/supplier.py
image_container = self.client.containers.run(
detach=True,
name=self.image_builder_container_name,
image=self.resolve_jdk_build_container_image(self.build_jdk),
Copy link
Copy Markdown
Contributor

@dliappis dliappis Sep 8, 2022

Choose a reason for hiding this comment

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

maybe we specify the new image name somewhere? atm it's nameless (and it helps troubleshooting)

Copy link
Copy Markdown
Member Author

@b-deam b-deam Sep 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I tested setting the image repository and tags locally, but because we pass in an explicit image ID when running the actual builder container, the name doesn't appear in the docker ps output:

rally/esrally/mechanic/supplier.py

Line 890 in 1b3415e

image=container_image.id,

@b-deam b-deam mentioned this pull request Sep 12, 2022
Closed
@b-deam b-deam merged commit eb28518 into elastic:master Sep 12, 2022
@b-deam b-deam added this to the 2.6.1 milestone Sep 12, 2022
@b-deam b-deam mentioned this pull request Sep 13, 2022
Merged
@pquentin pquentin added :internal Changes for internal, undocumented features: e.g. experimental, release scripts and removed :internal Changes for internal, undocumented features: e.g. experimental, release scripts labels Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pquentin pquentin Awaiting requested review from pquentin

1 more reviewer

@dliappis dliappis dliappis approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@b-deam b-deam

Labels

:Config Config file format changes, new properties, ... enhancement Improves the status quo :internal Changes for internal, undocumented features: e.g. experimental, release scripts

Projects

None yet

Milestone

2.7.0

Development

Successfully merging this pull request may close these issues.

3 participants

@b-deam @dliappis @pquentin