Skip to content

Commit ab3f6e9

Browse files
saschagrunertsaschagrunert
committed
Apply small cleanups and error check paths
Signed-off-by: Sascha Grunert <[email protected]>
1 parent f870c19 commit ab3f6e9

File tree

2 files changed

+40
-37
lines changed

2 files changed

+40
-37
lines changed

‎pkg/ocicni/ocicni.go

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7-
"io/ioutil"
87
"net"
98
"os"
109
"path"
@@ -221,6 +220,13 @@ func initCNI(exec cniinvoke.Exec, cacheDir, defaultNetName string, confDir strin
221220
binDirs = []string{DefaultBinDir}
222221
}
223222

223+
if exec == nil {
224+
exec = &cniinvoke.DefaultExec{
225+
RawExec: &cniinvoke.RawExec{Stderr: os.Stderr},
226+
PluginDecoder: cniversion.PluginDecoder{},
227+
}
228+
}
229+
224230
plugin := &cniNetworkPlugin{
225231
cniConfig: libcni.NewCNIConfigWithCacheDir(binDirs, cacheDir, exec),
226232
defaultNetName: netName{
@@ -239,20 +245,15 @@ func initCNI(exec cniinvoke.Exec, cacheDir, defaultNetName string, confDir strin
239245
cacheDir: cacheDir,
240246
}
241247

242-
if exec == nil {
243-
exec = &cniinvoke.DefaultExec{
244-
RawExec: &cniinvoke.RawExec{Stderr: os.Stderr},
245-
PluginDecoder: cniversion.PluginDecoder{},
246-
}
247-
}
248-
249248
nsm, err := newNSManager()
250249
if err != nil {
251250
return nil, err
252251
}
253252
plugin.nsManager = nsm
254253

255-
plugin.syncNetworkConfig()
254+
if err := plugin.syncNetworkConfig(); err != nil {
255+
logrus.Errorf("CNI sync network config failed: %v", err)
256+
}
256257

257258
if useInotify {
258259
plugin.watcher, err = newWatcher(append([]string{plugin.confDir}, binDirs...))
@@ -494,7 +495,7 @@ func (plugin *cniNetworkPlugin) forEachNetwork(podNetwork *PodNetwork, fromCache
494495

495496
rt, err := buildCNIRuntimeConf(podNetwork, ifName, podNetwork.RuntimeConfig[network.Name])
496497
if err != nil {
497-
logrus.Errorf("error building CNI runtime config: %v", err)
498+
logrus.Errorf("Error building CNI runtime config: %v", err)
498499
return err
499500
}
500501

@@ -503,8 +504,8 @@ func (plugin *cniNetworkPlugin) forEachNetwork(podNetwork *PodNetwork, fromCache
503504
var newRt *libcni.RuntimeConf
504505
cniNet, newRt, err = plugin.loadNetworkFromCache(network.Name, rt)
505506
if err != nil {
506-
logrus.Errorf("error loading cached network config: %v", err)
507-
logrus.Warningf("falling back to loading from existing plugins on disk")
507+
logrus.Errorf("Error loading cached network config: %v", err)
508+
logrus.Warningf("Falling back to loading from existing plugins on disk")
508509
} else {
509510
// Use the updated RuntimeConf
510511
rt = newRt
@@ -580,7 +581,7 @@ func (plugin *cniNetworkPlugin) getCachedNetworkInfo(containerID string) ([]NetA
580581
}
581582

582583
dirPath := filepath.Join(cacheDir, "results")
583-
entries, err := ioutil.ReadDir(dirPath)
584+
entries, err := os.ReadDir(dirPath)
584585
if err != nil {
585586
return nil, err
586587
}
@@ -600,9 +601,9 @@ func (plugin *cniNetworkPlugin) getCachedNetworkInfo(containerID string) ([]NetA
600601
}
601602

602603
cacheFile := filepath.Join(dirPath, fname)
603-
bytes, err := ioutil.ReadFile(cacheFile)
604+
bytes, err := os.ReadFile(cacheFile)
604605
if err != nil {
605-
logrus.Errorf("failed to read CNI cache file %s: %v", cacheFile, err)
606+
logrus.Errorf("Failed to read CNI cache file %s: %v", cacheFile, err)
606607
continue
607608
}
608609

@@ -614,11 +615,11 @@ func (plugin *cniNetworkPlugin) getCachedNetworkInfo(containerID string) ([]NetA
614615
}{}
615616

616617
if err := json.Unmarshal(bytes, &cachedInfo); err != nil {
617-
logrus.Errorf("failed to unmarshal CNI cache file %s: %v", cacheFile, err)
618+
logrus.Errorf("Failed to unmarshal CNI cache file %s: %v", cacheFile, err)
618619
continue
619620
}
620621
if cachedInfo.Kind != libcni.CNICacheV1 {
621-
logrus.Warningf("unknown CNI cache file %s kind %q", cacheFile, cachedInfo.Kind)
622+
logrus.Warningf("Unknown CNI cache file %s kind %q", cacheFile, cachedInfo.Kind)
622623
continue
623624
}
624625
if cachedInfo.ContainerID != containerID {
@@ -629,7 +630,7 @@ func (plugin *cniNetworkPlugin) getCachedNetworkInfo(containerID string) ([]NetA
629630
continue
630631
}
631632
if cachedInfo.IfName == "" || cachedInfo.NetName == "" {
632-
logrus.Warningf("missing CNI cache file %s ifname %q or netname %q", cacheFile, cachedInfo.IfName, cachedInfo.NetName)
633+
logrus.Warningf("Missing CNI cache file %s ifname %q or netname %q", cacheFile, cachedInfo.IfName, cachedInfo.NetName)
633634
continue
634635
}
635636

‎pkg/ocicni/ocicni_test.go

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"encoding/base64"
66
"encoding/json"
77
"fmt"
8-
"io/ioutil"
98
"net"
109
"os"
1110
"path/filepath"
@@ -32,7 +31,7 @@ func writeConfig(dir, fileName, netName, plugin string, version string) (string,
3231
"type": "%s",
3332
"cniVersion": "%s"
3433
}`, netName, plugin, version)
35-
return conf, confPath, ioutil.WriteFile(confPath, []byte(conf), 0644)
34+
return conf, confPath, os.WriteFile(confPath, []byte(conf), 0644)
3635
}
3736

3837
func writeCacheFile(dir, containerID, netName, ifname, config string) {
@@ -52,7 +51,7 @@ func writeCacheFile(dir, containerID, netName, ifname, config string) {
5251
Expect(err).NotTo(HaveOccurred())
5352

5453
filePath := filepath.Join(dirName, fmt.Sprintf("%s-%s-%s", netName, containerID, ifname))
55-
err = ioutil.WriteFile(filePath, []byte(cachedData), 0644)
54+
err = os.WriteFile(filePath, []byte(cachedData), 0644)
5655
Expect(err).NotTo(HaveOccurred())
5756
}
5857

@@ -146,7 +145,10 @@ func (f *fakeExec) ExecPlugin(ctx context.Context, pluginPath string, stdinData
146145
// SetUpPod We only care about a few fields
147146
testConf := &TestConf{}
148147
err = json.Unmarshal(stdinData, &testConf)
148+
Expect(err).NotTo(HaveOccurred())
149+
149150
testData, err := json.Marshal(testConf)
151+
Expect(err).NotTo(HaveOccurred())
150152
if plugin.expectedConf != "" {
151153
Expect(string(testData)).To(MatchJSON(plugin.expectedConf))
152154
}
@@ -193,11 +195,11 @@ var _ = Describe("ocicni operations", func() {
193195

194196
BeforeEach(func() {
195197
var err error
196-
tmpDir, err = ioutil.TempDir("", "ocicni_tmp")
198+
tmpDir, err = os.MkdirTemp("", "ocicni_tmp")
197199
Expect(err).NotTo(HaveOccurred())
198-
tmpBinDir, err = ioutil.TempDir("", "ocicni_tmp_bin")
200+
tmpBinDir, err = os.MkdirTemp("", "ocicni_tmp_bin")
199201
Expect(err).NotTo(HaveOccurred())
200-
cacheDir, err = ioutil.TempDir("", "ocicni_cache")
202+
cacheDir, err = os.MkdirTemp("", "ocicni_cache")
201203
Expect(err).NotTo(HaveOccurred())
202204

203205
networkNS, err = testutils.NewNS()
@@ -233,7 +235,7 @@ var _ = Describe("ocicni operations", func() {
233235
Expect(len(net.config.Plugins)).To(BeNumerically(">", 0))
234236
Expect(net.config.Plugins[0].Network.Type).To(Equal("myplugin"))
235237

236-
ocicni.Shutdown()
238+
Expect(ocicni.Shutdown()).NotTo(HaveOccurred())
237239
})
238240

239241
It("finds an asynchronously written default network configuration", func() {
@@ -255,7 +257,7 @@ var _ = Describe("ocicni operations", func() {
255257
Expect(len(net.config.Plugins)).To(BeNumerically(">", 0))
256258
Expect(net.config.Plugins[0].Network.Type).To(Equal("myplugin"))
257259

258-
ocicni.Shutdown()
260+
Expect(ocicni.Shutdown()).NotTo(HaveOccurred())
259261
})
260262

261263
It("finds an asynchronously written default network configuration whose plugin is written later", func() {
@@ -269,7 +271,7 @@ var _ = Describe("ocicni operations", func() {
269271

270272
// Write a file in the bindir to trigger the fsnotify code to resync
271273
fExec.failFind = false
272-
err = ioutil.WriteFile(filepath.Join(tmpBinDir, "myplugin"), []byte("adsfasdfsafd"), 0755)
274+
err = os.WriteFile(filepath.Join(tmpBinDir, "myplugin"), []byte("adsfasdfsafd"), 0755)
273275
Expect(err).NotTo(HaveOccurred())
274276

275277
tmp := ocicni.(*cniNetworkPlugin)
@@ -290,7 +292,7 @@ var _ = Describe("ocicni operations", func() {
290292
return nil
291293
}, 10).Should(Succeed())
292294

293-
ocicni.Shutdown()
295+
Expect(ocicni.Shutdown()).NotTo(HaveOccurred())
294296
})
295297

296298
It("should monitor the net conf dir for changes when default network is not specified", func() {
@@ -321,7 +323,7 @@ var _ = Describe("ocicni operations", func() {
321323
Expect(len(net.config.Plugins)).To(BeNumerically(">", 0))
322324
Expect(net.config.Plugins[0].Network.Type).To(Equal("testplugin"))
323325

324-
ocicni.Shutdown()
326+
Expect(ocicni.Shutdown()).NotTo(HaveOccurred())
325327
})
326328

327329
It("should monitor the net conf dir for changes when default network is specified", func() {
@@ -352,7 +354,7 @@ var _ = Describe("ocicni operations", func() {
352354
Expect(len(net.config.Plugins)).To(BeNumerically(">", 0))
353355
Expect(net.config.Plugins[0].Network.Type).To(Equal("testplugin"))
354356

355-
ocicni.Shutdown()
357+
Expect(ocicni.Shutdown()).NotTo(HaveOccurred())
356358
})
357359

358360
It("finds and refinds an asynchronously written default network configuration", func() {
@@ -382,7 +384,7 @@ var _ = Describe("ocicni operations", func() {
382384
Expect(err).NotTo(HaveOccurred())
383385
Eventually(ocicni.Status, 5).Should(Succeed())
384386

385-
ocicni.Shutdown()
387+
Expect(ocicni.Shutdown()).NotTo(HaveOccurred())
386388
})
387389

388390
It("finds an ASCIIbetically first network configuration as default real-time if given no default network name", func() {
@@ -414,7 +416,7 @@ var _ = Describe("ocicni operations", func() {
414416
Expect(len(net.config.Plugins)).To(BeNumerically(">", 0))
415417
Expect(net.config.Plugins[0].Network.Type).To(Equal("myplugin"))
416418

417-
ocicni.Shutdown()
419+
Expect(ocicni.Shutdown()).NotTo(HaveOccurred())
418420
})
419421

420422
It("returns correct default network from loadNetworks()", func() {
@@ -638,7 +640,7 @@ var _ = Describe("ocicni operations", func() {
638640
Expect(err).NotTo(HaveOccurred())
639641
Expect(fake.delIndex).To(Equal(len(fake.plugins)))
640642

641-
ocicni.Shutdown()
643+
Expect(ocicni.Shutdown()).NotTo(HaveOccurred())
642644
})
643645

644646
It("sets up and tears down a pod using specified networks", func() {
@@ -715,7 +717,7 @@ var _ = Describe("ocicni operations", func() {
715717
Expect(err).NotTo(HaveOccurred())
716718
Expect(fake.delIndex).To(Equal(len(fake.plugins)))
717719

718-
ocicni.Shutdown()
720+
Expect(ocicni.Shutdown()).NotTo(HaveOccurred())
719721
})
720722

721723
It("sets up and tears down a pod using specified v4 networks", func() {
@@ -801,7 +803,7 @@ var _ = Describe("ocicni operations", func() {
801803
Expect(err).NotTo(HaveOccurred())
802804
Expect(fake.delIndex).To(Equal(len(fake.plugins)))
803805

804-
ocicni.Shutdown()
806+
Expect(ocicni.Shutdown()).NotTo(HaveOccurred())
805807
})
806808

807809
Context("when tearing down a pod using cached info", func() {
@@ -855,7 +857,7 @@ var _ = Describe("ocicni operations", func() {
855857
})
856858

857859
AfterEach(func() {
858-
ocicni.Shutdown()
860+
Expect(ocicni.Shutdown()).NotTo(HaveOccurred())
859861
})
860862

861863
It("uses the specified networks", func() {
@@ -910,7 +912,7 @@ var _ = Describe("ocicni operations", func() {
910912

911913
ocicni, err := initCNI(fake, cacheDir, defaultNetName, tmpDir, true, "/opt/cni/bin")
912914
Expect(err).NotTo(HaveOccurred())
913-
defer ocicni.Shutdown()
915+
defer Expect(ocicni.Shutdown()).NotTo(HaveOccurred())
914916

915917
podNet := PodNetwork{
916918
Name: "pod1",

0 commit comments

Comments
 (0)