Merge pull request #80294 from klueska/upstream-fail-fast-cpumanager-topologymanager

Update the cpumanager and topologymanager to error out if an invalid policy is given
This commit is contained in:
Kubernetes Prow Robot 2019-07-23 11:40:14 -07:00 committed by GitHub
commit 304e2c247a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 32 additions and 18 deletions

View File

@ -132,8 +132,7 @@ func NewManager(cpuPolicyName string, reconcilePeriod time.Duration, machineInfo
policy = NewStaticPolicy(topo, numReservedCPUs) policy = NewStaticPolicy(topo, numReservedCPUs)
default: default:
klog.Errorf("[cpumanager] Unknown policy \"%s\", falling back to default policy \"%s\"", cpuPolicyName, PolicyNone) return nil, fmt.Errorf("unknown policy: \"%s\"", cpuPolicyName)
policy = NewNonePolicy()
} }
stateImpl, err := state.NewCheckpointState(stateFileDirectory, cpuManagerStateFileName, policy.Name()) stateImpl, err := state.NewCheckpointState(stateFileDirectory, cpuManagerStateFileName, policy.Name())

View File

@ -232,7 +232,7 @@ func TestCPUManagerGenerate(t *testing.T) {
description: "invalid policy name", description: "invalid policy name",
cpuPolicyName: "invalid", cpuPolicyName: "invalid",
nodeAllocatableReservation: nil, nodeAllocatableReservation: nil,
expectedPolicy: "none", expectedError: fmt.Errorf("unknown policy: \"invalid\""),
}, },
{ {
description: "static policy", description: "static policy",

View File

@ -17,6 +17,8 @@ limitations under the License.
package topologymanager package topologymanager
import ( import (
"fmt"
"k8s.io/api/core/v1" "k8s.io/api/core/v1"
"k8s.io/klog" "k8s.io/klog"
"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/socketmask" "k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/socketmask"
@ -71,7 +73,7 @@ type TopologyHint struct {
var _ Manager = &manager{} var _ Manager = &manager{}
//NewManager creates a new TopologyManager based on provided policy //NewManager creates a new TopologyManager based on provided policy
func NewManager(topologyPolicyName string) Manager { func NewManager(topologyPolicyName string) (Manager, error) {
klog.Infof("[topologymanager] Creating topology manager with %s policy", topologyPolicyName) klog.Infof("[topologymanager] Creating topology manager with %s policy", topologyPolicyName)
var policy Policy var policy Policy
@ -87,8 +89,7 @@ func NewManager(topologyPolicyName string) Manager {
policy = NewStrictPolicy() policy = NewStrictPolicy()
default: default:
klog.Errorf("[topologymanager] Unknown policy %s, using default policy %s", topologyPolicyName, PolicyNone) return nil, fmt.Errorf("unknown policy: \"%s\"", topologyPolicyName)
policy = NewNonePolicy()
} }
var hp []HintProvider var hp []HintProvider
@ -101,7 +102,7 @@ func NewManager(topologyPolicyName string) Manager {
policy: policy, policy: policy,
} }
return manager return manager, nil
} }
func (m *manager) GetAffinity(podUID string, containerName string) TopologyHint { func (m *manager) GetAffinity(podUID string, containerName string) TopologyHint {

View File

@ -17,7 +17,9 @@ limitations under the License.
package topologymanager package topologymanager
import ( import (
"fmt"
"reflect" "reflect"
"strings"
"testing" "testing"
"k8s.io/api/core/v1" "k8s.io/api/core/v1"
@ -39,28 +41,40 @@ func NewTestSocketMaskFull() socketmask.SocketMask {
func TestNewManager(t *testing.T) { func TestNewManager(t *testing.T) {
tcases := []struct { tcases := []struct {
name string description string
policyType string policyName string
expectedPolicy string
expectedError error
}{ }{
{ {
name: "Policy is set preferred", description: "Policy is set preferred",
policyType: "preferred", policyName: "preferred",
expectedPolicy: "preferred",
}, },
{ {
name: "Policy is set to strict", description: "Policy is set to strict",
policyType: "strict", policyName: "strict",
expectedPolicy: "strict",
}, },
{ {
name: "Policy is set to unknown", description: "Policy is set to unknown",
policyType: "unknown", policyName: "unknown",
expectedError: fmt.Errorf("unknown policy: \"unknown\""),
}, },
} }
for _, tc := range tcases { for _, tc := range tcases {
mngr := NewManager(tc.policyType) mngr, err := NewManager(tc.policyName)
if _, ok := mngr.(Manager); !ok { if tc.expectedError != nil {
t.Errorf("result is not Manager type") if !strings.Contains(err.Error(), tc.expectedError.Error()) {
t.Errorf("Unexpected error message. Have: %s wants %s", err.Error(), tc.expectedError.Error())
}
} else {
rawMgr := mngr.(*manager)
if rawMgr.policy.Name() != tc.expectedPolicy {
t.Errorf("Unexpected policy name. Have: %q wants %q", rawMgr.policy.Name(), tc.expectedPolicy)
}
} }
} }
} }