Skip to content

Commit 444d23b

Browse files
committed
dra: generated name for ResourceClaim from template
Generating the name avoids all potential name collisions. It's not clear how much of a problem that was because users can avoid them and the deterministic names for generic ephemeral volumes have not led to reports from users. But using generated names is not too hard either. What makes it relatively easy is that the new pod.status.resourceClaimStatus map stores the generated name for kubelet and node authorizer, i.e. the information in the pod is sufficient to determine the name of the ResourceClaim. The resource claim controller becomes a bit more complex and now needs permission to modify the pod status. The new failure scenario of "ResourceClaim created, updating pod status fails" is handled with the help of a new special "resource.kubernetes.io/pod-claim-name" annotation that together with the owner reference identifies exactly for what a ResourceClaim was generated, so updating the pod status can be retried for existing ResourceClaims. The transition from deterministic names is handled with a special case for that recovery code path: a ResourceClaim with no annotation and a name that follows the Kubernetes <= 1.27 naming pattern is assumed to be generated for that pod claim and gets added to the pod status. There's no immediate need for it, but just in case that it may become relevant, the name of the generated ResourceClaim may also be left unset to record that no claim was needed. Components processing such a pod can skip whatever they normally would do for the claim. To ensure that they do and also cover other cases properly ("no known field is set", "must check ownership"), resourceclaim.Name gets extended.
1 parent 19a25ba commit 444d23b

File tree

17 files changed

+669
-144
lines changed

17 files changed

+669
-144
lines changed

pkg/api/pod/util.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,10 @@ func dropDisabledPodStatusFields(podStatus, oldPodStatus *api.PodStatus, podSpec
540540
dropResourcesFields(podStatus.EphemeralContainerStatuses)
541541
podStatus.Resize = ""
542542
}
543+
544+
if !utilfeature.DefaultFeatureGate.Enabled(features.DynamicResourceAllocation) && !dynamicResourceAllocationInUse(oldPodSpec) {
545+
podStatus.ResourceClaimStatuses = nil
546+
}
543547
}
544548

545549
// dropDisabledDynamicResourceAllocationFields removes pod claim references from

