Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[docs-only] add log rotation to deploy examples #1377

Merged
merged 5 commits into from
Jan 23, 2021
Merged

[docs-only] add log rotation to deploy examples #1377

merged 5 commits into from
Jan 23, 2021

Conversation

wkloucek
Copy link
Contributor

Description

This PR adds log rotation for Docker logs to our deployment examples. This is helpful to prevent endless log accumulation on our (not yet available) test servers and failures which are caused by this.

Related Issue

  • Log accumulation already happend on ocis.owncloud.works and caused various errors when the disk was full.

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@update-docs
Copy link

update-docs bot commented Jan 21, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@wkloucek wkloucek requested a review from C0rby January 21, 2021 15:03
@C0rby
Copy link
Contributor

C0rby commented Jan 21, 2021

Do we really want to you the json driver instead of the local driver?
I'm not experienced so I don't know what's the best practice here but if I understood correctly this will wrap our json messages in another json object. Don't know why that would be useful.

@wkloucek
Copy link
Contributor Author

Do we really want to you the json driver instead of the local driver?
I'm not experienced so I don't know what's the best practice here but if I understood correctly this will wrap our json messages in another json object. Don't know why that would be useful.

Ok, switched to local driver. This has already log rotation enabled as default, so no need anymore fore the annoying yaml pointers to deduplicate config.

I wonder if we should leave it up to the docker host to configure the default logging driver properly (with log rotation)? But I worry that some people will not do this and having this configuration doesn't any harm. So I feel that we should add this.

C0rby
C0rby approved these changes Jan 22, 2021
View reviewed changes
Copy link
Contributor

@C0rby C0rby left a comment

Choose a reason for hiding this comment

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

Cool 👍

@wkloucek wkloucek merged commit eb634d2 into master Jan 23, 2021
@delete-merged-branch delete-merged-branch bot deleted the add_log_rotation_deploy_examples branch January 23, 2021 09:25
ownclouders pushed a commit that referenced this pull request Jan 23, 2021
@wkloucek
commit eb634d2
b66b4fc 
Merge: c2f154f 94c65c6
Author: Willy Kloucek <34452982+wkloucek@users.noreply.github.com>
Date:   Sat Jan 23 10:25:42 2021 +0100

    Merge pull request #1377 from owncloud/add_log_rotation_deploy_examples

    [docs-only] add log rotation to deploy examples
@wkloucek
Copy link
Contributor Author

@C0rby just ran into this: docker/compose#7420. Should be fixed with docker-compose 1.27.0 and up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@C0rby C0rby C0rby approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wkloucek @C0rby