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.

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

DPE-1799 Add rotation of mysqlrouter logs #143

Merged
merged 14 commits into from
Oct 25, 2023
Merged

Conversation

shayancanonical
Copy link
Contributor

@shayancanonical shayancanonical commented Oct 17, 2023

Issue

We do not currently have rotation of mysqlrouter logs

Solution

  • Add logrotate config to the workload container
  • Use pebble on the workload container to run a logrotate dispatcher script (which calls logrotate with the above config)
  • Add integration test

TODO

Attempt to run logrotate as non-root user

src/kubernetes_logrotate.py Show resolved Hide resolved
tox.ini Show resolved Hide resolved
metadata.yaml Show resolved Hide resolved
scripts/logrotate_dispatcher.py Outdated Show resolved Hide resolved
src/kubernetes_logrotate.py Outdated Show resolved Hide resolved
src/rock.py Outdated Show resolved Hide resolved
src/rock.py Outdated
)
layer = ops.pebble.Layer(
{
"summary": "Logrotate dispatcher layer",
Copy link
Contributor

Choose a reason for hiding this comment

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

question: will this override the summary for mysql router service since they're using the same layer?

also to confirm, does override:replace within the service only override the logrotate service?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/canonical/pebble#layer-specification indicates it only overrides the service with the same name. i ran a small test to confirm this

Copy link
Contributor

Choose a reason for hiding this comment

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

what about the summary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure what you mean to ask. a summary is a field of the service object, and thus gets overwritten when replace: override is specified

Copy link
Contributor

Choose a reason for hiding this comment

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

is it part of the service? it looks like it's part of the layer—and it seems like it will override the summary from the layer with the mysql router service

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to be honest, i don't think the layer summary is overwritten/persisted in that way.

the following is the pebble plan:

root@mysql-router-k8s-0:/# pebble plan 
services:
    logrotate_executor:
        summary: Logrotate executor
        startup: enabled
        override: replace
        command: python3 /logrotate_executor.py
        user: mysql
        group: mysql
    mysql_router:
        summary: MySQL Router
        startup: enabled
        override: replace
        command: mysqlrouter --config /etc/mysqlrouter/mysqlrouter.conf
        user: mysql
        group: mysql

there is no way to inspect the pebble layers

Copy link
Contributor

Choose a reason for hiding this comment

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

if it isn't persisted, should we be writing it at all?

Copy link
Contributor

Choose a reason for hiding this comment

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

no way to inspect the pebble layers

I think there might be in the filesystem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is possible that pebble keeps track of the summary of the layers, but it is not exposed to the user with any of the pebble commands. Either way, I removed the layer summary in 86e8516

src/logrotate.py Outdated Show resolved Hide resolved
src/kubernetes_logrotate.py Show resolved Hide resolved
src/kubernetes_logrotate.py Outdated Show resolved Hide resolved
Copy link
Contributor

@carlcsaposs-canonical carlcsaposs-canonical left a comment

Choose a reason for hiding this comment

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

looks great!

src/logrotate.py Outdated Show resolved Hide resolved
src/kubernetes_logrotate.py Outdated Show resolved Hide resolved
Copy link
Contributor

@carlcsaposs-canonical carlcsaposs-canonical left a comment

Choose a reason for hiding this comment

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

nits

src/logrotate.py Outdated Show resolved Hide resolved
src/logrotate.py Outdated Show resolved Hide resolved
@shayancanonical shayancanonical merged commit 1a1d06a into main Oct 25, 2023
8 checks passed
@shayancanonical shayancanonical deleted the feature/log_rotation branch October 25, 2023 19:01
github-actions bot added a commit to carlcsaposs-canonical/mysql-router-k8s-operator that referenced this pull request Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants