Reload TALs on SIGHUP

[hellerve]

This PR is a first attempt at handling SIGHUP (referencing #200) using crossbeam channels and signal-hook, which seems to be the current best practice. It allows us to get rid of IdleWait, which is nice. I’m not sure whether the threading model was what you were thinking of, which is why I only added the SIGHUP handler for now (though extending it should be fairly simple now!).

Let me know whether this is going in the right direction!

Cheers

[hellerve]

This seems to break the Windows build, somewhat unsurprisingly (I didn’t know Windows was supported TBH). Will take a look!

[hellerve]

Okay, this seems to have fixed it!

[partim]

Ah, right. That was the reason I created IdleWait in the first place: I wanted it as a wrapper to do this correctly for different OSes. #201 has a pointer for how to do this in Windows.

[hellerve]

For now I stubbed it out on Windows and it “works”, but I can try and figure out how to make it work across all platforms (but likely not anytime soone)!

[partim]

Leaving Windows for later is completely fine. I don’t think there’s too many people using Routinator as a Windows service, anyway.

However, how do you feel about wrapping the functionality into IdleWait in preparation for that? This could then later also be expanded to serve as a hook-up point for other control channels.

[hellerve]

we could do that, however then I’d maybe move to name it HandleSignals or similar, since it doesn’t really just wait idly anymore :) What do you think?

[partim]

Isn’t the main validation thread still sitting there idle waiting for something arrive in the channel or timing out? If so, the name is still fine. I think I mostly just stuck Idle on there because Wait was too generic. Admittedly, I when rereading the code I though it somehow came from tokio, so maybe it still is too generic. RefreshWait, maybe?

[hellerve]

@partim I forgot to notify you again but I moved the logic to SignalWait as requested!

[partim]

Ah, sorry, now I messed it up …

I think I would prefer if SignalWait just returned an enum detailing what to do next and then the actual reload happens in Server::run. This also has the advantage that SignalWait::wait doesn’t need repository and config anymore.

It also should be possible to have Repository::reload_tals instead of creating a whole new repository? This would also make it possible to just keep the current TALs if reloading fails. Not sure we want that, though.

[hellerve]

That sounds sensible.

About Repository::reload_tals: we can definitely do that! I did it the “brutal” way because you seemed to favor it in #200, but I can definitely try to do it a little less destructively!

[partim]

Ah, good point. That was about full reload including the config and everything. Maybe that should happen on SIGHUP? (That would also include shutting down the RTR and HTTP listeners so needs to be a bit further out.)

Reloading TALs is slightly less intrusive – it is basically an attended re-validation that therefore is okay to fail (that was the reason for not reloading TALs on every validation run). Maybe that should actually be wired to SIGUSR1?

[hellerve]

I shuffled the logic around a little bit; was that what you were thinking? If so, I‘ll change the signal type.

[partim]

Jupp, that’s exactly it.

[hellerve]

Alright, I just rebased and changed the signal. What do you think?

[partim]

Looks good. I’d like to see the CI actions run on your last commit but somehow it doesn’t seem to want to run them anymore.

[hellerve]

Yeah, I saw. I’d be happy if it did that, too, because I didn’t test the latest changes for Windows at all.

[partim]

Hm, maybe try adding a trivial commit now after the force-push?

[hellerve]

Seems like we’re good now!

[partim]

Awesome! Thank you for the effort and sorry for being picky.

[hellerve]

No problem, I’m happy it turned out to your liking!