diff --git a/pkg/kubelet/cm/dra/manager.go b/pkg/kubelet/cm/dra/manager.go index 4eca61103116..2596646c473c 100644 --- a/pkg/kubelet/cm/dra/manager.go +++ b/pkg/kubelet/cm/dra/manager.go @@ -67,23 +67,10 @@ func NewManagerImpl(kubeClient clientset.Interface, stateFileDirectory string) ( // containerResources on success. func (m *ManagerImpl) PrepareResources(pod *v1.Pod) error { for i := range pod.Spec.ResourceClaims { - claimName := resourceclaim.Name(pod, &pod.Spec.ResourceClaims[i]) + podClaim := &pod.Spec.ResourceClaims[i] + claimName := resourceclaim.Name(pod, podClaim) klog.V(3).InfoS("Processing resource", "claim", claimName, "pod", pod.Name) - // Resource is already prepared, add pod UID to it - if claimInfo := m.cache.get(claimName, pod.Namespace); claimInfo != nil { - // We delay checkpointing of this change until this call - // returns successfully. It is OK to do this because we - // will only return successfully from this call if the - // checkpoint has succeeded. That means if the kubelet is - // ever restarted before this checkpoint succeeds, the pod - // whose resources are being prepared would never have - // started, so it's OK (actually correct) to not include it - // in the cache. - claimInfo.addPodReference(pod.UID) - continue - } - // Query claim object from the API server resourceClaim, err := m.kubeClient.ResourceV1alpha2().ResourceClaims(pod.Namespace).Get( context.TODO(), @@ -99,6 +86,27 @@ func (m *ManagerImpl) PrepareResources(pod *v1.Pod) error { pod.Name, pod.UID, claimName, resourceClaim.UID) } + // If no container actually uses the claim, then we don't need + // to prepare it. + if !claimIsUsedByPod(podClaim, pod) { + klog.V(5).InfoS("Skipping unused resource", "claim", claimName, "pod", pod.Name) + continue + } + + // Is the resource already prepared? Then add the pod UID to it. + if claimInfo := m.cache.get(claimName, pod.Namespace); claimInfo != nil { + // We delay checkpointing of this change until this call + // returns successfully. It is OK to do this because we + // will only return successfully from this call if the + // checkpoint has succeeded. That means if the kubelet is + // ever restarted before this checkpoint succeeds, the pod + // whose resources are being prepared would never have + // started, so it's OK (actually correct) to not include it + // in the cache. + claimInfo.addPodReference(pod.UID) + continue + } + // Grab the allocation.resourceHandles. If there are no // allocation.resourceHandles, create a single resourceHandle with no // content. This will trigger processing of this claim by a single @@ -178,6 +186,34 @@ func (m *ManagerImpl) PrepareResources(pod *v1.Pod) error { return nil } +func claimIsUsedByPod(podClaim *v1.PodResourceClaim, pod *v1.Pod) bool { + if claimIsUsedByContainers(podClaim, pod.Spec.InitContainers) { + return true + } + if claimIsUsedByContainers(podClaim, pod.Spec.Containers) { + return true + } + return false +} + +func claimIsUsedByContainers(podClaim *v1.PodResourceClaim, containers []v1.Container) bool { + for i := range containers { + if claimIsUsedByContainer(podClaim, &containers[i]) { + return true + } + } + return false +} + +func claimIsUsedByContainer(podClaim *v1.PodResourceClaim, container *v1.Container) bool { + for _, c := range container.Resources.Claims { + if c.Name == podClaim.Name { + return true + } + } + return false +} + // GetResources gets a ContainerInfo object from the claimInfo cache. // This information is used by the caller to update a container config. func (m *ManagerImpl) GetResources(pod *v1.Pod, container *v1.Container) (*ContainerInfo, error) { diff --git a/test/e2e/dra/dra.go b/test/e2e/dra/dra.go index 59dc3376da3c..3d77fd93bd05 100644 --- a/test/e2e/dra/dra.go +++ b/test/e2e/dra/dra.go @@ -61,11 +61,11 @@ var _ = ginkgo.Describe("[sig-node] DRA [Feature:DynamicResourceAllocation]", fu nodes := NewNodes(f, 1, 1) driver := NewDriver(f, nodes, networkResources) // All tests get their own driver instance. b := newBuilder(f, driver) + ginkgo.It("registers plugin", func() { ginkgo.By("the driver is running") }) - // This test does not pass at the moment because kubelet doesn't retry. ginkgo.It("must retry NodePrepareResource", func(ctx context.Context) { // We have exactly one host. m := MethodInstance{driver.Nodenames()[0], NodePrepareResourceMethod} @@ -95,6 +95,7 @@ var _ = ginkgo.Describe("[sig-node] DRA [Feature:DynamicResourceAllocation]", fu framework.Fail("NodePrepareResource should have been called again") } }) + ginkgo.It("must not run a pod if a claim is not reserved for it", func(ctx context.Context) { parameters := b.parameters() claim := b.externalClaim(resourcev1alpha2.AllocationModeImmediate) @@ -118,6 +119,7 @@ var _ = ginkgo.Describe("[sig-node] DRA [Feature:DynamicResourceAllocation]", fu return nil }, 20*time.Second, 200*time.Millisecond).Should(gomega.BeNil()) }) + ginkgo.It("must unprepare resources for force-deleted pod", func(ctx context.Context) { parameters := b.parameters() claim := b.externalClaim(resourcev1alpha2.AllocationModeImmediate) @@ -140,6 +142,19 @@ var _ = ginkgo.Describe("[sig-node] DRA [Feature:DynamicResourceAllocation]", fu gomega.Eventually(plugin.GetPreparedResources).WithTimeout(time.Minute).Should(gomega.BeEmpty(), "prepared claims on host %s", host) } }) + + ginkgo.It("must skip NodePrepareResource if not used by any container", func(ctx context.Context) { + parameters := b.parameters() + pod, template := b.podInline(resourcev1alpha2.AllocationModeWaitForFirstConsumer) + for i := range pod.Spec.Containers { + pod.Spec.Containers[i].Resources.Claims = nil + } + b.create(ctx, parameters, pod, template) + framework.ExpectNoError(e2epod.WaitForPodRunningInNamespace(ctx, f.ClientSet, pod), "start pod") + for host, plugin := range b.driver.Nodes { + gomega.Expect(plugin.GetPreparedResources()).Should(gomega.BeEmpty(), "not claims should be prepared on host %s while pod is running", host) + } + }) }) ginkgo.Context("driver", func() {