From e789f2cda863cda6d0ed1e76d182671570bfe3da Mon Sep 17 00:00:00 2001 From: apostasie Date: Sun, 16 Jun 2024 16:53:37 -0700 Subject: [PATCH] Docker v26 compatibility & test fixes Signed-off-by: apostasie --- .github/workflows/test.yml | 11 +-- cmd/nerdctl/container_create_linux_test.go | 91 +++++++++++------- .../container_run_network_linux_test.go | 96 ++++++++++++------- cmd/nerdctl/network_inspect_test.go | 2 +- .../container_network_manager.go | 10 +- 5 files changed, 127 insertions(+), 83 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b66314c39f7..36e866aac0b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -251,8 +251,7 @@ jobs: go-version: ${{ env.GO_VERSION }} cache: true check-latest: true - # Docker >= v25 is still unsupported: https://github.com/containerd/nerdctl/issues/3088 - - name: "Install Docker v24" + - name: "Install Docker v26" run: | set -eux -o pipefail # Uninstall the preinstalled Docker @@ -264,10 +263,10 @@ jobs: cat /etc/docker/daemon.json # Download Docker packages curl -OSL https://download.docker.com/linux/ubuntu/dists/jammy/pool/stable/amd64/containerd.io_1.6.33-1_amd64.deb - curl -OSL https://download.docker.com/linux/ubuntu/dists/jammy/pool/stable/amd64/docker-ce_24.0.9-1~ubuntu.22.04~jammy_amd64.deb - curl -OSL https://download.docker.com/linux/ubuntu/dists/jammy/pool/stable/amd64/docker-ce-cli_24.0.9-1~ubuntu.22.04~jammy_amd64.deb - curl -OSL https://download.docker.com/linux/ubuntu/dists/jammy/pool/stable/amd64/docker-buildx-plugin_0.13.1-1~ubuntu.22.04~jammy_amd64.deb - curl -OSL https://download.docker.com/linux/ubuntu/dists/jammy/pool/stable/amd64/docker-compose-plugin_2.25.0-1~ubuntu.22.04~jammy_amd64.deb + curl -OSL https://download.docker.com/linux/ubuntu/dists/jammy/pool/stable/amd64/docker-ce_26.1.4-1~ubuntu.22.04~jammy_amd64.deb + curl -OSl https://download.docker.com/linux/ubuntu/dists/jammy/pool/stable/amd64/docker-ce-cli_26.1.4-1~ubuntu.22.04~jammy_amd64.deb + curl -OSL https://download.docker.com/linux/ubuntu/dists/jammy/pool/stable/amd64/docker-buildx-plugin_0.14.1-1~ubuntu.22.04~jammy_amd64.deb + curl -OSL https://download.docker.com/linux/ubuntu/dists/jammy/pool/stable/amd64/docker-compose-plugin_2.27.1-1~ubuntu.22.04~jammy_amd64.deb # Install Docker sudo apt-get install -y ./*.deb rm -f ./*.deb diff --git a/cmd/nerdctl/container_create_linux_test.go b/cmd/nerdctl/container_create_linux_test.go index ba70bda420b..960a2ff3222 100644 --- a/cmd/nerdctl/container_create_linux_test.go +++ b/cmd/nerdctl/container_create_linux_test.go @@ -19,6 +19,7 @@ package main import ( "fmt" "runtime" + "strings" "testing" "github.com/containerd/nerdctl/v2/pkg/testutil" @@ -57,69 +58,85 @@ func TestCreateWithMACAddress(t *testing.T) { networkBridge := "testNetworkBridge" + tID networkMACvlan := "testNetworkMACvlan" + tID networkIPvlan := "testNetworkIPvlan" + tID - base.Cmd("network", "create", networkBridge, "--driver", "bridge").AssertOK() - base.Cmd("network", "create", networkMACvlan, "--driver", "macvlan").AssertOK() - base.Cmd("network", "create", networkIPvlan, "--driver", "ipvlan").AssertOK() - t.Cleanup(func() { + + tearDown := func() { base.Cmd("network", "rm", networkBridge).Run() base.Cmd("network", "rm", networkMACvlan).Run() base.Cmd("network", "rm", networkIPvlan).Run() - }) + } + + tearDown() + t.Cleanup(tearDown) + + base.Cmd("network", "create", networkBridge, "--driver", "bridge").AssertOK() + base.Cmd("network", "create", networkMACvlan, "--driver", "macvlan").AssertOK() + base.Cmd("network", "create", networkIPvlan, "--driver", "ipvlan").AssertOK() + + defaultMac := base.Cmd("run", "--rm", "-i", "--network", "host", testutil.CommonImage). + CmdOption(testutil.WithStdin(strings.NewReader("ip addr show eth0 | grep ether | awk '{printf $2}'"))). + Run().Stdout() + + passedMac := "we expect the generated mac on the output" tests := []struct { Network string WantErr bool Expect string }{ - {"host", true, "conflicting options"}, - {"none", true, "can't open '/sys/class/net/eth0/address'"}, - {"container:whatever" + tID, true, "conflicting options"}, - {"bridge", false, ""}, - {networkBridge, false, ""}, - {networkMACvlan, false, ""}, + {"host", false, defaultMac}, // anything but the actual address being passed + {"none", false, ""}, + {"container:whatever" + tID, true, "container"}, // "No such container" vs. "could not find container" + {"bridge", false, passedMac}, + {networkBridge, false, passedMac}, + {networkMACvlan, false, passedMac}, {networkIPvlan, true, "not support"}, } for i, test := range tests { containerName := fmt.Sprintf("%s_%d", tID, i) testName := fmt.Sprintf("%s_container:%s_network:%s_expect:%s", tID, containerName, test.Network, test.Expect) + expect := test.Expect + network := test.Network + wantErr := test.WantErr t.Run(testName, func(tt *testing.T) { + tt.Parallel() + macAddress, err := nettestutil.GenerateMACAddress() if err != nil { tt.Errorf("failed to generate MAC address: %s", err) } - if test.Expect == "" && !test.WantErr { - test.Expect = macAddress + if expect == passedMac { + expect = macAddress } tt.Cleanup(func() { base.Cmd("rm", "-f", containerName).Run() }) - cmd := base.Cmd("create", "--network", test.Network, "--mac-address", macAddress, "--name", containerName, testutil.CommonImage, "cat", "/sys/class/net/eth0/address") - if !test.WantErr { - cmd.AssertOK() + // This is currently blocked by https://github.com/containerd/nerdctl/pull/3104 + // res := base.Cmd("create", "-i", "--network", network, "--mac-address", macAddress, testutil.CommonImage).Run() + res := base.Cmd("create", "--network", network, "--name", containerName, + "--mac-address", macAddress, testutil.CommonImage, + "sh", "-c", "--", "ip addr show eth0 | grep ether").Run() + + if !wantErr { + assert.Assert(t, res.ExitCode == 0, "Command should have succeeded", res.Combined()) + // This is currently blocked by: https://github.com/containerd/nerdctl/pull/3104 + // res = base.Cmd("start", "-i", containerName). + // CmdOption(testutil.WithStdin(strings.NewReader("ip addr show eth0 | grep ether | awk '{printf $2}'"))).Run() base.Cmd("start", containerName).AssertOK() - cmd = base.Cmd("logs", containerName) - cmd.AssertOK() - cmd.AssertOutContains(test.Expect) + res = base.Cmd("logs", containerName).Run() + assert.Assert(t, strings.Contains(res.Stdout(), expect), fmt.Sprintf("expected output to contain %q: %q", expect, res.Stdout())) + assert.Assert(t, res.ExitCode == 0, "Command should have succeeded", res.Combined()) } else { - if (testutil.GetTarget() == testutil.Docker && test.Network == networkIPvlan) || test.Network == "none" { - // 1. unlike nerdctl - // when using network ipvlan in Docker + if testutil.GetTarget() == testutil.Docker && + (network == networkIPvlan || network == "container:whatever"+tID) { + // unlike nerdctl + // when using network ipvlan or container in Docker // it delays fail on executing start command - // 2. start on network none will success in both - // nerdctl and Docker - cmd.AssertOK() - cmd = base.Cmd("start", containerName) - if test.Network == "none" { - // we check the result on logs command - cmd.AssertOK() - cmd = base.Cmd("logs", containerName) - } - } - cmd.AssertCombinedOutContains(test.Expect) - if test.Network == "none" { - cmd.AssertOK() - } else { - cmd.AssertFail() + assert.Assert(t, res.ExitCode == 0, "Command should have succeeded", res.Combined()) + res = base.Cmd("start", "-i", containerName). + CmdOption(testutil.WithStdin(strings.NewReader("ip addr show eth0 | grep ether | awk '{printf $2}'"))).Run() } + + assert.Assert(t, strings.Contains(res.Combined(), expect), fmt.Sprintf("expected output to contain %q: %q", expect, res.Combined())) + assert.Assert(t, res.ExitCode != 0, "Command should have failed", res.Combined()) } }) } diff --git a/cmd/nerdctl/container_run_network_linux_test.go b/cmd/nerdctl/container_run_network_linux_test.go index 58349cba619..0f6f8990371 100644 --- a/cmd/nerdctl/container_run_network_linux_test.go +++ b/cmd/nerdctl/container_run_network_linux_test.go @@ -354,12 +354,17 @@ func TestRunContainerWithStaticIP(t *testing.T) { useNetwork: true, checkTheIPAddress: false, }, - { - ip: "10.4.0.2", - shouldSuccess: true, - useNetwork: false, - checkTheIPAddress: false, - }, + // XXX see https://github.com/containerd/nerdctl/issues/3101 + // docker 24 silently ignored the ip - now, docker 26 is erroring out - furthermore, this ip only makes sense + // in the context of nerdctl bridge network, so, this test needs rewritting either way + /* + { + ip: "10.4.0.2", + shouldSuccess: true, + useNetwork: false, + checkTheIPAddress: false, + }, + */ } tID := testutil.Identifier(t) for i, tc := range testCases { @@ -453,43 +458,68 @@ func TestRunContainerWithMACAddress(t *testing.T) { networkBridge := "testNetworkBridge" + tID networkMACvlan := "testNetworkMACvlan" + tID networkIPvlan := "testNetworkIPvlan" + tID - base.Cmd("network", "create", networkBridge, "--driver", "bridge").AssertOK() - base.Cmd("network", "create", networkMACvlan, "--driver", "macvlan").AssertOK() - base.Cmd("network", "create", networkIPvlan, "--driver", "ipvlan").AssertOK() - t.Cleanup(func() { + tearDown := func() { base.Cmd("network", "rm", networkBridge).Run() base.Cmd("network", "rm", networkMACvlan).Run() base.Cmd("network", "rm", networkIPvlan).Run() - }) + } + + tearDown() + t.Cleanup(tearDown) + + base.Cmd("network", "create", networkBridge, "--driver", "bridge").AssertOK() + base.Cmd("network", "create", networkMACvlan, "--driver", "macvlan").AssertOK() + base.Cmd("network", "create", networkIPvlan, "--driver", "ipvlan").AssertOK() + + defaultMac := base.Cmd("run", "--rm", "-i", "--network", "host", testutil.CommonImage). + CmdOption(testutil.WithStdin(strings.NewReader("ip addr show eth0 | grep ether | awk '{printf $2}'"))). + Run().Stdout() + + passedMac := "we expect the generated mac on the output" + tests := []struct { Network string WantErr bool Expect string }{ - {"host", true, "conflicting options"}, - {"none", true, "can't open '/sys/class/net/eth0/address'"}, - {"container:whatever" + tID, true, "conflicting options"}, - {"bridge", false, ""}, - {networkBridge, false, ""}, - {networkMACvlan, false, ""}, + {"host", false, defaultMac}, // anything but the actual address being passed + {"none", false, ""}, // nothing + {"container:whatever" + tID, true, "container"}, // "No such container" vs. "could not find container" + {"bridge", false, passedMac}, + {networkBridge, false, passedMac}, + {networkMACvlan, false, passedMac}, {networkIPvlan, true, "not support"}, } - for _, test := range tests { - macAddress, err := nettestutil.GenerateMACAddress() - if err != nil { - t.Errorf("failed to generate MAC address: %s", err) - } - if test.Expect == "" && !test.WantErr { - test.Expect = macAddress - } - cmd := base.Cmd("run", "--rm", "--network", test.Network, "--mac-address", macAddress, testutil.CommonImage, "cat", "/sys/class/net/eth0/address") - if test.WantErr { - cmd.AssertFail() - cmd.AssertCombinedOutContains(test.Expect) - } else { - cmd.AssertOK() - cmd.AssertOutContains(test.Expect) - } + + for i, test := range tests { + containerName := fmt.Sprintf("%s_%d", tID, i) + testName := fmt.Sprintf("%s_container:%s_network:%s_expect:%s", tID, containerName, test.Network, test.Expect) + expect := test.Expect + network := test.Network + wantErr := test.WantErr + t.Run(testName, func(tt *testing.T) { + tt.Parallel() + + macAddress, err := nettestutil.GenerateMACAddress() + if err != nil { + t.Errorf("failed to generate MAC address: %s", err) + } + if expect == passedMac { + expect = macAddress + } + + res := base.Cmd("run", "--rm", "-i", "--network", network, "--mac-address", macAddress, testutil.CommonImage). + CmdOption(testutil.WithStdin(strings.NewReader("ip addr show eth0 | grep ether | awk '{printf $2}'"))).Run() + + if wantErr { + assert.Assert(t, res.ExitCode != 0, "Command should have failed", res.Combined()) + assert.Assert(t, strings.Contains(res.Combined(), expect), fmt.Sprintf("expected output to contain %q: %q", expect, res.Combined())) + } else { + assert.Assert(t, res.ExitCode == 0, "Command should have succeeded", res.Combined()) + assert.Assert(t, strings.Contains(res.Stdout(), expect), fmt.Sprintf("expected output to contain %q: %q", expect, res.Stdout())) + } + }) + } } diff --git a/cmd/nerdctl/network_inspect_test.go b/cmd/nerdctl/network_inspect_test.go index 5a43cab1ceb..bab22c998eb 100644 --- a/cmd/nerdctl/network_inspect_test.go +++ b/cmd/nerdctl/network_inspect_test.go @@ -34,7 +34,7 @@ func TestNetworkInspect(t *testing.T) { const ( testSubnet = "10.24.24.0/24" testGateway = "10.24.24.1" - testIPRange = "10.24.24.1/25" + testIPRange = "10.24.24.0/25" ) base := testutil.NewBase(t) diff --git a/pkg/containerutil/container_network_manager.go b/pkg/containerutil/container_network_manager.go index 1f746c2d4bb..f6db2e78d55 100644 --- a/pkg/containerutil/container_network_manager.go +++ b/pkg/containerutil/container_network_manager.go @@ -200,9 +200,9 @@ func (m *containerNetworkManager) VerifyNetworkOptions(_ context.Context) error return errors.New("conflicting options: only one network specification is allowed when using '--network=container:'") } + // Note that mac-address is accepted, though it is a no-op nonZeroParams := nonZeroMapValues(map[string]interface{}{ - "--hostname": m.netOpts.Hostname, - "--mac-address": m.netOpts.MACAddress, + "--hostname": m.netOpts.Hostname, // NOTE: an empty slice still counts as a non-zero value so we check its length: "-p/--publish": len(m.netOpts.PortMappings) != 0, "--dns": len(m.netOpts.DNSServers) != 0, @@ -286,6 +286,8 @@ func (m *containerNetworkManager) InternalNetworkingOptionLabels(ctx context.Con if m.netOpts.NetworkSlice == nil || len(m.netOpts.NetworkSlice) != 1 { return opts, fmt.Errorf("conflicting options: exactly one network specification is allowed when using '--network=container:'") } + // MacAddress is not allowed with container networking + opts.MACAddress = "" container, err := m.getNetworkingContainerForArgument(ctx, m.netOpts.NetworkSlice[0], m.client) if err != nil { @@ -357,10 +359,6 @@ func (m *hostNetworkManager) VerifyNetworkOptions(_ context.Context) error { return errors.New("cannot use host networking on Windows") } - if m.netOpts.MACAddress != "" { - return errors.New("conflicting options: mac-address and the network mode") - } - return validateUtsSettings(m.netOpts) }