From 72479b3c4c527bd985aed5f871dbe6faf5bf7a0b Mon Sep 17 00:00:00 2001 From: apostasie Date: Mon, 14 Oct 2024 21:13:18 -0700 Subject: [PATCH 01/10] 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 3828b178843ef1901d138c89904de15400a1fe54 Mon Sep 17 00:00:00 2001 From: apostasie Date: Mon, 14 Oct 2024 21:19:38 -0700 Subject: [PATCH 02/10] Mark flaky tests Signed-off-by: apostasie --- cmd/nerdctl/container/container_logs_test.go | 1 + cmd/nerdctl/image/image_inspect_test.go | 5 +++++ cmd/nerdctl/image/image_save_test.go | 5 +++++ cmd/nerdctl/ipfs/ipfs_compose_linux_test.go | 2 ++ 4 files changed, 13 insertions(+) diff --git a/cmd/nerdctl/container/container_logs_test.go b/cmd/nerdctl/container/container_logs_test.go index 085c5f840c6..71debda52e9 100644 --- a/cmd/nerdctl/container/container_logs_test.go +++ b/cmd/nerdctl/container/container_logs_test.go @@ -82,6 +82,7 @@ func TestLogsOutStreamsSeparated(t *testing.T) { } func TestLogsWithInheritedFlags(t *testing.T) { + // Seen flaky with Docker t.Parallel() base := testutil.NewBase(t) for k, v := range base.Args { diff --git a/cmd/nerdctl/image/image_inspect_test.go b/cmd/nerdctl/image/image_inspect_test.go index 792c3ddc81e..816b163abbd 100644 --- a/cmd/nerdctl/image/image_inspect_test.go +++ b/cmd/nerdctl/image/image_inspect_test.go @@ -18,6 +18,7 @@ package image import ( "encoding/json" + "runtime" "strings" "testing" @@ -63,6 +64,10 @@ func TestImageInspectSimpleCases(t *testing.T) { }, } + if runtime.GOOS == "windows" { + testCase.Require = nerdtest.IsFlaky("https://github.com/containerd/nerdctl/issues/3524") + } + testCase.Run(t) } diff --git a/cmd/nerdctl/image/image_save_test.go b/cmd/nerdctl/image/image_save_test.go index 23860cf443e..eb2617a1eb3 100644 --- a/cmd/nerdctl/image/image_save_test.go +++ b/cmd/nerdctl/image/image_save_test.go @@ -19,6 +19,7 @@ package image import ( "os" "path/filepath" + "runtime" "strings" "testing" @@ -70,6 +71,10 @@ func TestSave(t *testing.T) { // See https://github.com/containerd/nerdctl/issues/3425 and others for details. testCase.Require = nerdtest.Private + if runtime.GOOS == "windows" { + testCase.Require = nerdtest.IsFlaky("https://github.com/containerd/nerdctl/issues/3524") + } + testCase.SubTests = []*test.Case{ { Description: "Single image, by id", diff --git a/cmd/nerdctl/ipfs/ipfs_compose_linux_test.go b/cmd/nerdctl/ipfs/ipfs_compose_linux_test.go index a8c597de950..e1b78cc4c1e 100644 --- a/cmd/nerdctl/ipfs/ipfs_compose_linux_test.go +++ b/cmd/nerdctl/ipfs/ipfs_compose_linux_test.go @@ -46,6 +46,7 @@ func TestIPFSCompNoBuild(t *testing.T) { test.Not(nerdtest.Docker), nerdtest.Registry, nerdtest.IPFS, + nerdtest.IsFlaky("https://github.com/containerd/nerdctl/issues/3510"), // See note below // nerdtest.Private, ) @@ -153,6 +154,7 @@ services: WORDPRESS_DB_USER: exampleuser WORDPRESS_DB_PASSWORD: examplepass WORDPRESS_DB_NAME: exampledb + # FIXME: this is flaky and will make the container fail on occasions volumes: - wordpress:/var/www/html From c09972fad72c8bbdfee1a29b862e3ebfe5913bb1 Mon Sep 17 00:00:00 2001 From: apostasie Date: Tue, 15 Oct 2024 01:17:37 -0700 Subject: [PATCH 03/10] 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 0ab60d382d8..4bfcb57d8bf 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 8dd8a1edce6..6663e138c26 100644 --- a/pkg/testutil/testutil.go +++ b/pkg/testutil/testutil.go @@ -767,14 +767,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) } @@ -850,16 +842,6 @@ func RegisterBuildCacheCleanup(t *testing.T) { }) } -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, - } -} - // SetupDockerContainerBuilder creates a Docker builder using the docker-container driver // and adds cleanup steps to test cleanup. The builder name is returned as output. // From a3d740732ab82df1db6985778625641be30a209c Mon Sep 17 00:00:00 2001 From: apostasie Date: Tue, 15 Oct 2024 01:19:50 -0700 Subject: [PATCH 04/10] 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) -} From 031e532d0be972e35f9a5ac7ff801e283b225d55 Mon Sep 17 00:00:00 2001 From: apostasie Date: Tue, 15 Oct 2024 01:21:04 -0700 Subject: [PATCH 05/10] Fix semantic of Fail to not care about exit code Signed-off-by: apostasie --- pkg/testutil/test/helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/testutil/test/helpers.go b/pkg/testutil/test/helpers.go index 99d769d7c3e..06411df8e99 100644 --- a/pkg/testutil/test/helpers.go +++ b/pkg/testutil/test/helpers.go @@ -70,7 +70,7 @@ func (help *helpersInternal) Anyhow(args ...string) { // Fail will run a command and make sure it does fail func (help *helpersInternal) Fail(args ...string) { help.Command(args...).Run(&Expected{ - ExitCode: 1, + ExitCode: -1, }) } From c0b7f9a387247588184d9a10baa47a61493605ab Mon Sep 17 00:00:00 2001 From: apostasie Date: Tue, 15 Oct 2024 01:22:17 -0700 Subject: [PATCH 06/10] Chores: spurious comments and syntax simplification Signed-off-by: apostasie --- cmd/nerdctl/network/network_create_linux_test.go | 8 +------- cmd/nerdctl/network/network_list_linux_test.go | 3 --- pkg/cmd/image/ensure.go | 3 --- pkg/testutil/testutil.go | 9 ++------- 4 files changed, 3 insertions(+), 20 deletions(-) diff --git a/cmd/nerdctl/network/network_create_linux_test.go b/cmd/nerdctl/network/network_create_linux_test.go index 2d1274f19ea..596375f33db 100644 --- a/cmd/nerdctl/network/network_create_linux_test.go +++ b/cmd/nerdctl/network/network_create_linux_test.go @@ -35,8 +35,6 @@ func TestNetworkCreate(t *testing.T) { testCase.SubTests = []*test.Case{ { Description: "vanilla", - // #3491 and #3508 may have helped - commenting this out for now - // Require: nerdtest.IsFlaky("https://github.com/containerd/nerdctl/issues/3086"), Setup: func(data test.Data, helpers test.Helpers) { helpers.Ensure("network", "create", data.Identifier()) netw := nerdtest.InspectNetwork(helpers, data.Identifier()) @@ -66,8 +64,6 @@ func TestNetworkCreate(t *testing.T) { }, { Description: "with MTU", - // #3491 and #3508 may have helped - commenting this out for now - // Require: nerdtest.IsFlaky("https://github.com/containerd/nerdctl/issues/3086"), Setup: func(data test.Data, helpers test.Helpers) { helpers.Ensure("network", "create", data.Identifier(), "--driver", "bridge", "--opt", "com.docker.network.driver.mtu=9216") }, @@ -81,9 +77,7 @@ func TestNetworkCreate(t *testing.T) { }, { Description: "with ipv6", - // #3491 and #3508 may have helped - commenting this out for now - // Require: nerdtest.IsFlaky("https://github.com/containerd/nerdctl/issues/3086"), - Require: nerdtest.OnlyIPv6, + Require: nerdtest.OnlyIPv6, Setup: func(data test.Data, helpers test.Helpers) { subnetStr := "2001:db8:8::/64" data.Set("subnetStr", subnetStr) diff --git a/cmd/nerdctl/network/network_list_linux_test.go b/cmd/nerdctl/network/network_list_linux_test.go index af8c8d1ca0b..22620287a66 100644 --- a/cmd/nerdctl/network/network_list_linux_test.go +++ b/cmd/nerdctl/network/network_list_linux_test.go @@ -29,9 +29,6 @@ import ( func TestNetworkLsFilter(t *testing.T) { testCase := nerdtest.Setup() - // #3491 and #3508 may have helped - commenting this out for now - // testCase.Require = nerdtest.IsFlaky("https://github.com/containerd/nerdctl/issues/3086"), - testCase.Setup = func(data test.Data, helpers test.Helpers) { data.Set("identifier", data.Identifier()) data.Set("label", "mylabel=label-1") diff --git a/pkg/cmd/image/ensure.go b/pkg/cmd/image/ensure.go index de2634871ea..fe5f6ca88c5 100644 --- a/pkg/cmd/image/ensure.go +++ b/pkg/cmd/image/ensure.go @@ -65,9 +65,6 @@ func ensureOne(ctx context.Context, client *containerd.Client, rawRef string, ta if err != nil { return err } - // if platform == nil { - // platform = platforms.DefaultSpec() - //} pltf := []ocispec.Platform{platform} platformComparer := platformutil.NewMatchComparerFromOCISpecPlatformSlice(pltf) diff --git a/pkg/testutil/testutil.go b/pkg/testutil/testutil.go index 6663e138c26..eb70bfce5b0 100644 --- a/pkg/testutil/testutil.go +++ b/pkg/testutil/testutil.go @@ -180,9 +180,7 @@ func (b *Base) EnsureDaemonActive() { sleep = 3 * time.Second ) for i := 0; i < maxRetry; i++ { - cmd := exec.Command("systemctl", - append(systemctlArgs, - []string{"is-active", target}...)...) + cmd := exec.Command("systemctl", append(systemctlArgs, "is-active", target)...) out, err := cmd.CombinedOutput() b.T.Logf("(retry=%d) %s", i, string(out)) if err == nil { @@ -204,10 +202,7 @@ func (b *Base) DumpDaemonLogs(minutes int) { b.T.Helper() target := b.systemctlTarget() cmd := exec.Command("journalctl", - append(b.systemctlArgs(), - []string{"-u", target, - "--no-pager", - "-S", fmt.Sprintf("%d min ago", minutes)}...)...) + append(b.systemctlArgs(), "-u", target, "--no-pager", "-S", fmt.Sprintf("%d min ago", minutes))...) b.T.Logf("===== %v =====", cmd.Args) out, err := cmd.CombinedOutput() if err != nil { From ca6e165f01469fd41a629ad8c3be2b51e96e286e Mon Sep 17 00:00:00 2001 From: apostasie Date: Tue, 15 Oct 2024 01:22:54 -0700 Subject: [PATCH 07/10] Mark all legacy tests as flaky to preserve behavior Signed-off-by: apostasie --- pkg/testutil/testutil.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/testutil/testutil.go b/pkg/testutil/testutil.go index eb70bfce5b0..fe3fb0004ee 100644 --- a/pkg/testutil/testutil.go +++ b/pkg/testutil/testutil.go @@ -787,6 +787,9 @@ func newBase(t *testing.T, ns string, ipv6Compatible bool, kubernetesCompatible } else if !base.EnableKubernetes && base.KubernetesCompatible { t.Skip("runner skips Kubernetes compatible tests in the non-Kubernetes environment") } + if !GetFlakyEnvironment() && !GetEnableKubernetes() && !GetEnableIPv6() { + t.Skip("legacy tests are considered flaky by default and are skipped unless in the flaky environment") + } var err error switch base.Target { case Nerdctl: From 4dcf569db364987f8b39232dbceab49431d65238 Mon Sep 17 00:00:00 2001 From: apostasie Date: Tue, 15 Oct 2024 01:13:16 -0700 Subject: [PATCH 08/10] Cleanup actions and test integration triggers Signed-off-by: apostasie --- .../ghcr-image-build-and-publish.yml | 3 +- .github/workflows/lint.yml | 82 +++++++++++ .github/workflows/project.yml | 2 +- .github/workflows/test-canary.yml | 33 +---- .github/workflows/test-kube.yml | 5 +- .github/workflows/test.yml | 139 ++++++++---------- Dockerfile | 14 +- Makefile | 3 + docs/testing/README.md | 2 +- hack/build-integration-kubernetes.sh | 4 +- hack/test-integration.sh | 36 +++++ 11 files changed, 199 insertions(+), 124 deletions(-) create mode 100644 .github/workflows/lint.yml create mode 100755 hack/test-integration.sh diff --git a/.github/workflows/ghcr-image-build-and-publish.yml b/.github/workflows/ghcr-image-build-and-publish.yml index ebb35de53cb..bd7941780a2 100644 --- a/.github/workflows/ghcr-image-build-and-publish.yml +++ b/.github/workflows/ghcr-image-build-and-publish.yml @@ -1,4 +1,4 @@ -name: Container Image Build +name: image # This workflow uses actions that are not certified by GitHub. # They are provided by a third-party and are governed by @@ -21,7 +21,6 @@ env: # github.repository as / IMAGE_NAME: ${{ github.repository }} - jobs: build: diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 00000000000..211c284f128 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,82 @@ +name: lint + +on: + push: + branches: + - main + - 'release/**' + pull_request: + +env: + GO_VERSION: 1.23.x + +jobs: + go: + timeout-minutes: 5 + name: "go | ${{ matrix.goos }} | ${{ matrix.gonext }}" + runs-on: "${{ matrix.os }}" + strategy: + matrix: + include: + - os: ubuntu-24.04 + goos: linux + - os: ubuntu-24.04 + goos: freebsd + # FIXME: this is currently failing in a non-sensical way, so, running on linux instead... + # - os: windows-2022 + - os: ubuntu-24.04 + goos: windows + - os: ubuntu-24.04 + goos: linux + # This allows the canary script to select any upcoming golang alpha/beta/RC + gonext: next + - os: ubuntu-24.04 + goos: freebsd + gonext: next + - os: ubuntu-24.04 + goos: windows + gonext: next + env: + GOOS: "${{ matrix.goos }}" + steps: + - uses: actions/checkout@v4.2.1 + with: + fetch-depth: 1 + - name: Set GO env + run: | + # If gonext is specified, get the latest available golang pre-release instead of the major version + if [ "$gonext" != "" ]; then + . ./hack/build-integration-canary.sh + canary::golang::latest + fi + - uses: actions/setup-go@v5 + with: + go-version: ${{ env.GO_VERSION }} + check-latest: true + cache: true + - name: golangci-lint + uses: golangci/golangci-lint-action@v6 + with: + args: --verbose + + other: + timeout-minutes: 5 + name: yaml | shell | imports order + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v4.2.1 + with: + fetch-depth: 1 + - uses: actions/setup-go@v5 + with: + go-version: ${{ env.GO_VERSION }} + check-latest: true + cache: true + - name: yaml + run: make lint-yaml + - name: shell + run: make lint-shell + - name: go imports ordering + run: | + go install -v github.com/incu6us/goimports-reviser/v3@latest + make lint-imports diff --git a/.github/workflows/project.yml b/.github/workflows/project.yml index a03da712e02..6961f0fd565 100644 --- a/.github/workflows/project.yml +++ b/.github/workflows/project.yml @@ -9,7 +9,7 @@ on: jobs: project: - name: Project Checks + name: checks runs-on: ubuntu-24.04 timeout-minutes: 20 steps: diff --git a/.github/workflows/test-canary.yml b/.github/workflows/test-canary.yml index dc07fe92de7..f113c5ae86f 100644 --- a/.github/workflows/test-canary.yml +++ b/.github/workflows/test-canary.yml @@ -15,27 +15,6 @@ env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} jobs: - lint: - runs-on: "ubuntu-24.04" - timeout-minutes: 20 - steps: - - uses: actions/checkout@v4.2.1 - with: - fetch-depth: 1 - - name: Set GO env - run: | - . ./hack/build-integration-canary.sh - canary::golang::latest - - uses: actions/setup-go@v5 - with: - go-version: ${{ env.GO_VERSION }} - check-latest: true - cache: true - - name: golangci-lint - uses: golangci/golangci-lint-action@v6.1.1 - with: - args: --verbose - linux: runs-on: "ubuntu-24.04" timeout-minutes: 40 @@ -65,11 +44,13 @@ jobs: - name: "Run unit tests" run: go test -v ./pkg/... - name: "Run integration tests" - run: docker run -t --rm --privileged test-integration + run: docker run -t --rm --privileged test-integration ./hack/test-integration.sh + - name: "Run integration tests (flaky)" + run: docker run -t --rm --privileged test-integration ./hack/test-integration.sh -test.only-flaky windows: - runs-on: windows-latest timeout-minutes: 30 + runs-on: windows-latest defaults: run: shell: bash @@ -95,6 +76,7 @@ jobs: cache: true check-latest: true - run: go install ./cmd/nerdctl + - run: go install -v gotest.tools/gotestsum@v1 # This here is solely to get the cni install script, which has not been modified in 3+ years. # There is little to no reason to update this to latest containerd - uses: actions/checkout@v4.2.1 @@ -112,5 +94,6 @@ jobs: ctrdVersion: ${{ env.CONTAINERD_VERSION }} run: powershell hack/configure-windows-ci.ps1 - name: "Run integration tests" - # See https://github.com/containerd/nerdctl/blob/main/docs/testing/README.md#about-parallelization - run: go test -p 1 -v ./cmd/nerdctl/... + run: ./hack/test-integration.sh + - name: "Run integration tests (flaky)" + run: ./hack/test-integration.sh -test.only-flaky diff --git a/.github/workflows/test-kube.yml b/.github/workflows/test-kube.yml index c8e2ccda405..3c6faaaa457 100644 --- a/.github/workflows/test-kube.yml +++ b/.github/workflows/test-kube.yml @@ -10,13 +10,12 @@ on: paths-ignore: - '**.md' -env: - ROOTFUL: true - jobs: linux: runs-on: "ubuntu-24.04" timeout-minutes: 40 + env: + ROOTFUL: true steps: - uses: actions/checkout@v4.2.1 with: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4577a894cce..46318b58a49 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,37 +11,25 @@ on: env: GO_VERSION: 1.23.x + SHORT_TIMEOUT: 5 + LONG_TIMEOUT: 60 jobs: - lint: - runs-on: ubuntu-24.04 - timeout-minutes: 20 - steps: - - uses: actions/checkout@v4.2.1 - with: - fetch-depth: 1 - - uses: actions/setup-go@v5 - with: - go-version: ${{ env.GO_VERSION }} - check-latest: true - cache: true - - name: golangci-lint - uses: golangci/golangci-lint-action@v6.1.1 - with: - version: v1.60.1 - args: --verbose - - name: yamllint-lint - run: make lint-yaml - - name: shellcheck - run: make lint-shell - - name: go imports ordering - run: | - go install -v github.com/incu6us/goimports-reviser/v3@latest - make lint-imports - test-unit: - runs-on: ubuntu-24.04 - timeout-minutes: 20 + # Supposed to work: https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/evaluate-expressions-in-workflows-and-actions#example-returning-a-json-data-type + # Apparently does not + # timeout-minutes: ${{ fromJSON(env.SHORT_TIMEOUT) }} + timeout-minutes: 5 + name: unit | ${{ matrix.goos }} + runs-on: "${{ matrix.os }}" + strategy: + matrix: + include: + # FIXME: currently disabled as a lot more work is required to make these tests pass on windows + # - os: windows-2022 + # goos: windows + - os: ubuntu-24.04 + goos: linux steps: - uses: actions/checkout@v4.2.1 with: @@ -52,11 +40,12 @@ jobs: check-latest: true cache: true - name: "Run unit tests" - run: go test -v ./pkg/... + run: make test-unit test-integration: + timeout-minutes: 60 + name: rootful | ${{ matrix.containerd }} | ${{ matrix.runner }} runs-on: "${{ matrix.runner }}" - timeout-minutes: 40 strategy: fail-fast: false matrix: @@ -99,23 +88,23 @@ jobs: docker run --privileged --rm tonistiigi/binfmt --install linux/arm64 docker run --privileged --rm tonistiigi/binfmt --install linux/arm/v7 - name: "Run integration tests" - uses: nick-fields/retry@v3 - with: - timeout_minutes: 30 - max_attempts: 2 - retry_on: error - command: docker run -t --rm --privileged test-integration + run: | + docker run -t --rm --privileged test-integration ./hack/test-integration.sh + - name: "Run integration tests (flaky)" + run: | + docker run -t --rm --privileged test-integration ./hack/test-integration.sh -test.only-flaky test-integration-ipv6: + timeout-minutes: 60 + name: ipv6 | ${{ matrix.containerd }} | ${{ matrix.ubuntu }} runs-on: "ubuntu-${{ matrix.ubuntu }}" - timeout-minutes: 40 strategy: fail-fast: false matrix: # ubuntu-20.04: cgroup v1, ubuntu-22.04 and later: cgroup v2 include: - ubuntu: 24.04 - containerd: v1.7.23 + containerd: v2.0.0-rc.5 env: UBUNTU_VERSION: "${{ matrix.ubuntu }}" CONTAINERD_VERSION: "${{ matrix.containerd }}" @@ -133,7 +122,7 @@ jobs: echo '{"ipv6": true, "fixed-cidr-v6": "2001:db8:1::/64", "experimental": true, "ip6tables": true}' | sudo tee /etc/docker/daemon.json sudo systemctl restart docker - name: "Prepare integration test environment" - run: docker build -t test-integration-ipv6 --target test-integration-ipv6 --build-arg UBUNTU_VERSION=${UBUNTU_VERSION} --build-arg CONTAINERD_VERSION=${CONTAINERD_VERSION} . + run: docker build -t test-integration --target test-integration --build-arg UBUNTU_VERSION=${UBUNTU_VERSION} --build-arg CONTAINERD_VERSION=${CONTAINERD_VERSION} . - name: "Remove snap loopback devices (conflicts with our loopback devices in TestRunDevice)" run: | sudo systemctl disable --now snapd.service snapd.socket @@ -151,20 +140,16 @@ jobs: docker run --privileged --rm tonistiigi/binfmt --install linux/arm/v7 - name: "Run integration tests" # The nested IPv6 network inside docker and qemu is complex and needs a bunch of sysctl config. - # Therefore it's hard to debug why the IPv6 tests fail in such an isolation layer. + # Therefore, it's hard to debug why the IPv6 tests fail in such an isolation layer. # On the other side, using the host network is easier at configuration. # Besides, each job is running on a different instance, which means using host network here # is safe and has no side effects on others. - uses: nick-fields/retry@v3 - with: - timeout_minutes: 30 - max_attempts: 2 - retry_on: error - command: docker run --network host -t --rm --privileged test-integration-ipv6 + run: docker run --network host -t --rm --privileged test-integration ./hack/test-integration.sh -test.only-ipv6 test-integration-rootless: - runs-on: "ubuntu-${{ matrix.ubuntu }}" timeout-minutes: 60 + name: "${{ matrix.target }} | ${{ matrix.containerd }} | ${{ matrix.rootlesskit }} | ${{ matrix.ubuntu }}" + runs-on: "ubuntu-${{ matrix.ubuntu }}" strategy: fail-fast: false matrix: @@ -173,24 +158,24 @@ jobs: - ubuntu: 20.04 containerd: v1.6.36 rootlesskit: v1.1.1 # Deprecated - target: test-integration-rootless + target: rootless - ubuntu: 22.04 containerd: v1.7.23 rootlesskit: v2.3.1 - target: test-integration-rootless + target: rootless - ubuntu: 24.04 containerd: v2.0.0-rc.5 rootlesskit: v2.3.1 - target: test-integration-rootless + target: rootless - ubuntu: 24.04 containerd: v1.7.23 rootlesskit: v2.3.1 - target: test-integration-rootless-port-slirp4netns + target: rootless-port-slirp4netns env: UBUNTU_VERSION: "${{ matrix.ubuntu }}" CONTAINERD_VERSION: "${{ matrix.containerd }}" ROOTLESSKIT_VERSION: "${{ matrix.rootlesskit }}" - TEST_TARGET: "${{ matrix.target }}" + TEST_TARGET: "test-integration-${{ matrix.target }}" steps: - name: "Set up AppArmor" if: matrix.ubuntu == '24.04' @@ -230,16 +215,14 @@ jobs: fi echo "WORKAROUND_ISSUE_622=${WORKAROUND_ISSUE_622}" >> "$GITHUB_ENV" - name: "Test (network driver=slirp4netns, port driver=builtin)" - uses: nick-fields/retry@v3 - with: - timeout_minutes: 30 - max_attempts: 2 - retry_on: error - command: docker run -t --rm --privileged -e WORKAROUND_ISSUE_622=${WORKAROUND_ISSUE_622} ${TEST_TARGET} + run: docker run -t --rm --privileged -e WORKAROUND_ISSUE_622=${WORKAROUND_ISSUE_622} ${TEST_TARGET} /test-integration-rootless.sh ./hack/test-integration.sh + - name: "Test (network driver=slirp4netns, port driver=builtin) (flaky)" + run: docker run -t --rm --privileged -e WORKAROUND_ISSUE_622=${WORKAROUND_ISSUE_622} ${TEST_TARGET} /test-integration-rootless.sh ./hack/test-integration.sh -test.only-flaky - cross: + build: + timeout-minutes: 5 + name: "build | ${{ matrix.go-version }}" runs-on: ubuntu-24.04 - timeout-minutes: 40 strategy: matrix: go-version: ["1.22.x", "1.23.x"] @@ -252,12 +235,13 @@ jobs: go-version: ${{ matrix.go-version }} cache: true check-latest: true - - name: "Cross" + - name: "build" run: GO_VERSION="$(echo ${{ matrix.go-version }} | sed -e s/.x//)" make binaries test-integration-docker-compatibility: + timeout-minutes: 60 + name: docker runs-on: ubuntu-24.04 - timeout-minutes: 45 steps: - uses: actions/checkout@v4.2.1 with: @@ -284,26 +268,18 @@ jobs: - name: "Prepare integration test environment" run: | sudo apt-get install -y expect + go install -v gotest.tools/gotestsum@v1 - name: "Ensure that the integration test suite is compatible with Docker" - uses: nick-fields/retry@v3 - with: - timeout_minutes: 30 - max_attempts: 2 - retry_on: error - # See https://github.com/containerd/nerdctl/blob/main/docs/testing/README.md#about-parallelization - command: go test -p 1 -timeout 20m -v -exec sudo ./cmd/nerdctl/... -args -test.target=docker -test.allow-kill-daemon + run: ./hack/test-integration.sh -test.target=docker - name: "Ensure that the IPv6 integration test suite is compatible with Docker" - uses: nick-fields/retry@v3 - with: - timeout_minutes: 30 - max_attempts: 2 - retry_on: error - # See https://github.com/containerd/nerdctl/blob/main/docs/testing/README.md#about-parallelization - command: go test -p 1 -timeout 20m -v -exec sudo ./cmd/nerdctl/... -args -test.target=docker -test.allow-kill-daemon -test.only-ipv6 + run: ./hack/test-integration.sh -test.target=docker -test.only-ipv6 + - name: "Ensure that the integration test suite is compatible with Docker (flaky only)" + run: go test -p 1 -timeout 20m -v -exec sudo ./cmd/nerdctl/... -args -test.target=docker -test.allow-kill-daemon -test.only-flaky test-integration-windows: - runs-on: windows-2022 timeout-minutes: 30 + name: windows + runs-on: windows-2022 defaults: run: shell: bash @@ -317,6 +293,7 @@ jobs: cache: true check-latest: true - run: go install ./cmd/nerdctl + - run: go install -v gotest.tools/gotestsum@v1 - uses: actions/checkout@v4.2.1 with: repository: containerd/containerd @@ -330,16 +307,16 @@ jobs: env: ctrdVersion: 1.7.23 run: powershell hack/configure-windows-ci.ps1 - # TODO: Run unit tests - name: "Run integration tests" - # See https://github.com/containerd/nerdctl/blob/main/docs/testing/README.md#about-parallelization - run: go test -p 1 -v ./cmd/nerdctl/... + run: ./hack/test-integration.sh + - name: "Run integration tests (flaky)" + run: ./hack/test-integration.sh -test.only-flaky test-integration-freebsd: + timeout-minutes: 60 name: FreeBSD # ubuntu-24.04 lacks the vagrant package runs-on: ubuntu-22.04 - timeout-minutes: 20 steps: - uses: actions/checkout@v4.2.1 diff --git a/Dockerfile b/Dockerfile index a594358aee4..f1e32fd4153 100644 --- a/Dockerfile +++ b/Dockerfile @@ -276,7 +276,8 @@ ARG DEBIAN_FRONTEND=noninteractive # `expect` package contains `unbuffer(1)`, which is used for emulating TTY for testing RUN apt-get update -qq && apt-get install -qq --no-install-recommends \ expect \ - git + git \ + make COPY --from=goversion /GOVERSION /GOVERSION ARG TARGETARCH RUN curl -fsSL --proto '=https' --tlsv1.2 https://golang.org/dl/$(cat /GOVERSION).linux-${TARGETARCH:-amd64}.tar.gz | tar xzvC /usr/local @@ -314,8 +315,7 @@ RUN curl -o nydus-static.tgz -fsSL --proto '=https' --tlsv1.2 "https://github.co tar xzf nydus-static.tgz && \ mv nydus-static/nydus-image nydus-static/nydusd nydus-static/nydusify /usr/bin/ && \ rm nydus-static.tgz -CMD ["gotestsum", "--format=testname", "--rerun-fails=2", "--packages=./cmd/nerdctl/...", \ - "--", "-timeout=60m", "-p", "1", "-args", "-test.allow-kill-daemon"] +CMD ["./hack/test-integration.sh"] FROM test-integration AS test-integration-rootless # Install SSH for creating systemd user session. @@ -338,17 +338,11 @@ RUN systemctl disable test-integration-ipfs-offline VOLUME /home/rootless/.local/share COPY ./Dockerfile.d/test-integration-rootless.sh / RUN chmod a+rx /test-integration-rootless.sh -CMD ["/test-integration-rootless.sh", \ - "gotestsum", "--format=testname", "--rerun-fails=2", "--packages=./cmd/nerdctl/...", \ - "--", "-timeout=60m", "-p", "1", "-args", "-test.allow-kill-daemon"] +CMD ["/test-integration-rootless.sh", "./hack/test-integration.sh"] # test for CONTAINERD_ROOTLESS_ROOTLESSKIT_PORT_DRIVER=slirp4netns FROM test-integration-rootless AS test-integration-rootless-port-slirp4netns COPY ./Dockerfile.d/home_rootless_.config_systemd_user_containerd.service.d_port-slirp4netns.conf /home/rootless/.config/systemd/user/containerd.service.d/port-slirp4netns.conf RUN chown -R rootless:rootless /home/rootless/.config -FROM test-integration AS test-integration-ipv6 -CMD ["gotestsum", "--format=testname", "--rerun-fails=2", "--packages=./cmd/nerdctl/...", \ - "--", "-timeout=60m", "-p", "1", "-args", "-test.allow-kill-daemon", "-test.only-ipv6"] - FROM base AS demo diff --git a/Makefile b/Makefile index 0831c640047..ae4e18c94f3 100644 --- a/Makefile +++ b/Makefile @@ -86,6 +86,9 @@ lint-yaml: lint-shell: $(call recursive_wildcard,$(MAKEFILE_DIR)/,*.sh) shellcheck -a -x $^ +test-unit: + go test -v $(MAKEFILE_DIR)/pkg/... + binaries: nerdctl install: diff --git a/docs/testing/README.md b/docs/testing/README.md index c5bddc99285..7a678e2956b 100644 --- a/docs/testing/README.md +++ b/docs/testing/README.md @@ -58,7 +58,7 @@ explicitly allow it (with a call to `t.Parallel()`). ```bash docker build -t test-integration --target test-integration . -docker run -t --rm --privileged test-integration +docker run -t --rm --privileged test-integration ./hack/test-integration.sh ``` ### Principles diff --git a/hack/build-integration-kubernetes.sh b/hack/build-integration-kubernetes.sh index e41f13fcf7f..7fcea04c5cb 100755 --- a/hack/build-integration-kubernetes.sh +++ b/hack/build-integration-kubernetes.sh @@ -104,8 +104,10 @@ main(){ # Hack to get go into kind control plane exec::nerdctl rm -f go-kind 2>/dev/null || true - exec::nerdctl run -d --name go-kind golang:"$GO_VERSION" sleep Inf + exec::nerdctl pull --quiet golang:"$GO_VERSION" + exec::nerdctl run -d --pull never --name go-kind golang:"$GO_VERSION" sleep Inf exec::nerdctl cp go-kind:/usr/local/go /tmp/go + exec::nerdctl rm -f go-kind # Create fresh cluster log::info "Creating new cluster" diff --git a/hack/test-integration.sh b/hack/test-integration.sh new file mode 100755 index 00000000000..74f8b81f448 --- /dev/null +++ b/hack/test-integration.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash + +# Copyright The containerd Authors. + +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at + +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# shellcheck disable=SC2034,SC2015 +set -o errexit -o errtrace -o functrace -o nounset -o pipefail +root="$(cd "$(dirname "${BASH_SOURCE[0]:-$PWD}")" 2>/dev/null 1>&2 && pwd)" +readonly root +readonly timeout="60m" + +# See https://github.com/containerd/nerdctl/blob/main/docs/testing/README.md#about-parallelization +args=(--format=testname --jsonfile /tmp/test-integration.log --packages="$root"/../cmd/nerdctl/...) + +for arg in "$@"; do + if [ "$arg" == "-test.only-flaky" ]; then + args+=("--rerun-fails=2") + break + fi +done + +gotestsum "${args[@]}" -- -timeout="$timeout" -p 1 -args -test.allow-kill-daemon "$@" + +echo "These are the tests that took more than 20 seconds:" +gotestsum tool slowest --threshold 20s --jsonfile /tmp/test-integration.log From 13425b543f022c03be8ca09d9a101775bf2bffe9 Mon Sep 17 00:00:00 2001 From: apostasie Date: Tue, 15 Oct 2024 16:02:53 -0700 Subject: [PATCH 09/10] Mark flaky tests Signed-off-by: apostasie --- cmd/nerdctl/container/container_stop_linux_test.go | 1 + cmd/nerdctl/image/image_list_test.go | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/cmd/nerdctl/container/container_stop_linux_test.go b/cmd/nerdctl/container/container_stop_linux_test.go index 2c0e0bc2bea..a6ac329e18d 100644 --- a/cmd/nerdctl/container/container_stop_linux_test.go +++ b/cmd/nerdctl/container/container_stop_linux_test.go @@ -73,6 +73,7 @@ func TestStopStart(t *testing.T) { func TestStopWithStopSignal(t *testing.T) { t.Parallel() + // This is flaky with Docker base := testutil.NewBase(t) testContainerName := testutil.Identifier(t) defer base.Cmd("rm", "-f", testContainerName).Run() diff --git a/cmd/nerdctl/image/image_list_test.go b/cmd/nerdctl/image/image_list_test.go index 01acaf29a02..89d8bbaee47 100644 --- a/cmd/nerdctl/image/image_list_test.go +++ b/cmd/nerdctl/image/image_list_test.go @@ -18,6 +18,7 @@ package image import ( "fmt" + "runtime" "slices" "strings" "testing" @@ -117,6 +118,13 @@ func TestImages(t *testing.T) { }, } + if runtime.GOOS == "windows" { + testCase.Require = test.Require( + testCase.Require, + nerdtest.IsFlaky("https://github.com/containerd/nerdctl/issues/3524"), + ) + } + testCase.Run(t) } @@ -179,7 +187,7 @@ RUN echo "actually creating a layer so that docker sets the createdAt time" }, }, { - Description: "label=foo=bar label=version=0.1", + Description: "label=foo=bar label=version=0.2", Command: test.Command("images", "--filter", "label=foo=bar", "--filter", "label=version=0.2"), Expected: func(data test.Data, helpers test.Helpers) *test.Expected { return &test.Expected{ @@ -189,7 +197,6 @@ RUN echo "actually creating a layer so that docker sets the createdAt time" }, { Description: "label=version", - Require: nerdtest.IsFlaky("https://github.com/containerd/nerdctl/issues/3512"), Command: test.Command("images", "--filter", "label=version"), Expected: func(data test.Data, helpers test.Helpers) *test.Expected { return &test.Expected{ From aff9d083de3183cb49844381896a61853c826873 Mon Sep 17 00:00:00 2001 From: apostasie Date: Tue, 15 Oct 2024 18:08:53 -0700 Subject: [PATCH 10/10] Enable unit tests for windows Signed-off-by: apostasie --- .github/workflows/test.yml | 18 ++++++++++++++++-- pkg/cmd/builder/build_test.go | 5 +++++ pkg/composer/serviceparser/build_test.go | 6 ++++++ .../serviceparser/serviceparser_test.go | 13 +++++++++++++ .../credentialsstore_test.go | 4 ++++ .../dockercompat/dockercompat_test.go | 2 +- pkg/logging/cri_logger_test.go | 4 ++++ pkg/logging/json_logger_test.go | 4 ++++ pkg/mountutil/mountutil_windows_test.go | 2 +- 9 files changed, 54 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 46318b58a49..f07f9bc561a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -22,12 +22,15 @@ jobs: timeout-minutes: 5 name: unit | ${{ matrix.goos }} runs-on: "${{ matrix.os }}" + defaults: + run: + shell: bash strategy: matrix: include: # FIXME: currently disabled as a lot more work is required to make these tests pass on windows - # - os: windows-2022 - # goos: windows + - os: windows-2022 + goos: windows - os: ubuntu-24.04 goos: linux steps: @@ -39,6 +42,17 @@ jobs: go-version: ${{ env.GO_VERSION }} check-latest: true cache: true + - if: ${{ matrix.goos=='windows' }} + uses: actions/checkout@v4.2.1 + with: + repository: containerd/containerd + ref: v1.7.23 + path: containerd + fetch-depth: 1 + - if: ${{ matrix.goos=='windows' }} + name: "Set up CNI" + working-directory: containerd + run: GOPATH=$(go env GOPATH) script/setup/install-cni-windows - name: "Run unit tests" run: make test-unit diff --git a/pkg/cmd/builder/build_test.go b/pkg/cmd/builder/build_test.go index 954738cdbc6..5a0bbb3dcb5 100644 --- a/pkg/cmd/builder/build_test.go +++ b/pkg/cmd/builder/build_test.go @@ -18,6 +18,7 @@ package builder import ( "reflect" + "runtime" "testing" specs "github.com/opencontainers/image-spec/specs-go/v1" @@ -213,6 +214,10 @@ func TestParseBuildctlArgsForOCILayout(t *testing.T) { }, } + if runtime.GOOS == "windows" { + tests[1].expectedErr = "open D:\\tmp\\oci-layout\\index.json: The system cannot find the path specified." + } + for _, test := range tests { t.Run(test.name, func(t *testing.T) { args, err := parseBuildContextFromOCILayout(test.ociLayoutName, test.ociLayoutPath) diff --git a/pkg/composer/serviceparser/build_test.go b/pkg/composer/serviceparser/build_test.go index 2ffa8962515..34af7143aec 100644 --- a/pkg/composer/serviceparser/build_test.go +++ b/pkg/composer/serviceparser/build_test.go @@ -17,6 +17,7 @@ package serviceparser import ( + "runtime" "testing" "gotest.tools/v3/assert" @@ -30,6 +31,11 @@ func lastOf(ss []string) string { func TestParseBuild(t *testing.T) { t.Parallel() + + if runtime.GOOS == "windows" { + t.Skip("test is not compatible with windows") + } + const dockerComposeYAML = ` services: foo: diff --git a/pkg/composer/serviceparser/serviceparser_test.go b/pkg/composer/serviceparser/serviceparser_test.go index 65cf517cb67..337617a49f9 100644 --- a/pkg/composer/serviceparser/serviceparser_test.go +++ b/pkg/composer/serviceparser/serviceparser_test.go @@ -20,6 +20,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strconv" "testing" @@ -79,6 +80,11 @@ var in = strutil.InStringSlice func TestParse(t *testing.T) { t.Parallel() + + if runtime.GOOS == "windows" { + t.Skip("test is not compatible with windows") + } + const dockerComposeYAML = ` version: '3.1' @@ -333,6 +339,10 @@ services: func TestParseRelative(t *testing.T) { t.Parallel() + + if runtime.GOOS == "windows" { + t.Skip("test is not compatible with windows") + } const dockerComposeYAML = ` services: foo: @@ -408,6 +418,9 @@ services: func TestParseConfigs(t *testing.T) { t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("test is not compatible with windows") + } const dockerComposeYAML = ` services: foo: diff --git a/pkg/imgutil/dockerconfigresolver/credentialsstore_test.go b/pkg/imgutil/dockerconfigresolver/credentialsstore_test.go index a9890dffdcd..003fe1aa211 100644 --- a/pkg/imgutil/dockerconfigresolver/credentialsstore_test.go +++ b/pkg/imgutil/dockerconfigresolver/credentialsstore_test.go @@ -46,6 +46,10 @@ func TestBrokenCredentialsStore(t *testing.T) { // Anyhow, this test is about extreme cases & conditions (filesystem errors wrt credentials loading). t.Skip("skipping broken credential store tests for freebsd") } + if runtime.GOOS == "windows" { + // Same as above + t.Skip("test is not compatible with windows") + } testCases := []struct { description string diff --git a/pkg/inspecttypes/dockercompat/dockercompat_test.go b/pkg/inspecttypes/dockercompat/dockercompat_test.go index 483f7aed3d8..12814bc31fc 100644 --- a/pkg/inspecttypes/dockercompat/dockercompat_test.go +++ b/pkg/inspecttypes/dockercompat/dockercompat_test.go @@ -68,7 +68,7 @@ func TestContainerFromNative(t *testing.T) { expected: &Container{ Created: "0001-01-01T00:00:00Z", Platform: runtime.GOOS, - ResolvConfPath: tempStateDir + "/resolv.conf", + ResolvConfPath: filepath.Join(tempStateDir, "resolv.conf"), State: &ContainerState{ Status: "running", Running: true, diff --git a/pkg/logging/cri_logger_test.go b/pkg/logging/cri_logger_test.go index f497b436a44..6d45e4999bc 100644 --- a/pkg/logging/cri_logger_test.go +++ b/pkg/logging/cri_logger_test.go @@ -30,6 +30,7 @@ import ( "os" "path/filepath" "reflect" + "runtime" "testing" "time" ) @@ -234,6 +235,9 @@ func TestReadLogsLimitsWithTimestamps(t *testing.T) { func TestReadRotatedLog(t *testing.T) { tmpDir := t.TempDir() + if runtime.GOOS == "windows" { + t.Skip("windows implementation does not seem to work right now and should be fixed: https://github.com/containerd/nerdctl/issues/3554") + } file, err := os.CreateTemp(tmpDir, "logfile") if err != nil { t.Errorf("unable to create temp file, error: %s", err.Error()) diff --git a/pkg/logging/json_logger_test.go b/pkg/logging/json_logger_test.go index 53e6e434867..7d0be36285d 100644 --- a/pkg/logging/json_logger_test.go +++ b/pkg/logging/json_logger_test.go @@ -22,12 +22,16 @@ import ( "fmt" "os" "path/filepath" + "runtime" "testing" "time" ) func TestReadRotatedJSONLog(t *testing.T) { tmpDir := t.TempDir() + if runtime.GOOS == "windows" { + t.Skip("windows implementation does not seem to work right now and should be fixed: https://github.com/containerd/nerdctl/issues/3554") + } file, err := os.CreateTemp(tmpDir, "logfile") if err != nil { t.Errorf("unable to create temp file, error: %s", err.Error()) diff --git a/pkg/mountutil/mountutil_windows_test.go b/pkg/mountutil/mountutil_windows_test.go index 661c45c0f5a..05428b113c5 100644 --- a/pkg/mountutil/mountutil_windows_test.go +++ b/pkg/mountutil/mountutil_windows_test.go @@ -38,7 +38,7 @@ func TestParseVolumeOptions(t *testing.T) { vType: "bind", src: "dummy", optsRaw: "rw", - wants: []string{}, + wants: nil, }, { vType: "volume",