Skip to content

Commit 4943198

Browse files
paulyufan2paulyufan2
authored
[CNI] zap logger migration for store package (#2231)
* zap logger migration for store package
1 parent 3c6bb62 commit 4943198

File tree

8 files changed

+57
-24
lines changed

8 files changed

+57
-24
lines changed

‎aitelemetry/telemetrywrapper.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func getMetadata(th *telemetryHandle) {
111111
}
112112

113113
// Save metadata retrieved from wireserver to a file
114-
kvs, err := store.NewJsonFileStore(metadataFile, lockclient)
114+
kvs, err := store.NewJsonFileStore(metadataFile, lockclient, nil)
115115
if err != nil {
116116
debugLog("[AppInsights] Error initializing kvs store: %v", err)
117117
return

‎cni/plugin.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
)
2626

2727
var logger = log.CNILogger.With(zap.String("component", "cni-plugin"))
28+
var storeLogger = log.CNILogger.With(zap.String("component", "cni-store"))
2829

2930
var errEmptyContent = errors.New("read content is zero bytes")
3031

@@ -188,7 +189,7 @@ func (plugin *Plugin) InitializeKeyValueStore(config *common.PluginConfig) error
188189
return errors.Wrap(err, "error creating new filelock")
189190
}
190191

191-
plugin.Store, err = store.NewJsonFileStore(platform.CNIRuntimePath+plugin.Name+".json", lockclient)
192+
plugin.Store, err = store.NewJsonFileStore(platform.CNIRuntimePath+plugin.Name+".json", lockclient, storeLogger)
192193
if err != nil {
193194
logger.Error("Failed to create store", zap.Error(err))
194195
return err

‎cnm/plugin/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func main() {
167167

168168
// Create the key value store.
169169
storeFileName := storeFileLocation + name + ".json"
170-
config.Store, err = store.NewJsonFileStore(storeFileName, lockclient)
170+
config.Store, err = store.NewJsonFileStore(storeFileName, lockclient, nil)
171171
if err != nil {
172172
log.Errorf("Failed to create store file: %s, due to error %v\n", storeFileName, err)
173173
return

‎cnms/service/networkmonitor.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ func main() {
150150
return
151151
}
152152

153-
config.Store, err = store.NewJsonFileStore(platform.CNIRuntimePath+pluginName+".json", lockclient)
153+
config.Store, err = store.NewJsonFileStore(platform.CNIRuntimePath+pluginName+".json", lockclient, nil)
154154
if err != nil {
155155
fmt.Printf("[monitor] Failed to create store: %v\n", err)
156156
return

‎cns/restserver/api_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1671,7 +1671,7 @@ func startService() error {
16711671
config := common.ServiceConfig{}
16721672

16731673
// Create the key value fileStore.
1674-
fileStore, err := store.NewJsonFileStore(cnsJsonFileName, processlock.NewMockFileLock(false))
1674+
fileStore, err := store.NewJsonFileStore(cnsJsonFileName, processlock.NewMockFileLock(false), nil)
16751675
if err != nil {
16761676
logger.Errorf("Failed to create store file: %s, due to error %v\n", cnsJsonFileName, err)
16771677
return err

‎cns/service/main.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,7 @@ func main() {
641641

642642
// Create the key value store.
643643
storeFileName := storeFileLocation + name + ".json"
644-
config.Store, err = store.NewJsonFileStore(storeFileName, lockclient)
644+
config.Store, err = store.NewJsonFileStore(storeFileName, lockclient, nil)
645645
if err != nil {
646646
logger.Errorf("Failed to create store file: %s, due to error %v\n", storeFileName, err)
647647
return
@@ -664,7 +664,7 @@ func main() {
664664
}
665665
// Create the key value store.
666666
storeFileName := endpointStoreLocation + endpointStoreName + ".json"
667-
endpointStateStore, err = store.NewJsonFileStore(storeFileName, endpointStoreLock)
667+
endpointStateStore, err = store.NewJsonFileStore(storeFileName, endpointStoreLock, nil)
668668
if err != nil {
669669
logger.Errorf("Failed to create endpoint state store file: %s, due to error %v\n", storeFileName, err)
670670
return
@@ -903,7 +903,7 @@ func main() {
903903

904904
// Create the key value store.
905905
pluginStoreFile := storeFileLocation + pluginName + ".json"
906-
pluginConfig.Store, err = store.NewJsonFileStore(pluginStoreFile, lockclientCnm)
906+
pluginConfig.Store, err = store.NewJsonFileStore(pluginStoreFile, lockclientCnm, nil)
907907
if err != nil {
908908
logger.Errorf("Failed to create plugin store file %s, due to error : %v\n", pluginStoreFile, err)
909909
return

‎store/json.go‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/Azure/azure-container-networking/platform"
1717
"github.com/Azure/azure-container-networking/processlock"
1818
"github.com/pkg/errors"
19+
"go.uber.org/zap"
1920
)
2021

2122
const (
@@ -35,19 +36,21 @@ type jsonFileStore struct {
3536
inSync bool
3637
processLock processlock.Interface
3738
sync.Mutex
39+
logger *zap.Logger
3840
}
3941

4042
// NewJsonFileStore creates a new jsonFileStore object, accessed as a KeyValueStore.
4143
//
4244
//nolint:revive // ignoring name change
43-
func NewJsonFileStore(fileName string, lockclient processlock.Interface) (KeyValueStore, error) {
45+
func NewJsonFileStore(fileName string, lockclient processlock.Interface, logger *zap.Logger) (KeyValueStore, error) {
4446
if fileName == "" {
4547
return &jsonFileStore{}, errors.New("need to pass in a json file path")
4648
}
4749
kvs := &jsonFileStore{
4850
fileName: fileName,
4951
processLock: lockclient,
5052
data: make(map[string]*json.RawMessage),
53+
logger: logger,
5154
}
5255

5356
return kvs, nil
@@ -83,7 +86,12 @@ func (kvs *jsonFileStore) Read(key string, value interface{}) error {
8386
}
8487

8588
if len(b) == 0 {
86-
log.Printf("Unable to read file %s, was empty", kvs.fileName)
89+
if kvs.logger != nil {
90+
kvs.logger.Info("Unable to read empty file", zap.String("fileName", kvs.fileName))
91+
} else {
92+
log.Printf("Unable to read file %s, was empty", kvs.fileName)
93+
}
94+
8795
return ErrStoreEmpty
8896
}
8997

@@ -184,7 +192,12 @@ func (kvs *jsonFileStore) Lock(timeout time.Duration) error {
184192
afterTime := time.After(timeout)
185193
status := make(chan error)
186194

187-
log.Printf("Acquiring process lock")
195+
if kvs.logger != nil {
196+
kvs.logger.Info("Acquiring process lock")
197+
} else {
198+
log.Printf("Acquiring process lock")
199+
}
200+
188201
go kvs.lockUtil(status)
189202

190203
var err error
@@ -198,7 +211,12 @@ func (kvs *jsonFileStore) Lock(timeout time.Duration) error {
198211
return errors.Wrap(err, "processLock acquire error")
199212
}
200213

201-
log.Printf("Acquired process lock with timeout value of %v ", timeout)
214+
if kvs.logger != nil {
215+
kvs.logger.Info("Acquired process lock with timeout value of", zap.Any("timeout", timeout))
216+
} else {
217+
log.Printf("Acquired process lock with timeout value of %v", timeout)
218+
}
219+
202220
return nil
203221
}
204222

@@ -212,7 +230,12 @@ func (kvs *jsonFileStore) Unlock() error {
212230
return errors.Wrap(err, "unlock error")
213231
}
214232

215-
log.Printf("Released process lock")
233+
if kvs.logger != nil {
234+
kvs.logger.Info("Released process lock")
235+
} else {
236+
log.Printf("Released process lock")
237+
}
238+
216239
return nil
217240
}
218241

@@ -223,7 +246,12 @@ func (kvs *jsonFileStore) GetModificationTime() (time.Time, error) {
223246

224247
info, err := os.Stat(kvs.fileName)
225248
if err != nil {
226-
log.Printf("os.stat() for file %v failed: %v", kvs.fileName, err)
249+
if kvs.logger != nil {
250+
kvs.logger.Info("os.stat() for file", zap.String("fileName", kvs.fileName), zap.Error(err))
251+
} else {
252+
log.Printf("os.stat() for file %v failed: %v", kvs.fileName, err)
253+
}
254+
227255
return time.Time{}.UTC(), err
228256
}
229257

‎store/json_test.go‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"testing"
1010
"time"
1111

12+
"github.com/Azure/azure-container-networking/cni/log"
1213
"github.com/Azure/azure-container-networking/processlock"
1314
"github.com/stretchr/testify/require"
1415
)
@@ -49,7 +50,7 @@ func TestKeyValuePairsAreReinstantiatedFromJSONFile(t *testing.T) {
4950
defer os.Remove(testFileName)
5051

5152
// Create the store, initialized using the JSON file.
52-
kvs, err := NewJsonFileStore(testFileName, processlock.NewMockFileLock(false))
53+
kvs, err := NewJsonFileStore(testFileName, processlock.NewMockFileLock(false), nil)
5354
if err != nil {
5455
t.Fatalf("Failed to create KeyValueStore %v\n", err)
5556
}
@@ -74,7 +75,7 @@ func TestKeyValuePairsArePersistedToJSONFile(t *testing.T) {
7475
var actualPair string
7576

7677
// Create the store.
77-
kvs, err := NewJsonFileStore(testFileName, processlock.NewMockFileLock(false))
78+
kvs, err := NewJsonFileStore(testFileName, processlock.NewMockFileLock(false), nil)
7879
if err != nil {
7980
t.Fatalf("Failed to create KeyValueStore %v\n", err)
8081
}
@@ -119,8 +120,11 @@ func TestKeyValuePairsAreWrittenAndReadCorrectly(t *testing.T) {
119120
anotherValue := testType1{"any", 14}
120121
var readValue testType1
121122

123+
// Test when passing zap logger obj to NewJsonFileStore
124+
logger := log.CNILogger
125+
122126
// Create the store.
123-
kvs, err := NewJsonFileStore(testFileName, processlock.NewMockFileLock(false))
127+
kvs, err := NewJsonFileStore(testFileName, processlock.NewMockFileLock(false), logger)
124128
if err != nil {
125129
t.Fatalf("Failed to create KeyValueStore %v\n", err)
126130
}
@@ -155,12 +159,12 @@ func TestKeyValuePairsAreWrittenAndReadCorrectly(t *testing.T) {
155159

156160
// test case for testing newjsonfilestore idempotent
157161
func TestNewJsonFileStoreIdempotent(t *testing.T) {
158-
_, err := NewJsonFileStore(testLockFileName, processlock.NewMockFileLock(false))
162+
_, err := NewJsonFileStore(testLockFileName, processlock.NewMockFileLock(false), nil)
159163
if err != nil {
160164
t.Errorf("Failed to initialize store: %v", err)
161165
}
162166

163-
_, err = NewJsonFileStore(testLockFileName, processlock.NewMockFileLock(false))
167+
_, err = NewJsonFileStore(testLockFileName, processlock.NewMockFileLock(false), nil)
164168
if err != nil {
165169
t.Errorf("Failed to initialize same store second time: %v", err)
166170
}
@@ -177,7 +181,7 @@ func TestLock(t *testing.T) {
177181
{
178182
name: "Acquire Lock happy path",
179183
store: func() KeyValueStore {
180-
st, _ := NewJsonFileStore(testFileName, processlock.NewMockFileLock(false))
184+
st, _ := NewJsonFileStore(testFileName, processlock.NewMockFileLock(false), nil)
181185
return st
182186
}(),
183187
timeoutms: 10000,
@@ -186,7 +190,7 @@ func TestLock(t *testing.T) {
186190
{
187191
name: "Acquire Lock Fail",
188192
store: func() KeyValueStore {
189-
st, _ := NewJsonFileStore(testFileName, processlock.NewMockFileLock(true))
193+
st, _ := NewJsonFileStore(testFileName, processlock.NewMockFileLock(true), nil)
190194
return st
191195
}(),
192196
timeoutms: 10000,
@@ -196,7 +200,7 @@ func TestLock(t *testing.T) {
196200
{
197201
name: "Acquire Lock timeout error",
198202
store: func() KeyValueStore {
199-
st, _ := NewJsonFileStore(testFileName, processlock.NewMockFileLock(false))
203+
st, _ := NewJsonFileStore(testFileName, processlock.NewMockFileLock(false), nil)
200204
return st
201205
}(),
202206
timeoutms: 0,
@@ -223,12 +227,12 @@ func TestLock(t *testing.T) {
223227

224228
// test case for testing newjsonfilestore idempotent
225229
func TestFileName(t *testing.T) {
226-
_, err := NewJsonFileStore("", processlock.NewMockFileLock(false))
230+
_, err := NewJsonFileStore("", processlock.NewMockFileLock(false), nil)
227231
if err == nil {
228232
t.Errorf("This should have failed for empty file name")
229233
}
230234

231-
_, err = NewJsonFileStore("test.json", processlock.NewMockFileLock(false))
235+
_, err = NewJsonFileStore("test.json", processlock.NewMockFileLock(false), nil)
232236
if err != nil {
233237
t.Fatalf("This should not fail for a non-empty file %v", err)
234238
}

0 commit comments

Comments
 (0)