Fix report health for templated and renamed DRA claims

The Kubelet's DRA manager was failing to report device health status in a pod's status for certain types of resource claims. The logic incorrectly assumed that the claim name used within the container's spec (`container.resources.claims[*].name`) was the same as the metadata name of the actual ResourceClaim object.

This assumption is false in two key scenarios:
1.  When a claim is generated from a `ResourceClaimTemplate`, Kubernetes creates a `ResourceClaim` object with a randomized suffix in its name.
2.  When a user defines a pre-existing claim in `pod.spec.resourceClaims`, they can provide a local name that differs from the actual `ResourceClaim` object's name.

In both cases, the code would fail to find the claim's information in its internal cache, resulting in the health status not being populated, as reported in issue #134482.

This fix corrects the logic by using `pod.Status.ResourceClaimStatuses` as the authoritative map to look up the actual, generated name of the `ResourceClaim` object. This ensures that both templated and renamed claims are resolved correctly before their health status is retrieved from the cache.

Additionally, this change introduces a new node e2e test that specifically covers templated and renamed claims to prevent future regressions.
This commit is contained in:
John-Paul Sassine
2025-10-25 01:14:11 +00:00
parent 3ef0262766
commit 1e8fc543cc
3 changed files with 231 additions and 106 deletions

View File

