fixes log level, and adds a few tests

Signed-off-by: Skyler Clark <[email protected]>

Author: Skyler Clark

pkg/ocicni/ocicni.go

@@ -124,12 +124,12 @@ func newWatcher(confDir string) (*fsnotify.Watcher, error) {
 	// Ensure plugin directory exists, because the following monitoring logic
 	// relies on that.
 	if err := os.MkdirAll(confDir, 0755); err != nil {
-		return nil, fmt.Errorf("failed to create %q: %v", confDir, err)
+		return nil, fmt.Errorf("failed to create directory %q: %v", confDir, err)
 	}
 
 	watcher, err := fsnotify.NewWatcher()
 	if err != nil {
-		return nil, fmt.Errorf("could not create new watcher %v", err)
+		return nil, fmt.Errorf("failed to create new watcher %v", err)
 	}
 	defer func() {
 		// Close watcher on error
@@ -278,13 +278,13 @@ func loadNetworks(confDir string, cni *libcni.CNIConfig) (map[string]*cniNetwork
 		if strings.HasSuffix(confFile, ".conflist") {
 			confList, err = libcni.ConfListFromFile(confFile)
 			if err != nil {
-				logrus.Warningf("Error loading CNI config list file %s: %v", confFile, err)
+				logrus.Errorf("Error loading CNI config list file %s: %v", confFile, err)
 				continue
 			}
 		} else {
 			conf, err := libcni.ConfFromFile(confFile)
 			if err != nil {
-				logrus.Warningf("Error loading CNI config file %s: %v", confFile, err)
+				logrus.Errorf("Error loading CNI config file %s: %v", confFile, err)
 				continue
 			}
 			if conf.Network.Type == "" {
@@ -293,7 +293,7 @@ func loadNetworks(confDir string, cni *libcni.CNIConfig) (map[string]*cniNetwork
 			}
 			confList, err = libcni.ConfListFromConf(conf)
 			if err != nil {
-				logrus.Warningf("Error converting CNI config file %s to list: %v", confFile, err)
+				logrus.Errorf("Error converting CNI config file %s to list: %v", confFile, err)
 				continue
 			}
 		}
@@ -324,7 +324,7 @@ func loadNetworks(confDir string, cni *libcni.CNIConfig) (map[string]*cniNetwork
 		if _, ok := networks[confList.Name]; !ok {
 			networks[confList.Name] = cniNet
 		} else {
-			logrus.Infof("Ignore CNI network %s (type=%v) at %s because already exists", confList.Name, confList.Plugins[0].Network.Type, confFile)
+			logrus.Infof("Ignored CNI network %s (type=%v) at %s because already exists", confList.Name, confList.Plugins[0].Network.Type, confFile)
 		}
 
 		if defaultNetName == "" {
@@ -351,9 +351,9 @@ func (plugin *cniNetworkPlugin) syncNetworkConfig() error {
 	// Update defaultNetName if it is changeable
 	if plugin.defaultNetName.changeable {
 		plugin.defaultNetName.name = defaultNetName
-		logrus.Infof("Update default CNI network name to %s", defaultNetName)
+		logrus.Infof("Updated default CNI network name to %s", defaultNetName)
 	} else {
-		logrus.Warnf("Default CNI network name %s is unchangeable", plugin.defaultNetName.name)
+		logrus.Warningf("Default CNI network name %s is unchangeable", plugin.defaultNetName.name)
 	}
 
 	plugin.networks = networks
@@ -482,8 +482,8 @@ func (plugin *cniNetworkPlugin) forEachNetwork(podNetwork *PodNetwork, fromCache
 			var newRt *libcni.RuntimeConf
 			cniNet, newRt, err = plugin.loadNetworkFromCache(network.Name, rt)
 			if err != nil {
-				logrus.Debugf("error loading cached network config: %v", err)
-				logrus.Debugf("falling back to loading from existing plugins on disk")
+				logrus.Errorf("error loading cached network config: %v", err)
+				logrus.Warningf("falling back to loading from existing plugins on disk")
 			} else {
 				// Use the updated RuntimeConf
 				rt = newRt
@@ -519,10 +519,10 @@ func bringUpLoopback(netns string) error {
 			return err
 		}
 		if len(v4Addrs) != 0 {
-			// sanity check that this is a loopback address
+			// check that this is a loopback address
 			for _, addr := range v4Addrs {
 				if !addr.IP.IsLoopback() {
-					return fmt.Errorf("loopback interface found with non-loopback address %q", addr.IP)
+					return fmt.Warningf("loopback interface found with non-loopback address %q", addr.IP)
 				}
 			}
 		}
@@ -532,10 +532,10 @@ func bringUpLoopback(netns string) error {
 			return err
 		}
 		if len(v6Addrs) != 0 {
-			// sanity check that this is a loopback address
+			// check that this is a loopback address
 			for _, addr := range v6Addrs {
 				if !addr.IP.IsLoopback() {
-					return fmt.Errorf("loopback interface found with non-loopback address %q", addr.IP)
+					return fmt.Warningf("loopback interface found with non-loopback address %q", addr.IP)
 				}
 			}
 		}
@@ -616,7 +616,7 @@ func (plugin *cniNetworkPlugin) getCachedNetworkInfo(containerID string) ([]NetA
 		cacheFile := filepath.Join(dirPath, fname)
 		bytes, err := ioutil.ReadFile(cacheFile)
 		if err != nil {
-			logrus.Warningf("failed to read CNI cache file %s: %v", cacheFile, err)
+			logrus.Errorf("failed to read CNI cache file %s: %v", cacheFile, err)
 			continue
 		}
 
@@ -628,7 +628,7 @@ func (plugin *cniNetworkPlugin) getCachedNetworkInfo(containerID string) ([]NetA
 		}{}
 
 		if err := json.Unmarshal(bytes, &cachedInfo); err != nil {
-			logrus.Warningf("failed to unmarshal CNI cache file %s: %v", cacheFile, err)
+			logrus.Errorf("failed to unmarshal CNI cache file %s: %v", cacheFile, err)
 			continue
 		}
 		if cachedInfo.Kind != libcni.CNICacheV1 {
@@ -692,7 +692,7 @@ func (plugin *cniNetworkPlugin) TearDownPodWithContext(ctx context.Context, podN
 
 	if err := tearDownLoopback(podNetwork.NetNS); err != nil {
 		// ignore error
-		logrus.Errorf("Ignoring error tearing down loopback interface: %v", err)
+		logrus.Warningf("Ignoring error tearing down loopback interface: %v", err)
 	}
 
 	return plugin.forEachNetwork(&podNetwork, true, func(network *cniNetwork, podNetwork *PodNetwork, rt *libcni.RuntimeConf) error {
@@ -713,7 +713,7 @@ func checkLoopback(netns string) error {
 		}
 
 		if link.Attrs().Flags&net.FlagUp != net.FlagUp {
-			return fmt.Errorf("loopback interface is down")
+			return fmt.Warningf("loopback interface is down")
 		}
 
 		return nil
@@ -797,7 +797,7 @@ func (network *cniNetwork) checkNetwork(ctx context.Context, rt *libcni.RuntimeC
 
 	result, err = cni.GetNetworkListCachedResult(network.config, rt)
 	if err != nil {
-		logrus.Errorf("Error GetNetworkListCachedResult: %v", err)
+		logrus.Errorf("Error getting network list cached result: %v", err)
 		return nil, err
 	} else if result != nil {
 		return result, nil
@@ -850,7 +850,7 @@ func (network *cniNetwork) checkNetwork(ctx context.Context, rt *libcni.RuntimeC
 func (network *cniNetwork) deleteFromNetwork(ctx context.Context, rt *libcni.RuntimeConf, cni *libcni.CNIConfig) error {
 	logrus.Infof("About to del CNI network %s (type=%v)", network.name, network.config.Plugins[0].Network.Type)
 	if err := cni.DelNetworkList(ctx, network.config, rt); err != nil {
-		logrus.Errorf("Error deleting network: %v", err)
+		logrus.Errorf("Error deleting network %s: %v", network.name, err)
 		return err
 	}
 	return nil

pkg/ocicni/ocicni_test.go

@@ -758,6 +758,19 @@ var _ = Describe("ocicni operations", func() {
 			Expect(err).NotTo(HaveOccurred())
 			Expect(fake.delIndex).To(Equal(len(fake.plugins)))
 		})
+		It("verifies that network operations can be locked for a pod using cached networks", func() {
+			podNet.Networks = []NetAttachment{}
+			tmp := ocicni.(*cniNetworkPlugin)
+			Expect(len(tmp.pods)).To(Equal(0))
+			tmp.podLock(podNet)
+			Expect(len(tmp.pods)).To(Equal(1))
+		})
+		It("verifies that network operations can be unlocked for a pod using cached networks", func() {
+			podNet.Networks = []NetAttachment{}
+			tmp := ocicni.(*cniNetworkPlugin)
+			tmp.podUnlock(podNet)
+			Expect(len(tmp.pods)).To(Equal(0))
+		})
 	})
 
 	It("tears down a pod using specified networks when cached info is missing", func() {