diff --git a/pkg/kubelet/cm/topologymanager/topology_manager.go b/pkg/kubelet/cm/topologymanager/topology_manager.go index 03e9d80db01..6af26b6cedd 100644 --- a/pkg/kubelet/cm/topologymanager/topology_manager.go +++ b/pkg/kubelet/cm/topologymanager/topology_manager.go @@ -17,6 +17,8 @@ limitations under the License. package topologymanager import ( + "fmt" + "k8s.io/api/core/v1" "k8s.io/klog" "k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/socketmask" @@ -71,7 +73,7 @@ type TopologyHint struct { var _ Manager = &manager{} //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) var policy Policy @@ -87,8 +89,7 @@ func NewManager(topologyPolicyName string) Manager { policy = NewStrictPolicy() default: - klog.Errorf("[topologymanager] Unknown policy %s, using default policy %s", topologyPolicyName, PolicyNone) - policy = NewNonePolicy() + return nil, fmt.Errorf("unknown policy: \"%s\"", topologyPolicyName) } var hp []HintProvider @@ -101,7 +102,7 @@ func NewManager(topologyPolicyName string) Manager { policy: policy, } - return manager + return manager, nil } func (m *manager) GetAffinity(podUID string, containerName string) TopologyHint { diff --git a/pkg/kubelet/cm/topologymanager/topology_manager_test.go b/pkg/kubelet/cm/topologymanager/topology_manager_test.go index c14a1a17cb1..48c38f6209e 100644 --- a/pkg/kubelet/cm/topologymanager/topology_manager_test.go +++ b/pkg/kubelet/cm/topologymanager/topology_manager_test.go @@ -17,7 +17,9 @@ limitations under the License. package topologymanager import ( + "fmt" "reflect" + "strings" "testing" "k8s.io/api/core/v1" @@ -39,28 +41,40 @@ func NewTestSocketMaskFull() socketmask.SocketMask { func TestNewManager(t *testing.T) { tcases := []struct { - name string - policyType string + description string + policyName string + expectedPolicy string + expectedError error }{ { - name: "Policy is set preferred", - policyType: "preferred", + description: "Policy is set preferred", + policyName: "preferred", + expectedPolicy: "preferred", }, { - name: "Policy is set to strict", - policyType: "strict", + description: "Policy is set to strict", + policyName: "strict", + expectedPolicy: "strict", }, { - name: "Policy is set to unknown", - policyType: "unknown", + description: "Policy is set to unknown", + policyName: "unknown", + expectedError: fmt.Errorf("unknown policy: \"unknown\""), }, } for _, tc := range tcases { - mngr := NewManager(tc.policyType) + mngr, err := NewManager(tc.policyName) - if _, ok := mngr.(Manager); !ok { - t.Errorf("result is not Manager type") + if tc.expectedError != nil { + 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) + } } } }