libnetwork/osl: remove Sandbox.InterfaceOptions() etc.

InterfaceOptions() returned an IfaceOptionSetter interface, which contained
"methods" that returned functional options. Such a construct could have made
sense if the functional options returned would (e.g.) be pre-propagated with
information from the Sandbox (network namespace), but none of that was the case.

There was only one implementation of IfaceOptionSetter (networkNamespace),
which happened to be the same as the only implementation of Sandbox, so remove
the interface as well, to help networkNamespace with its multi-personality
disorder.

This patch:

- removes Sandbox.Bridge() and makes it a regular function (WithIsBridge)
- removes Sandbox.Master() and makes it a regular function (WithMaster)
- removes Sandbox.MacAddress() and makes it a regular function (WithMACAddress)
- removes Sandbox.Address() and makes it a regular function (WithIPv4Address)
- removes Sandbox.AddressIPv6() and makes it a regular function (WithIPv6Address)
- removes Sandbox.LinkLocalAddresses() and makes it a regular function (WithLinkLocalAddresses)
- removes Sandbox.Routes() and makes it a regular function (WithRoutes)
- removes Sandbox.InterfaceOptions().
- removes the IfaceOptionSetter interface.

Note that the IfaceOption signature was changes as well to allow returning
an error. This is not currently used, but will be used for some options
in the near future, so adding that in preparation.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

libnetwork/drivers/overlay/joinleave.go

@@ -11,6 +11,7 @@ import (
 	"github.com/containerd/containerd/log"
 	"github.com/docker/docker/libnetwork/driverapi"
 	"github.com/docker/docker/libnetwork/ns"
+	"github.com/docker/docker/libnetwork/osl"
 	"github.com/docker/docker/libnetwork/types"
 	"github.com/gogo/protobuf/proto"
 )
@@ -73,8 +74,7 @@ func (d *driver) Join(nid, eid string, sboxKey string, jinfo driverapi.JoinInfo,
 		return err
 	}
 
