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.

Sign up for GitHub

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

Add option --ipv4 #5599

Merged
merged 3 commits into from
Dec 12, 2024
+96 −25
Merged

Add option --ipv4 #5599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a5353e5
Docs: include --fixed-cidr-v6 and --bip6 docker0 options
robmry Dec 10, 2024
5c896c9
Docs: emphasise that some options that are for docker0
robmry Dec 10, 2024
6a2cde6
Add option '--ipv4'
robmry Dec 10, 2024
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
10 changes: 8 additions & 2 deletions cli/command/network/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type createOptions struct {
driverOpts opts.MapOpts
labels opts.ListOpts
internal bool
ipv4 *bool
ipv6 *bool
attachable bool
ingress bool
Expand All @@ -42,7 +43,7 @@ type ipamOptions struct {
}

func newCreateCommand(dockerCLI command.Cli) *cobra.Command {
var ipv6 bool
var ipv4, ipv6 bool
options := createOptions{
driverOpts: *opts.NewMapOpts(nil, nil),
labels: opts.NewListOpts(opts.ValidateLabel),
Expand All @@ -59,6 +60,9 @@ func newCreateCommand(dockerCLI command.Cli) *cobra.Command {
RunE: func(cmd *cobra.Command, args []string) error {
options.name = args[0]

if cmd.Flag("ipv4").Changed {
options.ipv4 = &ipv4
}
if cmd.Flag("ipv6").Changed {
options.ipv6 = &ipv6
}
Expand All @@ -73,7 +77,8 @@ func newCreateCommand(dockerCLI command.Cli) *cobra.Command {
flags.VarP(&options.driverOpts, "opt", "o", "Set driver specific options")
flags.Var(&options.labels, "label", "Set metadata on a network")
flags.BoolVar(&options.internal, "internal", false, "Restrict external access to the network")
flags.BoolVar(&ipv6, "ipv6", false, "Enable or disable IPv6 networking")
flags.BoolVar(&ipv4, "ipv4", true, "Enable or disable IPv4 address assignment")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
flags.BoolVar(&ipv4, "ipv4", true, "Enable or disable IPv4 address assignment")
flags.BoolVar(&ipv4, "ipv4", true, "Enable or disable IPv4 address assignment")

Wondering if we should force this to true on the CLI side.
The EnableIPv4 is *bool on the API side, so we could just keep it as missing/null if user haven't passed this option.
For example if in future we'd need to distinguish between "user explicitly doesn't want ipv4" or "user doesn't care" on the daemon side.

Does that make sense?

Copy link
Member

Choose a reason for hiding this comment

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

LOL, I just wrote a blurb below about "defaults".

Copy link
Contributor Author

@robmry robmry Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

so we could just keep it as missing/null if user haven't passed this option

Just above, lines 63-65 (copied from the IPv6 setting), the pointer in the API request is only set if the CLI flag is Changed ... so I think it's ok?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh right, I missed that!

flags.BoolVar(&ipv6, "ipv6", false, "Enable or disable IPv6 address assignment")
Copy link
Member

Choose a reason for hiding this comment

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

👍 for the revised help text.

flags.BoolVar(&options.attachable, "attachable", false, "Enable manual container attachment")
flags.SetAnnotation("attachable", "version", []string{"1.25"})
flags.BoolVar(&options.ingress, "ingress", false, "Create swarm routing-mesh network")
Expand Down Expand Up @@ -113,6 +118,7 @@ func runCreate(ctx context.Context, apiClient client.NetworkAPIClient, output io
Options: options.driverOpts.GetAll(),
IPAM: ipamCfg,
Internal: options.internal,
EnableIPv4: options.ipv4,
EnableIPv6: options.ipv6,
Attachable: options.attachable,
Ingress: options.ingress,
Expand Down
54 changes: 54 additions & 0 deletions cli/command/network/create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,60 @@ func TestNetworkCreateWithFlags(t *testing.T) {
assert.Check(t, is.Equal("banana", strings.TrimSpace(cli.OutBuffer().String())))
}

// TestNetworkCreateIPv4 verifies behavior of the "--ipv4" option. This option
// is an optional bool, and must default to "nil", not "true" or "false".
func TestNetworkCreateIPv4(t *testing.T) {
boolPtr := func(val bool) *bool { return &val }

tests := []struct {
doc, name string
flags []string
expected *bool
}{
{
doc: "IPv4 default",
name: "ipv4-default",
expected: nil,
},
{
doc: "IPv4 enabled",
name: "ipv4-enabled",
flags: []string{"--ipv4=true"},
expected: boolPtr(true),
},
{
doc: "IPv4 enabled (shorthand)",
name: "ipv4-enabled-shorthand",
flags: []string{"--ipv4"},
expected: boolPtr(true),
},
{
doc: "IPv4 disabled",
name: "ipv4-disabled",
flags: []string{"--ipv4=false"},
expected: boolPtr(false),
},
}

for _, tc := range tests {
t.Run(tc.doc, func(t *testing.T) {
cli := test.NewFakeCli(&fakeClient{
networkCreateFunc: func(ctx context.Context, name string, createBody network.CreateOptions) (network.CreateResponse, error) {
assert.Check(t, is.DeepEqual(createBody.EnableIPv4, tc.expected))
return network.CreateResponse{ID: name}, nil
},
})
cmd := newCreateCommand(cli)
cmd.SetArgs([]string{tc.name})
if tc.expected != nil {
assert.Check(t, cmd.ParseFlags(tc.flags))
}
assert.NilError(t, cmd.Execute())
assert.Check(t, is.Equal(tc.name, strings.TrimSpace(cli.OutBuffer().String())))
})
}
}

// TestNetworkCreateIPv6 verifies behavior of the "--ipv6" option. This option
// is an optional bool, and must default to "nil", not "true" or "false".
func TestNetworkCreateIPv6(t *testing.T) {
Expand Down
6 changes: 6 additions & 0 deletions cli/command/network/formatter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const (
defaultNetworkTableFormat = "table {{.ID}}\t{{.Name}}\t{{.Driver}}\t{{.Scope}}"

networkIDHeader = "NETWORK ID"
ipv4Header = "IPV4"
ipv6Header = "IPV6"
internalHeader = "INTERNAL"
)
Expand Down Expand Up @@ -51,6 +52,7 @@ func FormatWrite(ctx formatter.Context, networks []network.Summary) error {
"Name": formatter.NameHeader,
"Driver": formatter.DriverHeader,
"Scope": formatter.ScopeHeader,
"IPv4": ipv4Header,
"IPv6": ipv6Header,
"Internal": internalHeader,
"Labels": formatter.LabelsHeader,
Expand Down Expand Up @@ -88,6 +90,10 @@ func (c *networkContext) Scope() string {
return c.n.Scope
}

func (c *networkContext) IPv4() string {
return strconv.FormatBool(c.n.EnableIPv4)
}

func (c *networkContext) IPv6() string {
return strconv.FormatBool(c.n.EnableIPv6)
}
Expand Down
7 changes: 5 additions & 2 deletions cli/command/network/formatter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ func TestNetworkContext(t *testing.T) {
{networkContext{
n: network.Summary{Driver: "driver_name"},
}, "driver_name", ctx.Driver},
{networkContext{
n: network.Summary{EnableIPv4: true},
}, "true", ctx.IPv4},
{networkContext{
n: network.Summary{EnableIPv6: true},
}, "true", ctx.IPv6},
Expand Down Expand Up @@ -180,8 +183,8 @@ func TestNetworkContextWriteJSON(t *testing.T) {
{ID: "networkID2", Name: "foobar_bar"},
}
expectedJSONs := []map[string]any{
{"Driver": "", "ID": "networkID1", "IPv6": "false", "Internal": "false", "Labels": "", "Name": "foobar_baz", "Scope": "", "CreatedAt": "0001-01-01 00:00:00 +0000 UTC"},
{"Driver": "", "ID": "networkID2", "IPv6": "false", "Internal": "false", "Labels": "", "Name": "foobar_bar", "Scope": "", "CreatedAt": "0001-01-01 00:00:00 +0000 UTC"},
{"Driver": "", "ID": "networkID1", "IPv4": "false", "IPv6": "false", "Internal": "false", "Labels": "", "Name": "foobar_baz", "Scope": "", "CreatedAt": "0001-01-01 00:00:00 +0000 UTC"},
{"Driver": "", "ID": "networkID2", "IPv4": "false", "IPv6": "false", "Internal": "false", "Labels": "", "Name": "foobar_bar", "Scope": "", "CreatedAt": "0001-01-01 00:00:00 +0000 UTC"},
}

out := bytes.NewBufferString("")
Expand Down
44 changes: 23 additions & 21 deletions docs/reference/commandline/network_create.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ Create a network
| `--ip-range` | `stringSlice` | | Allocate container ip from a sub-range |
| `--ipam-driver` | `string` | `default` | IP Address Management Driver |
| `--ipam-opt` | `map` | `map[]` | Set IPAM driver specific options |
| `--ipv6` | `bool` | | Enable or disable IPv6 networking |
| `--ipv4` | `bool` | `true` | Enable or disable IPv4 address assignment |
| `--ipv6` | `bool` | | Enable or disable IPv6 address assignment |
| `--label` | `list` | | Set metadata on a network |
| `-o`, `--opt` | `map` | `map[]` | Set driver specific options |
| `--scope` | `string` | | Control the network's scope |
Expand Down Expand Up @@ -148,30 +149,31 @@ fails and Docker Engine returns an error.

### Bridge driver options

When creating a custom network, the default network driver (i.e. `bridge`) has
additional options that can be passed. The following are those options and the
equivalent Docker daemon flags used for docker0 bridge:
When creating a custom `bridge` network, the following additional options can
be passed. Some of these have equivalent flags that can be used on the dockerd
command line or in `daemon.json` to configure the default bridge, `docker0`:

| Option | Equivalent | Description |
|--------------------------------------------------|-------------|-------------------------------------------------------|
| `com.docker.network.bridge.name` | - | Bridge name to be used when creating the Linux bridge |
| `com.docker.network.bridge.enable_ip_masquerade` | `--ip-masq` | Enable IP masquerading |
| `com.docker.network.bridge.enable_icc` | `--icc` | Enable or Disable Inter Container Connectivity |
| `com.docker.network.bridge.host_binding_ipv4` | `--ip` | Default IP when binding container ports |
| `com.docker.network.driver.mtu` | `--mtu` | Set the containers network MTU |
| `com.docker.network.container_iface_prefix` | - | Set a custom prefix for container interfaces |
| Network create option | Daemon option for `docker0` | Description |
|--------------------------------------------------|-----------------------------|-------------------------------------------------------|
| `com.docker.network.bridge.name` | - | Bridge name to be used when creating the Linux bridge |
| `com.docker.network.bridge.enable_ip_masquerade` | `--ip-masq` | Enable IP masquerading |
| `com.docker.network.bridge.enable_icc` | `--icc` | Enable or Disable Inter Container Connectivity |
| `com.docker.network.bridge.host_binding_ipv4` | `--ip` | Default IP when binding container ports |
| `com.docker.network.driver.mtu` | `--mtu` | Set the containers network MTU |
| `com.docker.network.container_iface_prefix` | - | Set a custom prefix for container interfaces |

The following arguments can be passed to `docker network create` for any
network driver, again with their approximate equivalents to Docker daemon
flags used for the docker0 bridge:

| Argument | Equivalent | Description |
|--------------|----------------|--------------------------------------------|
| `--gateway` | - | IPv4 or IPv6 Gateway for the master subnet |
| `--ip-range` | `--fixed-cidr` | Allocate IPs from a range |
| `--internal` | - | Restrict external access to the network |
| `--ipv6` | `--ipv6` | Enable or disable IPv6 networking |
| `--subnet` | `--bip` | Subnet for network |
flags used for the `docker0` bridge:

| Network create option | Daemon option for `docker0` | Description |
|-----------------------|-----------------------------------|--------------------------------------------|
| `--gateway` | - | IPv4 or IPv6 Gateway for the master subnet |
| `--ip-range` | `--fixed-cidr`, `--fixed-cidr-v6` | Allocate IP addresses from a range |
| `--internal` | - | Restrict external access to the network |
| `--ipv4` | - | Enable or disable IPv4 address assignment |
| `--ipv6` | `--ipv6` | Enable or disable IPv6 address assignment |
| `--subnet` | `--bip`, `--bip6` | Subnet for network |

For example, let's use `-o` or `--opt` options to specify an IP address binding
when publishing ports:
Expand Down
Loading