Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable other containers to join network namespace of the none network #3443

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions cmd/nerdctl/container/container_run_network_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,41 @@ func TestSharedNetworkStack(t *testing.T) {
AssertOutContains(testutil.NginxAlpineIndexHTMLSnippet)
}

func TestSharedNetworkWithNone(t *testing.T) {
if runtime.GOOS != "linux" {
t.Skip("--network=container:<container name|id> only supports linux now")
}
base := testutil.NewBase(t)

containerName := testutil.Identifier(t)
t.Cleanup(func() {
base.Cmd("rm", "-f", containerName).Run()
})
base.Cmd("run", "-d", "--name", containerName, "--network", "none",
testutil.NginxAlpineImage).AssertOK()
base.EnsureContainerStarted(containerName)

containerNameJoin := testutil.Identifier(t) + "-network"
t.Cleanup(func() {
base.Cmd("rm", "-f", containerNameJoin).Run()
})
base.Cmd("run",
"-d",
"--name", containerNameJoin,
"--network=container:"+containerName,
testutil.CommonImage,
"sleep", "infinity").AssertOK()

base.Cmd("exec", containerNameJoin, "wget", "-qO-", "http://127.0.0.1:80").
AssertOutContains(testutil.NginxAlpineIndexHTMLSnippet)

base.Cmd("restart", containerName).AssertOK()
base.Cmd("stop", "--time=1", containerNameJoin).AssertOK()
base.Cmd("start", containerNameJoin).AssertOK()
base.Cmd("exec", containerNameJoin, "wget", "-qO-", "http://127.0.0.1:80").
AssertOutContains(testutil.NginxAlpineIndexHTMLSnippet)
}

func TestRunContainerInExistingNetNS(t *testing.T) {
if rootlessutil.IsRootless() {
t.Skip("Can't create new netns in rootless mode")
Expand Down Expand Up @@ -629,6 +664,8 @@ func TestHostsFileMounts(t *testing.T) {
"sh", "-euxc", "echo >> /etc/hosts").AssertOK()
base.Cmd("run", "--rm", "-v", "/etc/hosts:/etc/hosts", "--network", "host", testutil.CommonImage,
"sh", "-euxc", "head -n -1 /etc/hosts > temp && cat temp > /etc/hosts").AssertOK()
base.Cmd("run", "--rm", "--network", "none", testutil.CommonImage,
"sh", "-euxc", "echo >> /etc/hosts").AssertOK()

base.Cmd("run", "--rm", testutil.CommonImage,
"sh", "-euxc", "echo >> /etc/resolv.conf").AssertOK()
Expand All @@ -641,6 +678,8 @@ func TestHostsFileMounts(t *testing.T) {
"sh", "-euxc", "echo >> /etc/resolv.conf").AssertOK()
base.Cmd("run", "--rm", "-v", "/etc/resolv.conf:/etc/resolv.conf", "--network", "host", testutil.CommonImage,
"sh", "-euxc", "head -n -1 /etc/resolv.conf > temp && cat temp > /etc/resolv.conf").AssertOK()
base.Cmd("run", "--rm", "--network", "host", testutil.CommonImage,
"sh", "-euxc", "echo >> /etc/resolv.conf").AssertOK()
}

func TestRunContainerWithStaticIP6(t *testing.T) {
Expand Down Expand Up @@ -712,3 +751,39 @@ func TestRunContainerWithStaticIP6(t *testing.T) {
})
}
}

