Disable postgres and tar tasks when unused#26
Conversation
These tasks are only needed for the default backup prepare strategy. If you override it, you will probably want to use these 2 flags.
Know what? You can't self-reference variables in ansible. This is why I created some_var and _some_var.
| # Otherwise, switch them off when the template has changed. | ||
| _backups_role_sudoers_enabled: "{{ backups_role_sudoers_enabled | default(_template_is_default) }}" | ||
| _backups_role_postgresql_enabled: "{{ backups_role_postgresql_enabled | default(_template_is_default) }}" | ||
|
|
There was a problem hiding this comment.
I think this could seem a bit of black magic. Why don't we just throw a warning or an error when we detect that the default template is not being used?
There was a problem hiding this comment.
We don't want to throw an error when the default template is not being used. We want it to be overridable, that's ok. But at the same time, we need this feature not to enforce postgres to be installed and .
An alternative to this "black magic" is to just give the flags and enforce to user of the role to set them to false (don't run postgres) if they use another script template to prepare the backups. But in my opinion, the default behavior should be a bit smarter than that.
There was a problem hiding this comment.
I think that including postgres tasks by default is a bit arbitrary since it just comes from what we started to backup.
I also would decouple sudoers and postgresql tasks, they shouldn't share the same meaning of "default".
I'm starting to think that this role should do nothing by default and that the user should enable the things that she needs to backup. What do you think?
There was a problem hiding this comment.
I agree. I don't like the idea of assuming what users want to backup. By now I would just merge it, as we need it for opencell, but I would give it more time to think about it. Maybe placing one taskfile for postgres and one extra for tar? And then from the importer playbook "import tasks from" those taskfiles...
There was a problem hiding this comment.
OK, let's create a Trello card and an issue here in the repo and maybe a comment in the code. I think we should address this issue ASAP. Thanks!
2 participants
This feature is needed for playbooks that don't have postgres install in the host (at least directly, not inside container), and still want to use this role.
If you modify the
backups_role_script_prepare_templatevar, then it means that you are not usingthis script template and therefore we don't need to create postgres roles, etc.
and you may not need sudoer permissions for tar. Therefore, we disable
the related tasks by default. However, if you still would like to enable those tasks,
set to true the corresponding variables:
backups_role_postgresql_enabledbackups_role_sudoers_enabledDon't merge until #25 is merged. Otherwise it can overwrite some changes we didFunny enough, I did it in the reverse order and I had no problem. ough...