Skip to content

Commit

Permalink
feat: configure custom IP for default bridge network
Browse files Browse the repository at this point in the history
Signed-off-by: Swagat Bora <sbora@amazon.com>
  • Loading branch information
swagatbora90 committed Nov 4, 2024
1 parent c71e8d9 commit 308523a
Show file tree
Hide file tree
Showing 16 changed files with 148 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func TestHnsEndpointsExistDuringContainerLifecycle(t *testing.T) {
// does not work on Windows.
func getTestingNetwork() (*netutil.NetworkConfig, error) {
// NOTE: cannot currently `nerdctl network create` on Windows so we use a pre-existing network:
cniEnv, err := netutil.NewCNIEnv(defaults.CNIPath(), defaults.CNINetConfPath())
cniEnv, err := netutil.NewCNIEnv(defaults.CNIPath(), defaults.CNINetConfPath(), "")

Check failure on line 152 in cmd/nerdctl/container/container_run_network_windows_test.go

View workflow job for this annotation

GitHub Actions / windows

cannot use "" (untyped string constant) as netutil.CNIEnvOpt value in argument to netutil.NewCNIEnv

Check failure on line 152 in cmd/nerdctl/container/container_run_network_windows_test.go

View workflow job for this annotation

GitHub Actions / windows

cannot use "" (untyped string constant) as netutil.CNIEnvOpt value in argument to netutil.NewCNIEnv

Check failure on line 152 in cmd/nerdctl/container/container_run_network_windows_test.go

View workflow job for this annotation

GitHub Actions / go | windows |

cannot use "" (untyped string constant) as netutil.CNIEnvOpt value in argument to netutil.NewCNIEnv (typecheck)

Check failure on line 152 in cmd/nerdctl/container/container_run_network_windows_test.go

View workflow job for this annotation

GitHub Actions / windows

cannot use "" (untyped string constant) as netutil.CNIEnvOpt value in argument to netutil.NewCNIEnv

Check failure on line 152 in cmd/nerdctl/container/container_run_network_windows_test.go

View workflow job for this annotation

GitHub Actions / windows

cannot use "" (untyped string constant) as netutil.CNIEnvOpt value in argument to netutil.NewCNIEnv
if err != nil {
return nil, err
}
Expand Down
5 changes: 5 additions & 0 deletions cmd/nerdctl/helpers/flagutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ func ProcessRootCmdFlags(cmd *cobra.Command) (types.GlobalCommandOptions, error)
if err != nil {
return types.GlobalCommandOptions{}, err
}
bridgeIP, err := cmd.Flags().GetString("bridge-ip")
if err != nil {
return types.GlobalCommandOptions{}, err
}
return types.GlobalCommandOptions{
Debug: debug,
DebugFull: debugFull,
Expand All @@ -113,6 +117,7 @@ func ProcessRootCmdFlags(cmd *cobra.Command) (types.GlobalCommandOptions, error)
HostsDir: hostsDir,
Experimental: experimental,
HostGatewayIP: hostGatewayIP,
BridgeIP: bridgeIP,
}, nil
}

Expand Down
2 changes: 2 additions & 0 deletions cmd/nerdctl/internal/internal_oci_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ func internalOCIHookAction(cmd *cobra.Command, args []string) error {
}
cniPath := globalOptions.CNIPath
cniNetconfpath := globalOptions.CNINetConfPath
bridgeIP := globalOptions.BridgeIP
return ocihook.Run(os.Stdin, os.Stderr, event,
dataStore,
cniPath,
cniNetconfpath,
bridgeIP,
)
}
1 change: 1 addition & 0 deletions cmd/nerdctl/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ func initRootCmdFlags(rootCmd *cobra.Command, tomlPath string) (*pflag.FlagSet,
// Experimental enable experimental feature, see in https://github.com/containerd/nerdctl/blob/main/docs/experimental.md
helpers.AddPersistentBoolFlag(rootCmd, "experimental", nil, nil, cfg.Experimental, "NERDCTL_EXPERIMENTAL", "Control experimental: https://github.com/containerd/nerdctl/blob/main/docs/experimental.md")
helpers.AddPersistentStringFlag(rootCmd, "host-gateway-ip", nil, nil, nil, aliasToBeInherited, cfg.HostGatewayIP, "NERDCTL_HOST_GATEWAY_IP", "IP address that the special 'host-gateway' string in --add-host resolves to. Defaults to the IP address of the host. It has no effect without setting --add-host")
helpers.AddPersistentStringFlag(rootCmd, "bridge-ip", nil, nil, nil, aliasToBeInherited, cfg.BridgeIP, "NERDCTL_BRIDGE_IP", "IP address for the default nerdctl bridge network")
return aliasToBeInherited, nil
}

