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

Kube on the CI #3296

Merged
merged 1 commit into from
Aug 17, 2024
Merged

Kube on the CI #3296

merged 1 commit into from
Aug 17, 2024

Conversation

apostasie
Copy link
Contributor

@apostasie apostasie commented Aug 12, 2024 •
edited
Loading

This is meant to bring in kube testing (fix #3282):

  • using kind to spin-up kube
    • script usable locally or on the CI indifferently
    • most of it comes from what I committed recently to nydus
  • mimic the ipv6 mechanism: -test.kube will run only "kube-compatible" tests - see testutils
  • example / placeholder test inside container_commit_test, since this is where we will need the first one

Tagging @lingdie because of recent work on kube issues with #827 and #3268 and facing that question of how to test on kube in here.

@fahedouch @AkihiroSuda and other folks - thoughts/feeback?

@apostasie apostasie force-pushed the dev-kube branch 12 times, most recently from 9ab161f to 05b2421 Compare August 12, 2024 08:33
@apostasie apostasie changed the title [WIP] Kube on the CI Kube on the CI Aug 12, 2024
apostasie
apostasie commented Aug 12, 2024
View reviewed changes
hack/build-integration-canary.sh
@@ -67,141 +71,6 @@ STARGZ_SNAPSHOTTER_CHECKSUM=linux
# We specifically want the static ones
TINI_CHECKSUM=static


Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sharing that with the new kube script (moved to lib.sh)

@apostasie
Copy link
Contributor Author

Ok, target is green.

This is a bit rough in its current shape, but we can make it better / more integrated in the future as we start seeing usage for it (eg: actual kube tests).

@apostasie apostasie marked this pull request as ready for review August 12, 2024 08:40
@apostasie apostasie requested a review from AkihiroSuda August 12, 2024 08:40
@apostasie
Copy link
Contributor Author

FreeBSD failure is unrelated. Last push just removes two debugging print that are not consequential.

This was referenced Aug 13, 2024
Closed
Closed
@apostasie
Copy link
Contributor Author

@AkihiroSuda

To run kube tests:

go test ./cmd/nerdctl/ -test.kube

This must be run in the (containerd) context where kube containers have been created (by kubectl for example).
In the case of kind (on the CI) , I am running that command inside the kind control plane container.

There is now one test in https://github.com/containerd/nerdctl/pull/3296/files#diff-cb510ac1ecc8b03a6de395f8bfde3d161432dfbbf3d26814eec378cb350b41b7

It is an "anti" test - it assumes failure (because right now our code is broken), but MUST be transformed to assume success as soon as we fix the problem.

The gist of things is:

  • create stuff with kubectl
  • test that nerdctl can perform operations on these containers

BaseForKube puts you in the k8s.io namespace, where kube created containers are expected to live.

@apostasie apostasie force-pushed the dev-kube branch 4 times, most recently from 94eaea7 to c7a1032 Compare August 16, 2024 03:18
@apostasie
Copy link
Contributor Author

@AkihiroSuda from the comments above - most of them addressed, BUT:

  • kubectl wait will timeout with a busybox image - no idea why, and not particularly interested in debugging kube - I thus switched the image to testutil.CommonImage instead
  • kubectl namespaces are not containerd namespaces as far as I can tell, so, for now, we will operate on the standard namespace k8s.io (not sure how to change that)
  • moved the kube test to a linux only test file, as linking to testutil images would fail the tests on windows

Kube-CI is green.

@AkihiroSuda
Copy link
Member

kubectl namespaces are not containerd namespaces as far as I can tell, so, for now, we will operate on the standard namespace k8s.io (not sure how to change that)

The containerd objects should stay in containerd's k8s.io namespace, but the Kubernetes objects (Pods, etc.) should be in Kubernetes API's nerdctl-test namespace

@apostasie
Kube on the CI
97c5492 
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
@apostasie
Copy link
Contributor Author

Latest push:

  • rebased, conflict resolved
  • uses k8s namespace for kubectl created objects
  • tentative rename for the flags:
    • test.allow-kill-daemon
    • test.only-ipv6
    • test.only-kube

Let me know if you would like these flag names tweaked to something else.

@apostasie
Copy link
Contributor Author

CI failures are:

  1. github belly-up
  2. the usual TestIPFSComposeUp

If we can re-trigger these two - and eventually we need to figure out #3305

@apostasie
Copy link
Contributor Author

CI is green.

@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Aug 17, 2024
AkihiroSuda
AkihiroSuda approved these changes Aug 17, 2024
View reviewed changes
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit a44a1cb into containerd:main Aug 17, 2024
20 checks passed
@fahedouch
Copy link
Member

Sorry for late review! thanks for effort I recommend using k8s rather then kube to be more explicit.

fahedouch
fahedouch reviewed Aug 18, 2024
View reviewed changes
cmd/nerdctl/container_commit_linux_test.go Show resolved Hide resolved
cmd/nerdctl/container_commit_linux_test.go Show resolved Hide resolved
cmd/nerdctl/container_commit_linux_test.go
setup()

t.Run("test commit / push on Kube (https://github.com/containerd/nerdctl/issues/827)", func(t *testing.T) {
t.Log("This test is meant to verify that we can commit / push an image from a pod." +
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
t.Log("This test is meant to verify that we can commit / push an image from a pod." +
t.Log("This test is meant to verify that we can commit / push an image that belongs to containerd `k8s` namespace." +

Copy link
Member

Choose a reason for hiding this comment

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

BTW it is a nerdctl issue not only a kube

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean issue #827 can be reproduced in a non-kube environment?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a separate ticket, or do you have a simple reproducer for it that does not involve kube?

pkg/testutil/testutil.go Show resolved Hide resolved
@apostasie
Copy link
Contributor Author

Sorry for late review! thanks for effort I recommend using k8s rather then kube to be more explicit.

Thanks @fahedouch !

It is not too late - can definitely send a follow-up PR - and yes, I can rename kube -> k8s.

Just answered your comments above. Let me know what you think when you get a chance.

@ouyangningdong
Copy link

Has the issue of the save error prompt been resolved? Thank you very much! content digest sha256:a58ecd4f0c864650a4286c3c2d49c7219a3f2fc8d7a0bf478aa9834acfe14ae7: not found

@apostasie
Copy link
Contributor Author

@ouyangningdong #827 (comment)

@apostasie apostasie mentioned this pull request Aug 21, 2024
Merged
@apostasie apostasie mentioned this pull request Oct 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fahedouch fahedouch fahedouch left review comments

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

Assignees
No one assigned
Labels
area/ci e.g., CI failure area/kubernetes
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.

Add a Kube testing environment?
4 participants
@apostasie @AkihiroSuda @fahedouch @ouyangningdong