@@ -778,116 +778,116 @@ func (m *Manager) GetContainerClaimInfos(pod *v1.Pod, container *v1.Container) (
// UpdateAllocatedResourcesStatus updates the health status of allocated DRA resources in the pod's container statuses.
func (m *Manager) UpdateAllocatedResourcesStatus(pod *v1.Pod, status *v1.PodStatus) {
logger := klog.FromContext(context.Background())
for _, container := range pod.Spec.Containers {
// Get all the DRA claim details associated with this specific container.
claimInfos, err := m.GetContainerClaimInfos(pod, &container)
if err != nil {
logger.Error(err, "Failed to get claim infos for container", "pod", klog.KObj(pod), "container", container.Name)
for i := range status.ContainerStatuses {
containerStatus := &status.ContainerStatuses[i]
// Find the corresponding container spec in the pod spec.
var containerSpec *v1.Container
for j := range pod.Spec.Containers {
if pod.Spec.Containers[j].Name == containerStatus.Name {
containerSpec = &pod.Spec.Containers[j]
break
}
}
// Skip if there's no matching container spec or it has no resource claims.
if containerSpec == nil || len(containerSpec.Resources.Claims) == 0 {
continue
}
// Find the corresponding container status
for i, containerStatus := range status.ContainerStatuses {
if containerStatus.Name != container.Name {
resourceStatusMap := make(map[v1.ResourceName]*v1.ResourceStatus)
if containerStatus.AllocatedResourcesStatus != nil {
for j := range containerStatus.AllocatedResourcesStatus {
resourceStatusMap[containerStatus.AllocatedResourcesStatus[j].Name] = &containerStatus.AllocatedResourcesStatus[j]
}
}
// Iterate through the claims requested by this specific container.
for _, claim := range containerSpec.Resources.Claims {
// Find the actual name of the ResourceClaim object.
var actualClaimName string
for _, podClaimStatus := range pod.Status.ResourceClaimStatuses {
if podClaimStatus.Name == claim.Name {
if podClaimStatus.ResourceClaimName != nil {
actualClaimName = *podClaimStatus.ResourceClaimName
}
break
}
}
if actualClaimName == "" {
logger.V(4).Info("Could not find generated name for resource claim in pod status", "pod", klog.KObj(pod), "container", containerSpec.Name, "claimName", claim.Name)
continue
}
// Ensure the slice exists. Use a map for efficient updates by resource name.
resourceStatusMap := make(map[v1.ResourceName]*v1.ResourceStatus)
if status.ContainerStatuses[i].AllocatedResourcesStatus != nil {
for idx := range status.ContainerStatuses[i].AllocatedResourcesStatus {
// Store pointers to modify in place
resourceStatusMap[status.ContainerStatuses[i].AllocatedResourcesStatus[idx].Name] = &status.ContainerStatuses[i].AllocatedResourcesStatus[idx]
}
} else {
status.ContainerStatuses[i].AllocatedResourcesStatus = []v1.ResourceStatus{}
}
func() {
m.cache.RLock()
defer m.cache.RUnlock()
// Loop through each claim associated with the container
for _, claimInfo := range claimInfos {
var resourceName v1.ResourceName
foundClaimInSpec := false
for _, cClaim := range container.Resources.Claims {
if cClaim.Name == claimInfo.ClaimName {
if cClaim.Request == "" {
resourceName = v1.ResourceName(fmt.Sprintf("claim:%s", cClaim.Name))
} else {
resourceName = v1.ResourceName(fmt.Sprintf("claim:%s/%s", cClaim.Name, cClaim.Request))
}
foundClaimInSpec = true
break
}
}
if !foundClaimInSpec {
logger.V(4).Info("Could not find matching resource claim in container spec", "pod", klog.KObj(pod), "container", container.Name, "claimName", claimInfo.ClaimName)
continue
// Use the actual claim name to look up claim info.
claimInfo, exists := m.cache.get(actualClaimName, pod.Namespace)
if !exists {
logger.V(4).Info("Could not find claim info for resource claim", "pod", klog.KObj(pod), "container", containerSpec.Name, "claimName", actualClaimName)
return
}
resourceName := v1.ResourceName(fmt.Sprintf("claim:%s", claim.Name))
if claim.Request != "" {
resourceName = v1.ResourceName(fmt.Sprintf("claim:%s/%s", claim.Name, claim.Request))
}
// Get or create the ResourceStatus entry for this claim
resStatus, ok := resourceStatusMap[resourceName]
if !ok {
// Create a new entry and add it to the map and the slice
newStatus := v1.ResourceStatus{
Name: resourceName,
Resources: []v1.ResourceHealth{},
}
status.ContainerStatuses[i].AllocatedResourcesStatus = append(status.ContainerStatuses[i].AllocatedResourcesStatus, newStatus)
// Get pointer to the newly added element *after* appending
resStatus = &status.ContainerStatuses[i].AllocatedResourcesStatus[len(status.ContainerStatuses[i].AllocatedResourcesStatus)-1]
// Append and get a pointer to the new element.
if containerStatus.AllocatedResourcesStatus == nil {
containerStatus.AllocatedResourcesStatus = []v1.ResourceStatus{}
}
containerStatus.AllocatedResourcesStatus = append(containerStatus.AllocatedResourcesStatus, newStatus)
resStatus = &containerStatus.AllocatedResourcesStatus[len(containerStatus.AllocatedResourcesStatus)-1]
resourceStatusMap[resourceName] = resStatus
}
// Clear previous health entries for this resource before adding current ones
// Ensures we only report current health for allocated devices.
// Clear previous health entries before adding current ones.
resStatus.Resources = []v1.ResourceHealth{}
// Iterate through the map holding the state specific to each driver
for driverName, driverState := range claimInfo.DriverState {
// Iterate through each specific device allocated by this driver
for _, device := range driverState.Devices {
healthStr := m.healthInfoCache.getHealthInfo(driverName, device.PoolName, device.DeviceName)
// Convert internal health string to API type
var health v1.ResourceHealthStatus
switch healthStr {
case "Healthy":
health = v1.ResourceHealthStatusHealthy
case "Unhealthy":
health = v1.ResourceHealthStatusUnhealthy
default: // Catches "Unknown" or any other case
default:
health = v1.ResourceHealthStatusUnknown
}
// Create the ResourceHealth entry
resourceHealth := v1.ResourceHealth{
Health: health,
}
// Use first CDI device ID as ResourceID, with fallback
resourceHealth := v1.ResourceHealth{Health: health}
if len(device.CDIDeviceIDs) > 0 {
resourceHealth.ResourceID = v1.ResourceID(device.CDIDeviceIDs[0])
} else {
// Fallback ID if no CDI ID is present
resourceHealth.ResourceID = v1.ResourceID(fmt.Sprintf("%s/%s/%s", driverName, device.PoolName, device.DeviceName))
}
// Append the health status for this specific device/resource ID
resStatus.Resources = append(resStatus.Resources, resourceHealth)
}
}
}
// Rebuild the slice from the map values to ensure correctness
finalStatuses := make([]v1.ResourceStatus, 0, len(resourceStatusMap))
for _, rs := range resourceStatusMap {
// Only add if it actually has resource health entries populated
if len(rs.Resources) > 0 {
finalStatuses = append(finalStatuses, *rs)
}
}
status.ContainerStatuses[i].AllocatedResourcesStatus = finalStatuses
}()
}
// Rebuild the slice from map to ensure correctness and remove empty entries.
finalStatuses := make([]v1.ResourceStatus, 0, len(resourceStatusMap))
for _, rs := range resourceStatusMap {
if len(rs.Resources) > 0 {
finalStatuses = append(finalStatuses, *rs)
}
}
containerStatus.AllocatedResourcesStatus = finalStatuses
}
}

View File

@@ -1538,49 +1538,165 @@ func TestHandleWatchResourcesStream(t *testing.T) {
})
}
// TestUpdateAllocatedResourcesStatus checks if the manager correctly updates the
// PodStatus with the health information of allocated DRA resources. It populates
// the caches with known claim and health data, then calls the function and verifies the resulting PodStatus.
// TestUpdateAllocatedResourcesStatus verifies that the manager can correctly
// update the PodStatus with health information for different types of DRA claims.
// It covers the main scenarios that were difficult to test reliably in an e2e
// environment due to timing issues:
// 1. Direct claims: Where the pod's resource claim name directly matches the ResourceClaim object name.
// 2. Renamed claims: Where the pod uses a local name for a ResourceClaim that has a different object name.
// 3. Templated claims: Where the claim is generated from a template, and the pod status contains the
// dynamically generated claim name.
func TestUpdateAllocatedResourcesStatus(t *testing.T) {
tCtx := ktesting.Init(t)
directClaimName := "direct-claim-name"
renamedClaimObject := "renamed-claim-object"
templateName := "template-name"
templatedClaimName := "pod-templated-templated-claim"
// Setup Manager with caches
manager, err := NewManager(tCtx.Logger(), nil, t.TempDir())
require.NoError(t, err)
// Populate claimInfoCache
claimInfo := genTestClaimInfo(claimUID, []string{podUID}, true)
manager.cache.add(claimInfo)
// Populate healthInfoCache
healthyDevice := state.DeviceHealth{PoolName: poolName, DeviceName: deviceName, Health: "Healthy", LastUpdated: time.Now()}
_, err = manager.healthInfoCache.updateHealthInfo(driverName, []state.DeviceHealth{healthyDevice})
require.NoError(t, err)
// Create Pod and Status objects
pod := genTestPod()
require.NotEmpty(t, pod.Spec.Containers, "genTestPod should create at least one container")
// Ensure the container has a name for matching
pod.Spec.Containers[0].Name = containerName
podStatus := &v1.PodStatus{
ContainerStatuses: []v1.ContainerStatus{
{Name: containerName},
testCases := []struct {
name string
pod *v1.Pod
claimInfos []*ClaimInfo
initialStatus *v1.PodStatus
expectedAllocatedResourcesStatus []v1.ResourceStatus
}{
{
name: "Direct claim",
pod: &v1.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "pod-direct", UID: "pod-direct-uid"},
Spec: v1.PodSpec{
Containers: []v1.Container{
{Name: "container1", Resources: v1.ResourceRequirements{Claims: []v1.ResourceClaim{{Name: "claim1"}}}},
},
ResourceClaims: []v1.PodResourceClaim{
{Name: "claim1", ResourceClaimName: &directClaimName},
},
},
Status: v1.PodStatus{
ResourceClaimStatuses: []v1.PodResourceClaimStatus{
{Name: "claim1", ResourceClaimName: &directClaimName},
},
},
},
claimInfos: []*ClaimInfo{
{
ClaimInfoState: state.ClaimInfoState{
ClaimName: directClaimName,
PodUIDs: sets.New("pod-direct-uid"),
DriverState: map[string]state.DriverState{
"test-driver": {Devices: []state.Device{{PoolName: "pool", DeviceName: "dev-a"}}},
},
},
},
},
initialStatus: &v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{{Name: "container1"}}},
expectedAllocatedResourcesStatus: []v1.ResourceStatus{
{
Name: "claim:claim1",
Resources: []v1.ResourceHealth{
{ResourceID: "test-driver/pool/dev-a", Health: v1.ResourceHealthStatusHealthy},
},
},
},
},
{
name: "Renamed claim",
pod: &v1.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "pod-renamed", UID: "pod-renamed-uid"},
Spec: v1.PodSpec{
ResourceClaims: []v1.PodResourceClaim{{Name: "renamed-pod-claim", ResourceClaimName: &renamedClaimObject}},
Containers: []v1.Container{{Name: "container1", Resources: v1.ResourceRequirements{Claims: []v1.ResourceClaim{{Name: "renamed-pod-claim"}}}}},
},
Status: v1.PodStatus{
ResourceClaimStatuses: []v1.PodResourceClaimStatus{
{Name: "renamed-pod-claim", ResourceClaimName: &renamedClaimObject},
},
},
},
claimInfos: []*ClaimInfo{
{
ClaimInfoState: state.ClaimInfoState{
ClaimName: renamedClaimObject,
PodUIDs: sets.New("pod-renamed-uid"),
DriverState: map[string]state.DriverState{
"test-driver": {Devices: []state.Device{{PoolName: "pool", DeviceName: "dev-b"}}},
},
},
},
},
initialStatus: &v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{{Name: "container1"}}},
expectedAllocatedResourcesStatus: []v1.ResourceStatus{
{
Name: "claim:renamed-pod-claim",
Resources: []v1.ResourceHealth{
{ResourceID: "test-driver/pool/dev-b", Health: v1.ResourceHealthStatusHealthy},
},
},
},
},
{
name: "Templated claim",
pod: &v1.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "pod-templated", UID: "pod-templated-uid"},
Spec: v1.PodSpec{
ResourceClaims: []v1.PodResourceClaim{{Name: "templated-claim", ResourceClaimTemplateName: &templateName}},
Containers: []v1.Container{{Name: "container1", Resources: v1.ResourceRequirements{Claims: []v1.ResourceClaim{{Name: "templated-claim"}}}}},
},
Status: v1.PodStatus{
ResourceClaimStatuses: []v1.PodResourceClaimStatus{
{Name: "templated-claim", ResourceClaimName: &templatedClaimName},
},
},
},
claimInfos: []*ClaimInfo{
{
ClaimInfoState: state.ClaimInfoState{
ClaimName: templatedClaimName,
PodUIDs: sets.New("pod-templated-uid"),
DriverState: map[string]state.DriverState{
"test-driver": {Devices: []state.Device{{PoolName: "pool", DeviceName: "dev-c"}}},
},
},
},
},
initialStatus: &v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{{Name: "container1"}}},
expectedAllocatedResourcesStatus: []v1.ResourceStatus{
{
Name: "claim:templated-claim",
Resources: []v1.ResourceHealth{
{ResourceID: "test-driver/pool/dev-c", Health: v1.ResourceHealthStatusHealthy},
},
},
},
},
}
// Call the function under test
manager.UpdateAllocatedResourcesStatus(pod, podStatus)
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
tCtx := ktesting.Init(t)
manager, err := NewManager(tCtx.Logger(), nil, t.TempDir())
require.NoError(t, err)
require.Len(t, podStatus.ContainerStatuses, 1)
contStatus := podStatus.ContainerStatuses[0]
require.NotNil(t, contStatus.AllocatedResourcesStatus)
require.Len(t, contStatus.AllocatedResourcesStatus, 1, "Should have status for one resource claim")
for _, ci := range tc.claimInfos {
manager.cache.add(ci)
}
resourceStatus := contStatus.AllocatedResourcesStatus[0]
assert.Equal(t, v1.ResourceName("claim:"+claimName), resourceStatus.Name, "ResourceStatus Name mismatch")
// Check the Resources slice
require.Len(t, resourceStatus.Resources, 1, "Should have health info for one device")
resourceHealth := resourceStatus.Resources[0]
assert.Equal(t, v1.ResourceID(cdiID), resourceHealth.ResourceID, "ResourceHealth ResourceID mismatch")
assert.Equal(t, v1.ResourceHealthStatusHealthy, resourceHealth.Health, "ResourceHealth Health status mismatch")
// Set all devices to be healthy for the test.
devices := []state.DeviceHealth{}
for _, ci := range tc.claimInfos {
for driverName, ds := range ci.DriverState {
for _, dev := range ds.Devices {
devices = append(devices, state.DeviceHealth{PoolName: dev.PoolName, DeviceName: dev.DeviceName, Health: state.DeviceHealthStatusHealthy})
}
_, err := manager.healthInfoCache.updateHealthInfo(driverName, devices)
require.NoError(t, err)
}
}
status := tc.initialStatus.DeepCopy()
manager.UpdateAllocatedResourcesStatus(tc.pod, status)
require.Len(t, status.ContainerStatuses, 1)
assert.Equal(t, tc.expectedAllocatedResourcesStatus, status.ContainerStatuses[0].AllocatedResourcesStatus)
})
}
}

