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

Reload/restart firewall handlers; switching to service commands instead of init.d/ #46

Closed
wants to merge 0 commits into from

Conversation

donatengit
Copy link

Hey,

Thanks a lot for all your work, it is really helpful.

Couple of patches might be useful:

  1. Reload/restart firewall handlers; switching to service commands instead of init.d/
  2. Ignore .DS_Store and .idea in git

Thanks

@@ -13,7 +13,19 @@

- name: restart network
nohup:
command: /etc/init.d/network restart
command: service network restart
Copy link

@Hurricos Hurricos Aug 7, 2022

Choose a reason for hiding this comment

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

NACK. service is an OpenWrt ashism from service.sh.

Unlike with systemd, where functionality has been intentionally brought into systemctl, here the functionality all rests within procd. service realizes this by manually parsing arguments for the busybox start-stop-daemon applet, which is less direct and is thus less expected to be stable.

Copy link
Author

Choose a reason for hiding this comment

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

I was looking at your comments back in 2020 to a similar pull request. There you'd preferred service.
Not a big deal though, I'll change that soon along with additional playbooks on my router. Feel free to reject this pull request

Copy link

Choose a reason for hiding this comment

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

Oh, that was the maintainer (@gekmihesg), not me :^)

I like the pull request; I also think there should be more handlers available. Firewall is a great one to start with.

Copy link
Owner

Choose a reason for hiding this comment

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

What I meant there was using the service module instead of the nohup module. The existing handlers will interrupt the connection between the Ansible controller and the device, because the network (or wifi) gets shut down and brought up again. Ansible will fail because of that.
In the firewall case, this is unnecessary. You can just use the service module like this:

- name: reload firewall
  service:
    name: firewall
    state: reloaded
  when: "'openwrt' in group_names"

This will give you much better feedback if something goes wrong. So my change request stays the same ;-)

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.

3 participants