correct PopDevice (panic case)

update TestPopDevice
nokia · Mar 10, 2019 · 3c8a60f · 3c8a60f
1 parent 2645180
commit 3c8a60f
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 20 deletions.
diff --git a/pkg/danm/danm.go b/pkg/danm/danm.go
@@ -188,11 +188,7 @@ func extractConnections(args *cniArgs) error {
   return nil
 }
 
-func getAllocatedDevices(args *cniArgs, devicePool string)(*[]string, error){
-  checkpoint, err := checkpoint_utils.GetCheckpoint()
-  if err != nil {
-    return nil, errors.New("failed to instantiate checkpoint object due to:" + err.Error())
-  }
+func getAllocatedDevices(args *cniArgs, checkpoint checkpoint_utils.Checkpoint, devicePool string)(*[]string, error){
   resourceMap, err := checkpoint.GetComputeDeviceMap(string(args.podUid))
   if err != nil || len(resourceMap) == 0 {
     return nil, errors.New("failed to retrieve Pod info from checkpoint object due to:" + err.Error())
@@ -204,6 +200,7 @@ func getAllocatedDevices(args *cniArgs, devicePool string)(*[]string, error){
 }
 
 func popDevice(devicePool string, allocatedDevices map[string]*[]string)(string, error) {
+  if len(allocatedDevices) == 0 { return "", errors.New("allocatedDevices is empty") }
   devices := (*allocatedDevices[devicePool])
   if len(devices) == 0 { return "", errors.New("devicePool is empty") }
   device, devices := devices[len(devices)-1], devices[:len(devices)-1]
@@ -218,6 +215,11 @@ func setupNetworking(args *cniArgs) (*current.Result, error) {
   }
   syncher := syncher.NewSyncher(len(args.interfaces))
   allocatedDevices := make(map[string]*[]string)
+
+  checkpoint, err := checkpoint_utils.GetCheckpoint()
+  if err != nil {
+    return nil, errors.New("failed to instantiate checkpoint object due to:" + err.Error())
+  }
   var cniRes *current.Result
   for nicID, nicParams := range args.interfaces {
     isDelegationRequired, netInfo, err := cnidel.IsDelegationRequired(danmClient, nicParams.Network, args.nameSpace)
@@ -228,7 +230,7 @@ func setupNetworking(args *cniArgs) (*current.Result, error) {
     if isDelegationRequired {
       if cnidel.IsDeviceNeeded(netInfo.Spec.NetworkType) {
         if _, ok := allocatedDevices[netInfo.Spec.Options.DevicePool]; !ok {
-          allocatedDevices[netInfo.Spec.Options.DevicePool], err = getAllocatedDevices(args, netInfo.Spec.Options.DevicePool)
+          allocatedDevices[netInfo.Spec.Options.DevicePool], err = getAllocatedDevices(args, checkpoint, netInfo.Spec.Options.DevicePool)
           if err != nil {
             return cniRes, errors.New("failed to get allocated devices due to:" + err.Error())
           }

diff --git a/pkg/danm/danm_test.go b/pkg/danm/danm_test.go
@@ -7,23 +7,18 @@ import (
 var devicePool0 = "pool0"
 var devicePool1 = "pool1"
 
-var allocatedDevices = make(map[string]*[]string)
+func TestPopDevice(t *testing.T) {
+  allocatedDevices := make(map[string]*[]string)
 
-func TestPopDevicePanic(t *testing.T) {
-    defer func() {
-        if r := recover(); r == nil {
-            t.Errorf("Uninitialized map should expect error.")
-        }
-    }()
-
-    popDevice(devicePool0, allocatedDevices)
-}
+  device,err := popDevice(devicePool0, allocatedDevices)
+  if err == nil {
+    t.Errorf("Uninitialized map should expect error.")
+  }
 
-func TestPopDevice(t *testing.T) {  
   allocatedDevices[devicePool0] = &[]string{"device0"}
   allocatedDevices[devicePool1] = &[]string{"device1", "device2"}
-  
-  device,err := popDevice(devicePool1, allocatedDevices)
+
+  device,err = popDevice(devicePool1, allocatedDevices)
   if device != "device2"  || err != nil {
     t.Errorf("Received device or error does not match with expectation.")
   }
@@ -32,7 +27,7 @@ func TestPopDevice(t *testing.T) {
     t.Errorf("Received device or error does not match with expectation.")
   }
   device,err = popDevice(devicePool1, allocatedDevices)
-  if device == ""  && err == nil {
+  if err == nil {
     t.Errorf("Empty pool should expect error.")
   }
 }