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

Fixes #32175: Allow toggling task backup when cleaning them up #922

Merged
merged 1 commit into from
Mar 29, 2021

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Mar 23, 2021

Changes the default behavior to disable task backup when enabling
the task cleanup cron job. Users who still want backups kept
on disk will need to set the parameter to true.

@ehelms
Copy link
Member Author

ehelms commented Mar 23, 2021

Configuration for task-backups are also guided by https://github.com/theforeman/foreman-tasks/blob/master/config/foreman-tasks.yaml.example which is laid down by the RPM and not under Puppets control. This focuses on making it so that the cron job does not by default create backups as the environment variables override the base config. If a user chooses to run the cleanup by hand they have to adjust any specific configuration differences they would want.

Putting https://github.com/theforeman/foreman-tasks/blob/master/config/foreman-tasks.yaml.example under puppet control seemed like a much bigger change and handling the :rules: section as tricky given hashes are never fun and are further complicated when translated to installer parameters.

@ehelms
Copy link
Member Author

ehelms commented Mar 23, 2021

Needs #923

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Small nit, otherwise 👍

manifests/plugin/tasks.pp Outdated Show resolved Hide resolved
@ehelms
Copy link
Member Author

ehelms commented Mar 23, 2021

@adamruzicka Take a look if you have time, in case you have concerns around this approach and reasoning

@adamruzicka
Copy link
Contributor

I'm fine with this approach, +1 from me

Changes the default behavior to disable task backup when enabling
the task cleanup cron job. Users who still want backups kept
on disk will need to set the parameter to true.
@ehelms ehelms requested a review from ekohl March 23, 2021 17:16
@ekohl
Copy link
Member

ekohl commented Mar 29, 2021

I'm debating whether this is an enhancement or breaking change. It does change behavior so technically it would be. On the other hand, it is a plugin. I also think the next release should be major anyway since I want to finish #862.

@ehelms
Copy link
Member Author

ehelms commented Mar 29, 2021

I'm debating whether this is an enhancement

👍

@ehelms ehelms merged commit f69c9f1 into theforeman:master Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants