Allow disabling redis in registry

[roblabla]

There are bugs in the registry with regards to cache invalidation when deleting an image that can lead to some really problematic issues. See distribution/distribution#4269.

To work around that problem, add a new field to allow disabling redis in the registry.

[Vad1mo]

@roblabla, this seems like a serious bug in Distribution. I approve this PR.
Were you able to verify, first hand, that this solved the problem.
The reason I am asking is that Harbor has its own logic to garbage collect and is not relying on distribution.
But it might well be that we have the same blind spot as docker distribution.

[Vad1mo]

There are some errors in the CI

[Vad1mo]

@roblabla did open an issue in harbor, so we can track that issue in parallel. If not, can you do that.

[roblabla]

@roblabla did open an issue in harbor, so we can track that issue in parallel. If not, can you do that.

Done: goharbor/harbor#19988

Were you able to verify, first hand, that this solved the problem.
The reason I am asking is that Harbor has its own logic to garbage collect and is not relying on distribution.
But it might well be that we have the same blind spot as docker distribution.

As far as I can tell, harbor reuses distribution's garbage collector logic: https://github.com/goharbor/harbor/blob/main/src/registryctl/api/registry/manifest/manifest.go#L86

Anyways, I haven't validated this exact patch, but I did validate that purging the redis cache fixed the problem and allowed to reupload the image in harbor. So disabling redis was the obvious next step.

I'll look into the CI failure tomorrow (I'm not super familiar with helm ^^).

[Vad1mo]

btw there is a patch in distribution addressing this, distribution/distribution#3323 you might give it a try.

[zyyw]

Hi @roblabla , in this issue distribution/distribution#4269, to reproduce it by essentially running the following command:

# And run garbage collection. This will actually delete the data from the blobstore. However, the cache won't be cleaned!
./bin/registry garbage-collect config.yaml -m

Is it possible due to the implementation of ./bin/registry garbage-collect? Will you be able to reproduce it through an e2e scenario of Harbor? For example:

• deploy a Harbor instance with GC_TIME_WINDOW_HOURS set to 0
• push an image to Harbor
• delete the image
• run GC in Harbor
• push the image to Harbor again
• docker pull the image

[Vad1mo]

@zyyw here is the related issue on harbor side, with reproducible steps.

Related goharbor/harbor#19988

[roblabla]

Finally got around to getting this done. I can now confirm that this version works - I have it deployed and running.

[zyyw]

discussing it here. closing it now.
redis is a required component for distribution and GC does rely on redis.

[roblabla]

I'm... confused by this comment. Redis is an optional component of distribution, it's solely used as a cache to avoid S3 latency, and can be disabled in the config, as evidenced by this PR working... In my use-case, the performance hit of disabling redis is negligible. And on the distribution side, GC does not rely on redis - in fact that's the whole issue, distribution's GC ignores the redis, causing these issues.

So I'm not sure I understand the rationale here?

[Vad1mo]

discussing it here. closing it now. redis is a required component for distribution and GC does rely on redis.

I would like to reopen this PR.

1. #19988 is correctly addressing the problem.
   And this is obvious that the problem can't be resolved in a reasonable amount of time as it depends on docker distribution.

2. This is a fix for an issue, that can't be fixed otherwise.

3. Looking at this Helm chart in retrospect, it is not correctly designed (Most of the charts unfortunate aren't). The Configuration options are only exposed via variables without offering an escape hatch, to bypass values and just set whatever is needed directly in the config file.

[github-actions]

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

[roblabla]

Still relevant.

[roblabla]

It's been a while, and the issue is still not fixed upstream. Could this get merged in the interim so a workaround is at least available?

[roblabla]

Bump?

[github-actions]

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

[roblabla]

This is the last time I will bump this PR for the stalebot's sake. It's been half a year since I submitted this simple 10-line change. The upstream bug is still present, with no fix in sight. This PR provides a working and simple workaround to the problem. I do not understand what could be controversial about it.

I'll put it frankly: it feels insulting to have a bot claim my PR is "stale" and auto-close while it's waiting for review by the repo maintainers. I understand wanting to get rid of old PRs whose authors have disappeared, but this is just not a good solution to this problem.

If this PR gets closed by stalebot in whatever the timespan is, then so be it, I'll assume Harbor does not care about external contributions, and move on.