func TestNoneNetworkStaticConfigs(t *testing.T) {
testutil.DockerIncompatible(t)
base := testutil.NewBase(t)

//TODO: Add tests for --dns --dns-search --add-host

// If running on Linux, verify /etc/hostname is correctly set
if runtime.GOOS == "linux" {
containerHostName := testutil.Identifier(t)
cmd := base.Cmd("run", "--rm", "--net", "none", "--hostname", containerHostName, testutil.CommonImage, "cat", "/etc/hostname")
output := cmd.Run().Combined()
hostname := strings.TrimSpace(output)

if len(containerHostName) > 12 {
assert.Equal(t, containerHostName[:12], hostname[:12])
} else {
assert.Equal(t, containerHostName, hostname[:12])
}

containerName := testutil.Identifier(t) + "-id"
t.Cleanup(func() {
base.Cmd("rm", "-f", containerName).Run()
})
cmd = base.Cmd("run", "-d", "--net", "none", "--name", containerName, testutil.CommonImage, "sleep", "infinity")
output = cmd.Run().Combined()
containerIDShort := strings.TrimSpace(output)[:12]

cmd = base.Cmd("exec", containerName, "cat", "/etc/hostname")
output = cmd.Run().Combined()
containerHostName = strings.TrimSpace(output)

assert.Equal(t, containerHostName, containerIDShort)

}
}
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ require (
github.com/sirupsen/logrus v1.9.3 // indirect
github.com/spaolacci/murmur3 v1.1.0 // indirect
github.com/stefanberger/go-pkcs11uri v0.0.0-20230803200340-78284954bff6 // indirect
github.com/stretchr/objx v0.5.2 // indirect
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to run go mod tidy use the nerdctl test framework and fix a few things in code, just checking if my logic is correct to add the options DNS and host options for none network and probably need to be done for host network as well

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you added it initially with #3443 (review)

But really it should not be necessary.

Can you try removing the line from go.mod then doing go mod tidy again?

Will look at the rest later today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, the current version is more for making sure the direction is correct. I will be having another pr using the new test framework and updates for the host network config.

github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 // indirect
github.com/tinylib/msgp v1.2.0 // indirect
github.com/vbatts/tar-split v0.11.5 // indirect
Expand Down
3 changes: 2 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,9 @@ github.com/stefanberger/go-pkcs11uri v0.0.0-20230803200340-78284954bff6 h1:pnnLy
github.com/stefanberger/go-pkcs11uri v0.0.0-20230803200340-78284954bff6/go.mod h1:39R/xuhNgVhi+K0/zst4TLrJrVmbm6LVgl4A0+ZFS5M=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
Expand Down
143 changes: 139 additions & 4 deletions pkg/containerutil/container_network_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"github.com/containerd/nerdctl/v2/pkg/mountutil"
"github.com/containerd/nerdctl/v2/pkg/netutil"
"github.com/containerd/nerdctl/v2/pkg/netutil/nettype"
"github.com/containerd/nerdctl/v2/pkg/resolvconf"
"github.com/containerd/nerdctl/v2/pkg/rootlessutil"
"github.com/containerd/nerdctl/v2/pkg/strutil"
)
Expand Down Expand Up @@ -162,13 +163,81 @@ func (m *noneNetworkManager) VerifyNetworkOptions(_ context.Context) error {
}

