Propagate error up instead panic

Author: dixudx

pkg/kubelet/cm/cpumanager/cpu_manager.go

@@ -98,13 +98,7 @@ type manager struct {
 var _ Manager = &manager{}
 
 // NewManager creates new cpu manager based on provided policy
-func NewManager(
-	cpuPolicyName string,
-	reconcilePeriod time.Duration,
-	machineInfo *cadvisorapi.MachineInfo,
-	nodeAllocatableReservation v1.ResourceList,
-	stateFileDirecory string,
-) (Manager, error) {
+func NewManager(cpuPolicyName string, reconcilePeriod time.Duration, machineInfo *cadvisorapi.MachineInfo, nodeAllocatableReservation v1.ResourceList, stateFileDirecory string) (Manager, error) {
 	var policy Policy
 
 	switch policyName(cpuPolicyName) {
@@ -120,18 +114,16 @@ func NewManager(
 		glog.Infof("[cpumanager] detected CPU topology: %v", topo)
 		reservedCPUs, ok := nodeAllocatableReservation[v1.ResourceCPU]
 		if !ok {
-			// The static policy cannot initialize without this information. Panic!
-			panic("[cpumanager] unable to determine reserved CPU resources for static policy")
+			// The static policy cannot initialize without this information.
+			return nil, fmt.Errorf("[cpumanager] unable to determine reserved CPU resources for static policy")
 		}
 		if reservedCPUs.IsZero() {
-			// Panic!
-			//
 			// The static policy requires this to be nonzero. Zero CPU reservation
 			// would allow the shared pool to be completely exhausted. At that point
 			// either we would violate our guarantee of exclusivity or need to evict
 			// any pod that has at least one container that requires zero CPUs.
 			// See the comments in policy_static.go for more details.
-			panic("[cpumanager] the static policy requires systemreserved.cpu + kubereserved.cpu to be greater than zero")
+			return nil, fmt.Errorf("[cpumanager] the static policy requires systemreserved.cpu + kubereserved.cpu to be greater than zero")
 		}
 
 		// Take the ceiling of the reservation, since fractional CPUs cannot be

pkg/kubelet/cm/cpumanager/cpu_manager_test.go

@@ -234,7 +234,6 @@ func TestCPUManagerGenerate(t *testing.T) {
 		cpuPolicyName              string
 		nodeAllocatableReservation v1.ResourceList
 		isTopologyBroken           bool
-		panicMsg                   string
 		expectedPolicy             string
 		expectedError              error
 		skipIfPermissionsError     bool
@@ -270,14 +269,14 @@ func TestCPUManagerGenerate(t *testing.T) {
 			description:                "static policy - broken reservation",
 			cpuPolicyName:              "static",
 			nodeAllocatableReservation: v1.ResourceList{},
-			panicMsg:                   "unable to determine reserved CPU resources for static policy",
+			expectedError:              fmt.Errorf("unable to determine reserved CPU resources for static policy"),
 			skipIfPermissionsError:     true,
 		},
 		{
 			description:                "static policy - no CPU resources",
 			cpuPolicyName:              "static",
 			nodeAllocatableReservation: v1.ResourceList{v1.ResourceCPU: *resource.NewQuantity(0, resource.DecimalSI)},
-			panicMsg:                   "the static policy requires systemreserved.cpu + kubereserved.cpu to be greater than zero",
+			expectedError:              fmt.Errorf("the static policy requires systemreserved.cpu + kubereserved.cpu to be greater than zero"),
 			skipIfPermissionsError:     true,
 		},
 	}
@@ -319,19 +318,6 @@ func TestCPUManagerGenerate(t *testing.T) {
 				t.Errorf("cannot create state file: %s", err.Error())
 			}
 			defer os.RemoveAll(sDir)
-			defer func() {
-				if err := recover(); err != nil {
-					if testCase.panicMsg != "" {
-						if !strings.Contains(err.(string), testCase.panicMsg) {
-							t.Errorf("Unexpected panic message. Have: %q wants %q", err, testCase.panicMsg)
-						}
-					} else {
-						t.Errorf("Unexpected panic: %q", err)
-					}
-				} else if testCase.panicMsg != "" {
-					t.Error("Expected panic hasn't been raised")
-				}
-			}()
 
 			mgr, err := NewManager(testCase.cpuPolicyName, 5*time.Second, machineInfo, testCase.nodeAllocatableReservation, sDir)
 			if testCase.expectedError != nil {