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

Replace use of public.ecr.aws/nginx/nginx with ghcr.io/jitesoft/alpine image #254

Merged

Conversation

dimitar-kostadinov
Copy link
Contributor

@dimitar-kostadinov dimitar-kostadinov commented Sep 9, 2024

How to categorize this PR?

/area testing
/kind flake

What this PR does / why we need it:
There is an issue when distribution/distribution is used as a pull-through cache to Amazon Public ECR Gallery. Sometimes image manifests are successfully cached, but download of image layers blobs fails with Internal Server Error - 500.

With this PR public.ecr.aws/nginx/nginx image is replaced with ghcr.io/jitesoft/alpine. The VerifyRegistryCache func is extended to support various images.

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

Special notes for your reviewer:
N/A

Release note:

e2e tests are no longer using test images from public ECR as the Distribution project cannot pull blobs from it.

@gardener-prow gardener-prow bot added area/testing Testing related kind/flake Tracking or fixing a flaky test labels Sep 9, 2024
@gardener-prow gardener-prow bot added cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 9, 2024
@ialidzhikov
Copy link
Member

/assign

@gardener-prow gardener-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 9, 2024
docs/usage/registry-cache/configuration.md Outdated Show resolved Hide resolved
Comment on lines 43 to 44
// GitlabRegistryJitesoftAlpine31710Image is the registry.gitlab.com/jitesoft/dockerfiles/alpine:3.17.10 image.
GitlabRegistryJitesoftAlpine31710Image = "registry.gitlab.com/jitesoft/dockerfiles/alpine:3.17.10"
Copy link
Member

Choose a reason for hiding this comment

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

Nit: You could move it above to the group of the alpine images.

test/common/common.go Show resolved Hide resolved
test/common/common.go Outdated Show resolved Hide resolved
test/common/common.go Outdated Show resolved Hide resolved
test/common/common.go Outdated Show resolved Hide resolved
test/common/common.go Outdated Show resolved Hide resolved
Comment on lines 39 to 42
// ArtifactRegistryNginx1176Image is the europe-docker.pkg.dev/gardener-project/releases/3rd/nginx:1.17.6 image (copy of docker.io/library/nginx:1.17.6).
ArtifactRegistryNginx1176Image = "europe-docker.pkg.dev/gardener-project/releases/3rd/nginx:1.17.6"
// RegistryK8sNginx1154Image is the registry.k8s.io/e2e-test-images/nginx:1.15-4 image.
RegistryK8sNginx1154Image = "registry.k8s.io/e2e-test-images/nginx:1.15-4"
Copy link
Member

Choose a reason for hiding this comment

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

I see that we still have nginx image. Do we plan to switch them as well to alpine in a follow-up PR?
IIRC this is a band-aid PR to migration from ECR. But IMO in a follow-up PR we can also change the other nginx images to alpine. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can keep it flexible and use different container images in the tests.

@ialidzhikov ialidzhikov changed the title Replace use of public.ecr.aws/nginx/nginx with quay.io/jitesoft/apine image Replace use of public.ecr.aws/nginx/nginx with quay.io/jitesoft/alpine image Sep 9, 2024
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.

/lgtm

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

gardener-prow bot commented Sep 9, 2024

LGTM label has been added.

Git tree hash: 1028183232c885ec9735ef4c0d50fdce33ef4d1d

Copy link
Contributor

gardener-prow bot commented Sep 9, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ialidzhikov

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

The pull request process is described 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

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 9, 2024
@dimitar-kostadinov dimitar-kostadinov changed the title Replace use of public.ecr.aws/nginx/nginx with quay.io/jitesoft/alpine image Replace use of public.ecr.aws/nginx/nginx with ghcr.io/jitesoft/alpine image Sep 9, 2024
@gardener-prow gardener-prow bot merged commit a4fdb06 into gardener:main Sep 9, 2024
6 checks passed
@dimitar-kostadinov dimitar-kostadinov deleted the remove-public.ecr.aws branch September 17, 2024 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/testing Testing related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/flake Tracking or fixing a flaky test 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.

2 participants