Repository: kubernetes/kubernetes
File: pkg/controller/resourceclaim/controller.go (Go)

Code before:
						return err
					}
					if pod == nil || pod.UID != reservedFor.UID {
						logger.V(6).Info("Remove reservation because pod is gone or got replaced", "pod", klog.KObj(pod), "claim", klog.KRef(namespace, name))
						keepEntry = false
					}
				case pod.UID != reservedFor.UID:
					logger.V(6).Info("Remove reservation because pod got replaced with new instance", "pod", klog.KObj(pod), "claim", klog.KRef(namespace, name))
					keepEntry = false
				case isPodDone(pod):
					logger.V(6).Info("Remove reservation because pod will not run anymore", "pod", klog.KObj(pod), "claim", klog.KRef(namespace, name))
					keepEntry = false
				}
			}

			if keepEntry {
				valid = append(valid, reservedFor)
			}
			continue
		}

		// We don't know how to check this entry, so we just keep it to avoid
		// accidentally removing a reservation for a non-pod consumer that we
		// don't support yet.
		// TODO: support generic object lookup
		valid = append(valid, reservedFor)
	}

	builtinControllerFinalizer := slices.Index(claim.Finalizers, resourceapi.Finalizer)
	logger.V(5).Info("Claim reserved for counts", "currentCount", len(claim.Status.ReservedFor), "claim", klog.KRef(namespace, name), "updatedCount", len(valid), "builtinController", builtinControllerFinalizer >= 0)
	if len(valid) < len(claim.Status.ReservedFor) {
		// This is not using a patch because we want the update to fail if anything
		// changed in the meantime.
		claim := claim.DeepCopy()
		claim.Status.ReservedFor = valid

		// DRA always performs delayed allocations. Relatedly, it also
		// deallocates a claim as soon as the last consumer stops using
		// it. This ensures that the claim can be allocated again as needed by
		// some future consumer instead of trying to schedule that consumer
		// onto the node that was chosen for the previous consumer. It also
		// releases the underlying resources for use by other claims.
		//
		// This has to be triggered by the transition from "was being used" to
		// "is not used anymore" because a DRA driver is not required to set
		// `status.reservedFor` together with `status.allocation`, i.e. a claim
		// that is "currently unused" should not get deallocated.
		//
		// This does not matter for claims that were created for a pod. For
		// those, the resource claim controller will trigger deletion when the
		// pod is done. However, it doesn't hurt to also trigger deallocation

Review comment (suggestion):
You can remove the TODO. It and the error were there as a reminder that we need to decide what to do about unknown entries.

This PR now implements that decision: keep them and let other controllers handle them.


Code after:
						return err
					}
					if pod == nil || pod.UID != reservedFor.UID {
						logger.V(6).Info("Remove reservation because pod is gone or got replaced", "pod", klog.KObj(pod), "claim", klog.KRef(namespace, name))
						keepEntry = false
					}
				case pod.UID != reservedFor.UID:
					logger.V(6).Info("Remove reservation because pod got replaced with new instance", "pod", klog.KObj(pod), "claim", klog.KRef(namespace, name))
					keepEntry = false
				case isPodDone(pod):
					logger.V(6).Info("Remove reservation because pod will not run anymore", "pod", klog.KObj(pod), "claim", klog.KRef(namespace, name))
					keepEntry = false
				}
			}

			if keepEntry {
				remaining = append(remaining, reservedFor)
			}
			continue
		}

		// We don't know how to check this entry, so we just keep it to avoid
		// accidentally removing a reservation for a non-pod consumer that we
		// don't support yet.
		remaining = append(remaining, reservedFor)
	}

	builtinControllerFinalizer := slices.Index(claim.Finalizers, resourceapi.Finalizer)
	logger.V(5).Info("Claim reserved for counts", "currentCount", len(claim.Status.ReservedFor), "claim", klog.KRef(namespace, name), "updatedCount", len(remaining), "builtinController", builtinControllerFinalizer >= 0)
	if len(remaining) < len(claim.Status.ReservedFor) {
		// This is not using a patch because we want the update to fail if anything
		// changed in the meantime.
		claim := claim.DeepCopy()
		claim.Status.ReservedFor = remaining

		// DRA always performs delayed allocations. Relatedly, it also
		// deallocates a claim as soon as the last consumer stops using
		// it. This ensures that the claim can be allocated again as needed by
		// some future consumer instead of trying to schedule that consumer
		// onto the node that was chosen for the previous consumer. It also
		// releases the underlying resources for use by other claims.
		//
		// This has to be triggered by the transition from "was being used" to
		// "is not used anymore" because a DRA driver is not required to set
		// `status.reservedFor` together with `status.allocation`, i.e. a claim
		// that is "currently unused" should not get deallocated.
		//
		// This does not matter for claims that were created for a pod. For
		// those, the resource claim controller will trigger deletion when the
		// pod is done. However, it doesn't hurt to also trigger deallocation
		// for such claims and not checking for them keeps this code simpler.