From 36f7eb91311bb49865cbff96cc0afa2644936634 Mon Sep 17 00:00:00 2001 From: apostasie Date: Mon, 14 Oct 2024 21:13:18 -0700 Subject: [PATCH 1/3] Rewrite commit tests Signed-off-by: apostasie --- .../container/container_commit_linux_test.go | 124 +++++++++--------- .../container/container_commit_test.go | 81 ++++++++---- 2 files changed, 112 insertions(+), 93 deletions(-) diff --git a/cmd/nerdctl/container/container_commit_linux_test.go b/cmd/nerdctl/container/container_commit_linux_test.go index 8a4af41fdcd..710e37aeef4 100644 --- a/cmd/nerdctl/container/container_commit_linux_test.go +++ b/cmd/nerdctl/container/container_commit_linux_test.go @@ -21,76 +21,70 @@ import ( "testing" "github.com/containerd/nerdctl/v2/pkg/testutil" + "github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest" + "github.com/containerd/nerdctl/v2/pkg/testutil/test" ) -/* -This test below is meant to assert that https://github.com/containerd/nerdctl/issues/827 is NOT fixed. -Obviously, once we fix the issue, it should be replaced by something that assert it works. -Unfortunately, this is flaky. -It will regularly succeed or fail, making random PR fail the Kube check. -*/ - -func TestKubeCommitPush(t *testing.T) { - t.Parallel() - - base := testutil.NewBaseForKubernetes(t) - tID := testutil.Identifier(t) - - var containerID string - // var registryIP string - - setup := func() { - testutil.KubectlHelper(base, "run", "--image", testutil.CommonImage, tID, "--", "sleep", "Inf"). - AssertOK() - - testutil.KubectlHelper(base, "wait", "pod", tID, "--for=condition=ready", "--timeout=1m"). - AssertOK() - - testutil.KubectlHelper(base, "exec", tID, "--", "mkdir", "-p", "/tmp/whatever"). - AssertOK() - - cmd := testutil.KubectlHelper(base, "get", "pods", tID, "-o", "jsonpath={ .status.containerStatuses[0].containerID }") - cmd.Run() - containerID = strings.TrimPrefix(cmd.Out(), "containerd://") - - // This below is missing configuration to allow for plain http communication - // This is left here for future work to successfully start a registry usable in the cluster - /* - // Start a registry - testutil.KubectlHelper(base, "run", "--port", "5000", "--image", testutil.RegistryImageStable, "testregistry"). - AssertOK() - - testutil.KubectlHelper(base, "wait", "pod", "testregistry", "--for=condition=ready", "--timeout=1m"). - AssertOK() - - cmd = testutil.KubectlHelper(base, "get", "pods", tID, "-o", "jsonpath={ .status.hostIPs[0].ip }") - cmd.Run() - registryIP = cmd.Out() - - cmd = testutil.KubectlHelper(base, "apply", "-f", "-", fmt.Sprintf(`apiVersion: v1 - kind: ConfigMap - metadata: - name: local-registry - namespace: nerdctl-test - data: - localRegistryHosting.v1: | - host: "%s:5000" - help: "https://kind.sigs.k8s.io/docs/user/local-registry/" - `, registryIP)) - */ - +func TestKubeCommitSave(t *testing.T) { + testCase := nerdtest.Setup() + + testCase.Require = nerdtest.OnlyKubernetes + + testCase.Setup = func(data test.Data, helpers test.Helpers) { + containerID := "" + // NOTE: kubectl namespaces are not the same as containerd namespaces. + // We still want kube test objects segregated in their own Kube API namespace. + nerdtest.KubeCtlCommand(helpers, "create", "namespace", "nerdctl-test-k8s").Run(&test.Expected{}) + nerdtest.KubeCtlCommand(helpers, "run", "--image", testutil.CommonImage, data.Identifier(), "--", "sleep", "Inf").Run(&test.Expected{}) + nerdtest.KubeCtlCommand(helpers, "wait", "pod", data.Identifier(), "--for=condition=ready", "--timeout=1m").Run(&test.Expected{}) + nerdtest.KubeCtlCommand(helpers, "exec", data.Identifier(), "--", "mkdir", "-p", "/tmp/whatever").Run(&test.Expected{}) + nerdtest.KubeCtlCommand(helpers, "get", "pods", data.Identifier(), "-o", "jsonpath={ .status.containerStatuses[0].containerID }").Run(&test.Expected{ + Output: func(stdout string, info string, t *testing.T) { + containerID = strings.TrimPrefix(stdout, "containerd://") + }, + }) + data.Set("containerID", containerID) } - tearDown := func() { - testutil.KubectlHelper(base, "delete", "pod", "--all").Run() + testCase.Cleanup = func(data test.Data, helpers test.Helpers) { + nerdtest.KubeCtlCommand(helpers, "delete", "pod", "--all").Run(nil) } - tearDown() - t.Cleanup(tearDown) - setup() + testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand { + helpers.Ensure("commit", data.Get("containerID"), "testcommitsave") + return helpers.Command("save", "testcommitsave") + } - t.Run("test commit / push on Kube (https://github.com/containerd/nerdctl/issues/827)", func(t *testing.T) { - base.Cmd("commit", containerID, "testcommitsave").AssertOK() - base.Cmd("save", "testcommitsave").AssertOK() - }) + testCase.Expected = test.Expects(0, nil, nil) + + testCase.Run(t) + + // This below is missing configuration to allow for plain http communication + // This is left here for future work to successfully start a registry usable in the cluster + /* + // Start a registry + nerdtest.KubeCtlCommand(helpers, "run", "--port", "5000", "--image", testutil.RegistryImageStable, "testregistry"). + Run(&test.Expected{}) + + nerdtest.KubeCtlCommand(helpers, "wait", "pod", "testregistry", "--for=condition=ready", "--timeout=1m"). + AssertOK() + + cmd = nerdtest.KubeCtlCommand(helpers, "get", "pods", tID, "-o", "jsonpath={ .status.hostIPs[0].ip }") + cmd.Run(&test.Expected{ + Output: func(stdout string, info string, t *testing.T) { + registryIP = stdout + }, + }) + + cmd = nerdtest.KubeCtlCommand(helpers, "apply", "-f", "-", fmt.Sprintf(`apiVersion: v1 + kind: ConfigMap + metadata: + name: local-registry + namespace: nerdctl-test + data: + localRegistryHosting.v1: | + host: "%s:5000" + help: "https://kind.sigs.k8s.io/docs/user/local-registry/" + `, registryIP)) + */ } diff --git a/cmd/nerdctl/container/container_commit_test.go b/cmd/nerdctl/container/container_commit_test.go index f9f553d9ca1..9382a782d7a 100644 --- a/cmd/nerdctl/container/container_commit_test.go +++ b/cmd/nerdctl/container/container_commit_test.go @@ -17,39 +17,64 @@ package container import ( - "fmt" "testing" "github.com/containerd/nerdctl/v2/pkg/testutil" + "github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest" + "github.com/containerd/nerdctl/v2/pkg/testutil/test" ) func TestCommit(t *testing.T) { - t.Parallel() - base := testutil.NewBase(t) - switch base.Info().CgroupDriver { - case "none", "": - t.Skip("requires cgroup (for pausing)") - } - testContainer := testutil.Identifier(t) - testImage := testutil.Identifier(t) + "-img" - defer base.Cmd("rm", "-f", testContainer).Run() - defer base.Cmd("rmi", testImage).Run() - - for _, pause := range []string{ - "true", - "false", - } { - base.Cmd("run", "-d", "--name", testContainer, testutil.CommonImage, "sleep", "infinity").AssertOK() - base.EnsureContainerStarted(testContainer) - base.Cmd("exec", testContainer, "sh", "-euxc", `echo hello-test-commit > /foo`).AssertOK() - base.Cmd( - "commit", - "-c", `CMD ["/foo"]`, - "-c", `ENTRYPOINT ["cat"]`, - fmt.Sprintf("--pause=%s", pause), - testContainer, testImage).AssertOK() - base.Cmd("run", "--rm", testImage).AssertOutExactly("hello-test-commit\n") - base.Cmd("rm", "-f", testContainer).Run() - base.Cmd("rmi", testImage).Run() + testCase := nerdtest.Setup() + + testCase.SubTests = []*test.Case{ + { + Description: "with pause", + Require: nerdtest.CGroup, + Cleanup: func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Identifier()) + helpers.Anyhow("rmi", "-f", data.Identifier()) + }, + Setup: func(data test.Data, helpers test.Helpers) { + helpers.Ensure("run", "-d", "--name", data.Identifier(), testutil.CommonImage, "sleep", "infinity") + nerdtest.EnsureContainerStarted(helpers, data.Identifier()) + helpers.Ensure("exec", data.Identifier(), "sh", "-euxc", `echo hello-test-commit > /foo`) + }, + Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { + helpers.Ensure( + "commit", + "-c", `CMD ["/foo"]`, + "-c", `ENTRYPOINT ["cat"]`, + "--pause=true", + data.Identifier(), data.Identifier()) + return helpers.Command("run", "--rm", data.Identifier()) + }, + Expected: test.Expects(0, nil, test.Equals("hello-test-commit\n")), + }, + { + Description: "no pause", + Require: test.Not(test.Windows), + Cleanup: func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Identifier()) + helpers.Anyhow("rmi", "-f", data.Identifier()) + }, + Setup: func(data test.Data, helpers test.Helpers) { + helpers.Ensure("run", "-d", "--name", data.Identifier(), testutil.CommonImage, "sleep", "infinity") + nerdtest.EnsureContainerStarted(helpers, data.Identifier()) + helpers.Ensure("exec", data.Identifier(), "sh", "-euxc", `echo hello-test-commit > /foo`) + }, + Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { + helpers.Ensure( + "commit", + "-c", `CMD ["/foo"]`, + "-c", `ENTRYPOINT ["cat"]`, + "--pause=false", + data.Identifier(), data.Identifier()) + return helpers.Command("run", "--rm", data.Identifier()) + }, + Expected: test.Expects(0, nil, test.Equals("hello-test-commit\n")), + }, } + + testCase.Run(t) } From 914238eb6362052e8d4871e92138f069eda7c63b Mon Sep 17 00:00:00 2001 From: apostasie Date: Tue, 15 Oct 2024 01:17:37 -0700 Subject: [PATCH 2/3] Kuberneters testing tooling cleanup Signed-off-by: apostasie --- pkg/testutil/nerdtest/requirements.go | 7 ++++++- pkg/testutil/nerdtest/third-party.go | 8 ++++++++ pkg/testutil/testutil.go | 18 ------------------ 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/pkg/testutil/nerdtest/requirements.go b/pkg/testutil/nerdtest/requirements.go index 575270a129b..3c365095b03 100644 --- a/pkg/testutil/nerdtest/requirements.go +++ b/pkg/testutil/nerdtest/requirements.go @@ -64,8 +64,13 @@ var OnlyIPv6 = &test.Requirement{ var OnlyKubernetes = &test.Requirement{ Check: func(data test.Data, helpers test.Helpers) (ret bool, mess string) { helpers.Write(kubernetes, only) + if _, err := exec.LookPath("kubectl"); err != nil { + return false, fmt.Sprintf("kubectl is not in the path: %+v", err) + } ret = environmentHasKubernetes() - if !ret { + if ret { + helpers.Write(Namespace, "k8s.io") + } else { mess = "runner skips Kubernetes compatible tests in the non-Kubernetes environment" } return ret, mess diff --git a/pkg/testutil/nerdtest/third-party.go b/pkg/testutil/nerdtest/third-party.go index 21199f3a815..087ed50ed78 100644 --- a/pkg/testutil/nerdtest/third-party.go +++ b/pkg/testutil/nerdtest/third-party.go @@ -35,6 +35,14 @@ func BuildCtlCommand(helpers test.Helpers, args ...string) test.TestableCommand return cmd } +func KubeCtlCommand(helpers test.Helpers, args ...string) test.TestableCommand { + kubectl, _ := exec.LookPath("kubectl") + cmd := helpers.Custom(kubectl) + cmd.WithArgs("--namespace=nerdctl-test-k8s") + cmd.WithArgs(args...) + return cmd +} + func RegistryWithTokenAuth(data test.Data, helpers test.Helpers, user, pass string, port int, tls bool) (*registry.Server, *registry.TokenAuthServer) { rca := ca.New(data, helpers.T()) as := registry.NewCesantaAuthServer(data, helpers, rca, 0, user, pass, tls) diff --git a/pkg/testutil/testutil.go b/pkg/testutil/testutil.go index baa0de31e32..da87d67178c 100644 --- a/pkg/testutil/testutil.go +++ b/pkg/testutil/testutil.go @@ -762,14 +762,6 @@ func NewBaseWithIPv6Compatible(t *testing.T) *Base { return newBase(t, Namespace, true, false) } -func NewBaseForKubernetes(t *testing.T) *Base { - base := newBase(t, "k8s.io", false, true) - // NOTE: kubectl namespaces are not the same as containerd namespaces. - // We still want kube test objects segregated in their own Kube API namespace. - KubectlHelper(base, "create", "namespace", Namespace).Run() - return base -} - func NewBase(t *testing.T) *Base { return newBase(t, Namespace, false, false) } @@ -844,13 +836,3 @@ func RegisterBuildCacheCleanup(t *testing.T) { NewBase(t).Cmd("builder", "prune", "--all", "--force").Run() }) } - -func KubectlHelper(base *Base, args ...string) *Cmd { - base.T.Helper() - icmdCmd := icmd.Command("kubectl", append([]string{"--namespace", Namespace}, args...)...) - icmdCmd.Env = base.Env - return &Cmd{ - Cmd: icmdCmd, - Base: base, - } -} From 9603bf405ef4bfc0d42b2f3bd15b14a242093c9c Mon Sep 17 00:00:00 2001 From: apostasie Date: Tue, 15 Oct 2024 01:19:50 -0700 Subject: [PATCH 3/3] Registry testing tooling cleanup Signed-off-by: apostasie --- cmd/nerdctl/issues/issues_linux_test.go | 25 ++--- cmd/nerdctl/login/login_linux_test.go | 9 +- .../testregistry/testregistry_linux.go | 97 ------------------- 3 files changed, 18 insertions(+), 113 deletions(-) diff --git a/cmd/nerdctl/issues/issues_linux_test.go b/cmd/nerdctl/issues/issues_linux_test.go index 0a59126fc13..a371aaa6c0f 100644 --- a/cmd/nerdctl/issues/issues_linux_test.go +++ b/cmd/nerdctl/issues/issues_linux_test.go @@ -24,23 +24,24 @@ import ( "github.com/containerd/nerdctl/v2/pkg/testutil" "github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest" + "github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest/registry" "github.com/containerd/nerdctl/v2/pkg/testutil/test" - "github.com/containerd/nerdctl/v2/pkg/testutil/testregistry" ) func TestIssue3425(t *testing.T) { nerdtest.Setup() - var registry *testregistry.RegistryServer + var reg *registry.Server testCase := &test.Case{ + Require: nerdtest.Registry, Setup: func(data test.Data, helpers test.Helpers) { - base := testutil.NewBase(t) - registry = testregistry.NewWithNoAuth(base, 0, false) + reg = nerdtest.RegistryWithNoAuth(data, helpers, 0, false) + reg.Setup(data, helpers) }, Cleanup: func(data test.Data, helpers test.Helpers) { - if registry != nil { - registry.Cleanup(nil) + if reg != nil { + reg.Cleanup(data, helpers) } }, SubTests: []*test.Case{ @@ -52,14 +53,14 @@ func TestIssue3425(t *testing.T) { helpers.Ensure("run", "-d", "--name", data.Identifier(), testutil.CommonImage) helpers.Ensure("image", "rm", "-f", testutil.CommonImage) helpers.Ensure("image", "pull", testutil.CommonImage) - helpers.Ensure("tag", testutil.CommonImage, fmt.Sprintf("localhost:%d/%s", registry.Port, data.Identifier())) + helpers.Ensure("tag", testutil.CommonImage, fmt.Sprintf("localhost:%d/%s", reg.Port, data.Identifier())) }, Cleanup: func(data test.Data, helpers test.Helpers) { helpers.Anyhow("rm", "-f", data.Identifier()) - helpers.Anyhow("rmi", "-f", fmt.Sprintf("localhost:%d/%s", registry.Port, data.Identifier())) + helpers.Anyhow("rmi", "-f", fmt.Sprintf("localhost:%d/%s", reg.Port, data.Identifier())) }, Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { - return helpers.Command("push", fmt.Sprintf("localhost:%d/%s", registry.Port, data.Identifier())) + return helpers.Command("push", fmt.Sprintf("localhost:%d/%s", reg.Port, data.Identifier())) }, Expected: test.Expects(0, nil, nil), }, @@ -71,14 +72,14 @@ func TestIssue3425(t *testing.T) { helpers.Ensure("run", "-d", "--name", data.Identifier(), testutil.CommonImage, "touch", "/something") helpers.Ensure("image", "rm", "-f", testutil.CommonImage) helpers.Ensure("image", "pull", testutil.CommonImage) - helpers.Ensure("commit", data.Identifier(), fmt.Sprintf("localhost:%d/%s", registry.Port, data.Identifier())) + helpers.Ensure("commit", data.Identifier(), fmt.Sprintf("localhost:%d/%s", reg.Port, data.Identifier())) }, Cleanup: func(data test.Data, helpers test.Helpers) { helpers.Anyhow("rm", "-f", data.Identifier()) - helpers.Anyhow("rmi", "-f", fmt.Sprintf("localhost:%d/%s", registry.Port, data.Identifier())) + helpers.Anyhow("rmi", "-f", fmt.Sprintf("localhost:%d/%s", reg.Port, data.Identifier())) }, Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { - return helpers.Command("push", fmt.Sprintf("localhost:%d/%s", registry.Port, data.Identifier())) + return helpers.Command("push", fmt.Sprintf("localhost:%d/%s", reg.Port, data.Identifier())) }, Expected: test.Expects(0, nil, nil), }, diff --git a/cmd/nerdctl/login/login_linux_test.go b/cmd/nerdctl/login/login_linux_test.go index 0a53a6a623e..2ac21fa374f 100644 --- a/cmd/nerdctl/login/login_linux_test.go +++ b/cmd/nerdctl/login/login_linux_test.go @@ -31,6 +31,7 @@ import ( "github.com/containerd/nerdctl/v2/pkg/imgutil/dockerconfigresolver" "github.com/containerd/nerdctl/v2/pkg/testutil" + "github.com/containerd/nerdctl/v2/pkg/testutil/test" "github.com/containerd/nerdctl/v2/pkg/testutil/testca" "github.com/containerd/nerdctl/v2/pkg/testutil/testregistry" ) @@ -108,8 +109,8 @@ func TestLoginPersistence(t *testing.T) { t.Run(fmt.Sprintf("Server %s", tc.auth), func(t *testing.T) { t.Parallel() - username := testregistry.SafeRandomString(30) + "∞" - password := testregistry.SafeRandomString(30) + ":∞" + username := test.RandomStringBase64(30) + "∞" + password := test.RandomStringBase64(30) + ":∞" // Add the requested authentication var auth testregistry.Auth @@ -297,8 +298,8 @@ func TestLoginAgainstVariants(t *testing.T) { } // Generate credentials that are specific to each registry, so that we never cross hit another one - username := testregistry.SafeRandomString(30) + "∞" - password := testregistry.SafeRandomString(30) + ":∞" + username := test.RandomStringBase64(30) + "∞" + password := test.RandomStringBase64(30) + ":∞" // Get a CA if we want TLS var ca *testca.CA diff --git a/pkg/testutil/testregistry/testregistry_linux.go b/pkg/testutil/testregistry/testregistry_linux.go index 72701d5998d..d6610f9046a 100644 --- a/pkg/testutil/testregistry/testregistry_linux.go +++ b/pkg/testutil/testregistry/testregistry_linux.go @@ -17,8 +17,6 @@ package testregistry import ( - "crypto/rand" - "encoding/base64" "fmt" "net" "os" @@ -249,75 +247,6 @@ func (ba *BasicAuth) Params(base *testutil.Base) []string { return ret } -func NewIPFSRegistry(base *testutil.Base, ca *testca.CA, port int, auth Auth, boundCleanup func(error)) *RegistryServer { - EnsureImages(base) - - name := testutil.Identifier(base.T) - // listen on 0.0.0.0 to enable 127.0.0.1 - listenIP := net.ParseIP("0.0.0.0") - hostIP, err := nettestutil.NonLoopbackIPv4() - assert.NilError(base.T, err, fmt.Errorf("failed finding ipv4 non loopback interface: %w", err)) - port, err = portlock.Acquire(port) - assert.NilError(base.T, err, fmt.Errorf("failed acquiring port: %w", err)) - - containerName := fmt.Sprintf("ipfs-registry-%s-%d", name, port) - // Cleanup possible leftovers first - base.Cmd("rm", "-f", containerName).Run() - - args := []string{ - "run", - "--pull=never", - "-d", - "-p", fmt.Sprintf("%s:%d:%d", listenIP, port, port), - "--name", containerName, - "--entrypoint=/bin/sh", - testutil.KuboImage, - "-c", "--", - fmt.Sprintf("ipfs init && ipfs config Addresses.API /ip4/0.0.0.0/tcp/%d && ipfs daemon --offline", port), - } - - cleanup := func(err error) { - result := base.Cmd("rm", "-f", containerName).Run() - errPortRelease := portlock.Release(port) - if boundCleanup != nil { - boundCleanup(err) - } - if err == nil { - assert.NilError(base.T, result.Error, fmt.Errorf("failed removing container: %w", err)) - assert.NilError(base.T, errPortRelease, fmt.Errorf("failed releasing port: %w", err)) - } - } - - scheme := "http" - - err = func() error { - cmd := base.Cmd(args...).Run() - if cmd.Error != nil { - base.T.Logf("%s:\n%s\n%s\n-------\n%s", containerName, cmd.Cmd, cmd.Stdout(), cmd.Stderr()) - return cmd.Error - } - - if _, err = nettestutil.HTTPGet(fmt.Sprintf("%s://%s:%s/api/v0", scheme, hostIP.String(), strconv.Itoa(port)), 30, true); err != nil { - return err - } - - return nil - }() - - assert.NilError(base.T, err, fmt.Errorf("failed starting IPFS registry container in a timely manner: %w", err)) - - return &RegistryServer{ - IP: hostIP, - Port: port, - Scheme: scheme, - ListenIP: listenIP, - Cleanup: cleanup, - Logs: func() { - base.T.Logf("%s: %q", containerName, base.Cmd("logs", containerName).Run().String()) - }, - } -} - func NewRegistry(base *testutil.Base, ca *testca.CA, port int, auth Auth, boundCleanup func(error)) *RegistryServer { EnsureImages(base) @@ -469,29 +398,3 @@ func NewWithNoAuth(base *testutil.Base, port int, tls bool) *RegistryServer { } return NewRegistry(base, ca, port, &NoAuth{}, nil) } - -func NewWithBasicAuth(base *testutil.Base, user, pass string, port int, tls bool) *RegistryServer { - auth := &BasicAuth{ - Username: user, - Password: pass, - } - var ca *testca.CA - if tls { - ca = testca.New(base.T) - } - return NewRegistry(base, ca, port, auth, nil) -} - -func SafeRandomString(n int) string { - b := make([]byte, n) - l, err := rand.Read(b) - if err != nil { - panic(err) - } - if l != n { - panic(fmt.Errorf("expected %d bytes, got %d bytes", n, l)) - } - // XXX WARNING there is something in the registry (or more likely in the way we generate htpasswd files) - // that is broken and does not resist truly random strings - return base64.URLEncoding.EncodeToString(b) -}