Use Lockedfile api to acquire lock

aitelemetry/telemetrywrapper.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/Azure/azure-container-networking/common"
 	"github.com/Azure/azure-container-networking/log"
+	"github.com/Azure/azure-container-networking/processlock"
 	"github.com/Azure/azure-container-networking/store"
 	"github.com/microsoft/ApplicationInsights-Go/appinsights"
 )
@@ -101,17 +102,31 @@ func getMetadata(th *telemetryHandle) {
 	th.metadata = metadata
 	th.rwmutex.Unlock()
 
+	lockclient, err := processlock.NewFileLock(metadataFile + store.LockExtension)
+	if err != nil {
+		log.Printf("Error initializing file lock:%v", err)
+		return
+	}
+
 	// Save metadata retrieved from wireserver to a file
-	kvs, err := store.NewJsonFileStore(metadataFile)
+	kvs, err := store.NewJsonFileStore(metadataFile, lockclient)
 	if err != nil {
 		debugLog("[AppInsights] Error initializing kvs store: %v", err)
 		return
 	}
 
-	kvs.Lock(true)
-	err = common.SaveHostMetadata(th.metadata, metadataFile)
-	kvs.Unlock(true)
+	err = kvs.Lock(store.DefaultLockTimeout)
 	if err != nil {
+		log.Errorf("getMetadata: Not able to acquire lock:%v", err)
+		return
+	}
+	metadataErr := common.SaveHostMetadata(th.metadata, metadataFile)
+	err = kvs.Unlock()
+	if err != nil {
+		log.Errorf("getMetadata: Not able to release lock:%v", err)
+	}
+
+	if metadataErr != nil {
 		debugLog("[AppInsights] saving host metadata failed with :%v", err)
 	}
 }
@@ -284,6 +299,7 @@ func (th *telemetryHandle) TrackMetric(metric Metric) {
 		aimetric.Properties[versionStr] = th.appVersion
 		aimetric.Properties[resourceGroupStr] = th.metadata.ResourceGroupName
 		aimetric.Properties[vmIDStr] = metadata.VMID
+		aimetric.Properties[osStr] = runtime.GOOS
 		aimetric.Tags.Session().SetId(metadata.VMID)
 	}
 

cni/ipam/plugin/main.go

