employ docker-compose v3.4 and shared logging config #152

fmigneault · 2021-04-21T22:05:31Z

export only the specific changes of docker-compose version and logging config from #114

Change overview:

Cap container logs to 500m. Before there was no limit to it could be logging indefinitively and eating up precious disk space.
Remove unused volume_from: mongodb from solr and ncwms2 container.
Bump docker-compose file version to 3.4 to support sharing logging config.

Matching PR for Ouranos specific config bird-house/birdhouse-deploy-ouranos#10

Fixes #10

dbyrns

We'll wait for the tests to pass then it should be all good.

tlvu

A few questions. Not approving immediately because I'd like to try it on my dev server to see the difference with the new logging config but looks good overral, except the .gitignore part that I find unusual.

tlvu · 2021-04-22T15:14:02Z

birdhouse/docker-compose.yml

+  &default-logging
+  driver: "json-file"
+  options:
+    max-size: "200k"


So we have not set any of those config currently (driver, max-size, max-file). What's was their default?

It's very nice to have a unified logging config for all containers but can each container, if needed, not use this config and roll their own?

And to loop back to the original Weaver PR, Weaver need to "hook" into other components logs and that's why it prefers other components to have unified logging config or is there another reason to introduce this unified logging?

Just to be clear, I like it to have a default and unified logging config, I just want to understand its relation to Weaver.

Docker's default is json so that remains the same.
The difference here is that we add log rotation over 10 files.
This is a config we have found a long time ago when deploying on our instances since twitcher and magpie can generate an immense quantity of logs (on each request). This allows discarding them gradually instead of accumulating GB of useless logs.
Each container can define its own logging section by using the relevant config instead of the default-logging reference.
I don't think there is a nice way to automate that though. Best could be to have an optional-component, but I think it is better to control the logs.

Using Weaver, it is a bit more tricky since a different docker-compose file is employed.
Depending on how docker-compose merges the files, the reference could or won't work across files.
Basically, if files are merged, and then references resolved, it would work. But if internally it uses PyYAML or similar lib, to read each file individually and merge them, Weaver's compose will raise a missing reference. So the config would need to be duplicated in its own file.

So if I understand correctly, Weaver did not need this central logging and the log rotation to avoid accumulating infinite logs (which is indeed a nice thing).

From our recent "contributing guideline", this PR should not have been mixed with the Weaver PR to begin with. Or if mixed in, it should have been "well explained" that it is not required by Weaver?

Yes, this is why it has been split by itself now that we updated the contribution guideline.
Please consider that Weaver PR was started in January and review only started in March, both long before we even started discussions of contribution guidelines.

tlvu · 2021-04-22T15:15:34Z

birdhouse/docker-compose.yml

@@ -213,9 +233,8 @@ services:
      - "48983:9001"
    volumes:
      - /data/solr:/data
-    volumes_from:
-      - mongodb


From what I understood from our meeting, the removal of volumes_from directive was because of the update to format version 3.4, not because we want it or Weaver needs it?

Yes. volumes_from is deprecated for >=3
There are a lot of improvements in docker-compose since v2, so better bump it while problems are still few.

This needs to be tested, but if I understand the compose correctly, that volumes_from directive specifically is useless, since mongodb doesn't even have a volumes section. Starting with Weaver though, I require a data persistence volume in mongo, so I would like to have this adjusted with docker-compose v3 before it other images are not using it anyway (and don't start breaking obviously).

Agreed, this looks harmless but should be tested to be sure. Has you or Mat gone over the failed build to determine the error is not related to this? Currently only the ESGF notebook should be intermittently failing because their server seems flaky but everything else should be green.

docs/.gitignore

To match bird-house/birdhouse-deploy#152.

tlvu

Please confirm failing tests are okay.

Please let me do this merge since I need to since this merge with my other PR bird-house/birdhouse-deploy-ouranos#10 else our prod won't even start.

@nikola-rados I am pretty sure you will also need to do something like my PR bird-house/birdhouse-deploy-ouranos#10 above on your side. I can also sync with you for this merge. Let me know.

tlvu · 2021-04-29T01:21:45Z

birdhouse/docker-compose.yml

+  &default-logging
+  driver: "json-file"
+  options:
+    max-size: "200k"


So if I understand correctly, Weaver did not need this central logging and the log rotation to avoid accumulating infinite logs (which is indeed a nice thing).

From our recent "contributing guideline", this PR should not have been mixed with the Weaver PR to begin with. Or if mixed in, it should have been "well explained" that it is not required by Weaver?

tlvu · 2021-04-29T01:23:42Z

birdhouse/docker-compose.yml

@@ -213,9 +233,8 @@ services:
      - "48983:9001"
    volumes:
      - /data/solr:/data
-    volumes_from:
-      - mongodb


