Redo docker readme #2040

ingwinlu · 2018-06-04T12:23:21Z

Purpose

Redo the README.md and bring it up to date with the new Docker images. Also move the old README.md and related files into a legacy directory.

Checklist

Check relevant points but please do not remove entries.
For docu fixes, spell checking, and similar nothing
needs to be checked.

commit messages are fine ("module: short statement" syntax and refer to issues)
affected documentation is fixed
release notes are updated (doc/news/_preparation_next_release.md)

@markus2330 please review my pull request

markus2330

Works perfectly! This fills a huge gap in the documentation, thank you!

Only small remarks here.

markus2330 · 2018-06-04T16:08:15Z

doc/news/_preparation_next_release.md

-  - abort of previously queued but unfinished runs on new commits
+  - Better Docker image handling.
+  - Abort of previously queued but unfinished runs on new commits.
+  - Document how to locally replicate the Docker environment used for tests.


Please add your name

It is a sub bulletin of an entry that has my name. But apparently that is not enough for the current script.

Ahh, it is also not enough for me: I did not see it, too.

Thus you did not change any code, we can merge this even without the okay from the build server.

Please set "ready to merge" when you fixed everything from the review.

markus2330 · 2018-06-04T16:08:28Z

scripts/docker/README.md

@@ -1,34 +1,56 @@
-# Trying out Elektra with Docker
+# Elektra Docker Artifacts
+This folder contains all Docker related Artifacts.


markus2330 · 2018-06-04T16:10:35Z

scripts/docker/README.md

 ```sh
-$ mkdir data
-$ docker run --rm -v ${PWD}/data:/mnt/share -it buildelektra bash
+docker build -t buildelektra-stretch-full \


What about adding a script for this command?

E.g. scripts/docker-build

markus2330 · 2018-06-04T16:14:01Z

scripts/docker/README.md

 ```sh
-$ buildelektra -b elektra master
+docker run -it --rm \


again, what about a docker-run script?

markus2330 · 2018-06-04T16:17:57Z

scripts/docker/README.md

 ```

-When you exit the container you will find the created .deb package and the downloaded snapshots of Elektra in the data directory.
+Note since we used matching userid + groupid to your current user the container
+will write to your mounted directory with the correct permissions.


markus2330 · 2018-06-04T16:18:29Z

scripts/docker/README.md

+will write to your mounted directory with the correct permissions.
+
+Once you are running inside the container you can use the same commands as you
+would use normally to configure, compile and test Elektra.


please refer to BUILDSERVER.md and/or Jenkinsfile here.

markus2330 · 2018-06-04T16:19:22Z

scripts/docker/README.md

+Afterwards it executes each step via `docker exec`.
+
+There might be some more differences that might influence test outcomes.
+One such documented case is #2008 and #1973 where errors on the test


Please link using https://issues.libelektra.org/2008 https://issues.libelektra.org/1973

I reproducibly (always) get the error with TMPDIR=`pwd`/x ctest -R rypt.

@petermax2 however seems to not get this error with the Docker images. @petermax2 can you retry with the commands posted here?

I build with

cmake -DBUILD_DOCUMENTATION=OFF -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ .. make -j16 mkdir x TMPDIR=`pwd`/x ctest -R crypt. Test project /elektra/build Start 46: testmod_crypto_openssl 1/3 Test #46: testmod_crypto_openssl ........... Passed 1.32 sec Start 47: testmod_crypto_gcrypt 2/3 Test #47: testmod_crypto_gcrypt ............ Passed 1.16 sec Start 48: testmod_crypto_botan 3/3 Test #48: testmod_crypto_botan ............. Passed 1.52 sec 100% tests passed, 0 tests failed out of 3 Label Time Summary: memleak = 1.16 sec (1 test) Total Test time (real) = 4.01 sec

EDIT: after following the instructions posted above inside the docker container. The error does not occur.

Can we have this discussion maybe at the affected PR and not here? Code comments in PR's get buried so easy.

markus2330 · 2018-06-04T16:28:41Z

scripts/docker/README.md

-$ buildelektra "0.8.19"
-```
+Please note that the complete images used to test Elektra are quite big
+(~3.7GB) and take a some time (10min+) to build.


Is it really 10min? On my fast machine with excellent Internet connection (TU network) it seemed to be longer.

Image creation is usually not capped by network speed but disk IO. On my local VM I can build the stretch image we use in 10 minutes. On the build servers it is a lot slower (especially on a7). It is even slower than expected honestly.

markus2330 · 2018-06-04T16:30:19Z

scripts/docker/README.md

+docker run -it --rm \
+    -v `pwd`:/home/jenkins/workspace \
+    -w /home/jenkins/workspace \
+    buildelektra-stretch-full


Works excellent, but googletest-download was executed.

It does not for me, can you recheck if you maybe have an old version checked out?

Sorry, I have been on the branch omnidan-elektra-web1.5

