From 937d3303930e17a43f49cffd41e8910956685e85 Mon Sep 17 00:00:00 2001 From: Swati Sehgal Date: Fri, 3 Mar 2023 11:34:34 +0000 Subject: [PATCH] node: topologymgr: Remove ResourceAllocator as TM is always enabled With Topology Manager enabled by default, we no longer need `resourceAllocator` as Topology Manager serves as the main PodAdmitHandler completely responsible for admission check based on hints received from the hintProviders and the subsequent allocation of the corresponding resources to a pod as can be seen here: https://github.com/kubernetes/kubernetes/blob/v1.26.0/pkg/kubelet/cm/topologymanager/scope.go#L150 With regard to DRA, the passing of `cm.draManager` into resourceAllocator seems redundant as no admission checks (and allocation of resources handled by DRA) is taking place in `Admit` method of resourceAllocator. DRA has a completely different model to the rest of the resource managers where pod is only scheduled on a node once resources are reserved for it. Because of this, admission checks or waiting for resources to be provisioned after the pod has been scheduled on the node is not required. Before making the above change, it was verified that DRA Manager is instantiated in `NewContainerManager`: https://github.com/kubernetes/kubernetes/blob/v1.26.0/pkg/kubelet/cm/container_manager_linux.go#L318 Signed-off-by: Swati Sehgal --- pkg/kubelet/cm/container_manager_linux.go | 46 +---------------------- 1 file changed, 1 insertion(+), 45 deletions(-) diff --git a/pkg/kubelet/cm/container_manager_linux.go b/pkg/kubelet/cm/container_manager_linux.go index e54e12bfc5b..70e63c09028 100644 --- a/pkg/kubelet/cm/container_manager_linux.go +++ b/pkg/kubelet/cm/container_manager_linux.go @@ -51,7 +51,6 @@ import ( podresourcesapi "k8s.io/kubelet/pkg/apis/podresources/v1" kubefeatures "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/cadvisor" - "k8s.io/kubernetes/pkg/kubelet/cm/admission" "k8s.io/kubernetes/pkg/kubelet/cm/containermap" "k8s.io/kubernetes/pkg/kubelet/cm/cpumanager" "k8s.io/kubernetes/pkg/kubelet/cm/devicemanager" @@ -682,50 +681,7 @@ func (cm *containerManagerImpl) UpdatePluginResources(node *schedulerframework.N } func (cm *containerManagerImpl) GetAllocateResourcesPodAdmitHandler() lifecycle.PodAdmitHandler { - if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.TopologyManager) { - return cm.topologyManager - } - // TODO: we need to think about a better way to do this. This will work for - // now so long as we have only the cpuManager and deviceManager relying on - // allocations here. However, going forward it is not generalized enough to - // work as we add more and more hint providers that the TopologyManager - // needs to call Allocate() on (that may not be directly intstantiated - // inside this component). - return &resourceAllocator{cm.cpuManager, cm.memoryManager, cm.deviceManager, cm.draManager} -} - -type resourceAllocator struct { - cpuManager cpumanager.Manager - memoryManager memorymanager.Manager - deviceManager devicemanager.Manager - draManager dra.Manager -} - -func (m *resourceAllocator) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitResult { - pod := attrs.Pod - - for _, container := range append(pod.Spec.InitContainers, pod.Spec.Containers...) { - err := m.deviceManager.Allocate(pod, &container) - if err != nil { - return admission.GetPodAdmitResult(err) - } - - if m.cpuManager != nil { - err = m.cpuManager.Allocate(pod, &container) - if err != nil { - return admission.GetPodAdmitResult(err) - } - } - - if m.memoryManager != nil { - err = m.memoryManager.Allocate(pod, &container) - if err != nil { - return admission.GetPodAdmitResult(err) - } - } - } - - return admission.GetPodAdmitResult(nil) + return cm.topologyManager } func (cm *containerManagerImpl) SystemCgroupsLimit() v1.ResourceList {