troglobit / finit

Fast init for Linux. Cookies included
https://troglobit.com/projects/finit/
MIT License
632 stars 63 forks source link

finit doesn't properly reload config definitions when using initctl reload <service> #263

Closed hongkongkiwi closed 2 years ago

hongkongkiwi commented 2 years ago

I'm doing some development and testing on the device using finit.

I've found that when I change the bin that a service is pointing to, e.g. /tmp/mybin and change to something like /usr/bin/mybin issuing a initctl reload doesn't correctly change the service definition or atleast it appears to still refer to the old command in: initctl status

This isn't a problem as I just reboot after major service changes (conditions or command), but I think it's worth adding some additional tests for these cases there's definately a few minor bugs here.

troglobit commented 2 years ago

OK. Do you set name: and :id on these services? If not, the only handle Finit has to track the uniqueness is the command name.

Would be great if you could include the before and after shots of the config stanza.

hongkongkiwi commented 2 years ago

Yes I'm using name and id but it seems to not matter if I do or don't.

Perhaps this is not true, but logically I expect that initctl reload reloads all services and initctl reload <service> reloads that specific service.

task [12345789] name:sys :test env:/tmp/conftest /tmp/test -- test123

If I do a initctl reload it appears to pick up the config change to bin as well as reloading all other services, however if I do an "initctl reload sys:test" then it doesn't.

e.g. my config is now:

task [12345789] name:sys :test env:/tmp/conftest /tmp/test "$TEST" -- test123

But in status is still appears as the old config no matter how many times I do: initctl reload sys:test only initctl reload works.

I did find before that even a general reload didn't update (I just ended up rebooting), but I can't replicate that situation, so lets pretend it didn't exist until I can find a way to reproduce it, it may even be I was running to the above without realising it.

troglobit commented 2 years ago

Yeah this is one of those unfortunate anachronisms that snuck in when we developed the initctl tool.

Finit tracks changes to .conf files but it cannot know, before having read all changed files, where a service is set up (it could've moved from one file to another), so Finit cannot just reload a single service's .conf file. What initctl reload foo does is to mark the service as "dirty" (as if it had been changed) without reading any .conf file, and then spin the state machine to see if the service should be stop/started or sent SIGHUP. Essentially initctl reload foo is the same as initctl restart foo ~except that when marking the service dirty, all other dependent services are also marked dirty~ Update: and is only kept for backwards compatibility with other tooling.

In conclusion, only a plain initctl reload activates changes to .conf files (this is also what init q does internally). You can also send SIGHUP to PID 1 to achieve the same thing, only the std. caveat to UNIX signals apply, although it is sometimes very handy since they are asynchronous.

troglobit commented 2 years ago

Maybe this command should be dropped from the documentation and online help? As I mentioned, unfortunately we have to keep it to not break other tooling built around initctl, but if we remove it from the docs it's at least not confusing other users.

~I'll check with the other tooling, maybe we can check for -b as well, and if that is not set and reload has an argument we can give an "invalid command argument" error message or something.~ Update: nope, it's used as restart without any options.

hongkongkiwi commented 2 years ago

hmmm, I use initctl reload to send a reload signal to some services. It's helpful because then I don't need to worry about whether it supports HUP or not (finit will take care of hup or restart).

Perhaps it might be better to have a named command such as initctl hup <service_name> and keep initctl reload undocumented? the hup command could specify that it sends a hup signal or restarts depending on what kind of service it is.

(What happens if you send a reload to a task?)

Alternatively, we could make initctl reload to be something like initctl reconfigure? hah but reload is so nice to type and simple.

troglobit commented 2 years ago

Yeah I think that was the original use-case also for us (don't have to know what's supported).

I agree, we could expose the same functionality with another name. Dunno about hup, worry that folks may confuse it with signal HUP ...

(Dunno, and now you've got me worried, I'll try to investigate on the train to work :)

Nah, no more renaming stuff. We have what we have, let's try to tie up loose ends. I'd very much like to get the release out.

troglobit commented 2 years ago

Reloading a task allows it to run again in the same runlevel. So that was a bit of a surprise, but this is where we are now, apparently :rofl:

OK, I'll just update the docs and add a big disclaimer about the corresponding Finit .conf not being reloaded.

Thank you for bringing this to my attention!

troglobit commented 2 years ago

There, both the README and initctl(8) manual page updated.