pkg/api/pod/util_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,11 @@ func TestDropDynamicResourceAllocation(t *testing.T) {
822822
},
823823
},
824824
},
825+
Status: api.PodStatus{
826+
ResourceClaimStatuses: []api.PodResourceClaimStatus{
827+
{Name: "my-claim", ResourceClaimName: pointer.String("pod-my-claim")},
828+
},
829+
},
825830
}
826831
podWithoutClaims := &api.Pod{
827832
Spec: api.PodSpec{

pkg/apis/core/types.go

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3210,22 +3210,32 @@ type ClaimSource struct {
32103210
//
32113211
// The template will be used to create a new ResourceClaim, which will
32123212
// be bound to this pod. When this pod is deleted, the ResourceClaim
3213-
// will also be deleted. The name of the ResourceClaim will be <pod
3214-
// name>-<resource name>, where <resource name> is the
3215-
// PodResourceClaim.Name. Pod validation will reject the pod if the
3216-
// concatenated name is not valid for a ResourceClaim (e.g. too long).
3217-
//
3218-
// An existing ResourceClaim with that name that is not owned by the
3219-
// pod will not be used for the pod to avoid using an unrelated
3220-
// resource by mistake. Scheduling and pod startup are then blocked
3221-
// until the unrelated ResourceClaim is removed.
3213+
// will also be deleted. The pod name and resource name, along with a
3214+
// generated component, will be used to form a unique name for the
3215+
// ResourceClaim, which will be recorded in pod.status.resourceClaimStatuses.
32223216
//
32233217
// This field is immutable and no changes will be made to the
32243218
// corresponding ResourceClaim by the control plane after creating the
32253219
// ResourceClaim.
32263220
ResourceClaimTemplateName *string
32273221
}
32283222

3223+
// PodResourceClaimStatus is stored in the PodStatus for each PodResourceClaim
3224+
// which references a ResourceClaimTemplate. It stores the generated name for
3225+
// the corresponding ResourceClaim.
3226+
type PodResourceClaimStatus struct {
3227+
// Name uniquely identifies this resource claim inside the pod.
3228+
// This must match the name of an entry in pod.spec.resourceClaims,
3229+
// which implies that the string must be a DNS_LABEL.
3230+
Name string
3231+
3232+
// ResourceClaimName is the name of the ResourceClaim that was
3233+
// generated for the Pod in the namespace of the Pod. It this is
3234+
// unset, then generating a ResourceClaim was not necessary. The
3235+
// pod.spec.resourceClaims entry can be ignored in this case.
3236+
ResourceClaimName *string
3237+
}
3238+
32293239
// OSName is the set of OS'es that can be used in OS.
32303240
type OSName string
32313241

@@ -3661,6 +3671,11 @@ type PodStatus struct {
36613671
// +featureGate=InPlacePodVerticalScaling
36623672
// +optional
36633673
Resize PodResizeStatus
3674+
3675+
// Status of resource claims.
3676+
// +featureGate=DynamicResourceAllocation
3677+
// +optional
3678+
ResourceClaimStatuses []PodResourceClaimStatus
36643679
}
36653680

36663681
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

pkg/apis/core/validation/validation.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4845,6 +4845,7 @@ func ValidatePodStatusUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions
48454845
allErrs = append(allErrs, ValidateContainerStateTransition(newPod.Status.InitContainerStatuses, oldPod.Status.InitContainerStatuses, fldPath.Child("initContainerStatuses"), oldPod.Spec.RestartPolicy)...)
48464846
// The kubelet will never restart ephemeral containers, so treat them like they have an implicit RestartPolicyNever.
48474847
allErrs = append(allErrs, ValidateContainerStateTransition(newPod.Status.EphemeralContainerStatuses, oldPod.Status.EphemeralContainerStatuses, fldPath.Child("ephemeralContainerStatuses"), core.RestartPolicyNever)...)
4848+
allErrs = append(allErrs, validatePodResourceClaimStatuses(newPod.Status.ResourceClaimStatuses, newPod.Spec.ResourceClaims, fldPath.Child("resourceClaimStatuses"))...)
48484849

48494850
if newIPErrs := validatePodIPs(newPod); len(newIPErrs) > 0 {
48504851
allErrs = append(allErrs, newIPErrs...)
@@ -4866,6 +4867,36 @@ func validatePodConditions(conditions []core.PodCondition, fldPath *field.Path)
48664867
return allErrs
48674868
}
48684869

4870+
// validatePodResourceClaimStatuses validates the ResourceClaimStatuses slice in a pod status.
4871+
func validatePodResourceClaimStatuses(statuses []core.PodResourceClaimStatus, podClaims []core.PodResourceClaim, fldPath *field.Path) field.ErrorList {
4872+
var allErrs field.ErrorList
4873+
4874+
for i, status := range statuses {
4875+
idxPath := fldPath.Index(i)
4876+
// There's no need to check the content of the name. If it matches an entry,
4877+
// then it is valid, otherwise we reject it here.
4878+
if !havePodClaim(podClaims, status.Name) {
4879+
allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), status.Name, "must match the name of an entry in `spec.resourceClaims`"))
4880+
}
4881+
if status.ResourceClaimName != nil {
4882+
for _, detail := range ValidateResourceClaimName(*status.ResourceClaimName, false) {
4883+
allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), status.ResourceClaimName, detail))
4884+
}
4885+
}
4886+
}
4887+
4888+
return allErrs
4889+
}
4890+
4891+
func havePodClaim(podClaims []core.PodResourceClaim, name string) bool {
4892+
for _, podClaim := range podClaims {
4893+
if podClaim.Name == name {
4894+
return true
4895+
}
4896+
}
4897+
return false
4898+
}
4899+
48694900
// ValidatePodEphemeralContainersUpdate tests that a user update to EphemeralContainers is valid.
48704901
// newPod and oldPod must only differ in their EphemeralContainers.
48714902
func ValidatePodEphemeralContainersUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) field.ErrorList {

pkg/apis/core/validation/validation_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13629,6 +13629,82 @@ func TestValidatePodStatusUpdate(t *testing.T) {
1362913629
},
1363013630
"",
1363113631
"Container statuses all containers terminated",
13632+
}, {
13633+
core.Pod{
13634+
ObjectMeta: metav1.ObjectMeta{
13635+
Name: "foo",
13636+
},
13637+
Status: core.PodStatus{
13638+
ResourceClaimStatuses: []core.PodResourceClaimStatus{
13639+
{Name: "no-such-claim", ResourceClaimName: utilpointer.String("my-claim")},
13640+
},
13641+
},
13642+
},
13643+
core.Pod{
13644+
ObjectMeta: metav1.ObjectMeta{
13645+
Name: "foo",
13646+
},
13647+
},
13648+
"status.resourceClaimStatuses[0].name: Invalid value: \"no-such-claim\": must match the name of an entry in `spec.resourceClaims`",
13649+
"Non-existent PodResourceClaim",
13650+
}, {
13651+
core.Pod{
13652+
ObjectMeta: metav1.ObjectMeta{
13653+
Name: "foo",
13654+
},
13655+
Spec: core.PodSpec{
13656+
ResourceClaims: []core.PodResourceClaim{
13657+
{Name: "my-claim"},
13658+
},
13659+
},
13660+
Status: core.PodStatus{
13661+
ResourceClaimStatuses: []core.PodResourceClaimStatus{
13662+
{Name: "my-claim", ResourceClaimName: utilpointer.String("%$!#")},
13663+
},
13664+
},
13665+
},
13666+
core.Pod{
13667+
ObjectMeta: metav1.ObjectMeta{
13668+
Name: "foo",
13669+
},
13670+
Spec: core.PodSpec{
13671+
ResourceClaims: []core.PodResourceClaim{
13672+
{Name: "my-claim"},
13673+
},
13674+
},
13675+
},
13676+
`status.resourceClaimStatuses[0].name: Invalid value: "%$!#": a lowercase RFC 1123 subdomain must consist of`,
13677+
"Invalid ResourceClaim name",
13678+
}, {
13679+
core.Pod{
13680+
ObjectMeta: metav1.ObjectMeta{
13681+
Name: "foo",
13682+
},
13683+
Spec: core.PodSpec{
13684+
ResourceClaims: []core.PodResourceClaim{
13685+
{Name: "my-claim"},
13686+
{Name: "my-other-claim"},
13687+
},
13688+
},
13689+
Status: core.PodStatus{
13690+
ResourceClaimStatuses: []core.PodResourceClaimStatus{
13691+
{Name: "my-claim", ResourceClaimName: utilpointer.String("foo-my-claim-12345")},
13692+
{Name: "my-other-claim", ResourceClaimName: nil},
13693+
},
13694+
},
13695+
},
13696+
core.Pod{
13697+
ObjectMeta: metav1.ObjectMeta{
13698+
Name: "foo",
13699+
},
13700+
Spec: core.PodSpec{
13701+
ResourceClaims: []core.PodResourceClaim{
13702+
{Name: "my-claim"},
13703+
},
13704+
},
13705+
},
13706+
"",
13707+
"ResourceClaimStatuses okay",
1363213708
},
1363313709
}
1363413710

0 commit comments

Comments
 (0)