// SetupNetworking Performs setup actions required for the container with the given ID.
func (m *noneNetworkManager) SetupNetworking(_ context.Context, _ string) error {
func (m *noneNetworkManager) SetupNetworking(ctx context.Context, containerID string) error {

// Retrieve the container
container, err := m.client.ContainerService().Get(ctx, containerID)
if err != nil {
return err
}

// Get the dataStore
dataStore, err := clientutil.DataStore(m.globalOptions.DataRoot, m.globalOptions.Address)
if err != nil {
return err
}

// Get the hostsStore
hs, err := hostsstore.New(dataStore, container.Labels[labels.Namespace])
if err != nil {
return err
}

// Get extra-hosts
extraHostsJSON := container.Labels[labels.ExtraHosts]
var extraHosts []string
if err = json.Unmarshal([]byte(extraHostsJSON), &extraHosts); err != nil {
return err
}

hosts := make(map[string]string)
for _, host := range extraHosts {
if v := strings.SplitN(host, ":", 2); len(v) == 2 {
hosts[v[0]] = v[1]
}
}

// Prep the meta
hsMeta := hostsstore.Meta{
ID: container.ID,
Hostname: container.Labels[labels.Hostname],
ExtraHosts: hosts,
Name: container.Labels[labels.Name],
}

// Save the meta information
if err = hs.Acquire(hsMeta); err != nil {
return err
}

return nil
}

// CleanupNetworking Performs any required cleanup actions for the given container.
// Should only be called to revert any setup steps performed in SetupNetworking.
func (m *noneNetworkManager) CleanupNetworking(_ context.Context, _ containerd.Container) error {
func (m *noneNetworkManager) CleanupNetworking(ctx context.Context, container containerd.Container) error {
// Get the dataStore
dataStore, err := clientutil.DataStore(m.globalOptions.DataRoot, m.globalOptions.Address)
if err != nil {
return err
}

// Get labels
lbls, err := container.Labels(ctx)
if err != nil {
return err
}

// Get the hostsStore
hs, err := hostsstore.New(dataStore, lbls[labels.Namespace])
if err != nil {
return err
}

// Release
if err = hs.Release(container.ID()); err != nil {
return err
}
return nil
}

Expand All @@ -179,9 +248,75 @@ func (m *noneNetworkManager) InternalNetworkingOptionLabels(_ context.Context) (

// ContainerNetworkingOpts Returns a slice of `oci.SpecOpts` and `containerd.NewContainerOpts` which represent
// the network specs which need to be applied to the container with the given ID.
func (m *noneNetworkManager) ContainerNetworkingOpts(_ context.Context, _ string) ([]oci.SpecOpts, []containerd.NewContainerOpts, error) {
func (m *noneNetworkManager) ContainerNetworkingOpts(_ context.Context, containerID string) ([]oci.SpecOpts, []containerd.NewContainerOpts, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am increasingly uncomfortable with the amount of duplication between ContainerNetworkingOpts implementations (and even more so about the differences in there). But then, refactoring that should probably be a separate endeavor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really as I was writing it I too saw that so I plan to refactor it but before that just wanted to confirm the direction so that the effort is not put in the wrong place

// No options to return if no network settings are provided.
return []oci.SpecOpts{}, []containerd.NewContainerOpts{}, nil
dataStore, err := clientutil.DataStore(m.globalOptions.DataRoot, m.globalOptions.Address)
if err != nil {
return nil, nil, err
}

stateDir, err := ContainerStateDirPath(m.globalOptions.Namespace, dataStore, containerID)
if err != nil {
return nil, nil, err
}

resolvConfPath := filepath.Join(stateDir, "resolv.conf")
dns := []string{"127.0.0.1"}
dnsSearch := []string{}
dnsOptions := []string{}

if len(m.netOpts.DNSServers) > 0 {
dns = m.netOpts.DNSServers
}
if len(m.netOpts.DNSSearchDomains) > 0 {
dnsSearch = m.netOpts.DNSSearchDomains
}
if len(m.netOpts.DNSResolvConfOptions) > 0 {
dnsOptions = m.netOpts.DNSResolvConfOptions
}

// Call the Build function
_, err = resolvconf.Build(resolvConfPath, dns, dnsSearch, dnsOptions)
if err != nil {
return nil, nil, err
}

hs, err := hostsstore.New(dataStore, m.globalOptions.Namespace)
if err != nil {
return nil, nil, err
}

etcHostsPath, err := hs.AllocHostsFile(containerID, []byte{})
if err != nil {
return nil, nil, err
}

specs := []oci.SpecOpts{
withDedupMounts("/etc/hosts", withCustomHosts(etcHostsPath)),
withDedupMounts("/etc/resolv.conf", withCustomResolvConf(resolvConfPath)),
}

// `/etc/hostname` does not exist on FreeBSD
if runtime.GOOS == "linux" {
// If no hostname is set, default to first 12 characters of the container ID.
hostname := m.netOpts.Hostname
if hostname == "" {
hostname = containerID
if len(hostname) > 12 {
hostname = hostname[0:12]
}
}
m.netOpts.Hostname = hostname

hostnameOpts, err := writeEtcHostnameForContainer(m.globalOptions, m.netOpts.Hostname, containerID)
if err != nil {
return nil, nil, err
}
if hostnameOpts != nil {
specs = append(specs, hostnameOpts...)
}
}
return specs, []containerd.NewContainerOpts{}, nil
}

// types.NetworkOptionsManager implementation for container networking settings.
Expand Down
Loading