-	if err = sbox.AddInterface(overlayIfName, "veth",
-		sbox.InterfaceOptions().Master(s.brName)); err != nil {
+	if err = sbox.AddInterface(overlayIfName, "veth", osl.WithMaster(s.brName)); err != nil {
 		return fmt.Errorf("could not add veth pair inside the network sandbox: %v", err)
 	}
 

libnetwork/drivers/overlay/ov_network.go

@@ -426,9 +426,7 @@ func (n *network) setupSubnetSandbox(s *subnet, brName, vxlanName string) error
 	// create a bridge and vxlan device for this subnet and move it to the sandbox
 	sbox := n.sbox
 
-	if err := sbox.AddInterface(brName, "br",
-		sbox.InterfaceOptions().Address(s.gwIP),
-		sbox.InterfaceOptions().Bridge(true)); err != nil {
+	if err := sbox.AddInterface(brName, "br", osl.WithIPv4Address(s.gwIP), osl.WithIsBridge(true)); err != nil {
 		return fmt.Errorf("bridge creation in sandbox failed for subnet %q: %v", s.subnetIP.String(), err)
 	}
 
@@ -437,8 +435,7 @@ func (n *network) setupSubnetSandbox(s *subnet, brName, vxlanName string) error
 		return err
 	}
 
-	if err := sbox.AddInterface(vxlanName, "vxlan",
-		sbox.InterfaceOptions().Master(brName)); err != nil {
+	if err := sbox.AddInterface(vxlanName, "vxlan", osl.WithMaster(brName)); err != nil {
 		// If adding vxlan device to the overlay namespace fails, remove the bridge interface we
 		// already added to the namespace. This allows the caller to try the setup again.
 		for _, iface := range sbox.Interfaces() {

libnetwork/osl/interface_linux.go

@@ -190,7 +190,9 @@ func (n *networkNamespace) AddInterface(srcName, dstPrefix string, options ...If
 		dstName: dstPrefix,
 		ns:      n,
 	}
-	i.processInterfaceOptions(options...)
+	if err := i.processInterfaceOptions(options...); err != nil {
+		return err
+	}
 
 	if i.master != "" {
 		i.dstMaster = n.findDst(i.master, true)

libnetwork/osl/namespace_linux.go

@@ -343,11 +343,6 @@ func (n *networkNamespace) Interfaces() []Interface {
 	return ifaces
 }
 
-// InterfaceOptions an interface with methods to set interface options.
-func (n *networkNamespace) InterfaceOptions() IfaceOptionSetter {
-	return n
-}
-
 func (n *networkNamespace) loopbackUp() error {
 	iface, err := n.nlHandle.LinkByName("lo")
 	if err != nil {
@@ -493,7 +488,9 @@ func (n *networkNamespace) Restore(ifsopt map[Iface][]IfaceOption, routes []*typ
 			dstName: name.DstPrefix,
 			ns:      n,
 		}
-		i.processInterfaceOptions(opts...)
+		if err := i.processInterfaceOptions(opts...); err != nil {
+			return err
+		}
 		if i.master != "" {
 			i.dstMaster = n.findDst(i.master, true)
 			if i.dstMaster == "" {

libnetwork/osl/options_linux.go

@@ -24,61 +24,72 @@ func WithFamily(family int) NeighOption {
 	}
 }
 
-func (i *nwIface) processInterfaceOptions(options ...IfaceOption) {
+func (i *nwIface) processInterfaceOptions(options ...IfaceOption) error {
 	for _, opt := range options {
 		if opt != nil {
-			opt(i)
+			// TODO(thaJeztah): use multi-error instead of returning early.
+			if err := opt(i); err != nil {
+				return err
+			}
 		}
 	}
+	return nil
 }
 
-// Bridge returns an option setter to set if the interface is a bridge.
-func (n *networkNamespace) Bridge(isBridge bool) IfaceOption {
-	return func(i *nwIface) {
+// WithIsBridge sets whether the interface is a bridge.
+func WithIsBridge(isBridge bool) IfaceOption {
+	return func(i *nwIface) error {
 		i.bridge = isBridge
+		return nil
 	}
 }
 
-// Master returns an option setter to set the master interface if any for this
-// interface. The master interface name should refer to the srcname of a
-// previously added interface of type bridge.
-func (n *networkNamespace) Master(name string) IfaceOption {
-	return func(i *nwIface) {
+// WithMaster sets the master interface (if any) for this interface. The
+// master interface name should refer to the srcName of a previously added
+// interface of type bridge.
+func WithMaster(name string) IfaceOption {
+	return func(i *nwIface) error {
 		i.master = name
+		return nil
 	}
 }
 
-// MacAddress returns an option setter to set the MAC address.
-func (n *networkNamespace) MacAddress(mac net.HardwareAddr) IfaceOption {
-	return func(i *nwIface) {
+// WithMACAddress sets the interface MAC-address.
+func WithMACAddress(mac net.HardwareAddr) IfaceOption {
+	return func(i *nwIface) error {
 		i.mac = mac
+		return nil
 	}
 }
 
-// Address returns an option setter to set IPv4 address.
-func (n *networkNamespace) Address(addr *net.IPNet) IfaceOption {
-	return func(i *nwIface) {
+// WithIPv4Address sets the IPv4 address of the interface.
+func WithIPv4Address(addr *net.IPNet) IfaceOption {
+	return func(i *nwIface) error {
 		i.address = addr
+		return nil
 	}
 }
 
-// AddressIPv6 returns an option setter to set IPv6 address.
-func (n *networkNamespace) AddressIPv6(addr *net.IPNet) IfaceOption {
-	return func(i *nwIface) {
+// WithIPv6Address sets the IPv6 address of the interface.
+func WithIPv6Address(addr *net.IPNet) IfaceOption {
+	return func(i *nwIface) error {
 		i.addressIPv6 = addr
+		return nil
 	}
 }
 
-// LinkLocalAddresses returns an option setter to set the link-local IP addresses.
-func (n *networkNamespace) LinkLocalAddresses(list []*net.IPNet) IfaceOption {
-	return func(i *nwIface) {
+// WithLinkLocalAddresses set the link-local IP addresses of the interface.
+func WithLinkLocalAddresses(list []*net.IPNet) IfaceOption {
+	return func(i *nwIface) error {
 		i.llAddrs = list
+		return nil
 	}
 }
 
-// Routes returns an option setter to set interface routes.
-func (n *networkNamespace) Routes(routes []*net.IPNet) IfaceOption {
-	return func(i *nwIface) {
+// WithRoutes sets the interface routes.
+func WithRoutes(routes []*net.IPNet) IfaceOption {
+	return func(i *nwIface) error {
 		i.routes = routes
+		return nil
 	}
 }

libnetwork/osl/sandbox.go

@@ -22,7 +22,7 @@ type Iface struct {
 }
 
 // IfaceOption is a function option type to set interface options.
-type IfaceOption func(i *nwIface)
+type IfaceOption func(i *nwIface) error
 
 // NeighOption is a function option type to set neighbor options.
 type NeighOption func(nh *neigh)
@@ -77,9 +77,6 @@ type Sandbox interface {
 	// DeleteNeighbor deletes neighbor entry from the sandbox.
 	DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr, osDelete bool) error
 
-	// InterfaceOptions an interface with methods to set interface options.
-	InterfaceOptions() IfaceOptionSetter
-
 	// InvokeFunc invoke a function in the network namespace.
 	InvokeFunc(func()) error
 
@@ -95,32 +92,6 @@ type Sandbox interface {
 	Info
 }
 
-// IfaceOptionSetter interface defines the option setter methods for interface options.
-type IfaceOptionSetter interface {
-	// Bridge returns an option setter to set if the interface is a bridge.
-	Bridge(bool) IfaceOption
-
-	// MacAddress returns an option setter to set the MAC address.
-	MacAddress(net.HardwareAddr) IfaceOption
-
-	// Address returns an option setter to set IPv4 address.
-	Address(*net.IPNet) IfaceOption
-
-	// AddressIPv6 returns an option setter to set IPv6 address.
-	AddressIPv6(*net.IPNet) IfaceOption
-
-	// LinkLocalAddresses returns an option setter to set the link-local IP addresses.
-	LinkLocalAddresses([]*net.IPNet) IfaceOption
-
-	// Master returns an option setter to set the master interface if any for this
-	// interface. The master interface name should refer to the srcname of a
-	// previously added interface of type bridge.
-	Master(string) IfaceOption
-
-	// Routes returns an option setter to set interface routes.
-	Routes([]*net.IPNet) IfaceOption
-}
-
 // Info represents all possible information that
 // the driver wants to place in the sandbox which includes
 // interfaces, routes and gateway

libnetwork/osl/sandbox_linux_test.go

@@ -409,9 +409,9 @@ func TestSandboxCreate(t *testing.T) {
 
 	for _, i := range tbox.Interfaces() {
 		err = s.AddInterface(i.SrcName(), i.DstName(),
-			tbox.InterfaceOptions().Bridge(i.Bridge()),
-			tbox.InterfaceOptions().Address(i.Address()),
-			tbox.InterfaceOptions().AddressIPv6(i.AddressIPv6()))
+			WithIsBridge(i.Bridge()),
+			WithIPv4Address(i.Address()),
+			WithIPv6Address(i.AddressIPv6()))
 		if err != nil {
 			t.Fatalf("Failed to add interfaces to sandbox: %v", err)
 		}
@@ -508,9 +508,10 @@ func TestAddRemoveInterface(t *testing.T) {
 
 	for _, i := range tbox.Interfaces() {
 		err = s.AddInterface(i.SrcName(), i.DstName(),
-			tbox.InterfaceOptions().Bridge(i.Bridge()),
-			tbox.InterfaceOptions().Address(i.Address()),
-			tbox.InterfaceOptions().AddressIPv6(i.AddressIPv6()))
+			WithIsBridge(i.Bridge()),
+			WithIPv4Address(i.Address()),
+			WithIPv6Address(i.AddressIPv6()),
+		)
 		if err != nil {
 			t.Fatalf("Failed to add interfaces to sandbox: %v", err)
 		}
@@ -526,10 +527,12 @@ func TestAddRemoveInterface(t *testing.T) {
 	verifySandbox(t, s, []string{"1", "2"})
 
 	i := tbox.Interfaces()[0]
-	if err := s.AddInterface(i.SrcName(), i.DstName(),
-		tbox.InterfaceOptions().Bridge(i.Bridge()),
-		tbox.InterfaceOptions().Address(i.Address()),
-		tbox.InterfaceOptions().AddressIPv6(i.AddressIPv6())); err != nil {
+	err = s.AddInterface(i.SrcName(), i.DstName(),
+		WithIsBridge(i.Bridge()),
+		WithIPv4Address(i.Address()),
+		WithIPv6Address(i.AddressIPv6()),
+	)
+	if err != nil {
 		t.Fatalf("Failed to add interfaces to sandbox: %v", err)
 	}
 

libnetwork/sandbox_linux.go

@@ -199,17 +199,17 @@ func (sb *Sandbox) restoreOslSandbox() error {
 		}
 
 		ifaceOptions := []osl.IfaceOption{
-			sb.osSbox.InterfaceOptions().Address(i.addr),
-			sb.osSbox.InterfaceOptions().Routes(i.routes),
+			osl.WithIPv4Address(i.addr),
+			osl.WithRoutes(i.routes),
 		}
 		if i.addrv6 != nil && i.addrv6.IP.To16() != nil {
-			ifaceOptions = append(ifaceOptions, sb.osSbox.InterfaceOptions().AddressIPv6(i.addrv6))
+			ifaceOptions = append(ifaceOptions, osl.WithIPv6Address(i.addrv6))
 		}
 		if i.mac != nil {
-			ifaceOptions = append(ifaceOptions, sb.osSbox.InterfaceOptions().MacAddress(i.mac))
+			ifaceOptions = append(ifaceOptions, osl.WithMACAddress(i.mac))
 		}
 		if len(i.llAddrs) != 0 {
-			ifaceOptions = append(ifaceOptions, sb.osSbox.InterfaceOptions().LinkLocalAddresses(i.llAddrs))
+			ifaceOptions = append(ifaceOptions, osl.WithLinkLocalAddresses(i.llAddrs))
 		}
 		interfaces[osl.Iface{SrcName: i.srcName, DstPrefix: i.dstPrefix}] = ifaceOptions
 		if joinInfo != nil {
@@ -251,15 +251,15 @@ func (sb *Sandbox) populateNetworkResources(ep *Endpoint) error {
 	if i != nil && i.srcName != "" {
 		var ifaceOptions []osl.IfaceOption
 
-		ifaceOptions = append(ifaceOptions, sb.osSbox.InterfaceOptions().Address(i.addr), sb.osSbox.InterfaceOptions().Routes(i.routes))
+		ifaceOptions = append(ifaceOptions, osl.WithIPv4Address(i.addr), osl.WithRoutes(i.routes))
 		if i.addrv6 != nil && i.addrv6.IP.To16() != nil {
-			ifaceOptions = append(ifaceOptions, sb.osSbox.InterfaceOptions().AddressIPv6(i.addrv6))
+			ifaceOptions = append(ifaceOptions, osl.WithIPv6Address(i.addrv6))
 		}
 		if len(i.llAddrs) != 0 {
-			ifaceOptions = append(ifaceOptions, sb.osSbox.InterfaceOptions().LinkLocalAddresses(i.llAddrs))
+			ifaceOptions = append(ifaceOptions, osl.WithLinkLocalAddresses(i.llAddrs))
 		}
 		if i.mac != nil {
-			ifaceOptions = append(ifaceOptions, sb.osSbox.InterfaceOptions().MacAddress(i.mac))
+			ifaceOptions = append(ifaceOptions, osl.WithMACAddress(i.mac))
 		}
 
 		if err := sb.osSbox.AddInterface(i.srcName, i.dstPrefix, ifaceOptions...); err != nil {