markus2330 · 2018-06-05T08:23:13Z

Thank you, much better now!

An overview about which Dockerfiles we have now would be great. I added it to #1974

ingwinlu · 2018-06-05T08:32:38Z

Theres always an up2date list in Jenkinsfile. I would hate to have the information doubled as it is prone to be forgotten when updating one of them.

…

On 5 June 2018 at 10:23, markus2330 ***@***.***> wrote: Thank you, much better now! An overview about which Dockerfiles we have now would be great. I added it to #1974 <#1974> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2040 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEOv-n4Yxo_5N5N_Ts9_lzACRjDWsfCZks5t5j_ygaJpZM4UY_K5> .

markus2330 · 2018-06-05T08:51:36Z

Then a cross-reference to the Jenkinsfile + some info about what the Dockerfiles are about directly at the definition is the better solution.

ingwinlu · 2018-06-05T11:25:55Z

Then a cross-reference to the Jenkinsfile

That makes sense.

some info about what the Dockerfiles are about

Not sure what information you want here. Note you can easily grab the tests that run on an image by searching for (as an example) DOCKER_IMAGES.sid in the Jenkinsfile.

If you want to solve the issue of 'where do I need to install packages of my new dependency' then I do not have an answer. Because it will always depend on where you want to test it.

And it will always be the contributors responsibility to make sure all additions are actually tested. Similarly like you would have to modify cmake to include your new files in builds.

ingwinlu · 2018-06-05T11:49:41Z

Similarly I would like to argue that it is the maintainers responsibility to check if the tests ran where every it is appropriate.

A task made significantly harder because of the 'silent removal policy' in some parts of the cmake setup.

ingwinlu · 2018-06-05T13:03:06Z

I just had another look at it and it seems like I already referenced the Jenkinsfile as a source for commands executed in the containers.

- add reference to Jenkinsfile - use https links - add empty lines after sections - put note as quotes

ingwinlu added 4 commits June 4, 2018 12:07

docker: move legacy artifacts

8ed71d4

docker: Add new README.md

a0ebc89

docker: Add release note

33afa56

docker: fixup spelling in release notes

861c1be

ingwinlu added usability low priority labels Jun 4, 2018

markus2330 approved these changes Jun 4, 2018

View reviewed changes

petermax2 mentioned this pull request Jun 4, 2018

Tests behave oddly in certain environments (debian packages) #1973

Closed

docker: implement feedback

0a2edcd

ingwinlu added the ready to merge label Jun 4, 2018

markus2330 merged commit f4174fe into ElektraInitiative:master Jun 5, 2018

ingwinlu deleted the redo_docker_readme branch June 5, 2018 09:13

markus2330 mentioned this pull request Jun 5, 2018

Make removal of test cases non-silent #2042

Closed

markus2330 pushed a commit that referenced this pull request Jun 5, 2018

doc: update as discussed in #2040

4acbed8

- add reference to Jenkinsfile - use https links - add empty lines after sections - put note as quotes

omnidan pushed a commit to omnidan/libelektra that referenced this pull request Jun 18, 2018

doc: update as discussed in ElektraInitiative#2040

df0c357

- add reference to Jenkinsfile - use https links - add empty lines after sections - put note as quotes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redo docker readme #2040

Redo docker readme #2040

ingwinlu commented Jun 4, 2018

markus2330 left a comment

markus2330 Jun 4, 2018

ingwinlu Jun 4, 2018

markus2330 Jun 4, 2018

markus2330 Jun 4, 2018

markus2330 Jun 4, 2018

markus2330 Jun 4, 2018

markus2330 Jun 4, 2018

markus2330 Jun 4, 2018

markus2330 Jun 4, 2018

petermax2 Jun 4, 2018 •

edited

Loading

ingwinlu Jun 4, 2018

markus2330 Jun 4, 2018 •

edited

Loading

ingwinlu Jun 4, 2018

markus2330 Jun 4, 2018

ingwinlu Jun 4, 2018

markus2330 Jun 4, 2018

markus2330 commented Jun 5, 2018

ingwinlu commented Jun 5, 2018 via email

markus2330 commented Jun 5, 2018

ingwinlu commented Jun 5, 2018

ingwinlu commented Jun 5, 2018

ingwinlu commented Jun 5, 2018

Redo docker readme #2040

Redo docker readme #2040

Conversation

ingwinlu commented Jun 4, 2018

Purpose

Checklist

markus2330 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

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

petermax2 Jun 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markus2330 Jun 4, 2018 • edited Loading

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

markus2330 commented Jun 5, 2018

ingwinlu commented Jun 5, 2018 via email

markus2330 commented Jun 5, 2018

ingwinlu commented Jun 5, 2018

ingwinlu commented Jun 5, 2018

ingwinlu commented Jun 5, 2018

petermax2 Jun 4, 2018 •

edited

Loading

markus2330 Jun 4, 2018 •

edited

Loading