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

CI: switch to registry.k8s.io #23821

Merged
merged 1 commit into from
Feb 23, 2023

Conversation

ameukam
Copy link
Contributor

@ameukam ameukam commented Feb 16, 2023

The main Kubernetes container registry is now defaulted to registry.k8s.io. See: https://kubernetes.io/blog/2022/11/28/registry-k8s-io-faster-cheaper-ga/.
Switching to the new registry.

@ameukam ameukam requested review from a team as code owners February 16, 2023 14:26
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 16, 2023
@ameukam ameukam requested a review from aanm February 16, 2023 14:26
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Feb 16, 2023
@ameukam ameukam force-pushed the switch-to-registry-k8s-io branch 2 times, most recently from a040db6 to ec62aea Compare February 16, 2023 18:11
@ameukam ameukam changed the title Switch to registry.k8s.io CI: switch to registry.k8s.io Feb 17, 2023
@joestringer
Copy link
Member

joestringer commented Feb 17, 2023

/test

Job 'Cilium-PR-K8s-1.16-kernel-4.19' failed:

Click to show.

Test Name

K8sUpdates Tests upgrade and downgrade from a Cilium stable image to master

Failure Output

FAIL: DNS entry is not ready after timeout

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.16-kernel-4.19 so I can create one.

Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Thanks a lot for looking into this!

See: https://kubernetes.io/blog/2022/11/28/registry-k8s-io-faster-cheaper-ga/.

I'd really like to see that reference in the commit description, to avoid having to look up for the GitHub PR to get the link to this context.

Old images will remain on the old registry past April 3rd, but this should not be considered a permanent or long term option and may be subject to pruning in the future.

Joe, André, Maciej: I don't know what “long term” means here, with regards to support for our older branch. I suppose that we should backport this PR to all branches?

@qmonnet qmonnet added area/CI Continuous Integration testing issue or flake release-note/ci This PR makes changes to the CI. labels Feb 17, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 17, 2023
@ameukam
Copy link
Contributor Author

ameukam commented Feb 17, 2023

@qmonnet Thank you for taking the time to look at the PR! I amended the commit.

@qmonnet
Copy link
Member

qmonnet commented Feb 17, 2023

Thanks!
/test

@@ -88,26 +88,26 @@ items:
sizeBytes: 459801943
- names:
- docker.io/library/import-2022-09-02@sha256:f27fe306ea3c0e3ef176dda243bcbcc2bd8fa0f9d19f87285b0fb47e96d85f93
- k8s.gcr.io/kube-proxy:v1.23.10
- registry.k8s.io/kube-proxy:v1.23.10
Copy link
Member

Choose a reason for hiding this comment

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

These changes here are generated from script, and mainly depends on kind + workder node versions, I am not sure if we should update these files, as they can be reverted back next time we run the script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sayboras Can you link the script that generate those files ? I'm not familiar with the codebase.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

You might be able to try this out with make -C test/controlplane/ generate-input-files, but it seems that it is sensitive to some local environment settings.

@nebril
Copy link
Member

nebril commented Feb 21, 2023

@qmonnet based on this line:

Container images for Kubernetes releases from 1.25 onward are no longer published to k8s.gcr.io, only to registry.k8s.io.

I think we need to backport this change to Cilium 1.13, as 1.13 supports Kubernetes 1.25 and 1.26. Older Cilium branches are probably fine using the old registry.

@nebril nebril added the needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch label Feb 21, 2023
@ameukam ameukam force-pushed the switch-to-registry-k8s-io branch 2 times, most recently from c75a54b to d81e7bf Compare February 21, 2023 22:01
The main Kubernetes container registry is now defaulted to registry.k8s.io.
See: kubernetes.io/blog/2022/11/28/registry-k8s-io-faster-cheaper-ga

Signed-off-by: Arnaud Meukam <ameukam@gmail.com>
@ameukam
Copy link
Contributor Author

ameukam commented Feb 21, 2023

@qmonnet based on this line:

Container images for Kubernetes releases from 1.25 onward are no longer published to k8s.gcr.io, only to registry.k8s.io.

I think we need to backport this change to Cilium 1.13, as 1.13 supports Kubernetes 1.25 and 1.26. Older Cilium branches are probably fine using the old registry.

@nebril @qmonnet Happy to do the backports once this is merged.

@qmonnet
Copy link
Member

qmonnet commented Feb 21, 2023

Thanks! But we usually handle backports with a semi-automated process (there's a weekly rotation for the person handling backports), so unless some major conflict arises, which sounds unlikely on this PR, you shouldn't have to worry about it :)

@michi-covalent
Copy link
Contributor

/test

@sayboras
Copy link
Member

/test-1.16-4.19

@michi-covalent
Copy link
Contributor

do we need to rebase to pick up #23847, or should i just ignore the pending ci-verifier and mark it ready-to-merge? 👀

@sayboras
Copy link
Member

/ci-verifier

@sayboras
Copy link
Member

do we need to rebase to pick up #23847, or should i just ignore the pending ci-verifier and mark it ready-to-merge? eyes

The mentioned PR didn't work for me in one case #23847 (comment), so I just trigger ci-verifier manually, let's see how it's going.

Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

Apart from my above comment on controlplane manifest, which can be considered as nitpick.

The rest looks good to me. Thanks.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 23, 2023
@sayboras sayboras merged commit 865c1df into cilium:master Feb 23, 2023
@ameukam
Copy link
Contributor Author

ameukam commented Feb 23, 2023

Ref:

@sayboras sayboras mentioned this pull request Feb 28, 2023
10 tasks
@sayboras sayboras added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Feb 28, 2023
@julianwiedmann julianwiedmann added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants