Skip to content

Commit eddb00e

Browse files
author
Kubernetes Submit Queue
authored
Merge pull request kubernetes#57247 from dixudx/cm_return_err
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. cpumanager: Propagate error up instead panic **What this PR does / why we need it**: **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes kubernetes#57239 **Special notes for your reviewer**: /assign @sjenning **Release note**: ```release-note None ```
2 parents 1711705 + d474b86 commit eddb00e

File tree

2 files changed

+6
-28
lines changed

2 files changed

+6
-28
lines changed

pkg/kubelet/cm/cpumanager/cpu_manager.go

+4-12
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,7 @@ type manager struct {
9898
var _ Manager = &manager{}
9999

100100
// NewManager creates new cpu manager based on provided policy
101-
func NewManager(
102-
cpuPolicyName string,
103-
reconcilePeriod time.Duration,
104-
machineInfo *cadvisorapi.MachineInfo,
105-
nodeAllocatableReservation v1.ResourceList,
106-
stateFileDirecory string,
107-
) (Manager, error) {
101+
func NewManager(cpuPolicyName string, reconcilePeriod time.Duration, machineInfo *cadvisorapi.MachineInfo, nodeAllocatableReservation v1.ResourceList, stateFileDirecory string) (Manager, error) {
108102
var policy Policy
109103

110104
switch policyName(cpuPolicyName) {
@@ -120,18 +114,16 @@ func NewManager(
120114
glog.Infof("[cpumanager] detected CPU topology: %v", topo)
121115
reservedCPUs, ok := nodeAllocatableReservation[v1.ResourceCPU]
122116
if !ok {
123-
// The static policy cannot initialize without this information. Panic!
124-
panic("[cpumanager] unable to determine reserved CPU resources for static policy")
117+
// The static policy cannot initialize without this information.
118+
return nil, fmt.Errorf("[cpumanager] unable to determine reserved CPU resources for static policy")
125119
}
126120
if reservedCPUs.IsZero() {
127-
// Panic!
128-
//
129121
// The static policy requires this to be nonzero. Zero CPU reservation
130122
// would allow the shared pool to be completely exhausted. At that point
131123
// either we would violate our guarantee of exclusivity or need to evict
132124
// any pod that has at least one container that requires zero CPUs.
133125
// See the comments in policy_static.go for more details.
134-
panic("[cpumanager] the static policy requires systemreserved.cpu + kubereserved.cpu to be greater than zero")
126+
return nil, fmt.Errorf("[cpumanager] the static policy requires systemreserved.cpu + kubereserved.cpu to be greater than zero")
135127
}
136128

137129
// Take the ceiling of the reservation, since fractional CPUs cannot be

pkg/kubelet/cm/cpumanager/cpu_manager_test.go

+2-16
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,6 @@ func TestCPUManagerGenerate(t *testing.T) {
234234
cpuPolicyName string
235235
nodeAllocatableReservation v1.ResourceList
236236
isTopologyBroken bool
237-
panicMsg string
238237
expectedPolicy string
239238
expectedError error
240239
skipIfPermissionsError bool
@@ -270,14 +269,14 @@ func TestCPUManagerGenerate(t *testing.T) {
270269
description: "static policy - broken reservation",
271270
cpuPolicyName: "static",
272271
nodeAllocatableReservation: v1.ResourceList{},
273-
panicMsg: "unable to determine reserved CPU resources for static policy",
272+
expectedError: fmt.Errorf("unable to determine reserved CPU resources for static policy"),
274273
skipIfPermissionsError: true,
275274
},
276275
{
277276
description: "static policy - no CPU resources",
278277
cpuPolicyName: "static",
279278
nodeAllocatableReservation: v1.ResourceList{v1.ResourceCPU: *resource.NewQuantity(0, resource.DecimalSI)},
280-
panicMsg: "the static policy requires systemreserved.cpu + kubereserved.cpu to be greater than zero",
279+
expectedError: fmt.Errorf("the static policy requires systemreserved.cpu + kubereserved.cpu to be greater than zero"),
281280
skipIfPermissionsError: true,
282281
},
283282
}
@@ -319,19 +318,6 @@ func TestCPUManagerGenerate(t *testing.T) {
319318
t.Errorf("cannot create state file: %s", err.Error())
320319
}
321320
defer os.RemoveAll(sDir)
322-
defer func() {
323-
if err := recover(); err != nil {
324-
if testCase.panicMsg != "" {
325-
if !strings.Contains(err.(string), testCase.panicMsg) {
326-
t.Errorf("Unexpected panic message. Have: %q wants %q", err, testCase.panicMsg)
327-
}
328-
} else {
329-
t.Errorf("Unexpected panic: %q", err)
330-
}
331-
} else if testCase.panicMsg != "" {
332-
t.Error("Expected panic hasn't been raised")
333-
}
334-
}()
335321

336322
mgr, err := NewManager(testCase.cpuPolicyName, 5*time.Second, machineInfo, testCase.nodeAllocatableReservation, sDir)
337323
if testCase.expectedError != nil {

0 commit comments

Comments
 (0)