Skip to content

aws: Add support for using ECR as pull-through image cache#16593

Merged
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
rsafonseca:containerd_ecr_mirrors
Jul 29, 2025
Merged

aws: Add support for using ECR as pull-through image cache#16593
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
rsafonseca:containerd_ecr_mirrors

Conversation

@rsafonseca
Copy link
Copy Markdown
Contributor

@rsafonseca rsafonseca commented May 30, 2024 •
edited
Loading

This PR introduces a simple way to enable using ECR as a Pull-through image cache, without having to mutate images on the cluster using tools like Kyverno.

Containerd already has support for specifying registry mirrors in kops, but since ECR uses short lived tokens, it's not trivial (or even impossible without adding a few extra hacks on top of it) to use it as a pull-through cache.

This PR also bumps the ecr-credential-provider binary, which before version 1.29.0 specifically tried to parse an ECR repo URL from the image passed, leading to not being possible to enable this feature. This is now resolved in the latest versions.

This PR uses a flag to enable the feature when needed, and adds any server addresses configured in the mirrors to be allowed on the CredentialProviderConfig object that kops configures on the kubelet.

To configure this:

example:

spec:
  containerd:
    useECRCredentialsForMirrors: true
    registryMirrors:
      docker.io:
        - https://<MY-AWS-ACCOUNT>.dkr.ecr.us-east-1.amazonaws.com/v2/<MY cache rule namespace, e.g docker-hub>

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 30, 2024
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented May 30, 2024

Hi @rsafonseca. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 30, 2024
@k8s-ci-robot k8s-ci-robot requested review from hakman and johngmyers May 30, 2024 12:54
@k8s-ci-robot k8s-ci-robot added area/api area/nodeup size/M Denotes a PR that changes 30-99 lines, ignoring generated files. area/documentation size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 30, 2024
@hakman
Copy link
Copy Markdown
Member

hakman commented May 30, 2024

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 30, 2024
@rsafonseca
Copy link
Copy Markdown
Contributor Author

rsafonseca commented May 30, 2024 •
edited
Loading

Any idea why these e2e tests might be failing @hakman?
It seems odd that they never come up and the log dump references not finding an unrelated role that should belong to another job
e.g.
pull-kops-e2e-cni-amazonvpc:

0530 19:12:47.346328   14816 dumplogs.go:51] /home/prow/go/src/k8s.io/kops/.build/dist/linux/amd64/kops toolbox dump --name e2e-pr16593.pull-kops-e2e-cni-amazonvpc.test-cncf-aws.k8s.io --dir /logs/artifacts --private-key /tmp/kops/e2e-pr16593.pull-kops-e2e-cni-amazonvpc.test-cncf-aws.k8s.io/id_ed25519 --ssh-user ubuntu
W0530 19:12:59.466784   50124 aws.go:2055] could not find role "masters.e2e-pr16593.pull-kops-e2e-cni-cilium-ipv6.test-cn-40qra0". Resource may already have been deleted: operation error IAM: GetInstanceProfile, https response error StatusCode: 404, RequestID: 013035b7-7eba-43a9-98dd-abf842145433, NoSuchEntity: Instance Profile masters.e2e-pr16593.pull-kops-e2e-cni-cilium-ipv6.test-cn-40qra0 cannot be found.
W0530 19:13:02.485316   50124 aws.go:2055] could not find role "nodes.e2e-pr16593.pull-kops-e2e-cni-cilium-ipv6.test-cncf-47o1f8". Resource may already have been deleted: operation error IAM: GetInstanceProfile, https response error StatusCode: 404, RequestID: 4bc7f2c4-9c38-4723-85bd-b493cf0794c8, NoSuchEntity: Instance Profile nodes.e2e-pr16593.pull-kops-e2e-cni-cilium-ipv6.test-cncf-47o1f8 cannot be found.

could something be leaking somewhere? or do you think there's any change here that could be somehow related to this weird behaviour?

EDIT: nvm, found the problem... hidden tabs in string literal.. damn vscode 😂
the above behaviour is still odd though, seems like the logs are leaking across jobs when they fail

@hakman
Copy link
Copy Markdown
Member

hakman commented May 31, 2024

Please ignore the IPv6 tests

@rsafonseca
Copy link
Copy Markdown
Contributor Author

rsafonseca commented May 31, 2024

All good then, though it looks like pull-kops-e2e-cni-cilium-eni is flaky, passed on retest.
Any thoughts on this PR @hakman ? Does it make sense to you to add the config option under containerd on the CRD? :)

@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. do-not-merge/contains-merge-commits and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 9, 2024
@rsafonseca rsafonseca force-pushed the containerd_ecr_mirrors branch from a77071a to a7dbaac Compare June 10, 2024 11:18
@rsafonseca rsafonseca force-pushed the containerd_ecr_mirrors branch from 45e0800 to f682340 Compare June 10, 2024 17:00
@k8s-ci-robot k8s-ci-robot added the area/provider/aws Issues or PRs related to aws provider label Jun 10, 2024
@rsafonseca rsafonseca force-pushed the containerd_ecr_mirrors branch 2 times, most recently from 615a93f to a7dbaac Compare June 10, 2024 18:20
@sczelo
Copy link
Copy Markdown

