Stop proxy scheduler on system exit

[dimitar-kostadinov]

For pull through cache registry with configured TTL, a scheduler responsible for blobs garbage collection is started here: registry/proxy/proxyregistry.go#L109, but its Stop method is never invoked.

Scheduler state is written to scheduler-state.json if a state change is detected every 5 seconds.
When terminating the registry process, this can result in an inconsistent/corrupted scheduler-state.json:

• recent state changes are not saved at all
• scheduler-state.json has been truncated, but no new content has been written

This PR explicitly invokes scheduler Stop method on SIGINT & SIGTERM to ensure that scheduler state is preserved.

Fixes #2374

[milosgajdos]

Unless I've overloooked something this LGTM. PTAL @thaJeztah

[thaJeztah]

Slightly wondering if we must define our own interface for this, or if io.Closer (same signature) would do 🤔

However, looking at the changes in this PR, it looks like we're differentiating the "proxy-registry" here. Usually I'm for "small interfaces", but I did notice there's a TODO for the Repository interface as a whole, so wondering if we should go that direction instead, and expand the Repository interface to require a Close() method?

distribution/registry.go

Lines 116 to 118 in bc6e81e

	// TODO(stevvooe): Must add close methods to all these. May want to change the
	// way instances are created to better reflect internal dependency
	// relationships.

[milosgajdos]

Slightly wondering if we must define our own interface for this, or if io.Closer (same signature) would do 🤔

I was thinking about this exactly, but I figured at the time it'd be semantically confusing 🤷‍♂️

[ialidzhikov]

Slightly wondering if we must define our own interface for this, or if io.Closer (same signature) would do 🤔

I agree with @milosgajdos. I also prefer to do not use io.Closer as the registry is not an IO resource, it is a different abstraction.

---

@thaJeztah , thanks for noticing the TODO. Actually, we don't have a strong opinion on the topic. We are at most familiar with the proxy registry codebase and we are using the proxy scenario only. With this PR our main goal is to fix #2374 and not do design/redesign the Repository interface. If you as a maintainer have a strong opinion that this should instead added in the Repository interface instead, we can do it. I think we are not that familiar with all of the existing codebase to decide whether the other Repository implementations need to close as well. The proxyRegistry needs such Close method for sure to be able to stop the scheduler (which saves the scheduler-state.json file).

[Jamstah]

This now conflicts because #4338 got merged.

Should probably move the OnExit call from a defer into the new registry shutdown method.

[milosgajdos]

@dimitar-kostadinov can you rebase

[Jamstah]

Can we now move this into registry.Shutdown so its with other shutdown actions?

I expect if an embedded registry is shutdown, closing this would also be appropriate.