Update the topologymanager to error out if an invalid policy is given

Previously, the topologymanager would simply fall back to the None() policy
if an invalid policy was specified. This patch updates this to return an
error when an invalid policy is passed, forcing the kubelet to fail
fast when this occurs.

These semantics should be preferable because an invalid policy likely
indicates operator error in setting the policy flag on the kubelet
correctly (e.g. misspelling 'strict' as 'striict'). In this case it is
better to fail fast so the operator can detect this and correct the
mistake, than to mask the error and essentially disable the
topologymanager unexpectedly.

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 {

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)
+			}
 		}
 	}
 }