Caddy: onevent bug: need to deregister previous hooks after graceful restart

[UladzimirTrehubenka]

Observed with CoreDNS that uses latest https://github.com/mholt/caddy/tree/master/onevent

On start with Corefile:

.:53 {
    forward . 8.8.8.8:53
    errors
    on startup touch /tmp/ready
}

In logs:

[INFO] Blocking Command "touch /tmp/ready" with ID 0b73050f-c09b-4188-9df3-422fdec0bf5b

Change Corefile to:

.:53 {
    forward . 8.8.8.8:53
    errors
    on startup touch /tmp/ready2
}

and make graceful restart: killall -USR1 coredns
In logs:

[INFO] Blocking Command "touch /tmp/ready" with ID 0b73050f-c09b-4188-9df3-422fdec0bf5b
[INFO] Blocking Command "touch /tmp/ready2" with ID de1d2119-8822-41a5-92eb-f9d0b10d558b

[mholt]

Pinging @elcore. Something that will help with this is a change in #2015, which introduces instance-scoped storage. You can "store" the event callbacks in there, which will be disposed of when the instance is disposed of (which happens during a reload: a whole new instance is made in its place).

This is what I am referring to: https://github.com/mholt/caddy/blob/a03eba6fbc2770ef6bbe7defa184ec4a6817f0c2/caddy.go#L116-L122

And this is how to use it in the setup function - to get: https://github.com/mholt/caddy/blob/a03eba6fbc2770ef6bbe7defa184ec4a6817f0c2/caddytls/setup.go#L52

And to set:

https://github.com/mholt/caddy/blob/a03eba6fbc2770ef6bbe7defa184ec4a6817f0c2/caddytls/setup.go#L55

That should be of some help!

[elcore]

Hi @mholt,

Unfortunately, I do not have enough time to work on this :(

I kindly ask you or somebody else to work on this.

P.S.: I am not sure if a instance-scoped storage should be implemented in onevent, as onevent registers an EventHook, which is currently stored at https://github.com/mholt/caddy/blob/master/plugins.go#L42

[mholt]

@elcore You have a point; not all events correspond to a particular instance. We will need a way to clear out the relevant event hooks when an instance is disposed of...

[elcore]

Hello @UladzimirTrehubenka,

according to my tests, my PR should fix your issue, it would be nice if you would try it for yourself :)

[UladzimirTrehubenka]

I have tested - PR fix the issue.