Merge pull request #116738 from AxeZhan/TopologyManagerPolicy

When TopologyManagerPolicy is None, skip checks in NewManager.

pkg/kubelet/cm/topologymanager/scope.go

@@ -31,6 +31,8 @@ const (
 	containerTopologyScope = "container"
 	// podTopologyScope specifies the TopologyManagerScope per pod.
 	podTopologyScope = "pod"
+	// noneTopologyScope specifies the TopologyManagerScope when topologyPolicyName is none.
+	noneTopologyScope = "none"
 )
 
 type podTopologyHints map[string]map[string]TopologyHint

pkg/kubelet/cm/topologymanager/scope_container.go

@@ -45,11 +45,6 @@ func NewContainerScope(policy Policy) Scope {
 }
 
 func (s *containerScope) Admit(pod *v1.Pod) lifecycle.PodAdmitResult {
-	// Exception - Policy : none
-	if s.policy.Name() == PolicyNone {
-		return s.admitPolicyNone(pod)
-	}
-
 	for _, container := range append(pod.Spec.InitContainers, pod.Spec.Containers...) {
 		bestHint, admit := s.calculateAffinity(pod, &container)
 		klog.InfoS("Best TopologyHint", "bestHint", bestHint, "pod", klog.KObj(pod), "containerName", container.Name)

pkg/kubelet/cm/topologymanager/scope_none.go

@@ -0,0 +1,46 @@
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package topologymanager
+
+import (
+	"k8s.io/api/core/v1"
+	"k8s.io/kubernetes/pkg/kubelet/cm/containermap"
+	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
+)
+
+type noneScope struct {
+	scope
+}
+
+// Ensure noneScope implements Scope interface
+var _ Scope = &noneScope{}
+
+// NewNoneScope returns a none scope.
+func NewNoneScope() Scope {
+	return &noneScope{
+		scope{
+			name:             noneTopologyScope,
+			podTopologyHints: podTopologyHints{},
+			policy:           NewNonePolicy(),
+			podMap:           containermap.NewContainerMap(),
+		},
+	}
+}
+
+func (s *noneScope) Admit(pod *v1.Pod) lifecycle.PodAdmitResult {
+	return s.admitPolicyNone(pod)
+}

pkg/kubelet/cm/topologymanager/scope_pod.go

@@ -45,11 +45,6 @@ func NewPodScope(policy Policy) Scope {
 }
 
 func (s *podScope) Admit(pod *v1.Pod) lifecycle.PodAdmitResult {
-	// Exception - Policy : none
-	if s.policy.Name() == PolicyNone {
-		return s.admitPolicyNone(pod)
-	}
-
 	bestHint, admit := s.calculateAffinity(pod)
 	klog.InfoS("Best TopologyHint", "bestHint", bestHint, "pod", klog.KObj(pod))
 	if !admit {

pkg/kubelet/cm/topologymanager/topology_manager.go

@@ -135,6 +135,11 @@ var _ Manager = &manager{}
 func NewManager(topology []cadvisorapi.Node, topologyPolicyName string, topologyScopeName string, topologyPolicyOptions map[string]string) (Manager, error) {
 	klog.InfoS("Creating topology manager with policy per scope", "topologyPolicyName", topologyPolicyName, "topologyScopeName", topologyScopeName)
 
+	// When policy is none, the scope is not relevant, so we can short circuit here.
+	if topologyPolicyName == PolicyNone {
+		return &manager{scope: NewNoneScope()}, nil
+	}
+
 	opts, err := NewPolicyOptions(topologyPolicyOptions)
 	if err != nil {
 		return nil, err
@@ -152,9 +157,6 @@ func NewManager(topology []cadvisorapi.Node, topologyPolicyName string, topology
 	var policy Policy
 	switch topologyPolicyName {
 
-	case PolicyNone:
-		policy = NewNonePolicy()
-
 	case PolicyBestEffort:
 		policy = NewBestEffortPolicy(numaInfo, opts)
 

pkg/kubelet/cm/topologymanager/topology_manager_test.go

@@ -98,9 +98,14 @@ func TestNewManager(t *testing.T) {
 			}
 		} else {
 			rawMgr := mngr.(*manager)
-			rawScope := rawMgr.scope.(*containerScope)
+			var policyName string
-			if rawScope.policy.Name() != tc.expectedPolicy {
+			if rawScope, ok := rawMgr.scope.(*containerScope); ok {
-				t.Errorf("Unexpected policy name. Have: %q wants %q", rawScope.policy.Name(), tc.expectedPolicy)
+				policyName = rawScope.policy.Name()
+			} else if rawScope, ok := rawMgr.scope.(*noneScope); ok {
+				policyName = rawScope.policy.Name()
+			}
+			if policyName != tc.expectedPolicy {
+				t.Errorf("Unexpected policy name. Have: %q wants %q", policyName, tc.expectedPolicy)
 			}
 		}
 	}