Skip to content

Commit cdab7d0

Browse files
tamilmani1989tamilmani1989
authored
Use Lockedfile api to acquire lock (#1070)
* added lockedfileapi support for CNI * fixed interface changes * addressed comments fixed ut * addressed comments * fixed copy to buffer part in writer api * fixed copy to buffer part in writer api * keeping old code not changing it.
1 parent 077e11d commit cdab7d0

36 files changed

+2000
-410
lines changed

‎aitelemetry/telemetrywrapper.go‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/Azure/azure-container-networking/common"
99
"github.com/Azure/azure-container-networking/log"
10+
"github.com/Azure/azure-container-networking/processlock"
1011
"github.com/Azure/azure-container-networking/store"
1112
"github.com/microsoft/ApplicationInsights-Go/appinsights"
1213
)
@@ -101,17 +102,31 @@ func getMetadata(th *telemetryHandle) {
101102
th.metadata = metadata
102103
th.rwmutex.Unlock()
103104

105+
lockclient, err := processlock.NewFileLock(metadataFile + store.LockExtension)
106+
if err != nil {
107+
log.Printf("Error initializing file lock:%v", err)
108+
return
109+
}
110+
104111
// Save metadata retrieved from wireserver to a file
105-
kvs, err := store.NewJsonFileStore(metadataFile)
112+
kvs, err := store.NewJsonFileStore(metadataFile, lockclient)
106113
if err != nil {
107114
debugLog("[AppInsights] Error initializing kvs store: %v", err)
108115
return
109116
}
110117

111-
kvs.Lock(true)
112-
err = common.SaveHostMetadata(th.metadata, metadataFile)
113-
kvs.Unlock(true)
118+
err = kvs.Lock(store.DefaultLockTimeout)
114119
if err != nil {
120+
log.Errorf("getMetadata: Not able to acquire lock:%v", err)
121+
return
122+
}
123+
metadataErr := common.SaveHostMetadata(th.metadata, metadataFile)
124+
err = kvs.Unlock()
125+
if err != nil {
126+
log.Errorf("getMetadata: Not able to release lock:%v", err)
127+
}
128+
129+
if metadataErr != nil {
115130
debugLog("[AppInsights] saving host metadata failed with :%v", err)
116131
}
117132
}
@@ -284,6 +299,7 @@ func (th *telemetryHandle) TrackMetric(metric Metric) {
284299
aimetric.Properties[versionStr] = th.appVersion
285300
aimetric.Properties[resourceGroupStr] = th.metadata.ResourceGroupName
286301
aimetric.Properties[vmIDStr] = metadata.VMID
302+
aimetric.Properties[osStr] = runtime.GOOS
287303
aimetric.Tags.Session().SetId(metadata.VMID)
288304
}
289305

‎cni/ipam/plugin/main.go‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,20 +43,12 @@ func main() {
4343

4444
if err := ipamPlugin.Plugin.InitializeKeyValueStore(&config); err != nil {
4545
fmt.Printf("Failed to initialize key-value store of ipam plugin, err:%v.\n", err)
46-
47-
if isSafe, _ := ipamPlugin.Plugin.IsSafeToRemoveLock(ipamPlugin.Plugin.Name); isSafe {
48-
log.Printf("[IPAM] Removing lock file as process holding lock exited")
49-
if errUninit := ipamPlugin.Plugin.UninitializeKeyValueStore(true); errUninit != nil {
50-
log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit)
51-
}
52-
}
53-
5446
os.Exit(1)
5547
}
5648

