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

Make firewalld operations idempotent #173

Merged
merged 5 commits into from
Oct 4, 2023
Merged

Make firewalld operations idempotent #173

merged 5 commits into from
Oct 4, 2023

Conversation

sanatsathaye
Copy link
Contributor

I haven't worked much with ansible.posix.firewalld before, so let me know if there's something which needs changing. Think I've translated it over correctly though.

Copy link
Collaborator

@codyro codyro left a comment

Choose a reason for hiding this comment

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

We should add a requirements.yml file with the ansible.posix collection as well.

---
collections:
  - name: ansible.posix

lemmy-almalinux.yml Show resolved Hide resolved
@sanatsathaye
Copy link
Contributor Author

Sorry for the delay I've been very busy IRL with work and exams, I'll resume working on this some time next week

@codyro
Copy link
Collaborator

codyro commented Sep 30, 2023

No rush! Thank you for your time on improving these playbooks and the Lemmy ecosystem as a whole!

@sanatsathaye
Copy link
Contributor Author

No problem, need a good reddit alternative 😄
Btw I suppose I'll have to put instructions to run requirements.yml in the readme somewhere?

With immediate:true in ansible.posix.firewalld we don't need reload.

Thanks @codyro
@codyro
Copy link
Collaborator

codyro commented Oct 3, 2023

I suppose I'll have to put instructions to run requirements.yml in the readme somewhere?

If you don't mind, that'd be fantastic. While doing that, could you add that these playbooks require Ansible >= 2.10? We ran into an issue with notify not being supported in blocks in older versions (#177).

Thank you!

@ticoombs ticoombs added this to the 1.3.0 milestone Oct 4, 2023
@sanatsathaye
Copy link
Contributor Author

While doing that, could you add that these playbooks require Ansible >= 2.10? We ran into an issue with notify not being supported in blocks in older versions (#177).

I believe it's possible to use ansible itself to check which version is running the playbook using {{ ansible_version }} -- I can do another PR to add that into lemmy.yml and lemmy-almalinux.yml, and add the requirement in the readme as well if that's alright. Don't think it should be needed for the uninstall playbook.

Btw, should we be tying down the collections in the requirements.yml to any specific version number? Just realised I'll need to add community.docker as well as that's part of the Ubuntu install playbook, so I can point that as well as ansible.posix to the current latest version of both if that is required. If not, then I'll just add as is.

@dessalines dessalines merged commit d7be6c3 into LemmyNet:main Oct 4, 2023
@sanatsathaye sanatsathaye deleted the firewalld_idempotency_changes branch December 3, 2023 07:26
snowe2010 pushed a commit to programming-dot-dev/lemmy-ansible that referenced this pull request Jan 30, 2024
* Use firewalld module instead of command module

* Reload firewalld with systemd module

* Convert firewalld reload to a handler

* Explicit firewalld reload is not needed

With immediate:true in ansible.posix.firewalld we don't need reload.

Thanks @codyro

* Add requirements.yml
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.

4 participants