Minor cleanup with concurrency and locking

abhi · abhi · commit 1695039bdc44 · 2018-06-21T18:15:20.000Z
Signed-off-by: Abhinandan Prativadi &lt;abhi@docker.com&gt;
diff --git a/cni.go b/cni.go
@@ -72,6 +72,8 @@ func New(config ...CNIOpt) (CNI, error) {
 
 func (c *libcni) Load(opts ...CNIOpt) error {
 	var err error
+	c.Lock()
+	defer c.Unlock()
 	// Reset the networks on a load operation to ensure
 	// config happens on a clean slate
 	c.reset()
@@ -81,30 +83,27 @@ func (c *libcni) Load(opts ...CNIOpt) error {
 			return errors.Wrapf(ErrLoad, fmt.Sprintf("cni config load failed: %v", err))
 		}
 	}
-	return c.Status()
+	return nil
 }
 
 func (c *libcni) Status() error {
 	c.RLock()
 	defer c.RUnlock()
-	if len(c.networks) < c.networkCount {
-		return ErrCNINotInitialized
-	}
-	return nil
+	return c.status()
 }
 
 // Setup setups the network in the namespace
 func (c *libcni) Setup(id string, path string, opts ...NamespaceOpts) (*CNIResult, error) {
-	if err := c.Status(); err != nil {
+	c.RLock()
+	defer c.RUnlock()
+	if err := c.status(); err != nil {
 		return nil, err
 	}
 	ns, err := newNamespace(id, path, opts...)
 	if err != nil {
 		return nil, err
 	}
 	var results []*current.Result
-	c.RLock()
-	defer c.RUnlock()
 	for _, network := range c.networks {
 		r, err := network.Attach(ns)
 		if err != nil {
@@ -117,15 +116,15 @@ func (c *libcni) Setup(id string, path string, opts ...NamespaceOpts) (*CNIResul
 
 // Remove removes the network config from the namespace
 func (c *libcni) Remove(id string, path string, opts ...NamespaceOpts) error {
-	if err := c.Status(); err != nil {
+	c.RLock()
+	defer c.RUnlock()
+	if err := c.status(); err != nil {
 		return err
 	}
 	ns, err := newNamespace(id, path, opts...)
 	if err != nil {
 		return err
 	}
-	c.RLock()
-	defer c.RUnlock()
 	for _, network := range c.networks {
 		if err := network.Remove(ns); err != nil {
 			return err
@@ -135,7 +134,12 @@ func (c *libcni) Remove(id string, path string, opts ...NamespaceOpts) error {
 }
 
 func (c *libcni) reset() {
-	c.Lock()
-	defer c.Unlock()
 	c.networks = nil
 }
+
+func (c *libcni) status() error {
+	if len(c.networks) < c.networkCount {
+		return ErrCNINotInitialized
+	}
+	return nil
+}
diff --git a/opts.go b/opts.go
@@ -75,8 +75,6 @@ func WithLoNetwork(c *libcni) error {
 }]
 }`))
 
-	c.Lock()
-	defer c.Unlock()
 	c.networks = append(c.networks, &Network{
 		cni:    c.cniConfig,
 		config: loConfig,
@@ -97,8 +95,6 @@ func WithConf(bytes []byte) CNIOpt {
 		if err != nil {
 			return err
 		}
-		c.Lock()
-		defer c.Unlock()
 		c.networks = append(c.networks, &Network{
 			cni:    c.cniConfig,
 			config: confList,
@@ -122,8 +118,6 @@ func WithConfFile(fileName string) CNIOpt {
 		if err != nil {
 			return err
 		}
-		c.Lock()
-		defer c.Unlock()
 		c.networks = append(c.networks, &Network{
 			cni:    c.cniConfig,
 			config: confList,
@@ -142,8 +136,6 @@ func WithConfListFile(fileName string) CNIOpt {
 		if err != nil {
 			return err
 		}
-		c.Lock()
-		defer c.Unlock()
 		c.networks = append(c.networks, &Network{
 			cni:    c.cniConfig,
 			config: confList,
@@ -174,8 +166,6 @@ func WithDefaultConf(c *libcni) error {
 	// interface provided during init as the network interface for this default
 	// network. For every other network use a generated interface id.
 	i := 0
-	c.Lock()
-	defer c.Unlock()
 	for _, confFile := range files {
 		var confList *cnilibrary.NetworkConfigList
 		if strings.HasSuffix(confFile, ".conflist") {