sczelo commented Jun 21, 2024

Any update about this PR? Do you need any help or test, to be merged faster?
I built the images locally from this repo and tested it on AWS. It's worked for me.

@rsafonseca
Copy link
Copy Markdown
Contributor Author

rsafonseca commented Jun 21, 2024

Did you get a chance to review this yet @hakman @johngmyers ? :)

@hakman hakman removed the area/provider/scaleway Issues or PRs related to Scaleway provider label Jul 27, 2025
@hakman hakman removed the request for review from johngmyers July 27, 2025 05:06
@hakman
Copy link
Copy Markdown
Member

hakman commented Jul 28, 2025

The tests are broken, so please ignore for now. Will fix them and merge this.

@hakman
Copy link
Copy Markdown
Member

hakman commented Jul 28, 2025

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 28, 2025
@hakman
Copy link
Copy Markdown
Member

hakman commented Jul 28, 2025

/hold for tests to be fixed

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 28, 2025
@rsafonseca
Add support for using ECR as pull-through image cache
1794614 
Signed-off-by: Rafael da Fonseca <rafael.fonseca@wildlifestudios.com>
@rsafonseca rsafonseca force-pushed the containerd_ecr_mirrors branch from 808f9b5 to 1794614 Compare July 28, 2025 11:46
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 28, 2025
@hakman
Copy link
Copy Markdown
Member

hakman commented Jul 28, 2025

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 28, 2025
@hakman
Copy link
Copy Markdown
Member

hakman commented Jul 29, 2025

/unhold
/retest

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 29, 2025
@hakman
Copy link
Copy Markdown
Member

hakman commented Jul 29, 2025

/test all

@hakman
Copy link
Copy Markdown
Member

hakman commented Jul 29, 2025

/approve

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jul 29, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hakman

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

The pull request process is described here

Details 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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 29, 2025
@hakman
Copy link
Copy Markdown
Member

hakman commented Jul 29, 2025

/retest

@hakman hakman changed the title Add support for using ECR as pull-through image cache aws: Add support for using ECR as pull-through image cache Jul 29, 2025
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jul 29, 2025 •
edited
Loading

@rsafonseca: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-kops-e2e-aws-upgrade-k130-ko130-to-klatest-kolatest-many-addons 8b06aa4 link false /test pull-kops-e2e-aws-upgrade-k130-ko130-to-klatest-kolatest-many-addons
pull-kops-kubernetes-e2e-ubuntu-gce-build 8b06aa4 link false /test pull-kops-kubernetes-e2e-ubuntu-gce-build
pull-kops-e2e-cni-kuberouter 8b06aa4 link false /test pull-kops-e2e-cni-kuberouter
pull-kops-e2e-cni-flannel a783928 link true /test pull-kops-e2e-cni-flannel
pull-kops-e2e-gce-cni-kindnet a783928 link true /test pull-kops-e2e-gce-cni-kindnet
pull-kops-e2e-k8s-aws-amazonvpc-u2404 a783928 link true /test pull-kops-e2e-k8s-aws-amazonvpc-u2404
pull-kops-e2e-gce-cni-calico 1794614 link false /test pull-kops-e2e-gce-cni-calico

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@hakman
Copy link
Copy Markdown
Member

hakman commented Jul 29, 2025

/override pull-kops-e2e-cni-amazonvpc

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jul 29, 2025

@hakman: Overrode contexts on behalf of hakman: pull-kops-e2e-cni-amazonvpc

Details

In response to this:

/override pull-kops-e2e-cni-amazonvpc

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@hakman
Copy link
Copy Markdown
Member

hakman commented Jul 29, 2025

/override pull-kops-e2e-k8s-gce-ipalias

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jul 29, 2025

@hakman: Overrode contexts on behalf of hakman: pull-kops-e2e-k8s-gce-ipalias

Details

In response to this:

/override pull-kops-e2e-k8s-gce-ipalias

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@hakman
Copy link
Copy Markdown
Member

hakman commented Jul 29, 2025

/override pull-kops-e2e-cni-cilium-eni

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jul 29, 2025

@hakman: Overrode contexts on behalf of hakman: pull-kops-e2e-cni-cilium-eni

Details

In response to this:

/override pull-kops-e2e-cni-cilium-eni

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot merged commit dde2cfd into kubernetes:master Jul 29, 2025
35 of 36 checks passed
@hakman hakman mentioned this pull request Aug 9, 2025
Merged
@BrewTestBot BrewTestBot mentioned this pull request Oct 21, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@justinsb justinsb justinsb left review comments

@hakman hakman hakman requested changes

+1 more reviewer

@jim-barber-he jim-barber-he jim-barber-he left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@hakman hakman

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/addons area/api area/channels area/documentation area/kops-controller area/nodeup area/rolling-update cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@rsafonseca @k8s-ci-robot @hakman @sczelo @SzilardS @elliotdobson @k8s-triage-robot @itskingori @justinsb @jim-barber-he