-
Notifications
You must be signed in to change notification settings - Fork 3
sync planet-winter changes #29
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
base: develop
Are you sure you want to change the base?
Conversation
get up to date with latest CoffeITWorks master to add zfs_script
fix bug introduced in latest updates
update molecule to v3
update tests and support with debian 10
…S and set sbindir according to LSB
| @@ -1,16 +0,0 @@ | |||
| ******* | |||
| Install | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@planet-winter why is this file deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no clue... ;-)
| shell: cd {{ download_dir }}/{{ burpsrc }} && make install creates={{ burp_bin_path }} | ||
| notify: | ||
| - restart burp server | ||
| - restart burp server systemd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During upgrades, it is required
@planet-winter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During upgrades, it is required
Yes!
However ansible could not find the given handlers as there is no handlers directory in the client role. If this is in the server role only, the client role only runs together with the server role. -> should add a handlers dir and handlers main.yml
| @@ -0,0 +1,8 @@ | |||
| --- | |||
|
|
|||
| - name: freebsd | install logrotate | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like you forgot to call this file from tasks/main.yml ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! It is obsolete anyway as it does the same as the freebsd.yml which is called for all freebsd versions.
I probably was on autopilot, as I usually use a similar structure to this to flexibly get custom variables. It is easy to enhance:
- name: gather os specific variables
include_vars: "{{ item }}"
with_first_found:
- files:
- "{{ ansible_distribution|lower }}-{{ ansible_distribution_version }}.yml"
- "{{ ansible_distribution|lower }}-{{ ansible_distribution_release }}.yml"
- "{{ ansible_distribution|lower }}-{{ ansible_distribution_major_version }}.yml"
- "{{ ansible_distribution|lower }}.yml"
- "{{ ansible_os_family|lower }}.yml"
- defaults.yml
paths:
- ../vars
skip: True
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just seen you are using this already.
Same could be done with tasks though.
| src: "zfs_script" | ||
| owner: root | ||
| group: "{{ root_group }}" | ||
| mode: 0755 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when statment missing, only copy when burp_client_backup_script is true, using | bool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct!
| {% endfor %} | ||
| {% endif %} | ||
|
|
||
| {% if burp_client_backup_script is defined %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should be burp_client_backup_script_bool with false by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! sounds sensible.
However please note I'm having issues using ZFS backup over FIFO with this script. It's a long time ago would need to look into this to be more precise. Maybe not of use for you.
| @@ -1,35 +0,0 @@ | |||
| --- | |||
| # file: vars/Debian.yml | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleting this file will break support to others Debian based distributions like most Ubuntu versions.
Best approach is:
Use Debian.yml to support all the latest Debian based distributions.
Add only Debian-Mayorversion.yml or Ubuntu-Mayorversion.yml in case when you need to add compatibility to older versions that have different package names for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm. Lousy of me to delete it ;-) makes no sense.
I usually keep a file for each mayor version anyway in my roles to be explicit for what OS versions it works and I have it tested with.
| @@ -1,5 +1,5 @@ | |||
| --- | |||
| # file: vars/Debian-8.yml | |||
| # file: vars/Debian.yml | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File is still Debian-8.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
| - libc6 # from package dependencies | ||
| - libacl1 # from package dependencies | ||
| - libssl1.0.0 # from package dependencies | ||
| - libssl1.1 # from package dependencies (debian9,debian10,Ubuntu18+) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debian-8 will not work with this library
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok! true.
|
ERROR: Idempotence test failed because of the following tasks:
|
|
manual_delete should be mode: '0750' to sync with burp_server role. |
Is this due to the missing binary to delete in the new |
@planet-winter
Do you think we should sync our reports?
I always prefer to be in sync to not have duble work.
There are some changes that I would prefer to do different, I will explain on merge request.
refs #28