Agreed, this looks harmless but should be tested to be sure. Has you or Mat gone over the failed build to determine the error is not related to this? Currently only the ESGF notebook should be intermittently failing because their server seems flaky but everything else should be green.

fmigneault · 2021-04-29T02:21:48Z

@tlvu
I have no problem waiting for your merge on this.
If anything, I prefer you do it so that it is validated against a full stack.

I'm not too sure what was the problem on the last build, it was stopped for some reason.
Manual trigger produced the following outputs:

=> http://daccs-jenkins.crim.ca/job/DACCS-iac/401
=> http://daccs-jenkins.crim.ca/job/PAVICS-e2e-workflow-tests/job/master/316

22:12:17  notebooks/hummingbird.ipynb ............                                 [  8%]
22:12:25  notebooks-auth/test_thredds.ipynb ..........                             [ 15%]
22:12:32  pavics-sdi-master/docs/source/notebooks/WCS_example.ipynb .......        [ 20%]
22:13:06  pavics-sdi-master/docs/source/notebooks/WFS_example.ipynb .FFFF.         [ 24%]
22:13:15  pavics-sdi-master/docs/source/notebooks/WMS_example.ipynb ........       [ 30%]
22:13:17  pavics-sdi-master/docs/source/notebooks/WPS_example.ipynb ..........     [ 37%]
22:13:31  pavics-sdi-master/docs/source/notebooks/esgf-dap.ipynb .                 [ 37%]
22:13:32  pavics-sdi-master/docs/source/notebooks/jupyter_extensions.ipynb .       [ 38%]
22:13:40  pavics-sdi-master/docs/source/notebooks/opendap.ipynb .......            [ 43%]
22:13:44  pavics-sdi-master/docs/source/notebooks/pavics_thredds.ipynb ....F       [ 46%]
22:16:52  pavics-sdi-master/docs/source/notebooks/regridding.ipynb ...FF.FF.FFF.FF [ 57%]
22:16:56  FFFFFF...FFFFF.                                                          [ 67%]
22:17:00  pavics-sdi-master/docs/source/notebooks/rendering.ipynb ....             [ 70%]
22:17:05  pavics-sdi-master/docs/source/notebooks/subsetting.ipynb ......          [ 74%]
22:17:17  finch-master/docs/source/notebooks/dap_subset.ipynb ..........           [ 81%]
22:17:24  finch-master/docs/source/notebooks/finch-usage.ipynb ......              [ 86%]
22:17:51  finch-master/docs/source/notebooks/subset.ipynb ...FF....FFFFF....FF     [100%]

fmigneault · 2021-04-29T03:03:27Z

Just to confirm, it reran the whole thing, one time targeting master and the other docker-compose-update

master
http://daccs-jenkins.crim.ca/job/DACCS-iac/402
http://daccs-jenkins.crim.ca/job/PAVICS-e2e-workflow-tests/job/master/317

docker-compose-update
http://daccs-jenkins.crim.ca/job/DACCS-iac/403
http://daccs-jenkins.crim.ca/job/PAVICS-e2e-workflow-tests/job/master/318

Results are the same in both cases, as presented below.
Previous run of above #152 (comment) just hit a connection timeout with boreas.ouranos.ca that trickled down failing many tests.

22:43:28  notebooks/hummingbird.ipynb ............                                 [  8%]
22:43:35  notebooks-auth/test_thredds.ipynb ..........                             [ 15%]
22:43:51  pavics-sdi-master/docs/source/notebooks/WCS_example.ipynb .......        [ 20%]
22:44:00  pavics-sdi-master/docs/source/notebooks/WFS_example.ipynb ......         [ 24%]
22:44:08  pavics-sdi-master/docs/source/notebooks/WMS_example.ipynb ........       [ 30%]
22:44:10  pavics-sdi-master/docs/source/notebooks/WPS_example.ipynb ..........     [ 37%]
22:44:25  pavics-sdi-master/docs/source/notebooks/esgf-dap.ipynb .                 [ 37%]
22:44:25  pavics-sdi-master/docs/source/notebooks/jupyter_extensions.ipynb .       [ 38%]
22:44:31  pavics-sdi-master/docs/source/notebooks/opendap.ipynb .......            [ 43%]
22:44:35  pavics-sdi-master/docs/source/notebooks/pavics_thredds.ipynb ....F       [ 46%]
22:46:28  pavics-sdi-master/docs/source/notebooks/regridding.ipynb ............... [ 57%]
22:47:12  ...............                                                          [ 67%]
22:47:18  pavics-sdi-master/docs/source/notebooks/rendering.ipynb ....             [ 70%]
22:47:23  pavics-sdi-master/docs/source/notebooks/subsetting.ipynb ......          [ 74%]
22:47:34  finch-master/docs/source/notebooks/dap_subset.ipynb ..........           [ 81%]
22:47:42  finch-master/docs/source/notebooks/finch-usage.ipynb ......              [ 86%]
22:48:08  finch-master/docs/source/notebooks/subset.ipynb ...FF....FFFFF....FF     [100%]