@@ -43,20 +43,12 @@ func main() {
 
 	if err := ipamPlugin.Plugin.InitializeKeyValueStore(&config); err != nil {
 		fmt.Printf("Failed to initialize key-value store of ipam plugin, err:%v.\n", err)
-
-		if isSafe, _ := ipamPlugin.Plugin.IsSafeToRemoveLock(ipamPlugin.Plugin.Name); isSafe {
-			log.Printf("[IPAM] Removing lock file as process holding lock exited")
-			if errUninit := ipamPlugin.Plugin.UninitializeKeyValueStore(true); errUninit != nil {
-				log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit)
-			}
-		}
-
 		os.Exit(1)
 	}
 
 	defer func() {
-		if errUninit := ipamPlugin.Plugin.UninitializeKeyValueStore(false); errUninit != nil {
-			fmt.Printf("Failed to uninitialize key-value store of ipam plugin, err:%v.\n", err)
+		if errUninit := ipamPlugin.Plugin.UninitializeKeyValueStore(); errUninit != nil {
+			fmt.Printf("Failed to uninitialize key-value store of ipam plugin, err:%v.\n", errUninit)
 		}
 
 		if recover() != nil {

cni/ipam/pluginv6/main.go

@@ -43,20 +43,12 @@ func main() {
 
 	if err := ipamPlugin.Plugin.InitializeKeyValueStore(&config); err != nil {
 		fmt.Printf("Failed to initialize key-value store of ipam plugin, err:%v.\n", err)
-
-		if isSafe, _ := ipamPlugin.Plugin.IsSafeToRemoveLock(ipamPlugin.Plugin.Name); isSafe {
-			log.Printf("[IPAM] Removing lock file as process holding lock exited")
-			if errUninit := ipamPlugin.Plugin.UninitializeKeyValueStore(true); errUninit != nil {
-				log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit)
-			}
-		}
-
 		os.Exit(1)
 	}
 
 	defer func() {
-		if errUninit := ipamPlugin.Plugin.UninitializeKeyValueStore(false); errUninit != nil {
-			fmt.Printf("Failed to uninitialize key-value store of ipam plugin, err:%v.\n", err)
+		if errUninit := ipamPlugin.Plugin.UninitializeKeyValueStore(); errUninit != nil {
+			fmt.Printf("Failed to uninitialize key-value store of ipam plugin, err:%v.\n", errUninit)
 		}
 
 		if recover() != nil {

cni/network/plugin/main.go

@@ -11,15 +11,18 @@ import (
 	"reflect"
 	"time"
 
+	"github.com/Azure/azure-container-networking/aitelemetry"
 	"github.com/Azure/azure-container-networking/cni"
 	"github.com/Azure/azure-container-networking/cni/network"
 	"github.com/Azure/azure-container-networking/common"
 	"github.com/Azure/azure-container-networking/log"
 	acnnetwork "github.com/Azure/azure-container-networking/network"
 	"github.com/Azure/azure-container-networking/nns"
 	"github.com/Azure/azure-container-networking/platform"
+	"github.com/Azure/azure-container-networking/store"
 	"github.com/Azure/azure-container-networking/telemetry"
 	"github.com/containernetworking/cni/pkg/skel"
+	"github.com/pkg/errors"
 )
 
 const (
@@ -139,14 +142,13 @@ func main() {
 
 	var (
 		config       common.PluginConfig
-		err          error
 		logDirectory string // This sets empty string i.e. current location
 		tb           *telemetry.TelemetryBuffer
 	)
 
 	log.SetName(name)
 	log.SetLevel(log.LevelInfo)
-	if err = log.SetTargetLogDirectory(log.TargetLogfile, logDirectory); err != nil {
+	if err := log.SetTargetLogDirectory(log.TargetLogfile, logDirectory); err != nil {
 		fmt.Printf("Failed to setup cni logging: %v\n", err)
 		return
 	}
@@ -187,32 +189,42 @@ func main() {
 
 		cniReport.GetReport(pluginName, version, ipamQueryURL)
 
-		upTime, err := platform.GetLastRebootTime()
+		var upTime time.Time
+		upTime, err = platform.GetLastRebootTime()
 		if err == nil {
 			cniReport.VMUptime = upTime.Format("2006-01-02 15:04:05")
 		}
 
 		// CNI Acquires lock
 		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()
-			if tberr := tb.Connect(); tberr == nil {
-				reportPluginError(reportManager, tb, err)
-				tb.Close()
+			tb = telemetry.NewTelemetryBuffer()
+			if tberr := tb.Connect(); tberr != nil {
+				log.Errorf("Cannot connect to telemetry service:%v", tberr)
+				return
 			}
 
-			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)
+			reportPluginError(reportManager, tb, err)
+
+			if errors.Is(err, store.ErrTimeoutLockingStore) {
+				var cniMetric telemetry.AIMetric
+				cniMetric.Metric = aitelemetry.Metric{
+					Name:             telemetry.CNILockTimeoutStr,
+					Value:            1.0,
+					CustomDimensions: make(map[string]string),
+				}
+				err = telemetry.SendCNIMetric(&cniMetric, tb)
+				if err != nil {
+					log.Errorf("Couldn't send cnilocktimeout metric: %v", err)
 				}
 			}
 
+			tb.Close()
 			return
 		}
 
 		defer func() {
-			if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(false); errUninit != nil {
+			if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(); errUninit != nil {
 				log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit)
 			}
 
@@ -270,7 +282,7 @@ func main() {
 	netPlugin.Stop()
 
 	// release cni lock
-	if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(false); errUninit != nil {
+	if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(); errUninit != nil {
 		log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit)
 	}
 

cni/plugin.go

@@ -6,15 +6,14 @@ package cni
 import (
 	"context"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"runtime"
 
 	"github.com/Azure/azure-container-networking/common"
 	"github.com/Azure/azure-container-networking/log"
 	"github.com/Azure/azure-container-networking/platform"
+	"github.com/Azure/azure-container-networking/processlock"
 	"github.com/Azure/azure-container-networking/store"
-
 	cniInvoke "github.com/containernetworking/cni/pkg/invoke"
 	cniSkel "github.com/containernetworking/cni/pkg/skel"
 	cniTypes "github.com/containernetworking/cni/pkg/types"
@@ -155,16 +154,21 @@ func (plugin *Plugin) Errorf(format string, args ...interface{}) *cniTypes.Error
 func (plugin *Plugin) InitializeKeyValueStore(config *common.PluginConfig) error {
 	// Create the key value store.
 	if plugin.Store == nil {
-		var err error
-		plugin.Store, err = store.NewJsonFileStore(platform.CNIRuntimePath + plugin.Name + ".json")
+		lockclient, err := processlock.NewFileLock(platform.CNILockPath + plugin.Name + store.LockExtension)
+		if err != nil {
+			log.Printf("[cni] Error initializing file lock:%v", err)
+			return errors.Wrap(err, "error creating new filelock")
+		}
+
+		plugin.Store, err = store.NewJsonFileStore(platform.CNIRuntimePath+plugin.Name+".json", lockclient)
 		if err != nil {
 			log.Printf("[cni] Failed to create store: %v.", err)
 			return err
 		}
 	}
 
 	// Acquire store lock.
-	if err := plugin.Store.Lock(true); err != nil {
+	if err := plugin.Store.Lock(store.DefaultLockTimeout); err != nil {
 		log.Printf("[cni] Failed to lock store: %v.", err)
 		return err
 	}
@@ -175,9 +179,9 @@ func (plugin *Plugin) InitializeKeyValueStore(config *common.PluginConfig) error
 }
 
 // Uninitialize key-value store
-func (plugin *Plugin) UninitializeKeyValueStore(force bool) error {
+func (plugin *Plugin) UninitializeKeyValueStore() error {
 	if plugin.Store != nil {
-		err := plugin.Store.Unlock(force)
+		err := plugin.Store.Unlock()
 		if err != nil {
 			log.Printf("[cni] Failed to unlock store: %v.", err)
 			return err
@@ -187,67 +191,3 @@ func (plugin *Plugin) UninitializeKeyValueStore(force bool) error {
 
 	return nil
 }
-
-// check if safe to remove lockfile
-func (plugin *Plugin) IsSafeToRemoveLock(processName string) (bool, error) {
-	if plugin != nil && plugin.Store != nil {
-		// check if get process command supported
-		if cmdErr := platform.GetProcessSupport(); cmdErr != nil {
-			log.Errorf("Get process cmd not supported. Error %v", cmdErr)
-			return false, cmdErr
-		}
-
-		lockFileName := plugin.Store.GetLockFileName()
-		// Read pid from lockfile
-		lockFilePid, err := plugin.readLockFile(lockFileName)
-		if err != nil {
-			return false, errors.Wrap(err, "IsSafeToRemoveLock lockfile read failed")
-		}
-
-		log.Printf("Read from lockfile:%s", lockFilePid)
-		// Get the process name if running and
-		// check if that matches with our expected process
-		// if it returns non-nil error then process is not running
-		pName, err := platform.GetProcessNameByID(lockFilePid)
-		if err != nil {
-			var content string
-			content, err = plugin.readLockFile(lockFileName)
-			if err != nil {
-				return false, errors.Wrap(err, "IsSafeToRemoveLock lockfile 2nd read failed")
-			}
-
-			// pid in lockfile changed after getprocessnamebyid call. so some other process acquired lockfile in between.
-			// so its not safe to remove lockfile
-			if string(content) != lockFilePid {
-				log.Printf("Lockfile content changed from %s to %s. So not safe to remove lockfile", lockFilePid, content)
-				return false, nil
-			}
-
-			return true, nil
-		}
-
-		log.Printf("[CNI] Process name is %s", pName)
-
-		if pName != processName {
-			return true, nil
-		}
-	}
-
-	log.Errorf("Plugin store is nil")
-	return false, fmt.Errorf("plugin store nil")
-}
-
-func (plugin *Plugin) readLockFile(filename string) (string, error) {
-	content, err := ioutil.ReadFile(filename)
-	if err != nil {
-		log.Errorf("Failed to read lockfile :%v", err)
-		return "", fmt.Errorf("readLockFile error:%w", err)
-	}
-
-	if len(content) == 0 {
-		log.Errorf("Num bytes read from lockfile is 0")
-		return "", errEmptyContent
-	}
-
-	return string(content), nil
-}