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.

Sign up for GitHub

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

Jump to bottom

add containerd to e2e tests #226

Merged
merged 4 commits into from
Apr 17, 2023
Merged

add containerd to e2e tests #226

merged 4 commits into from
Apr 17, 2023

Conversation

BenTheElder
Copy link
Member

@BenTheElder BenTheElder commented Apr 8, 2023 •
edited
Loading

With a little bit of hackery we can run containerd as the current, unprivileged user and pull images to it without unpacking.

TODO:

  • expand to N stable releases (and maybe nightly?) Added!
  • add other critical clients where feasible (docker, cri-o ... must be able to get them running and pulling in unprivileged envrionments)

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 8, 2023
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 8, 2023
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 8, 2023
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 8, 2023
@BenTheElder BenTheElder changed the title [WIP] add containerd to e2e tests add containerd to e2e tests Apr 8, 2023
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 8, 2023
@BenTheElder BenTheElder force-pushed the e2e-again branch 7 times, most recently from 7291774 to 5fc1fae Compare April 8, 2023 06:54
@BenTheElder
Copy link
Member Author

Still very fast, and we're checking v1.6.20 and v1.7.0 now (both stable releases)

https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/registry.k8s.io/226/pull-registry-e2e/1644594644308201472

BenTheElder
BenTheElder commented Apr 8, 2023
View reviewed changes
hack/make-rules/test.sh
else
go_test_opts+=('-tags=noe2e')
Copy link
Member Author

Choose a reason for hiding this comment

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

this was a bug in #32

BenTheElder
BenTheElder commented Apr 8, 2023
View reviewed changes
cmd/archeio/internal/e2e/e2e_containerd_linux_test.go

func TestE2EContainerdPull(t *testing.T) {
t.Parallel()
containerdVersions := []string{"1.6.20", "1.7.0"}
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: keeping these up to date ...

With crane we're floating at the latest tagged release

We can maybe figure out something reasonable to detect the most recent patch release on the current and N-1 minor versions ...

@BenTheElder BenTheElder mentioned this pull request Apr 8, 2023
Merged
@BenTheElder
Copy link
Member Author

This should be a solid starting point.

@BenTheElder BenTheElder requested review from ameukam and removed request for hh and riaankleinhans April 17, 2023 16:50
@BenTheElder BenTheElder mentioned this pull request Apr 17, 2023
Closed
upodroid
upodroid approved these changes Apr 17, 2023
View reviewed changes
Copy link
Member

@upodroid upodroid left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 17, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, upodroid

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

@k8s-ci-robot k8s-ci-robot merged commit c0a3102 into kubernetes:main Apr 17, 2023
@BenTheElder BenTheElder deleted the e2e-again branch April 18, 2023 00:20
@BenTheElder
Copy link
Member Author

Interesting, this fails in post-submit because wget is not available in that image. Easy enough to fix.

https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/post-registry-push-images/1648111849591279616

@BenTheElder BenTheElder mentioned this pull request Apr 18, 2023
Merged
@BenTheElder
Copy link
Member Author

filed #230 for wget => curl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dims dims dims left review comments

@upodroid upodroid upodroid approved these changes

@ameukam ameukam Awaiting requested review from ameukam

Assignees

@upodroid upodroid

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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. 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.
4 participants
@BenTheElder @k8s-ci-robot @dims @upodroid