5749
defer func() {
58-
if errUninit := ipamPlugin.Plugin.UninitializeKeyValueStore(false); errUninit != nil {
59-
fmt.Printf("Failed to uninitialize key-value store of ipam plugin, err:%v.\n", err)
50+
if errUninit := ipamPlugin.Plugin.UninitializeKeyValueStore(); errUninit != nil {
51+
fmt.Printf("Failed to uninitialize key-value store of ipam plugin, err:%v.\n", errUninit)
6052
}
6153

6254
if recover() != nil {

‎cni/ipam/pluginv6/main.go‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,20 +43,12 @@ func main() {
4343

4444
if err := ipamPlugin.Plugin.InitializeKeyValueStore(&config); err != nil {
4545
fmt.Printf("Failed to initialize key-value store of ipam plugin, err:%v.\n", err)
46-
47-
if isSafe, _ := ipamPlugin.Plugin.IsSafeToRemoveLock(ipamPlugin.Plugin.Name); isSafe {
48-
log.Printf("[IPAM] Removing lock file as process holding lock exited")
49-
if errUninit := ipamPlugin.Plugin.UninitializeKeyValueStore(true); errUninit != nil {
50-
log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit)
51-
}
52-
}
53-
5446
os.Exit(1)
5547
}
5648

5749
defer func() {
58-
if errUninit := ipamPlugin.Plugin.UninitializeKeyValueStore(false); errUninit != nil {
59-
fmt.Printf("Failed to uninitialize key-value store of ipam plugin, err:%v.\n", err)
50+
if errUninit := ipamPlugin.Plugin.UninitializeKeyValueStore(); errUninit != nil {
51+
fmt.Printf("Failed to uninitialize key-value store of ipam plugin, err:%v.\n", errUninit)
6052
}
6153

6254
if recover() != nil {

‎cni/network/plugin/main.go‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,18 @@ import (
1111
"reflect"
1212
"time"
1313

14+
"github.com/Azure/azure-container-networking/aitelemetry"
1415
"github.com/Azure/azure-container-networking/cni"
1516
"github.com/Azure/azure-container-networking/cni/network"
1617
"github.com/Azure/azure-container-networking/common"
1718
"github.com/Azure/azure-container-networking/log"
1819
acnnetwork "github.com/Azure/azure-container-networking/network"
1920
"github.com/Azure/azure-container-networking/nns"
2021
"github.com/Azure/azure-container-networking/platform"
22+
"github.com/Azure/azure-container-networking/store"
2123
"github.com/Azure/azure-container-networking/telemetry"
2224
"github.com/containernetworking/cni/pkg/skel"
25+
"github.com/pkg/errors"
2326
)
2427

2528
const (
@@ -139,14 +142,13 @@ func main() {
139142

140143
var (
141144
config common.PluginConfig
142-
err error
143145
logDirectory string // This sets empty string i.e. current location
144146
tb *telemetry.TelemetryBuffer
145147
)
146148

147149
log.SetName(name)
148150
log.SetLevel(log.LevelInfo)
149-
if err = log.SetTargetLogDirectory(log.TargetLogfile, logDirectory); err != nil {
151+
if err := log.SetTargetLogDirectory(log.TargetLogfile, logDirectory); err != nil {
150152
fmt.Printf("Failed to setup cni logging: %v\n", err)
151153
return
152154
}
@@ -187,32 +189,42 @@ func main() {
187189

188190
cniReport.GetReport(pluginName, version, ipamQueryURL)
189191

190-
upTime, err := platform.GetLastRebootTime()
192+
var upTime time.Time
193+
upTime, err = platform.GetLastRebootTime()
191194
if err == nil {
192195
cniReport.VMUptime = upTime.Format("2006-01-02 15:04:05")
193196
}
194197

195198
// CNI Acquires lock
196199
if err = netPlugin.Plugin.InitializeKeyValueStore(&config); err != nil {
197200
log.Errorf("Failed to initialize key-value store of network plugin, err:%v.\n", err)
198-
tb := telemetry.NewTelemetryBuffer()
199-
if tberr := tb.Connect(); tberr == nil {
200-
reportPluginError(reportManager, tb, err)
201-
tb.Close()
201+
tb = telemetry.NewTelemetryBuffer()
202+
if tberr := tb.Connect(); tberr != nil {
203+
log.Errorf("Cannot connect to telemetry service:%v", tberr)
204+
return
202205
}
203206

204-
if isSafe, _ := netPlugin.Plugin.IsSafeToRemoveLock(name); isSafe {
205-
log.Printf("[CNI] Removing lock file as process holding lock exited")
206-
if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(true); errUninit != nil {
207-
log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit)
207+
reportPluginError(reportManager, tb, err)
208+
209+
if errors.Is(err, store.ErrTimeoutLockingStore) {
210+
var cniMetric telemetry.AIMetric
211+
cniMetric.Metric = aitelemetry.Metric{
212+
Name: telemetry.CNILockTimeoutStr,
213+
Value: 1.0,
214+
CustomDimensions: make(map[string]string),
215+
}
216+
err = telemetry.SendCNIMetric(&cniMetric, tb)
217+
if err != nil {
218+
log.Errorf("Couldn't send cnilocktimeout metric: %v", err)
208219
}
209220
}
210221

222+
tb.Close()
211223
return
212224
}
213225

214226
defer func() {
215-
if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(false); errUninit != nil {
227+
if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(); errUninit != nil {
216228
log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit)
217229
}
218230

@@ -270,7 +282,7 @@ func main() {
270282
netPlugin.Stop()
271283

272284
// release cni lock
273-
if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(false); errUninit != nil {
285+
if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(); errUninit != nil {
274286
log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit)
275287
}
276288

‎cni/plugin.go‎

Lines changed: 11 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,14 @@ package cni
66
import (
77
"context"
88
"fmt"
9-
"io/ioutil"
109
"os"
1110
"runtime"
1211

1312
"github.com/Azure/azure-container-networking/common"
1413
"github.com/Azure/azure-container-networking/log"
1514
"github.com/Azure/azure-container-networking/platform"
15+
"github.com/Azure/azure-container-networking/processlock"
1616
"github.com/Azure/azure-container-networking/store"
17-
1817
cniInvoke "github.com/containernetworking/cni/pkg/invoke"
1918
cniSkel "github.com/containernetworking/cni/pkg/skel"
2019
cniTypes "github.com/containernetworking/cni/pkg/types"
@@ -155,16 +154,21 @@ func (plugin *Plugin) Errorf(format string, args ...interface{}) *cniTypes.Error
155154
func (plugin *Plugin) InitializeKeyValueStore(config *common.PluginConfig) error {
156155
// Create the key value store.
157156
if plugin.Store == nil {
158-
var err error
159-
plugin.Store, err = store.NewJsonFileStore(platform.CNIRuntimePath + plugin.Name + ".json")
157+
lockclient, err := processlock.NewFileLock(platform.CNILockPath + plugin.Name + store.LockExtension)
158+
if err != nil {
159+
log.Printf("[cni] Error initializing file lock:%v", err)
160+
return errors.Wrap(err, "error creating new filelock")
161+
}
162+
163+
plugin.Store, err = store.NewJsonFileStore(platform.CNIRuntimePath+plugin.Name+".json", lockclient)
160164
if err != nil {
161165
log.Printf("[cni] Failed to create store: %v.", err)
162166
return err
163167
}
164168
}
165169

166170
// Acquire store lock.
167-
if err := plugin.Store.Lock(true); err != nil {
171+
if err := plugin.Store.Lock(store.DefaultLockTimeout); err != nil {
168172
log.Printf("[cni] Failed to lock store: %v.", err)
169173
return err
170174
}
@@ -175,9 +179,9 @@ func (plugin *Plugin) InitializeKeyValueStore(config *common.PluginConfig) error
175179
}
176180

177181
// Uninitialize key-value store
178-
func (plugin *Plugin) UninitializeKeyValueStore(force bool) error {
182+
func (plugin *Plugin) UninitializeKeyValueStore() error {
179183
if plugin.Store != nil {
180-
err := plugin.Store.Unlock(force)
184+
err := plugin.Store.Unlock()
181185
if err != nil {
182186
log.Printf("[cni] Failed to unlock store: %v.", err)
183187
return err
@@ -187,67 +191,3 @@ func (plugin *Plugin) UninitializeKeyValueStore(force bool) error {
187191

188192
return nil
189193
}
190-
191-
// check if safe to remove lockfile
192-
func (plugin *Plugin) IsSafeToRemoveLock(processName string) (bool, error) {
193-
if plugin != nil && plugin.Store != nil {
194-
// check if get process command supported
195-
if cmdErr := platform.GetProcessSupport(); cmdErr != nil {
196-
log.Errorf("Get process cmd not supported. Error %v", cmdErr)
197-
return false, cmdErr
198-
}
199-
200-
lockFileName := plugin.Store.GetLockFileName()
201-
// Read pid from lockfile
202-
lockFilePid, err := plugin.readLockFile(lockFileName)
203-
if err != nil {
204-
return false, errors.Wrap(err, "IsSafeToRemoveLock lockfile read failed")
205-
}
206-
207-
log.Printf("Read from lockfile:%s", lockFilePid)
208-
// Get the process name if running and
209-
// check if that matches with our expected process
210-
// if it returns non-nil error then process is not running
211-
pName, err := platform.GetProcessNameByID(lockFilePid)
212-
if err != nil {
213-
var content string
214-
content, err = plugin.readLockFile(lockFileName)
215-
if err != nil {
216-
return false, errors.Wrap(err, "IsSafeToRemoveLock lockfile 2nd read failed")
217-
}
218-
219-
// pid in lockfile changed after getprocessnamebyid call. so some other process acquired lockfile in between.
220-
// so its not safe to remove lockfile
221-
if string(content) != lockFilePid {
222-
log.Printf("Lockfile content changed from %s to %s. So not safe to remove lockfile", lockFilePid, content)
223-
return false, nil
224-
}
225-
226-
return true, nil
227-
}
228-
229-
log.Printf("[CNI] Process name is %s", pName)
230-
231-
if pName != processName {
232-
return true, nil
233-
}
234-
}
235-
236-
log.Errorf("Plugin store is nil")
237-
return false, fmt.Errorf("plugin store nil")
238-
}
239-
240-
func (plugin *Plugin) readLockFile(filename string) (string, error) {
241-
content, err := ioutil.ReadFile(filename)
242-
if err != nil {
243-
log.Errorf("Failed to read lockfile :%v", err)
244-
return "", fmt.Errorf("readLockFile error:%w", err)
245-
}
246-
247-
if len(content) == 0 {
248-
log.Errorf("Num bytes read from lockfile is 0")
249-
return "", errEmptyContent
250-
}
251-
252-
return string(content), nil
253-
}

‎cni/plugin_test.go‎

Lines changed: 0 additions & 69 deletions
This file was deleted.

0 commit comments

Comments
 (0)