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

Stop daemonizing #14

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

leoplo
Copy link
Contributor

@leoplo leoplo commented Mar 31, 2022

No description provided.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@leoplo leoplo force-pushed the stop_daemonizing branch 2 times, most recently from ef28219 to 5d520b7 Compare April 4, 2022 13:05
@pedrofran12 pedrofran12 force-pushed the master branch 3 times, most recently from 4070f07 to 5a45b31 Compare April 10, 2022 21:49
@leoplo leoplo force-pushed the stop_daemonizing branch from 5d520b7 to c5995ae Compare May 3, 2022 13:05
@pedrofran12
Copy link
Owner

pedrofran12 commented May 26, 2022

Hi Léo-Paul,

Thank you again for your PR and sorry for the delay.

So far I have tested your PR and the start of the process looks ok in the foreground process as well on the background with "&" and "setsid".
The procedure for "systemd-run" or "systemctl" didn't work on my setup. Not sure if it is because I am using on a docker container... I copied the file to the directory you mention but I still get errors running those commands:
image
image

Also noticed that command "-instances" is broken:
image
It looks like client_socket is returning None and causing an exception.

I will also send some comments to the code. You moved the code in some places and wrote for loops in a different way but the code looks ok.

Thank you again :)

@pedrofran12 pedrofran12 added the enhancement New feature or request label May 30, 2022
@leoplo leoplo force-pushed the stop_daemonizing branch 6 times, most recently from bf3c47a to d24bf42 Compare October 25, 2022 15:57
Return a tuple for vrfs instead of a list
On stop command call Main.stop() runs
`remove_interface("*", pim=True, membership=True, ipv4=True, ipv6=True)`.

Therefore if a PIM interface was added for network interface eth0, the call
`remove_interface('eth0', True, True)` will evaluate the condition
`membership and not membership_interface` as True and the PIM interface will
never be removed.

Furthermore
`remove_interface("*", pim=True, membership=True, ipv4=True, ipv6=True)` will
do calls like `remove_interface('lo', True, True)` then calling
`remove_virtual_interface('lo')` and the line
`index = self.vif_name_to_index_dic.pop(interface_name, None)` will cause a
KeyError when running
`del self.vif_name_to_index_dic[self.vif_index_to_name_dic[index]]` or
`mif_index = self.vif_name_to_index_dic.pop(interface_name)`.
As running stop and then start is the default behavior for daemon managing tools
restart command, the current restart command looks pointless.
exit class can be used to handle signals and do proper cleaning before killing
a process
Run pim-dm in foreground
Exit with an error if config command fails because 'PyYAML' is missing.
Wait for thread loop to return when exiting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants