Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add proxy support for registry cache #246

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

dergeberl
Copy link

@dergeberl dergeberl commented Aug 28, 2024

How to categorize this PR?

/area usability
/kind enhancement

What this PR does / why we need it:

This PR adds the possibility to set a proxy for a registry cache. The setting will be added with the HTTP_PROXY, HTTPS_PROXY and NO_PROXY as env in the statefulset.

We have the usecase where a registry is only reachable via a proxy. As it not easy/possible to set a proxy within containerd and also have the NO_PROXY configured to include all internal IPs/domains (also in cases where there are multiple proxies). Therefore we want to use the registry cache for that usecase.

I know that the registry cache gets in that use cases more critical, as this is the only way to get images for that registry (if only reachable without proxy). For that reason I will add an second PR to make it possible (optional) to scale the statefulset with the High Availability feature. #247

Which issue(s) this PR fixes:
N/A

Special notes for your reviewer:
N/A

Release note:

The registry cache now support optional proxy settings.

@gardener-prow gardener-prow bot added area/usability Usability related kind/enhancement Enhancement, improvement, extension labels Aug 28, 2024
@gardener-prow gardener-prow bot added cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 28, 2024
@gardener-prow gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 4, 2024
@ialidzhikov
Copy link
Member

@dergeberl, thanks for the PR and sorry for the delay! We will try to provide a review next week!

Copy link
Member

@ialidzhikov ialidzhikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dergeberl ,

Sorry for the delay for the review!
I posted a feedback.

Do you also have a setup where we could test the feature locally? For example a proxy running in-cluster? If you don't, we can try to invest in such setup.
I see we rather rely that the go clients in the Distribution project respect the corresponding env vars. I just wanted to see this working in example setup before we merge this.

pkg/apis/registry/v1alpha3/types.go Outdated Show resolved Hide resolved
pkg/component/registrycaches/registry_caches.go Outdated Show resolved Hide resolved
pkg/apis/registry/v1alpha3/types.go Outdated Show resolved Hide resolved
docs/usage/registry-cache/configuration.md Outdated Show resolved Hide resolved
docs/usage/registry-cache/configuration.md Outdated Show resolved Hide resolved
pkg/apis/registry/v1alpha3/types.go Outdated Show resolved Hide resolved
pkg/apis/registry/v1alpha3/types.go Outdated Show resolved Hide resolved
@gardener-ci-robot
Copy link

The Gardener project currently lacks enough active contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:

  • After 15d of inactivity, lifecycle/stale is applied
  • After 15d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 7d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Mark this PR as rotten with /lifecycle rotten
  • Close this PR with /close

/lifecycle stale

@gardener-prow gardener-prow bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 4, 2024
@timebertt
Copy link
Member

Sorry for the delay, @dergeberl will soon return from vacation and continue working on this PR.
/remove-lifecycle stale

@gardener-prow gardener-prow bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 10, 2024
@gardener-prow gardener-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 16, 2024
@dergeberl
Copy link
Author

Do you also have a setup where we could test the feature locally? For example a proxy running in-cluster? If you don't, we can try to invest in such setup.
I see we rather rely that the go clients in the Distribution project respect the corresponding env vars. I just wanted to see this working in example setup before we merge this.

Yes, we have tested it and are already using it.

@ialidzhikov ialidzhikov self-assigned this Oct 21, 2024
@ialidzhikov
Copy link
Member

Yes, we have tested it and are already using it.

Sure. I was rather wondering about how we can test it. But I will try to invest these days in a local setup to test it out.

Copy link
Contributor

gardener-prow bot commented Oct 31, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from ialidzhikov. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dergeberl
Copy link
Author

Sure. I was rather wondering about how we can test it. But I will try to invest these days in a local setup to test it out.

Sorry I got that wrong. I will have a look next week for a local setup. Let me know if you got a test setup already.

@gardener-ci-robot
Copy link