Expand Down
1 change: 1 addition & 0 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ experimental = true
| `hosts_dir` | `--hosts-dir` | | `certs.d` directory | Since 0.16.0 |
| `experimental` | `--experimental` | `NERDCTL_EXPERIMENTAL` | Enable [experimental features](experimental.md) | Since 0.22.3 |
| `host_gateway_ip` | `--host-gateway-ip` | `NERDCTL_HOST_GATEWAY_IP` | IP address that the special 'host-gateway' string in --add-host resolves to. Defaults to the IP address of the host. It has no effect without setting --add-host | Since 1.3.0 |
| `bridge_ip` | `--bridge-ip` | `NERDCTL_BRIDGE_IP` | IP address for the default nerdctl bridge network | Since 1.7.8 |

The properties are parsed in the following precedence:
1. CLI flag
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/compose/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func New(client *containerd.Client, globalOptions types.GlobalCommandOptions, op
return nil, err
}

cniEnv, err := netutil.NewCNIEnv(globalOptions.CNIPath, globalOptions.CNINetConfPath, netutil.WithNamespace(globalOptions.Namespace), netutil.WithDefaultNetwork())
cniEnv, err := netutil.NewCNIEnv(globalOptions.CNIPath, globalOptions.CNINetConfPath, netutil.WithNamespace(globalOptions.Namespace), netutil.WithDefaultNetwork(globalOptions.BridgeIP))
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/container/kill.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func cleanupNetwork(ctx context.Context, container containerd.Container, globalO
case nettype.Host, nettype.None, nettype.Container, nettype.Namespace:
// NOP
case nettype.CNI:
e, err := netutil.NewCNIEnv(globalOpts.CNIPath, globalOpts.CNINetConfPath, netutil.WithNamespace(globalOpts.Namespace), netutil.WithDefaultNetwork())
e, err := netutil.NewCNIEnv(globalOpts.CNIPath, globalOpts.CNINetConfPath, netutil.WithNamespace(globalOpts.Namespace), netutil.WithDefaultNetwork(globalOpts.BridgeIP))
if err != nil {
return err
}
Expand Down
1 change: 1 addition & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type Config struct {
HostsDir []string `toml:"hosts_dir"`
Experimental bool `toml:"experimental"`
HostGatewayIP string `toml:"host_gateway_ip"`
BridgeIP string `toml:"bridge_ip, omitempty"`
}

// New creates a default Config object statically,
Expand Down
2 changes: 1 addition & 1 deletion pkg/containerutil/container_network_manager_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type cniNetworkManagerPlatform struct {

// Verifies that the internal network settings are correct.
func (m *cniNetworkManager) VerifyNetworkOptions(_ context.Context) error {
e, err := netutil.NewCNIEnv(m.globalOptions.CNIPath, m.globalOptions.CNINetConfPath, netutil.WithNamespace(m.globalOptions.Namespace), netutil.WithDefaultNetwork())
e, err := netutil.NewCNIEnv(m.globalOptions.CNIPath, m.globalOptions.CNINetConfPath, netutil.WithNamespace(m.globalOptions.Namespace), netutil.WithDefaultNetwork(m.globalOptions.BridgeIP))
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/containerutil/container_network_manager_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type cniNetworkManagerPlatform struct {

// Verifies that the internal network settings are correct.
func (m *cniNetworkManager) VerifyNetworkOptions(_ context.Context) error {
e, err := netutil.NewCNIEnv(m.globalOptions.CNIPath, m.globalOptions.CNINetConfPath, netutil.WithNamespace(m.globalOptions.Namespace), netutil.WithDefaultNetwork())
e, err := netutil.NewCNIEnv(m.globalOptions.CNIPath, m.globalOptions.CNINetConfPath, netutil.WithNamespace(m.globalOptions.Namespace), netutil.WithDefaultNetwork(m.globalOptions.BridgeIP))
if err != nil {
return err
}
Expand Down Expand Up @@ -67,7 +67,7 @@ func (m *cniNetworkManager) VerifyNetworkOptions(_ context.Context) error {
}

func (m *cniNetworkManager) getCNI() (gocni.CNI, error) {
e, err := netutil.NewCNIEnv(m.globalOptions.CNIPath, m.globalOptions.CNINetConfPath, netutil.WithNamespace(m.globalOptions.Namespace), netutil.WithDefaultNetwork())
e, err := netutil.NewCNIEnv(m.globalOptions.CNIPath, m.globalOptions.CNINetConfPath, netutil.WithNamespace(m.globalOptions.Namespace), netutil.WithDefaultNetwork(m.globalOptions.BridgeIP))
if err != nil {
return nil, fmt.Errorf("failed to instantiate CNI env: %s", err)
}
Expand Down
26 changes: 19 additions & 7 deletions pkg/netutil/netutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,9 @@ func namespaceUsedNetworks(ctx context.Context, containers []containerd.Containe
return used, nil
}

func WithDefaultNetwork() CNIEnvOpt {
func WithDefaultNetwork(bridgeIP string) CNIEnvOpt {
return func(e *CNIEnv) error {
return e.ensureDefaultNetworkConfig()
return e.ensureDefaultNetworkConfig(bridgeIP)
}
}

Expand Down Expand Up @@ -323,7 +323,6 @@ func (e *CNIEnv) CreateNetwork(opts types.NetworkCreateOptions) (*NetworkConfig,
if _, ok := netMap[opts.Name]; ok {
return errdefs.ErrAlreadyExists
}

ipam, err := e.generateIPAM(opts.IPAMDriver, opts.Subnets, opts.Gateway, opts.IPRange, opts.IPAMOptions, opts.IPv6)
if err != nil {
return err
Expand Down Expand Up @@ -406,31 +405,44 @@ func (e *CNIEnv) GetDefaultNetworkConfig() (*NetworkConfig, error) {
return nil, nil
}

func (e *CNIEnv) ensureDefaultNetworkConfig() error {
func (e *CNIEnv) ensureDefaultNetworkConfig(bridgeIP string) error {
defaultNet, err := e.GetDefaultNetworkConfig()
if err != nil {
return fmt.Errorf("failed to check for default network: %s", err)
}
if defaultNet == nil {
if err := e.createDefaultNetworkConfig(); err != nil {
if err := e.createDefaultNetworkConfig(bridgeIP); err != nil {
return fmt.Errorf("failed to create default network: %s", err)
}
}
return nil
}

func (e *CNIEnv) createDefaultNetworkConfig() error {
func (e *CNIEnv) createDefaultNetworkConfig(bridgeIP string) error {
filename := e.getConfigPathForNetworkName(DefaultNetworkName)
if _, err := os.Stat(filename); err == nil {
return fmt.Errorf("already found existing network config at %q, cannot create new network named %q", filename, DefaultNetworkName)
}

bridgeCIDR := DefaultCIDR
bridgeGatewayIP := DefaultGatewayIP
if bridgeIP != "" {
bIP, bCIDR, err := net.ParseCIDR(bridgeIP)
if err != nil {
return fmt.Errorf("invalid bridge ip %s: %s", bridgeIP, err)
}
bridgeGatewayIP = bIP.String()
bridgeCIDR = bCIDR.String()
}
opts := types.NetworkCreateOptions{
Name: DefaultNetworkName,
Driver: DefaultNetworkName,
Subnets: []string{DefaultCIDR},
Subnets: []string{bridgeCIDR},
Gateway: bridgeGatewayIP,
IPAMDriver: "default",
Labels: []string{fmt.Sprintf("%s=true", labels.NerdctlDefaultNetwork)},
}

_, err := e.CreateNetwork(opts)
if err != nil && !errdefs.IsAlreadyExists(err) {
return err
Expand Down
1 change: 1 addition & 0 deletions pkg/netutil/netutil_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,5 @@ func TestDefaultNetworkCreation(t *testing.T) {
}

testDefaultNetworkCreation(t)
testDefaultNetworkCreationWithBridgeIP(t)
}
110 changes: 106 additions & 4 deletions pkg/netutil/netutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package netutil

import (
"bytes"
"encoding/json"
"fmt"
"net"
"os"
Expand All @@ -33,6 +34,8 @@ import (
"github.com/containerd/nerdctl/v2/pkg/testutil"
)

const testBridgeIP = "10.1.100.1/24"

Check failure on line 37 in pkg/netutil/netutil_test.go

View workflow job for this annotation

GitHub Actions / go | freebsd |

const `testBridgeIP` is unused (unused)

const preExistingNetworkConfigTemplate = `
{
"cniVersion": "0.2.0",
Expand Down Expand Up @@ -160,10 +163,77 @@ func testDefaultNetworkCreation(t *testing.T) {
assert.Assert(t, defaultNetConf == nil)

// Attempt to create the default network.
err = cniEnv.ensureDefaultNetworkConfig()
err = cniEnv.ensureDefaultNetworkConfig("")
assert.NilError(t, err)

// Ensure default network config is present now.
defaultNetConf, err = cniEnv.GetDefaultNetworkConfig()
assert.NilError(t, err)
assert.Assert(t, defaultNetConf != nil)

// Check network config file present.
stat, err := os.Stat(defaultNetConf.File)
assert.NilError(t, err)
firstConfigModTime := stat.ModTime()

// Check default network label present.
assert.Assert(t, defaultNetConf.NerdctlLabels != nil)
lstr, ok := (*defaultNetConf.NerdctlLabels)[labels.NerdctlDefaultNetwork]
assert.Assert(t, ok)
boolv, err := strconv.ParseBool(lstr)
assert.NilError(t, err)
assert.Assert(t, boolv)

// Ensure network isn't created twice or accidentally re-created.
err = cniEnv.ensureDefaultNetworkConfig("")
assert.NilError(t, err)

// Check for any other network config files.
files := []os.FileInfo{}
walkF := func(p string, info os.FileInfo, err error) error {
files = append(files, info)
return nil
}
err = filepath.Walk(cniConfTestDir, walkF)
assert.NilError(t, err)
assert.Assert(t, len(files) == 2) // files[0] is the entry for '.'
assert.Assert(t, filepath.Join(cniConfTestDir, files[1].Name()) == defaultNetConf.File)
assert.Assert(t, firstConfigModTime == files[1].ModTime())
}

// Tests whether nerdctl properly creates the default network
// with a custom bridge IP and subnet.
// nolint:unused
func testDefaultNetworkCreationWithBridgeIP(t *testing.T) {
// To prevent subnet collisions when attempting to recreate the default network
// in the isolated CNI config dir we'll be using, we must first delete
// the network in the default CNI config dir.
defaultCniEnv := CNIEnv{
Path: ncdefaults.CNIPath(),
NetconfPath: ncdefaults.CNINetConfPath(),
}
defaultNet, err := defaultCniEnv.GetDefaultNetworkConfig()
assert.NilError(t, err)
if defaultNet != nil {
assert.NilError(t, defaultCniEnv.RemoveNetwork(defaultNet))
}

// Ensure no default network config is present now.
// We create a tempdir for the CNI conf path to ensure an empty env for this test.
cniConfTestDir := t.TempDir()
cniEnv := CNIEnv{
Path: ncdefaults.CNIPath(),
NetconfPath: cniConfTestDir,
}
// Ensure no default network config is not present.
defaultNetConf, err := cniEnv.GetDefaultNetworkConfig()
assert.NilError(t, err)
assert.Assert(t, defaultNetConf == nil)

// Attempt to create the default network with a test bridgeIP
err = cniEnv.ensureDefaultNetworkConfig(testBridgeIP)
assert.NilError(t, err)

// Ensure default network config is present now.
defaultNetConf, err = cniEnv.GetDefaultNetworkConfig()
assert.NilError(t, err)
assert.Assert(t, defaultNetConf != nil)
Expand All @@ -181,8 +251,40 @@ func testDefaultNetworkCreation(t *testing.T) {
assert.NilError(t, err)
assert.Assert(t, boolv)

// Check bridge IP is set.
assert.Assert(t, defaultNetConf.Plugins != nil)
assert.Assert(t, len(defaultNetConf.Plugins) > 0)
bridgePlugin := defaultNetConf.Plugins[0]
var bridgeConfig struct {
Type string `json:"type"`
Bridge string `json:"bridge"`
IPAM struct {
Ranges [][]struct {
Gateway string `json:"gateway"`
Subnet string `json:"subnet"`
} `json:"ranges"`
Routes []struct {
Dst string `json:"dst"`
} `json:"routes"`
Type string `json:"type"`
} `json:"ipam"`
}

err = json.Unmarshal(bridgePlugin.Bytes, &bridgeConfig)
if err != nil {
t.Fatalf("Failed to parse bridge plugin config: %v", err)
}

// Assert on bridge plugin configuration
assert.Equal(t, "bridge", bridgeConfig.Type)
// Assert on IPAM configuration
assert.Equal(t, "10.1.100.1", bridgeConfig.IPAM.Ranges[0][0].Gateway)
assert.Equal(t, "10.1.100.0/24", bridgeConfig.IPAM.Ranges[0][0].Subnet)
assert.Equal(t, "0.0.0.0/0", bridgeConfig.IPAM.Routes[0].Dst)
assert.Equal(t, "host-local", bridgeConfig.IPAM.Type)

// Ensure network isn't created twice or accidentally re-created.
err = cniEnv.ensureDefaultNetworkConfig()
err = cniEnv.ensureDefaultNetworkConfig(testBridgeIP)
assert.NilError(t, err)

// Check for any other network config files.
Expand Down Expand Up @@ -249,7 +351,7 @@ func TestNetworkWithDefaultNameAlreadyExists(t *testing.T) {
assert.Assert(t, defaultNetConf != nil)
assert.Assert(t, defaultNetConf.File == testConfFile)

err = cniEnv.ensureDefaultNetworkConfig()
err = cniEnv.ensureDefaultNetworkConfig("")
assert.NilError(t, err)

netConfs, err = cniEnv.NetworkList()
Expand Down
1 change: 1 addition & 0 deletions pkg/netutil/netutil_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import (
const (
DefaultNetworkName = "bridge"
DefaultCIDR = "10.4.0.0/24"
DefaultGatewayIP = "10.4.0.1"
DefaultIPAMDriver = "host-local"

// When creating non-default network without passing in `--subnet` option,
Expand Down
1 change: 1 addition & 0 deletions pkg/netutil/netutil_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
const (
DefaultNetworkName = "nat"
DefaultCIDR = "10.4.0.0/24"
DefaultGatewayIP = "10.4.0.1"

// When creating non-default network without passing in `--subnet` option,
// nerdctl assigns subnet address for the creation starting from `StartingCIDR`
Expand Down
8 changes: 4 additions & 4 deletions pkg/ocihook/ocihook.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const (
NetworkNamespace = labels.Prefix + "network-namespace"
)

func Run(stdin io.Reader, stderr io.Writer, event, dataStore, cniPath, cniNetconfPath string) error {
func Run(stdin io.Reader, stderr io.Writer, event, dataStore, cniPath, cniNetconfPath, bridgeIP string) error {
if stdin == nil || event == "" || dataStore == "" || cniPath == "" || cniNetconfPath == "" {
return errors.New("got insufficient args")
}
Expand Down Expand Up @@ -113,7 +113,7 @@ func Run(stdin io.Reader, stderr io.Writer, event, dataStore, cniPath, cniNetcon
}
defer lockutil.Unlock(lock)

opts, err := newHandlerOpts(&state, dataStore, cniPath, cniNetconfPath)
opts, err := newHandlerOpts(&state, dataStore, cniPath, cniNetconfPath, bridgeIP)
if err != nil {
return err
}
Expand All @@ -128,7 +128,7 @@ func Run(stdin io.Reader, stderr io.Writer, event, dataStore, cniPath, cniNetcon
}
}

func newHandlerOpts(state *specs.State, dataStore, cniPath, cniNetconfPath string) (*handlerOpts, error) {
func newHandlerOpts(state *specs.State, dataStore, cniPath, cniNetconfPath, bridgeIP string) (*handlerOpts, error) {
o := &handlerOpts{
state: state,
dataStore: dataStore,
Expand Down Expand Up @@ -173,7 +173,7 @@ func newHandlerOpts(state *specs.State, dataStore, cniPath, cniNetconfPath strin
case nettype.Host, nettype.None, nettype.Container, nettype.Namespace:
// NOP
case nettype.CNI:
e, err := netutil.NewCNIEnv(cniPath, cniNetconfPath, netutil.WithNamespace(namespace), netutil.WithDefaultNetwork())
e, err := netutil.NewCNIEnv(cniPath, cniNetconfPath, netutil.WithNamespace(namespace), netutil.WithDefaultNetwork(bridgeIP))
if err != nil {
return nil, err
}
Expand Down

0 comments on commit 308523a

Please sign in to comment.