Clarify gid used by docker image process and bind-mount method #49529
Conversation
dliappis
added
>docs
|
Pinging @elastic/es-docs (>docs) |
|
Pinging @elastic/es-core-infra (:Core/Infra/Packaging) |
|
Note: not assigning reviewers yet, until packaging CI turns green. |
|
Packaging-matrix job is now green, assigned reviewers. |
| // Restart the container | ||
| final Map<Path, Path> volumes = Map.of(tempEsDataDir.toAbsolutePath(), Path.of("/usr/share/elasticsearch/data")); | ||
|
|
||
| runContainer(distribution(), volumes, Map.of("ES_JAVA_OPTS", "-Xms512m -Xmx512m")); |
There was a problem hiding this comment.
Do we need the extra env vars here? I don't see any assertions that the values are in effect. You can pass null otherwise.
There was a problem hiding this comment.
This is only because I wanted to keep the memory usage to a minimum for this type of test. Default is 1GB which I thought is unnecessary.
There was a problem hiding this comment.
None of the Docker tests do anything exciting right now, or spin up multiple nodes, so we could make this the default. I think the tests should all work the same way though, unless they need to change a setting for the sake of the test.
There was a problem hiding this comment.
I opted for not setting the heap in this test: 289f776
We can reduce the footprint for all tests, by default, in a future PR.
qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java
Outdated
Show resolved
Hide resolved
qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java
Outdated
Show resolved
Hide resolved
qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java
Outdated
Show resolved
Hide resolved
qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java
Outdated
Show resolved
Hide resolved
qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java
Outdated
Show resolved
Hide resolved
qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java
Outdated
Show resolved
Hide resolved
and use of installation.data instead of hardcoded values.
* Switch to int in signature * Address PR comment about redundant .toString()
|
@pugnascotia Thanks for the comments! I think I addressed them with the exception of #49529 (comment) for the reason suggested there. Could you take another pass when there's time? |
qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java
Outdated
Show resolved
Hide resolved
…47929-test-docker-uid-gid
Fix reference about the uid:gid that Elasticsearch runs as inside the Docker container and add a packaging test to ensure that bind mounting a data dir with a random uid and gid:0 works as expected. Relates elastic#49529 Closes elastic#47929
Elasticsearch runs as uid:gid
1000:0inside the Docker image:Clarify docs to reflect this and add required packaging test.
Finally, remove unnecessary trailing whitespace characters from the
Docker docs page.
Closes #47929