Browse code

Refactor NetworkController interface

- To reflect work flow. NewDriver() => ConfigureDriver()
and no NetworkDriver returned.
libnetwork clients would refer to a driver/network type, then
internally controller will retrieve the correspondent driver
instance, but this is not a concern of the clients.
- Remove NetworkDriver interface
- Removed stale blank dependency on bridge in libnetwork_test.go

Signed-off-by: Alessandro Boch <aboch@docker.com>

Alessandro Boch authored on 2015/04/23 08:47:07
Showing 6 changed files
... ...
@@ -10,15 +10,17 @@ func main() {
10 10
 	// Create a new controller instance
11 11
 	controller := libnetwork.New()
12 12
 
13
+	// Select and configure the network driver
14
+	networkType := "bridge"
13 15
 	option := options.Generic{}
14
-	driver, err := controller.NewNetworkDriver("bridge", option)
16
+	err := controller.ConfigureNetworkDriver(networkType, option)
15 17
 	if err != nil {
16 18
 		return
17 19
 	}
18 20
 
19 21
 	netOptions := options.Generic{}
20 22
 	// Create a network for containers to join.
21
-	network, err := controller.NewNetwork(driver, "network1", netOptions)
23
+	network, err := controller.NewNetwork(networkType, "network1", netOptions)
22 24
 	if err != nil {
23 25
 		return
24 26
 	}
... ...
@@ -15,8 +15,9 @@ func main() {
15 15
 
16 16
 	options := options.Generic{"AddressIPv4": net}
17 17
 	controller := libnetwork.New()
18
-	driver, _ := controller.NewNetworkDriver("bridge", options)
19
-	netw, err := controller.NewNetwork(driver, "dummy", "")
18
+	netType := "bridge"
19
+	err := controller.ConfigureNetworkDriver(netType, options)
20
+	netw, err := controller.NewNetwork(netType, "dummy", "")
20 21
 	if err != nil {
21 22
 		log.Fatal(err)
22 23
 	}
... ...
@@ -39,4 +39,7 @@ type Driver interface {
39 39
 	// DeleteEndpoint invokes the driver method to delete an endpoint
40 40
 	// passing the network id and endpoint id.
41 41
 	DeleteEndpoint(nid, eid types.UUID) error
42
+
43
+	// Type returns the the type of this driver, the network type this driver manages
44
+	Type() string
42 45
 }
... ...
@@ -475,6 +475,10 @@ func (d *driver) DeleteEndpoint(nid, eid types.UUID) error {
475 475
 	return nil
476 476
 }
477 477
 
478
+func (d *driver) Type() string {
479
+	return networkType
480
+}
481
+
478 482
 func parseEndpointOptions(epOptions interface{}) (*EndpointConfiguration, error) {
479 483
 	if epOptions == nil {
480 484
 		return nil, nil
... ...
@@ -6,22 +6,24 @@ import (
6 6
 
7 7
 	log "github.com/Sirupsen/logrus"
8 8
 	"github.com/docker/libnetwork"
9
-	_ "github.com/docker/libnetwork/drivers/bridge"
10 9
 	"github.com/docker/libnetwork/netutils"
11 10
 	"github.com/docker/libnetwork/pkg/options"
12 11
 )
13 12
 
14
-var bridgeName = "dockertest0"
13
+const (
14
+	netType    = "bridge"
15
+	bridgeName = "dockertest0"
16
+)
15 17
 
16 18
 func createTestNetwork(networkType, networkName string, option options.Generic) (libnetwork.Network, error) {
17 19
 	controller := libnetwork.New()
18 20
 
19
-	driver, err := controller.NewNetworkDriver(networkType, option)
21
+	err := controller.ConfigureNetworkDriver(networkType, option)
20 22
 	if err != nil {
21 23
 		return nil, err
22 24
 	}
23 25
 
24
-	network, err := controller.NewNetwork(driver, networkName, "")
26
+	network, err := controller.NewNetwork(networkType, networkName, "")
25 27
 	if err != nil {
26 28
 		return nil, err
27 29
 	}
... ...
@@ -62,7 +64,7 @@ func Testbridge(t *testing.T) {
62 62
 		"EnableIPForwarding":    true,
63 63
 		"AllowNonDefaultBridge": true}
64 64
 
65
-	network, err := createTestNetwork("bridge", "testnetwork", option)
65
+	network, err := createTestNetwork(netType, "testnetwork", option)
66 66
 	if err != nil {
67 67
 		t.Fatal(err)
68 68
 	}
... ...
@@ -106,12 +108,12 @@ func TestNilDriver(t *testing.T) {
106 106
 	controller := libnetwork.New()
107 107
 
108 108
 	option := options.Generic{}
109
-	_, err := controller.NewNetwork(nil, "dummy", option)
109
+	_, err := controller.NewNetwork("framerelay", "dummy", option)
110 110
 	if err == nil {
111 111
 		t.Fatal("Expected to fail. But instead succeeded")
112 112
 	}
113 113
 
114
-	if err != libnetwork.ErrNilNetworkDriver {
114
+	if err != libnetwork.ErrInvalidNetworkDriver {
115 115
 		t.Fatalf("Did not fail with expected error. Actual error: %v", err)
116 116
 	}
117 117
 }
... ...
@@ -120,7 +122,7 @@ func TestNoInitDriver(t *testing.T) {
120 120
 	controller := libnetwork.New()
121 121
 
122 122
 	option := options.Generic{}
123
-	_, err := controller.NewNetwork(&libnetwork.NetworkDriver{}, "dummy", option)
123
+	_, err := controller.NewNetwork("ppp", "dummy", option)
124 124
 	if err == nil {
125 125
 		t.Fatal("Expected to fail. But instead succeeded")
126 126
 	}
... ...
@@ -135,17 +137,17 @@ func TestDuplicateNetwork(t *testing.T) {
135 135
 	controller := libnetwork.New()
136 136
 
137 137
 	option := options.Generic{}
138
-	driver, err := controller.NewNetworkDriver("bridge", option)
138
+	err := controller.ConfigureNetworkDriver(netType, option)
139 139
 	if err != nil {
140 140
 		t.Fatal(err)
141 141
 	}
142 142
 
143
-	_, err = controller.NewNetwork(driver, "testnetwork", "")
143
+	_, err = controller.NewNetwork(netType, "testnetwork", "")
144 144
 	if err != nil {
145 145
 		t.Fatal(err)
146 146
 	}
147 147
 
148
-	_, err = controller.NewNetwork(driver, "testnetwork", "")
148
+	_, err = controller.NewNetwork(netType, "testnetwork", "")
149 149
 	if err == nil {
150 150
 		t.Fatal("Expected to fail. But instead succeeded")
151 151
 	}
... ...
@@ -158,7 +160,7 @@ func TestDuplicateNetwork(t *testing.T) {
158 158
 func TestNetworkName(t *testing.T) {
159 159
 	networkName := "testnetwork"
160 160
 
161
-	n, err := createTestNetwork("bridge", networkName, options.Generic{})
161
+	n, err := createTestNetwork(netType, networkName, options.Generic{})
162 162
 	if err != nil {
163 163
 		t.Fatal(err)
164 164
 	}
... ...
@@ -169,7 +171,7 @@ func TestNetworkName(t *testing.T) {
169 169
 }
170 170
 
171 171
 func TestNetworkType(t *testing.T) {
172
-	networkType := "bridge"
172
+	networkType := netType
173 173
 
174 174
 	n, err := createTestNetwork(networkType, "testnetwork", options.Generic{})
175 175
 	if err != nil {
... ...
@@ -182,7 +184,7 @@ func TestNetworkType(t *testing.T) {
182 182
 }
183 183
 
184 184
 func TestNetworkID(t *testing.T) {
185
-	networkType := "bridge"
185
+	networkType := netType
186 186
 
187 187
 	n, err := createTestNetwork(networkType, "testnetwork", options.Generic{})
188 188
 	if err != nil {
... ...
@@ -200,7 +202,7 @@ func TestDeleteNetworkWithActiveEndpoints(t *testing.T) {
200 200
 		"BridgeName":            bridgeName,
201 201
 		"AllowNonDefaultBridge": true}
202 202
 
203
-	network, err := createTestNetwork("bridge", "testnetwork", option)
203
+	network, err := createTestNetwork(netType, "testnetwork", option)
204 204
 	if err != nil {
205 205
 		t.Fatal(err)
206 206
 	}
... ...
@@ -235,7 +237,7 @@ func TestUnknownNetwork(t *testing.T) {
235 235
 		"BridgeName":            bridgeName,
236 236
 		"AllowNonDefaultBridge": true}
237 237
 
238
-	network, err := createTestNetwork("bridge", "testnetwork", option)
238
+	network, err := createTestNetwork(netType, "testnetwork", option)
239 239
 	if err != nil {
240 240
 		t.Fatal(err)
241 241
 	}
... ...
@@ -268,7 +270,7 @@ func TestUnknownEndpoint(t *testing.T) {
268 268
 		"AddressIPv4":           subnet,
269 269
 		"AllowNonDefaultBridge": true}
270 270
 
271
-	network, err := createTestNetwork("bridge", "testnetwork", option)
271
+	network, err := createTestNetwork(netType, "testnetwork", option)
272 272
 	if err != nil {
273 273
 		t.Fatal(err)
274 274
 	}
... ...
@@ -50,11 +50,11 @@ import (
50 50
 // NetworkController provides the interface for controller instance which manages
51 51
 // networks.
52 52
 type NetworkController interface {
53
-	// NOTE: This method will go away when moving to plugin infrastructure
54
-	NewNetworkDriver(networkType string, options interface{}) (*NetworkDriver, error)
53
+	// ConfigureNetworkDriver applies the passed options to the driver instance for the specified network type
54
+	ConfigureNetworkDriver(networkType string, options interface{}) error
55 55
 	// Create a new network. The options parameter carries network specific options.
56 56
 	// Labels support will be added in the near future.
57
-	NewNetwork(d *NetworkDriver, name string, options interface{}) (Network, error)
57
+	NewNetwork(networkType, name string, options interface{}) (Network, error)
58 58
 }
59 59
 
60 60
 // A Network represents a logical connectivity zone that containers may
... ...
@@ -99,12 +99,6 @@ type Endpoint interface {
99 99
 	Delete() error
100 100
 }
101 101
 
102
-// NetworkDriver provides a reference to driver and way to push driver specific config
103
-type NetworkDriver struct {
104
-	networkType    string
105
-	internalDriver driverapi.Driver
106
-}
107
-
108 102
 type endpoint struct {
109 103
 	name        string
110 104
 	id          types.UUID
... ...
@@ -117,7 +111,7 @@ type network struct {
117 117
 	name        string
118 118
 	networkType string
119 119
 	id          types.UUID
120
-	driver      *NetworkDriver
120
+	driver      driverapi.Driver
121 121
 	endpoints   endpointTable
122 122
 	sync.Mutex
123 123
 }
... ...
@@ -136,26 +130,24 @@ func New() NetworkController {
136 136
 	return &controller{networkTable{}, enumerateDrivers(), sync.Mutex{}}
137 137
 }
138 138
 
139
-func (c *controller) NewNetworkDriver(networkType string, options interface{}) (*NetworkDriver, error) {
139
+func (c *controller) ConfigureNetworkDriver(networkType string, options interface{}) error {
140 140
 	d, ok := c.drivers[networkType]
141 141
 	if !ok {
142
-		return nil, NetworkTypeError(networkType)
143
-	}
144
-
145
-	if err := d.Config(options); err != nil {
146
-		return nil, err
142
+		return NetworkTypeError(networkType)
147 143
 	}
148
-
149
-	return &NetworkDriver{networkType: networkType, internalDriver: d}, nil
144
+	return d.Config(options)
150 145
 }
151 146
 
152
-// NewNetwork creates a new network of the specified NetworkDriver. The options
153
-// are driver specific and modeled in a generic way.
154
-func (c *controller) NewNetwork(nd *NetworkDriver, name string, options interface{}) (Network, error) {
155
-	if nd == nil {
156
-		return nil, ErrNilNetworkDriver
147
+// NewNetwork creates a new network of the specified network type. The options
148
+// are network specific and modeled in a generic way.
149
+func (c *controller) NewNetwork(networkType, name string, options interface{}) (Network, error) {
150
+	// Check if a driver for the specified network type is available
151
+	d, ok := c.drivers[networkType]
152
+	if !ok {
153
+		return nil, ErrInvalidNetworkDriver
157 154
 	}
158 155
 
156
+	// Check if a network already exists with the specified network name
159 157
 	c.Lock()
160 158
 	for _, n := range c.networks {
161 159
 		if n.name == name {
... ...
@@ -165,22 +157,21 @@ func (c *controller) NewNetwork(nd *NetworkDriver, name string, options interfac
165 165
 	}
166 166
 	c.Unlock()
167 167
 
168
+	// Construct the network object
168 169
 	network := &network{
169
-		name:   name,
170
-		id:     types.UUID(stringid.GenerateRandomID()),
171
-		ctrlr:  c,
172
-		driver: nd}
173
-	network.endpoints = make(endpointTable)
174
-
175
-	d := network.driver.internalDriver
176
-	if d == nil {
177
-		return nil, ErrInvalidNetworkDriver
170
+		name:      name,
171
+		id:        types.UUID(stringid.GenerateRandomID()),
172
+		ctrlr:     c,
173
+		driver:    d,
174
+		endpoints: endpointTable{},
178 175
 	}
179 176
 
177
+	// Create the network
180 178
 	if err := d.CreateNetwork(network.id, options); err != nil {
181 179
 		return nil, err
182 180
 	}
183 181
 
182
+	// Store the network handler in controller
184 183
 	c.Lock()
185 184
 	c.networks[network.id] = network
186 185
 	c.Unlock()
... ...
@@ -201,7 +192,7 @@ func (n *network) Type() string {
201 201
 		return ""
202 202
 	}
203 203
 
204
-	return n.driver.networkType
204
+	return n.driver.Type()
205 205
 }
206 206
 
207 207
 func (n *network) Delete() error {
... ...
@@ -232,8 +223,7 @@ func (n *network) Delete() error {
232 232
 		}
233 233
 	}()
234 234
 
235
-	d := n.driver.internalDriver
236
-	err = d.DeleteNetwork(n.id)
235
+	err = n.driver.DeleteNetwork(n.id)
237 236
 	return err
238 237
 }
239 238
 
... ...
@@ -242,7 +232,7 @@ func (n *network) CreateEndpoint(name string, sboxKey string, options interface{
242 242
 	ep.id = types.UUID(stringid.GenerateRandomID())
243 243
 	ep.network = n
244 244
 
245
-	d := n.driver.internalDriver
245
+	d := n.driver
246 246
 	sinfo, err := d.CreateEndpoint(n.id, ep.id, sboxKey, options)
247 247
 	if err != nil {
248 248
 		return nil, err
... ...
@@ -310,7 +300,6 @@ func (ep *endpoint) Delete() error {
310 310
 		}
311 311
 	}()
312 312
 
313
-	d := n.driver.internalDriver
314
-	err = d.DeleteEndpoint(n.id, ep.id)
313
+	err = n.driver.DeleteEndpoint(n.id, ep.id)
315 314
 	return err
316 315
 }