RFC: Proposal to replace fixtures with docker-compose #35651

alpar-t · 2018-11-16T16:48:19Z

This is a non complete example to request feedback on the general
direction before mooing forward with these changes.
It's lacking detect and skip functionality and causes the containers to go up even when they are not needed, but these are workable if we find the general approach ok.

We can easily support random ports with this setup as the Gradle plugin exposes a map of the randomly assigned ports, it's just a matter of passing them to the tests.

The plugin being used is documented here: https://github.com/avast/gradle-docker-compose-plugin

Relates to #34095

This is a non working example to request feedback on the general direction before mooving forward with thease tools. We can easily support random ports with this setup as the Gradle plugin exposes a map of the randomly assigned ports.

elasticmachine · 2018-11-16T16:48:20Z

Pinging @elastic/es-core-infra

nik9000

I like the idea. It'd be nice to know:

If docker isn't installed, can we still skip the tests like we do with vagrant?
Is it possible that it'd work on Windows? It looks like Docker has a thing for Windows.

I think a good next thing to check is "how do random ports work?"

nik9000 · 2018-11-16T17:43:31Z

x-pack/test/smb-fixture/build.gradle

+ * Allows for other subprojects to depend on the project level and not care about the details
+ * of composeUp
+ */
+jar {


This seems funky. I think I'd prefer to depend on the task.

I wasn't fully convinced either, just wanted to bring it up. Let's do the task it's more straight forward.

alpar-t · 2018-11-19T12:10:52Z

We can still ski the tests if docker-composed is not installed. By the same means as we do for vagrant, it's just not implemented yet.
This will work on windows. I have used the plugin on windows, and it's documentation said it handles differences gratefully. I know there are some differences when dealing with volumes, I don't remember the specifics, I think it was something like it's possible to mount a single file on Linux but not on Windows - something easy to work around.

alpar-t · 2018-11-19T16:34:13Z

Thanks for taking a look @nik9000 I'm glad you like it.

I made changes to show how remote ports could work.
I'm getting test failures now, but these don't seem to be related. I'll have to check if I also get them on master.

I'm thinking of pulling it into a plugin, to avoid repetition throughout the build.
Detecting docker-compose would be part of that and I think we can figure out from the plugin what needs to be disabled for the most part. I'll look at that next.

nik9000

I love the idea.

One thing that bothers me a little is that we're already actively moving away from tasks to manage test clusters and this seems to use tasks to manage docker. The docker images are going to be much lighter than our test clusters so it is probably ok, but it does feel bad.

nik9000 · 2018-11-19T16:34:29Z

x-pack/qa/third-party/active-directory/build.gradle

  systemProperty 'es.set.netty.runtime.available.processors', 'false'
  include '**/*IT.class'
  include '**/*Tests.class'
+  doFirst {
+    fixtureProject.dockerCompose.servicesInfos.fixture.ports.forEach { container, host ->
+      logger.info(" adding system property fixture.smb-fixture.ports.$container -> $host")


Do you think we should move this to debug? It is nice to have but maybe not all the time.

nik9000 · 2018-11-19T16:36:33Z

...c/test/java/org/elasticsearch/xpack/security/authc/ldap/AbstractActiveDirectoryTestCase.java

@@ -46,13 +46,14 @@
    // as we cannot control the URL of the referral which may contain a non-resolvable DNS name as
    // this name would be served by the samba4 instance
    public static final Boolean FOLLOW_REFERRALS = Booleans.parseBoolean(getFromEnv("TESTS_AD_FOLLOW_REFERRALS", "false"));
-    public static final String AD_LDAP_URL = getFromEnv("TESTS_AD_LDAP_URL", "ldaps://localhost:61636");
-    public static final String AD_LDAP_GC_URL = getFromEnv("TESTS_AD_LDAP_GC_URL", "ldaps://localhost:63269");
+    public static final String AD_LDAP_URL = getFromEnv("TESTS_AD_LDAP_URL", "ldaps://localhost:" + getFromProperty("636"));


Maybe name them rather than use the original port number? As much as I love port numbers I think it'd be nicer to call them LDAP and LDAP_GC or something.

nik9000 · 2018-11-19T16:36:56Z

x-pack/test/smb-fixture/build.gradle

-    environmentVars vagrantEnvVars
+buildscript {
+    repositories {
+        jcenter()


I think jcenter is already in our build script.

nik9000 · 2018-11-19T16:37:22Z

x-pack/test/smb-fixture/build.gradle

-    dependsOn halt
+task info() {
+    dependsOn composeUp
+    doLast { println dockerCompose.servicesInfos.fixture.ports }


Should this do compose down after?

nik9000 · 2018-11-19T16:40:56Z

I'm thinking of pulling it into a plugin, to avoid repetition throughout the build.
Detecting docker-compose would be part of that and I think we can figure out from the plugin what needs to be disabled for the most part. I'll look at that next.

That makes sense to me!

I think before we merge this we should get that docker detection.

rjernst

I'm very happy with this as a better approach than vagrant, especially because it fixes the longstanding issue of hardcoded ports. Other than making sure this works in elasticsearch-ci, I just have one little nit.

rjernst · 2018-11-19T22:35:55Z

x-pack/qa/third-party/active-directory/build.gradle

  systemProperty 'es.set.netty.runtime.available.processors', 'false'
  include '**/*IT.class'
  include '**/*Tests.class'
+  doFirst {
+    fixtureProject.dockerCompose.servicesInfos.fixture.ports.forEach { container, host ->
+      logger.info(" adding system property fixture.smb-fixture.ports.$container -> $host")


nit: can you use ${container} so it is clear where the substitution ends vs string value? Same below with the actual system property.

…tures-docker-compose

jasontedor

LGTM.

alpar-t · 2018-11-20T18:29:09Z

I muted the failing tests since CI already has docker-compose.

Added the composePull command to the packer cache script to cache the container images when the CI image is built.

alpar-t · 2018-11-21T11:15:31Z

@elasticmachine test this please

…tures-docker-compose

The conainer images are cached making subsequent runs faster.

alpar-t · 2018-11-26T16:47:53Z

Note to self. When this is merged, should add an issue to add fixtures for some projects like: x-pack/qa/third-party/jira/build.gradle which require an external one so these can run in regular CI.

alpar-t · 2018-11-27T10:38:11Z

@rjernst do you want to take another look ? I made some changes after you approved.

rjernst

LGTM, one nit

rjernst · 2018-11-27T20:20:45Z

x-pack/test/smb-fixture/Dockerfile

@@ -0,0 +1,12 @@
+FROM ubuntu:16.04
+RUN apt-get update -qqy && apt-get install -qqy samba ldap-utils


Now that installsmb.sh is being run in the docker file, I think installing samba and ldap-utils should move back into there?

I don't have a strong opinion on that, and don't mind doing it in a follow up.
One advantage of keeping these separate is that one can iterate on the install script without waiting for apt all the time, but having everything in the same script is also nice.

Creates a new plugin to manage docker-compose based test fixtures. Convert the smb-fixture as a first example.

alpar-t added 3 commits November 16, 2018 18:33

Add port mappings

268665a

Remove vagrantfile

4416463

alpar-t added >non-issue :Delivery/Build Build or test infrastructure v7.0.0 v6.6.0 labels Nov 16, 2018

alpar-t requested review from nik9000, rjernst, spinscale, jaymode, albertzaharovits and jasontedor November 16, 2018 16:48

nik9000 reviewed Nov 16, 2018

View reviewed changes

Add port detection

0623db7

nik9000 reviewed Nov 19, 2018

View reviewed changes

rjernst approved these changes Nov 19, 2018

View reviewed changes

alpar-t added 5 commits November 20, 2018 07:57

Merge remote-tracking branch 'origin/master' into replace-vagrant-fix…

a232329

…tures-docker-compose

Add a test fixtures plugin

b8db838

Add docker-compose detection

1c009ca

Add a note about docker-compose

7f52fe3

Merge remote-tracking branch 'origin/master' into replace-vagrant-fix…

9bd8e55

…tures-docker-compose

jasontedor approved these changes Nov 20, 2018

View reviewed changes

Mute failing tests and stage images

df045ef

Mute more failing tests

d18603d

alpar-t added 3 commits November 21, 2018 13:41

Merge remote-tracking branch 'origin/master' into replace-vagrant-fix…

1306d8f

…tures-docker-compose

Move package installation to dockerfile

4bc31ba

Move building the image into the dockerfile

06f606b

The conainer images are cached making subsequent runs faster.

jaymode approved these changes Nov 26, 2018

View reviewed changes

rjernst approved these changes Nov 27, 2018

View reviewed changes

alpar-t merged commit 45db829 into elastic:master Nov 29, 2018

alpar-t deleted the replace-vagrant-fixtures-docker-compose branch November 29, 2018 07:43

alpar-t added a commit that referenced this pull request Nov 29, 2018

Replace fixtures with docker-compose (#35651)

5c28147

Creates a new plugin to manage docker-compose based test fixtures. Convert the smb-fixture as a first example.

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: Proposal to replace fixtures with docker-compose #35651

RFC: Proposal to replace fixtures with docker-compose #35651

alpar-t commented Nov 16, 2018 •

edited

Loading

elasticmachine commented Nov 16, 2018

nik9000 left a comment

nik9000 Nov 16, 2018

alpar-t Nov 19, 2018

alpar-t commented Nov 19, 2018

alpar-t commented Nov 19, 2018

nik9000 left a comment

nik9000 Nov 19, 2018

nik9000 Nov 19, 2018

nik9000 Nov 19, 2018

nik9000 Nov 19, 2018

nik9000 commented Nov 19, 2018

rjernst left a comment

rjernst Nov 19, 2018

jasontedor left a comment

alpar-t commented Nov 20, 2018

alpar-t commented Nov 21, 2018

alpar-t commented Nov 26, 2018

alpar-t commented Nov 27, 2018

rjernst left a comment

rjernst Nov 27, 2018

alpar-t Nov 29, 2018

		@@ -0,0 +1,12 @@
		FROM ubuntu:16.04
		RUN apt-get update -qqy && apt-get install -qqy samba ldap-utils

RFC: Proposal to replace fixtures with docker-compose #35651

RFC: Proposal to replace fixtures with docker-compose #35651

Conversation

alpar-t commented Nov 16, 2018 • edited Loading

elasticmachine commented Nov 16, 2018

nik9000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alpar-t commented Nov 19, 2018

alpar-t commented Nov 19, 2018

nik9000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nik9000 commented Nov 19, 2018

rjernst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasontedor left a comment

Choose a reason for hiding this comment

alpar-t commented Nov 20, 2018

alpar-t commented Nov 21, 2018

alpar-t commented Nov 26, 2018

alpar-t commented Nov 27, 2018

rjernst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alpar-t commented Nov 16, 2018 •

edited

Loading