The Gardener project currently lacks enough active contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:

  • After 15d of inactivity, lifecycle/stale is applied
  • After 15d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 7d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Mark this PR as rotten with /lifecycle rotten
  • Close this PR with /close

/lifecycle stale

@gardener-prow gardener-prow bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 15, 2024
@dimitar-kostadinov
Copy link
Contributor

/remove-lifecycle stale

@gardener-prow gardener-prow bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 15, 2024
@dimitar-kostadinov
Copy link
Contributor

Adding steps to test the proxy scenario.

  1. In the local setup deploy registry cache extension and create a Shoot cluster:
make extension-up

k create -f example/shoot-registry-cache.yaml
  1. Deploy Squd proxy to the Shoot cluster in kube-system namespace.

  2. Edit Shoot and configure proxy for docker.io registry:

 % k -n garden-local edit shoots local

  - providerConfig:
      apiVersion: registry.extensions.gardener.cloud/v1alpha3
      caches:
      - upstream: docker.io
        volume:
          size: 500Mi
        proxy:
          httpsProxy: http://my-squid
  1. After shoot reconciliation make sure the registry cache is working and traffic is passing through the squid proxy:
% k -n kube-system logs my-squid-d96b984c7-mxjlk
...
1732259852.141    422 10.3.0.26 TCP_TUNNEL/200 4931 CONNECT registry-1.docker.io:443 - HIER_DIRECT/54.198.86.24 -
1732259854.690    361 10.3.0.26 TCP_TUNNEL/200 10197 CONNECT auth.docker.io:443 - HIER_DIRECT/54.236.113.205 -
1732259854.851    250 10.3.0.26 TCP_TUNNEL/200 39 CONNECT registry-1.docker.io:443 - HIER_DIRECT/54.198.86.24 -
1732259854.977   2404 10.3.0.26 TCP_TUNNEL/200 12313 CONNECT registry-1.docker.io:443 - HIER_DIRECT/54.198.86.24 -
1732259861.132   6440 10.3.0.26 TCP_TUNNEL/200 6391 CONNECT registry-1.docker.io:443 - HIER_DIRECT/54.198.86.24 -
1732259861.132   8990 10.3.0.26 TCP_TUNNEL/200 49961 CONNECT auth.docker.io:443 - HIER_DIRECT/54.236.113.205 -
1732259861.555   6978 10.3.0.26 TCP_TUNNEL/200 6353 CONNECT registry-1.docker.io:443 - HIER_DIRECT/54.198.86.24 -