View File

@@ -1584,6 +1584,15 @@ func createHealthTestPodAndClaim(ctx context.Context, f *framework.Framework, dr
e2epod.DeletePodOrFail(ctx, f.ClientSet, createdPod.Namespace, createdPod.Name)
})
// Update the Pod's status to include the ResourceClaimStatuses, mimicking the scheduler.
podToUpdate, err := f.ClientSet.CoreV1().Pods(f.Namespace.Name).Get(ctx, createdPod.Name, metav1.GetOptions{})
framework.ExpectNoError(err)
podToUpdate.Status.ResourceClaimStatuses = []v1.PodResourceClaimStatus{
{Name: claimName, ResourceClaimName: &claimName},
}
_, err = f.ClientSet.CoreV1().Pods(f.Namespace.Name).UpdateStatus(ctx, podToUpdate, metav1.UpdateOptions{})
framework.ExpectNoError(err, "failed to update Pod status with ResourceClaimStatuses")
ginkgo.By(fmt.Sprintf("Allocating claim %q to pod %q with its real UID", claimName, podName))
// Get the created claim to ensure the latest version before updating.
claimToUpdate, err := f.ClientSet.ResourceV1().ResourceClaims(f.Namespace.Name).Get(ctx, claimName, metav1.GetOptions{})