1. docker
  2. Commit 9d3b1f941966a0b6bf5e8ac077e8792d084ddbc6
Browse code

libnetwork/osl: make constructing Interfaces more atomic

It's still not "great", but implement a `newInterface()` constructor
to create a new Interface instance, instead of creating a partial
instance and applying "options" after the fact.

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

Sebastiaan van Stijn authored on 2023/08/21 19:24:46
Showing 3 changed files
libnetwork/osl/interface_linux.go
History View file @ 9d3b1f9
... ... 
@@ -14,6 +14,31 @@ import (
14 14 
 	"github.com/vishvananda/netns"
15 15 
 )
16 16
17 
+// newInterface creates a new interface in the given namespace using the
18 
+// provided options.
19 
+func newInterface(ns *Namespace, srcName, dstPrefix string, options ...IfaceOption) (*Interface, error) {
20 
+	i := &Interface{
21 
+		srcName: srcName,
22 
+		dstName: dstPrefix,
23 
+		ns:      ns,
24 
+	}
25 
+	for _, opt := range options {
26 
+		if opt != nil {
27 
+			// TODO(thaJeztah): use multi-error instead of returning early.
28 
+			if err := opt(i); err != nil {
29 
+				return nil, err
30 
+			}
31 
+		}
32 
+	}
33 
+	if i.master != "" {
34 
+		i.dstMaster = ns.findDst(i.master, true)
35 
+		if i.dstMaster == "" {
36 
+			return nil, fmt.Errorf("could not find an appropriate master %q for %q", i.master, i.srcName)
37 
+		}
38 
+	}
39 
+	return i, nil
40 
+}
41 
+
17 42 
 // Interface represents the settings and identity of a network device.
18 43 
 // It is used as a return type for Network.Link, and it is common practice
19 44 
 // for the caller to use this information when moving interface SrcName from
... ... 
@@ -135,23 +160,11 @@ func (n *Namespace) findDst(srcName string, isBridge bool) string {
135 135 
 // to only provide a prefix for DstName. The AddInterface api will auto-generate
136 136 
 // an appropriate suffix for the DstName to disambiguate.
137 137 
 func (n *Namespace) AddInterface(srcName, dstPrefix string, options ...IfaceOption) error {
138 
-	i := &Interface{
139 
-		srcName: srcName,
140 
-		dstName: dstPrefix,
141 
-		ns:      n,
142 
-	}
143 
-	if err := i.processInterfaceOptions(options...); err != nil {
138 
+	i, err := newInterface(n, srcName, dstPrefix, options...)
139 
+	if err != nil {
144 140 
 		return err
145 141 
 	}
146 142
147 
-	if i.master != "" {
148 
-		i.dstMaster = n.findDst(i.master, true)
149 
-		if i.dstMaster == "" {
150 
-			return fmt.Errorf("could not find an appropriate master %q for %q",
151 
-				i.master, i.srcName)
152 
-		}
153 
-	}
154 
-
155 143 
 	n.mu.Lock()
156 144 
 	if n.isDefault {
157 145 
 		i.dstName = i.srcName
libnetwork/osl/namespace_linux.go
History View file @ 9d3b1f9
... ... 
@@ -482,20 +482,10 @@ func (n *Namespace) Destroy() error {
482 482 
 func (n *Namespace) Restore(interfaces map[Iface][]IfaceOption, routes []*types.StaticRoute, gw net.IP, gw6 net.IP) error {
483 483 
 	// restore interfaces
484 484 
 	for iface, opts := range interfaces {
485 
-		i := &Interface{
486 
-			srcName: iface.SrcName,
487 
-			dstName: iface.DstPrefix,
488 
-			ns:      n,
489 
-		}
490 
-		if err := i.processInterfaceOptions(opts...); err != nil {
485 
+		i, err := newInterface(n, iface.SrcName, iface.DstPrefix, opts...)
486 
+		if err != nil {
491 487 
 			return err
492 488 
 		}
493 
-		if i.master != "" {
494 
-			i.dstMaster = n.findDst(i.master, true)
495 
-			if i.dstMaster == "" {
496 
-				return fmt.Errorf("could not find an appropriate master %q for %q", i.master, i.srcName)
497 
-			}
498 
-		}
499 489 
 		if n.isDefault {
500 490 
 			i.dstName = i.srcName
501 491 
 		} else {
libnetwork/osl/options_linux.go
History View file @ 9d3b1f9
... ... 
@@ -24,18 +24,6 @@ func WithFamily(family int) NeighOption {
24 24 
 	}
25 25 
 }
26 26
27 
-func (i *Interface) processInterfaceOptions(options ...IfaceOption) error {
28 
-	for _, opt := range options {
29 
-		if opt != nil {
30 
-			// TODO(thaJeztah): use multi-error instead of returning early.
31 
-			if err := opt(i); err != nil {
32 
-				return err
33 
-			}
34 
-		}
35 
-	}
36 
-	return nil
37 
-}
38 
-
39 27 
 // WithIsBridge sets whether the interface is a bridge.
40 28 
 func WithIsBridge(isBridge bool) IfaceOption {
41 29 
 	return func(i *Interface) error {