From 0d68bffd03f1af15fc8cbb3eaeda9ff95a47eeab Mon Sep 17 00:00:00 2001 From: Kevin Klues Date: Sun, 2 Feb 2020 22:03:01 +0000 Subject: [PATCH] Change GetTopologyPodAdmitHandler() to be more general GetTopologyPodAdmitHandler() now returns a lifecycle.PodAdmitHandler type instead of the TopologyManager directly. The handler it returns is generally responsible for attempting to allocate any resources that require a pod admission check. When the TopologyManager feature gate is on, this comes directly from the TopologyManager. When it is off, we simply attempt the allocations ourselves and fail the admission on an unexpected error. The higher level kubelet.go feature gate check will be removed in an upcoming PR. --- pkg/kubelet/cm/container_manager.go | 5 +-- pkg/kubelet/cm/container_manager_linux.go | 48 ++++++++++++++++++--- pkg/kubelet/cm/container_manager_stub.go | 4 +- pkg/kubelet/cm/container_manager_windows.go | 2 +- 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/pkg/kubelet/cm/container_manager.go b/pkg/kubelet/cm/container_manager.go index caa3de666c7..4e9501da4e3 100644 --- a/pkg/kubelet/cm/container_manager.go +++ b/pkg/kubelet/cm/container_manager.go @@ -25,7 +25,6 @@ import ( internalapi "k8s.io/cri-api/pkg/apis" podresourcesapi "k8s.io/kubernetes/pkg/kubelet/apis/podresources/v1alpha1" "k8s.io/kubernetes/pkg/kubelet/cm/cpuset" - "k8s.io/kubernetes/pkg/kubelet/cm/topologymanager" "k8s.io/kubernetes/pkg/kubelet/config" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" evictionapi "k8s.io/kubernetes/pkg/kubelet/eviction/api" @@ -111,8 +110,8 @@ type ContainerManager interface { // due to node recreation. ShouldResetExtendedResourceCapacity() bool - // GetTopologyPodAdmitHandler returns an instance of the TopologyManager for Pod Admission - GetTopologyPodAdmitHandler() topologymanager.Manager + // GetTopologyPodAdmitHandler returns an instance of a PodAdmitHandler responsible for allocating pod resources. + GetTopologyPodAdmitHandler() lifecycle.PodAdmitHandler // UpdateAllocatedDevices frees any Devices that are bound to terminated pods. UpdateAllocatedDevices() diff --git a/pkg/kubelet/cm/container_manager_linux.go b/pkg/kubelet/cm/container_manager_linux.go index 5de1d0a25d3..f962c298c7c 100644 --- a/pkg/kubelet/cm/container_manager_linux.go +++ b/pkg/kubelet/cm/container_manager_linux.go @@ -672,15 +672,51 @@ func (cm *containerManagerImpl) GetResources(pod *v1.Pod, container *v1.Containe } func (cm *containerManagerImpl) UpdatePluginResources(node *schedulernodeinfo.NodeInfo, attrs *lifecycle.PodAdmitAttributes) error { - err := cm.deviceManager.Allocate(attrs.Pod) - if err != nil { - return err - } return cm.deviceManager.UpdatePluginResources(node, attrs) } -func (cm *containerManagerImpl) GetTopologyPodAdmitHandler() topologymanager.Manager { - return cm.topologyManager +func (cm *containerManagerImpl) GetTopologyPodAdmitHandler() 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.deviceManager} +} + +type resourceAllocator struct { + cpuManager cpumanager.Manager + deviceManager devicemanager.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 lifecycle.PodAdmitResult{ + Message: fmt.Sprintf("Allocate failed due to %v, which is unexpected", err), + Reason: "UnexpectedAdmissionError", + Admit: false, + } + } + + err = m.cpuManager.Allocate(pod, &container) + if err != nil { + return lifecycle.PodAdmitResult{ + Message: fmt.Sprintf("Allocate failed due to %v, which is unexpected", err), + Reason: "UnexpectedAdmissionError", + Admit: false, + } + } + } + + return lifecycle.PodAdmitResult{Admit: true} } func (cm *containerManagerImpl) SystemCgroupsLimit() v1.ResourceList { diff --git a/pkg/kubelet/cm/container_manager_stub.go b/pkg/kubelet/cm/container_manager_stub.go index c21e8cce0dc..5c0caa635eb 100644 --- a/pkg/kubelet/cm/container_manager_stub.go +++ b/pkg/kubelet/cm/container_manager_stub.go @@ -117,8 +117,8 @@ func (cm *containerManagerStub) ShouldResetExtendedResourceCapacity() bool { return cm.shouldResetExtendedResourceCapacity } -func (cm *containerManagerStub) GetTopologyPodAdmitHandler() topologymanager.Manager { - return nil +func (cm *containerManagerStub) GetTopologyPodAdmitHandler() lifecycle.PodAdmitHandler { + return topologymanager.NewFakeManager() } func (cm *containerManagerStub) UpdateAllocatedDevices() { diff --git a/pkg/kubelet/cm/container_manager_windows.go b/pkg/kubelet/cm/container_manager_windows.go index 13271b98c91..813d13620a7 100644 --- a/pkg/kubelet/cm/container_manager_windows.go +++ b/pkg/kubelet/cm/container_manager_windows.go @@ -178,7 +178,7 @@ func (cm *containerManagerImpl) ShouldResetExtendedResourceCapacity() bool { return false } -func (cm *containerManagerImpl) GetTopologyPodAdmitHandler() topologymanager.Manager { +func (cm *containerManagerImpl) GetTopologyPodAdmitHandler() lifecycle.PodAdmitHandler { return nil }