-
Notifications
You must be signed in to change notification settings - Fork 31
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
fix: make enabling/disabling non-existent services not fail in check mode #153
fix: make enabling/disabling non-existent services not fail in check mode #153
Conversation
- Addresses linux-system-roles#146 - firewall_lib.py - check if service exists before running firewalld methods that would cause failure - fails if service does not exist and in diff mode, warns if in check mode and service does not exist - README.md - reflects changes and explains interaction with check mode - tests/tests_service.yml - add integration test case for adding non-existent services in check mode - tests/unit/test_firewall_lib.py - Mock necessary output from fw.config().getServiceNames()
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #153 +/- ##
==========================================
+ Coverage 53.39% 53.62% +0.23%
==========================================
Files 2 2
Lines 796 800 +4
==========================================
+ Hits 425 429 +4
Misses 371 371
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
ansible-lint: add |
[citest] |
Co-authored-by: Richard Megginson <richm@stanfordalumni.org>
Co-authored-by: Richard Megginson <richm@stanfordalumni.org>
Co-authored-by: Richard Megginson <richm@stanfordalumni.org>
Co-authored-by: Richard Megginson <richm@stanfordalumni.org>
Co-authored-by: Richard Megginson <richm@stanfordalumni.org>
Co-authored-by: Richard Megginson <richm@stanfordalumni.org>
You might find this useful - https://linux-system-roles.github.io/documentation/howto/working-with-ansible-jinja-code-and-filters.html - "How to solve some common ansible-lint issues" |
[citest] |
Make MockAnisbleModule.check_mode mutable in test cases
Should we add this in .github for all roles or am I missing something? |
yes, we should - I don't see any other way around this, unfortunately |
Great work, thank you so much! |
Enhancement:
Reason:
Better compliance with Ansible best practices for check mode (not failing in check mode, especially where they would not fail in diff mode)
Reason for this particular solution - We cannot track changes from previous check modes without overhauling how check mode is handled throughout the entire system role.
Result:
Undefined services being enabled or disabled will not result in failure while in check mode, but a warning will be displayed intended to prompt the user to confirm that the service is defined in a previous play, since the same action could result in failure when run in diff mode.
Issue Tracker Tickets (Jira or BZ if any):