% k logs registry-docker-io-0
...
10.3.0.1 - - [22/Nov/2024:07:17:31 +0000] "HEAD /v2/library/nginx/manifests/1.25.0?ns=docker.io HTTP/1.1" 200 0 "" "containerd/v1.7.18"
10.3.0.1 - - [22/Nov/2024:07:17:33 +0000] "GET /v2/library/nginx/manifests/sha256:b997b0db9c2bc0a2fb803ced5fb9ff3a757e54903a28ada3e50412cc3ab7822f?ns=docker.io HTTP/1.1" 200 1862 "" "containerd/v1.7.18"
time="2024-11-22T07:17:33.168908543Z" level=info msg="response completed" go.version=go1.22.4 http.request.host="10.4.210.83:5000" http.request.id=74ab8007-ae3a-474f-920d-5f1861c7c4b0 http.request.method=GET http.request.remoteaddr="10.3.0.1:59289" http.request.uri="/v2/library/nginx/manifests/sha256:b997b0db9c2bc0a2fb803ced5fb9ff3a757e54903a28ada3e50412cc3ab7822f?ns=docker.io" http.request.useragent=containerd/v1.7.18 http.response.contenttype=application/vnd.docker.distribution.manifest.list.v2+json http.response.duration="532.083µs" http.response.status=200 http.response.written=1862 instance.id=8f696df7-5fac-4aae-8a61-c8f56bf644bd service=registry vars.name=library/nginx vars.reference="sha256:b997b0db9c2bc0a2fb803ced5fb9ff3a757e54903a28ada3e50412cc3ab7822f" version=3.0.0-beta.1
time="2024-11-22T07:17:33.473614377Z" level=info msg="Adding new scheduler entry for library/nginx@sha256:f2ab27de75f97311b87e6287a14fad819302070a62f9b946ccf45e1fc6a508af with ttl=167h59m59.999997542s" go.version=go1.22.4 instance.id=8f696df7-5fac-4aae-8a61-c8f56bf644bd service=registry version=3.0.0-beta.1
...
10.3.0.1 - - [22/Nov/2024:07:17:34 +0000] "GET /v2/library/nginx/blobs/sha256:92ad4775570054c645678402c8b75eb489b8e05313c9ccd7867bb591266db4d8?ns=docker.io HTTP/1.1" 200 30062834 "" "containerd/v1.7.18"
time="2024-11-22T07:17:36.610049045Z" level=info msg="response completed" go.version=go1.22.4 http.request.host="10.4.210.83:5000" http.request.id=6e8acc99-8123-4427-9340-145098056e7a http.request.method=GET http.request.remoteaddr="10.3.0.1:30192" http.request.uri="/v2/library/nginx/blobs/sha256:92ad4775570054c645678402c8b75eb489b8e05313c9ccd7867bb591266db4d8?ns=docker.io" http.request.useragent=containerd/v1.7.18 http.response.contenttype=application/octet-stream http.response.duration=2.279087542s http.response.status=200 http.response.written=30062834 instance.id=8f696df7-5fac-4aae-8a61-c8f56bf644bd service=registry vars.digest="sha256:92ad4775570054c645678402c8b75eb489b8e05313c9ccd7867bb591266db4d8" vars.name=library/nginx version=3.0.0-beta.1
time="2024-11-22T07:17:36.610740087Z" level=info msg="Adding new scheduler entry for library/nginx@sha256:624cd930b6aa85205d147d21d2df46eb4dd013511dec330ee4351b554c0a2845 with ttl=167h59m59.999997583s" go.version=go1.22.4 instance.id=8f696df7-5fac-4aae-8a61-c8f56bf644bd service=registry version=3.0.0-beta.1
10.3.0.1 - - [22/Nov/2024:07:17:35 +0000] "GET /v2/library/nginx/blobs/sha256:624cd930b6aa85205d147d21d2df46eb4dd013511dec330ee4351b554c0a2845?ns=docker.io HTTP/1.1" 200 1400 "" "containerd/v1.7.18"
time="2024-11-22T07:17:36.610850878Z" level=info msg="response completed" go.version=go1.22.4 http.request.host="10.4.210.83:5000" http.request.id=6296aac8-6fe7-4601-be14-64bbabecbf91 http.request.method=GET http.request.remoteaddr="10.3.0.1:46793" http.request.uri="/v2/library/nginx/blobs/sha256:624cd930b6aa85205d147d21d2df46eb4dd013511dec330ee4351b554c0a2845?ns=docker.io" http.request.useragent=containerd/v1.7.18 http.response.contenttype=application/octet-stream http.response.duration=647.959875ms http.response.status=200 http.response.written=1400 instance.id=8f696df7-5fac-4aae-8a61-c8f56bf644bd service=registry vars.digest="sha256:624cd930b6aa85205d147d21d2df46eb4dd013511dec330ee4351b554c0a2845" vars.name=library/nginx version=3.0.0-beta.1

/lgtm

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Nov 22, 2024
Copy link
Contributor

gardener-prow bot commented Nov 22, 2024

LGTM label has been added.

Git tree hash: ed856326b83e8bc416b6092bba83098e771e7e2b

@ialidzhikov
Copy link
Member

/hold
until v0.12 is released

@gardener-prow gardener-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/usability Usability related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants