Merge pull request #5225 from thaJeztah/network_cleanups

cli/command/network: some cleanup and pass smaller interfaces

cli/command/network/connect.go

@@ -25,7 +25,7 @@ type connectOptions struct {
 	driverOpts   []string
 }
 
-func newConnectCommand(dockerCli command.Cli) *cobra.Command {
+func newConnectCommand(dockerCLI command.Cli) *cobra.Command {
 	options := connectOptions{
 		links: opts.NewListOpts(opts.ValidateLink),
 	}
@@ -37,14 +37,14 @@ func newConnectCommand(dockerCli command.Cli) *cobra.Command {
 		RunE: func(cmd *cobra.Command, args []string) error {
 			options.network = args[0]
 			options.container = args[1]
-			return runConnect(cmd.Context(), dockerCli.Client(), options)
+			return runConnect(cmd.Context(), dockerCLI.Client(), options)
 		},
 		ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 			if len(args) == 0 {
-				return completion.NetworkNames(dockerCli)(cmd, args, toComplete)
+				return completion.NetworkNames(dockerCLI)(cmd, args, toComplete)
 			}
 			nw := args[0]
-			return completion.ContainerNames(dockerCli, true, not(isConnected(nw)))(cmd, args, toComplete)
+			return completion.ContainerNames(dockerCLI, true, not(isConnected(nw)))(cmd, args, toComplete)
 		},
 	}
 

cli/command/network/create.go

@@ -3,6 +3,7 @@ package network
 import (
 	"context"
 	"fmt"
+	"io"
 	"net"
 	"strings"
 
@@ -11,6 +12,7 @@ import (
 	"github.com/docker/cli/cli/command/completion"
 	"github.com/docker/cli/opts"
 	"github.com/docker/docker/api/types/network"
+	"github.com/docker/docker/client"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
@@ -27,22 +29,27 @@ type createOptions struct {
 	ingress    bool
 	configOnly bool
 	configFrom string
-
+	ipam       ipamOptions
-	ipamDriver  string
-	ipamSubnet  []string
-	ipamIPRange []string
-	ipamGateway []string
-	ipamAux     opts.MapOpts
-	ipamOpt     opts.MapOpts
 }
 
-func newCreateCommand(dockerCli command.Cli) *cobra.Command {
+type ipamOptions struct {
+	driver       string
+	subnets      []string
+	ipRanges     []string
+	gateways     []string
+	auxAddresses opts.MapOpts
+	driverOpts   opts.MapOpts
+}
+
+func newCreateCommand(dockerCLI command.Cli) *cobra.Command {
 	var ipv6 bool
 	options := createOptions{
 		driverOpts: *opts.NewMapOpts(nil, nil),
 		labels:     opts.NewListOpts(opts.ValidateLabel),
-		ipamAux:    *opts.NewMapOpts(nil, nil),
+		ipam: ipamOptions{
-		ipamOpt:    *opts.NewMapOpts(nil, nil),
+			auxAddresses: *opts.NewMapOpts(nil, nil),
+			driverOpts:   *opts.NewMapOpts(nil, nil),
+		},
 	}
 
 	cmd := &cobra.Command{
@@ -56,7 +63,7 @@ func newCreateCommand(dockerCli command.Cli) *cobra.Command {
 				options.ipv6 = &ipv6
 			}
 
-			return runCreate(cmd.Context(), dockerCli, options)
+			return runCreate(cmd.Context(), dockerCLI.Client(), dockerCLI.Out(), options)
 		},
 		ValidArgsFunction: completion.NoComplete,
 	}
@@ -78,21 +85,19 @@ func newCreateCommand(dockerCli command.Cli) *cobra.Command {
 	flags.StringVar(&options.configFrom, "config-from", "", "The network from which to copy the configuration")
 	flags.SetAnnotation("config-from", "version", []string{"1.30"})
 
-	flags.StringVar(&options.ipamDriver, "ipam-driver", "default", "IP Address Management Driver")
+	flags.StringVar(&options.ipam.driver, "ipam-driver", "default", "IP Address Management Driver")
-	flags.StringSliceVar(&options.ipamSubnet, "subnet", []string{}, "Subnet in CIDR format that represents a network segment")
+	flags.StringSliceVar(&options.ipam.subnets, "subnet", []string{}, "Subnet in CIDR format that represents a network segment")
-	flags.StringSliceVar(&options.ipamIPRange, "ip-range", []string{}, "Allocate container ip from a sub-range")
+	flags.StringSliceVar(&options.ipam.ipRanges, "ip-range", []string{}, "Allocate container ip from a sub-range")
-	flags.StringSliceVar(&options.ipamGateway, "gateway", []string{}, "IPv4 or IPv6 Gateway for the master subnet")
+	flags.StringSliceVar(&options.ipam.gateways, "gateway", []string{}, "IPv4 or IPv6 Gateway for the master subnet")
 
-	flags.Var(&options.ipamAux, "aux-address", "Auxiliary IPv4 or IPv6 addresses used by Network driver")
+	flags.Var(&options.ipam.auxAddresses, "aux-address", "Auxiliary IPv4 or IPv6 addresses used by Network driver")
-	flags.Var(&options.ipamOpt, "ipam-opt", "Set IPAM driver specific options")
+	flags.Var(&options.ipam.driverOpts, "ipam-opt", "Set IPAM driver specific options")
 
 	return cmd
 }
 
-func runCreate(ctx context.Context, dockerCli command.Cli, options createOptions) error {
+func runCreate(ctx context.Context, apiClient client.NetworkAPIClient, output io.Writer, options createOptions) error {
-	client := dockerCli.Client()
+	ipamCfg, err := createIPAMConfig(options.ipam)
-
-	ipamCfg, err := consolidateIpam(options.ipamSubnet, options.ipamIPRange, options.ipamGateway, options.ipamAux.GetAll())
 	if err != nil {
 		return err
 	}
@@ -103,14 +108,10 @@ func runCreate(ctx context.Context, dockerCli command.Cli, options createOptions
 			Network: options.configFrom,
 		}
 	}
-	resp, err := client.NetworkCreate(ctx, options.name, network.CreateOptions{
+	resp, err := apiClient.NetworkCreate(ctx, options.name, network.CreateOptions{
 		Driver:     options.driver,
 		Options:    options.driverOpts.GetAll(),
-		IPAM: &network.IPAM{
+		IPAM:       ipamCfg,
-			Driver:  options.ipamDriver,
-			Config:  ipamCfg,
-			Options: options.ipamOpt.GetAll(),
-		},
 		Internal:   options.internal,
 		EnableIPv6: options.ipv6,
 		Attachable: options.attachable,
@@ -123,25 +124,25 @@ func runCreate(ctx context.Context, dockerCli command.Cli, options createOptions
 	if err != nil {
 		return err
 	}
-	fmt.Fprintf(dockerCli.Out(), "%s\n", resp.ID)
+	_, _ = fmt.Fprintf(output, "%s\n", resp.ID)
 	return nil
 }
 
 // Consolidates the ipam configuration as a group from different related configurations
 // user can configure network with multiple non-overlapping subnets and hence it is
 // possible to correlate the various related parameters and consolidate them.
-// consolidateIpam consolidates subnets, ip-ranges, gateways and auxiliary addresses into
+// createIPAMConfig consolidates subnets, ip-ranges, gateways and auxiliary addresses into
 // structured ipam data.
 //
 //nolint:gocyclo
-func consolidateIpam(subnets, ranges, gateways []string, auxaddrs map[string]string) ([]network.IPAMConfig, error) {
+func createIPAMConfig(options ipamOptions) (*network.IPAM, error) {
-	if len(subnets) < len(ranges) || len(subnets) < len(gateways) {
+	if len(options.subnets) < len(options.ipRanges) || len(options.subnets) < len(options.gateways) {
 		return nil, errors.Errorf("every ip-range or gateway must have a corresponding subnet")
 	}
 	iData := map[string]*network.IPAMConfig{}
 
 	// Populate non-overlapping subnets into consolidation map
-	for _, s := range subnets {
+	for _, s := range options.subnets {
 		for k := range iData {
 			ok1, err := subnetMatches(s, k)
 			if err != nil {
@@ -159,9 +160,9 @@ func consolidateIpam(subnets, ranges, gateways []string, auxaddrs map[string]str
 	}
 
 	// Validate and add valid ip ranges
-	for _, r := range ranges {
+	for _, r := range options.ipRanges {
 		match := false
-		for _, s := range subnets {
+		for _, s := range options.subnets {
 			ok, err := subnetMatches(s, r)
 			if err != nil {
 				return nil, err
@@ -182,9 +183,9 @@ func consolidateIpam(subnets, ranges, gateways []string, auxaddrs map[string]str
 	}
 
 	// Validate and add valid gateways
-	for _, g := range gateways {
+	for _, g := range options.gateways {
 		match := false
-		for _, s := range subnets {
+		for _, s := range options.subnets {
 			ok, err := subnetMatches(s, g)
 			if err != nil {
 				return nil, err
@@ -205,9 +206,9 @@ func consolidateIpam(subnets, ranges, gateways []string, auxaddrs map[string]str
 	}
 
 	// Validate and add aux-addresses
-	for key, aa := range auxaddrs {
+	for key, aa := range options.auxAddresses.GetAll() {
 		match := false
-		for _, s := range subnets {
+		for _, s := range options.subnets {
 			ok, err := subnetMatches(s, aa)
 			if err != nil {
 				return nil, err
@@ -223,11 +224,16 @@ func consolidateIpam(subnets, ranges, gateways []string, auxaddrs map[string]str
 		}
 	}
 
-	idl := []network.IPAMConfig{}
+	idl := make([]network.IPAMConfig, 0, len(iData))
 	for _, v := range iData {
 		idl = append(idl, *v)
 	}
-	return idl, nil
+
+	return &network.IPAM{
+		Driver:  options.driver,
+		Config:  idl,
+		Options: options.driverOpts.GetAll(),
+	}, nil
 }
 
 func subnetMatches(subnet, data string) (bool, error) {

cli/command/network/disconnect.go

@@ -7,6 +7,7 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/client"
 	"github.com/spf13/cobra"
 )
 
@@ -26,7 +27,7 @@ func newDisconnectCommand(dockerCli command.Cli) *cobra.Command {
 		RunE: func(cmd *cobra.Command, args []string) error {
 			opts.network = args[0]
 			opts.container = args[1]
-			return runDisconnect(cmd.Context(), dockerCli, opts)
+			return runDisconnect(cmd.Context(), dockerCli.Client(), opts)
 		},
 		ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 			if len(args) == 0 {
@@ -43,10 +44,8 @@ func newDisconnectCommand(dockerCli command.Cli) *cobra.Command {
 	return cmd
 }
 
-func runDisconnect(ctx context.Context, dockerCli command.Cli, opts disconnectOptions) error {
+func runDisconnect(ctx context.Context, apiClient client.NetworkAPIClient, opts disconnectOptions) error {
-	client := dockerCli.Client()
+	return apiClient.NetworkDisconnect(ctx, opts.network, opts.container, opts.force)
-
-	return client.NetworkDisconnect(ctx, opts.network, opts.container, opts.force)
 }
 
 func isConnected(network string) func(types.Container) bool {

cli/command/network/inspect.go

@@ -5,6 +5,7 @@ package network
 
 import (
 	"context"
+	"io"
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
@@ -12,6 +13,7 @@ import (
 	"github.com/docker/cli/cli/command/inspect"
 	flagsHelper "github.com/docker/cli/cli/flags"
 	"github.com/docker/docker/api/types/network"
+	"github.com/docker/docker/client"
 	"github.com/spf13/cobra"
 )
 
@@ -21,7 +23,7 @@ type inspectOptions struct {
 	verbose bool
 }
 
-func newInspectCommand(dockerCli command.Cli) *cobra.Command {
+func newInspectCommand(dockerCLI command.Cli) *cobra.Command {
 	var opts inspectOptions
 
 	cmd := &cobra.Command{
@@ -30,9 +32,9 @@ func newInspectCommand(dockerCli command.Cli) *cobra.Command {
 		Args:  cli.RequiresMinArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			opts.names = args
-			return runInspect(cmd.Context(), dockerCli, opts)
+			return runInspect(cmd.Context(), dockerCLI.Client(), dockerCLI.Out(), opts)
 		},
-		ValidArgsFunction: completion.NetworkNames(dockerCli),
+		ValidArgsFunction: completion.NetworkNames(dockerCLI),
 	}
 
 	cmd.Flags().StringVarP(&opts.format, "format", "f", "", flagsHelper.InspectFormatHelp)
@@ -41,12 +43,8 @@ func newInspectCommand(dockerCli command.Cli) *cobra.Command {
 	return cmd
 }
 
-func runInspect(ctx context.Context, dockerCli command.Cli, opts inspectOptions) error {
+func runInspect(ctx context.Context, apiClient client.NetworkAPIClient, output io.Writer, opts inspectOptions) error {
-	client := dockerCli.Client()
+	return inspect.Inspect(output, opts.names, opts.format, func(name string) (any, []byte, error) {
-
+		return apiClient.NetworkInspectWithRaw(ctx, name, network.InspectOptions{Verbose: opts.verbose})
-	getNetFunc := func(name string) (any, []byte, error) {
+	})
-		return client.NetworkInspectWithRaw(ctx, name, network.InspectOptions{Verbose: opts.verbose})
-	}
-
-	return inspect.Inspect(dockerCli.Out(), opts.names, opts.format, getNetFunc)
 }

cli/command/network/prune.go

@@ -31,7 +31,7 @@ func NewPruneCommand(dockerCli command.Cli) *cobra.Command {
 				return err
 			}
 			if output != "" {
-				fmt.Fprintln(dockerCli.Out(), output)
+				_, _ = fmt.Fprintln(dockerCli.Out(), output)
 			}
 			return nil
 		},

cli/command/network/remove.go

@@ -60,11 +60,11 @@ func runRemove(ctx context.Context, dockerCli command.Cli, networks []string, op
 			if opts.force && errdefs.IsNotFound(err) {
 				continue
 			}
-			fmt.Fprintf(dockerCli.Err(), "%s\n", err)
+			_, _ = fmt.Fprintf(dockerCli.Err(), "%s\n", err)
 			status = 1
 			continue
 		}
-		fmt.Fprintf(dockerCli.Out(), "%s\n", name)
+		_, _ = fmt.Fprintf(dockerCli.Out(), "%s\n", name)
 	}
 
 	if status != 0 {