removed lock for version command (#929)

* removed lock for version command

* updated variable name

* catch and return error on lock

* added log for release lock file

* separated out lock and store initialization

* addressed comments

Author: tamilmani1989

cni/network/network.go

@@ -41,6 +41,7 @@ const (
 	ipVersion             = "4"
 	ipamV6                = "azure-vnet-ipamv6"
 	defaultRequestTimeout = 15 * time.Second
+	azureCniName          = "azure-vnet"
 )
 
 // CNI Operation Types
@@ -145,12 +146,6 @@ func (plugin *netPlugin) Start(config *common.PluginConfig) error {
 	common.LogNetworkInterfaces()
 
 	// Initialize network manager.
-	err = plugin.nm.Initialize(config, rehydrateNetworkInfoOnReboot)
-	if err != nil {
-		log.Printf("[cni-net] Failed to initialize network manager, err:%v.", err)
-		return err
-	}
-
 	log.Printf("[cni-net] Plugin started.")
 
 	return nil
@@ -282,7 +277,6 @@ func (plugin *netPlugin) setCNIReportDetails(nwCfg *cni.NetworkConfig, opType st
 	plugin.report.SubContext = fmt.Sprintf("%+v", nwCfg)
 	plugin.report.EventMessage = msg
 	plugin.report.BridgeDetails.NetworkMode = nwCfg.Mode
-	plugin.report.InterfaceDetails.SecondaryCAUsedCount = plugin.nm.GetNumberOfEndpoints("", nwCfg.Name)
 }
 
 func addNatIPV6SubnetInfo(nwCfg *cni.NetworkConfig,
@@ -301,6 +295,33 @@ func addNatIPV6SubnetInfo(nwCfg *cni.NetworkConfig,
 	}
 }
 
+func acquireLockForStore(config *common.PluginConfig, plugin *netPlugin) error {
+	var err error
+	if err = plugin.Store.Lock(true); err != nil {
+		log.Printf("[CNI] Failed to lock store: %v. check if process running", err)
+		if isSafe, _ := plugin.IsSafeToRemoveLock(azureCniName); isSafe {
+			log.Printf("[CNI] Removing lock file as process holding lock exited")
+			if err = releaseLockForStore(plugin); err != nil {
+				log.Errorf("Failed to release lock file, err:%v.\n", err)
+			}
+		}
+	}
+
+	return err
+}
+
+func releaseLockForStore(plugin *netPlugin) error {
+	if plugin.Store != nil {
+		err := plugin.Store.Unlock(false)
+		if err != nil {
+			log.Printf("[cni] Failed to unlock store: %v.", err)
+			return err
+		}
+	}
+
+	log.Printf("Released lock file")
+	return nil
+}
 //
 // CNI implementation
 // https://github.com/containernetworking/cni/blob/master/SPEC.md
@@ -349,6 +370,22 @@ func (plugin *netPlugin) Add(args *cniSkel.CmdArgs) error {
 	iptables.DisableIPTableLock = nwCfg.DisableIPTableLock
 	plugin.setCNIReportDetails(nwCfg, CNI_ADD, "")
 
+	// acquire cni lock file
+	// TODO: check if we need pluginconfig and if not remove it
+	config := &common.PluginConfig{Store: plugin.Store}
+	if err := acquireLockForStore(config, plugin); err != nil {
+		log.Errorf("Couldn't acquire lock: %+v", err)
+		return err
+	}
+
+	defer releaseLockForStore(plugin)
+
+	// restore network state
+	if err = plugin.nm.Initialize(config, rehydrateNetworkInfoOnReboot); err != nil {
+		log.Printf("[cni-net] Failed to initialize network manager, err:%+v.", err)
+		return err
+	}
+
 	defer func() {
 		operationTimeMs := time.Since(startTime).Milliseconds()
 		cniMetric.Metric = aitelemetry.Metric{
@@ -774,6 +811,20 @@ func (plugin *netPlugin) Get(args *cniSkel.CmdArgs) error {
 
 	iptables.DisableIPTableLock = nwCfg.DisableIPTableLock
 
+	config := &common.PluginConfig{Store: plugin.Store}
+	if err := acquireLockForStore(config, plugin); err != nil {
+		log.Errorf("Couldn't acquire lock: %+v", err)
+		return err
+	}
+
+	defer releaseLockForStore(plugin)
+
+	// restore network state
+	if err = plugin.nm.Initialize(config, rehydrateNetworkInfoOnReboot); err != nil {
+		log.Printf("[cni-net] Failed to initialize network manager, err:%+v.", err)
+		return err
+	}
+
 	// Parse Pod arguments.
 	if k8sPodName, k8sNamespace, err = plugin.getPodInfo(args.Args); err != nil {
 		return err
@@ -859,6 +910,21 @@ func (plugin *netPlugin) Delete(args *cniSkel.CmdArgs) error {
 
 	log.Printf("[cni-net] Read network configuration %+v.", nwCfg)
 
+	// acquire cni lock file
+	config := &common.PluginConfig{Store: plugin.Store}
+	if err := acquireLockForStore(config, plugin); err != nil {
+		log.Errorf("Couldn't acquire lock: %+v", err)
+		return err
+	}
+
+	defer releaseLockForStore(plugin)
+
+	// restore network state
+	if err = plugin.nm.Initialize(config, rehydrateNetworkInfoOnReboot); err != nil {
+		log.Printf("[cni-net] Failed to initialize network manager, err:%+v.", err)
+		return err
+	}
+
 	// Parse Pod arguments.
 	if k8sPodName, k8sNamespace, err = plugin.getPodInfo(args.Args); err != nil {
 		log.Printf("[cni-net] Failed to get POD info due to error: %v", err)
@@ -1021,6 +1087,21 @@ func (plugin *netPlugin) Update(args *cniSkel.CmdArgs) error {
 	iptables.DisableIPTableLock = nwCfg.DisableIPTableLock
 	plugin.setCNIReportDetails(nwCfg, CNI_UPDATE, "")
 
+	// acquire cni lock file
+	config := &common.PluginConfig{Store: plugin.Store}
+	if err := acquireLockForStore(config, plugin); err != nil {
+		log.Errorf("Couldn't acquire lock: %+v", err)
+		return err
+	}
+
+	defer releaseLockForStore(plugin)
+
+	// restore network state
+	if err = plugin.nm.Initialize(config, rehydrateNetworkInfoOnReboot); err != nil {
+		log.Printf("[cni-net] Failed to initialize network manager, err:%+v.", err)
+		return err
+	}
+
 	defer func() {
 		operationTimeMs := time.Since(startTime).Milliseconds()
 		cniMetric.Metric = aitelemetry.Metric{

cni/network/plugin/main.go

@@ -180,8 +180,7 @@ func main() {
 		log.Printf("Failed to create network plugin, err:%v.\n", err)
 		return
 	}
-
-	// CNI Acquires lock
+	// CNI initializes store
 	if err = netPlugin.Plugin.InitializeKeyValueStore(&config); err != nil {
 		log.Errorf("Failed to initialize key-value store of network plugin, err:%v.\n", err)
 		tb := telemetry.NewTelemetryBuffer()
@@ -190,13 +189,6 @@ func main() {
 			tb.Close()
 		}
 
-		if isSafe, _ := netPlugin.Plugin.IsSafeToRemoveLock(name); isSafe {
-			log.Printf("[CNI] Removing lock file as process holding lock exited")
-			if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(true); errUninit != nil {
-				log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit)
-			}
-		}
-
 		return
 	}
 

cni/plugin.go

@@ -163,28 +163,13 @@ func (plugin *Plugin) InitializeKeyValueStore(config *common.PluginConfig) error
 		removeLockFileAfterReboot(plugin)
 	}
 
-	// Acquire store lock.
-	if err := plugin.Store.Lock(true); err != nil {
-		log.Printf("[cni] Failed to lock store: %v.", err)
-		return err
-	}
-
 	config.Store = plugin.Store
-
 	return nil
 }
 
 // Uninitialize key-value store
 func (plugin *Plugin) UninitializeKeyValueStore(force bool) error {
-	if plugin.Store != nil {
-		err := plugin.Store.Unlock(force)
-		if err != nil {
-			log.Printf("[cni] Failed to unlock store: %v.", err)
-			return err
-		}
-	}
 	plugin.Store = nil
-
 	return nil
 }
 

store/json.go

@@ -221,8 +221,15 @@ func (kvs *jsonFileStore) Lock(block bool) error {
 
 	defer lockFile.Close()
 
+	currentPid := os.Getpid()
+	log.Printf("Write pid %d to lockfile", currentPid)
 	// Write the process ID for easy identification.
-	if _, err = lockFile.WriteString(strconv.Itoa(os.Getpid())); err != nil {
+	if _, err = lockFile.WriteString(strconv.Itoa(currentPid)); err != nil {
+		// remove lockfile
+		log.Errorf("Write to lockfile failed:%+v", err)
+		if errRem := os.Remove(kvs.lockFileName); errRem != nil {
+			log.Errorf("removing lockfile failed:%+v", errRem)
+		}
 		return err
 	}