tlvu · 2021-05-05T14:35:19Z

@nikola-rados I am pretty sure you will also need to do something like my PR for Ouranos custom config bird-house/birdhouse-deploy-ouranos#10 your side. Let me know once you are done so we can sync this merge.

nikola-rados · 2021-05-05T22:08:55Z

@tlvu I have updated our config.

tlvu · 2021-05-06T02:30:29Z

@tlvu I have updated our config.

@nikola-rados Thanks so I'll merge this during the day tomorrow so you can go-live manually if required.

update docker-compose file version to 3.4 To match bird-house/birdhouse-deploy#152, otherwise our deployment will not start.

Forgot to push during PR #152.

tlvu · 2021-05-06T19:12:02Z

Default logging retention should have been bumped to something more reasonable for prod. I forgot to push the bump here so bumping in PR #165

…on-to-something-more-reasonable-for-prod bump default log retention to 500m instead of 2m, more suitable for prod # Overview Bump default log retention to 500m instead of 2m, more suitable for prod Forgot to push during PR #152. ## Changes **Non-breaking changes** - Bump default log retention to 500m instead of 2m, more suitable for prod

employ docker-compose v3.4 and shared logging config

783ae3d

fmigneault requested review from huard, dbyrns and tlvu April 21, 2021 22:05

fmigneault mentioned this pull request Apr 21, 2021

Feature/dac 300 weaver compose #114

Merged

fmigneault added 2 commits April 21, 2021 18:44

remove unrequired and breaking 'volumes_from' with docker-compose v3

4387b01

fix other docker-compose version mismatches

55dc6b8

fmigneault mentioned this pull request Apr 21, 2021

Utility docker-compose optional components #153

Merged

dbyrns approved these changes Apr 22, 2021

View reviewed changes

huard approved these changes Apr 22, 2021

View reviewed changes

tlvu reviewed Apr 22, 2021

View reviewed changes

tlvu added a commit to bird-house/birdhouse-deploy-ouranos that referenced this pull request Apr 23, 2021

update docker-compose file version to 3.4

c773c95

To match bird-house/birdhouse-deploy#152.

update docs gitignore to only ignore build dir

577326a

tlvu mentioned this pull request Apr 26, 2021

update docker-compose file version to 3.4 bird-house/birdhouse-deploy-ouranos#10

Merged

tlvu approved these changes Apr 29, 2021

View reviewed changes

This was referenced Apr 29, 2021

Tracking CHANGES #157

Closed

add gitignore for build dir of docs #162

Merged

Merge remote-tracking branch 'origin/master' into docker-compose-update

9c6ca64

tlvu merged commit 0179962 into master May 6, 2021

tlvu deleted the docker-compose-update branch May 6, 2021 19:00

tlvu added a commit that referenced this pull request May 6, 2021

bump default log retention to 500m instead of 2m, more suitable for prod

f002bbc

Forgot to push during PR #152.

tlvu mentioned this pull request May 6, 2021

bump default log retention to 500m instead of 2m, more suitable for prod #165

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

employ docker-compose v3.4 and shared logging config #152

employ docker-compose v3.4 and shared logging config #152

fmigneault commented Apr 21, 2021 •

edited by tlvu

Loading

dbyrns left a comment

tlvu left a comment •

edited

Loading

tlvu Apr 22, 2021

fmigneault Apr 22, 2021

fmigneault Apr 22, 2021

tlvu Apr 29, 2021

fmigneault Apr 29, 2021

tlvu Apr 22, 2021

fmigneault Apr 22, 2021

tlvu Apr 29, 2021

tlvu left a comment

tlvu Apr 29, 2021

tlvu Apr 29, 2021

fmigneault commented Apr 29, 2021

fmigneault commented Apr 29, 2021

tlvu commented May 5, 2021

nikola-rados commented May 5, 2021

tlvu commented May 6, 2021

tlvu commented May 6, 2021

employ docker-compose v3.4 and shared logging config #152

employ docker-compose v3.4 and shared logging config #152

Conversation

fmigneault commented Apr 21, 2021 • edited by tlvu Loading

dbyrns left a comment

Choose a reason for hiding this comment

tlvu left a comment • 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

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

tlvu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmigneault commented Apr 29, 2021

fmigneault commented Apr 29, 2021

tlvu commented May 5, 2021

nikola-rados commented May 5, 2021

tlvu commented May 6, 2021

tlvu commented May 6, 2021

fmigneault commented Apr 21, 2021 •

edited by tlvu

Loading

tlvu left a comment •

edited

Loading