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

feat: adds IP address configuration for default bridge network #3640

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
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 |
swagatbora90 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| `bridge_ip` | `--bridge-ip` | `NERDCTL_BRIDGE_IP` | IP address for the default nerdctl bridge network | Since 1.7.8 |
| `bridge_ip` | `--bridge-ip` | `NERDCTL_BRIDGE_IP` | IP address for the default nerdctl bridge network | Since 2.1.0 |


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 := ""
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" // nolint: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
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