From c438f8a983c7437dba9f42cd7b55eaf3b472ef65 Mon Sep 17 00:00:00 2001 From: fj-naji Date: Mon, 20 Oct 2025 09:45:37 +0000 Subject: [PATCH] scheduler: Add BindingTimeout args to DynamicResources plugin Add a new `bindingTimeout` field to DynamicResources plugin args and wire it into PreBind. Changes: - API: add `bindingTimeout` to DynamicResourcesArgs (staging + internal types). - Defaults: default to 600 seconds when BOTH DRADeviceBindingConditions and DRAResourceClaimDeviceStatus are enabled. - Validation: require >= 1s; forbid when either feature gate is disabled. - Plugin: plumbs args into `pl.bindingTimeout` and uses it in `wait.PollUntilContextTimeout` for binding-condition wait logic. - Plugin: remove legacy `BindingTimeoutDefaultSeconds`. Tests: - Add/adjust unit tests for validation and PreBind timeout path. - Ensure <1s and negative values are rejected; forbids when gates disabled. --- pkg/generated/openapi/zz_generated.openapi.go | 6 ++ .../apis/config/scheme/scheme_test.go | 6 +- pkg/scheduler/apis/config/types_pluginargs.go | 27 +++++++ pkg/scheduler/apis/config/v1/defaults.go | 6 ++ pkg/scheduler/apis/config/v1/defaults_test.go | 12 +++ .../apis/config/v1/zz_generated.conversion.go | 2 + .../validation/validation_pluginargs.go | 18 +++++ .../validation/validation_pluginargs_test.go | 78 +++++++++++++++++-- .../apis/config/zz_generated.deepcopy.go | 5 ++ .../dynamicresources/dynamicresources.go | 28 +++---- .../dynamicresources/dynamicresources_test.go | 5 +- .../k8s.io/kube-scheduler/config/v1/types.go | 27 +++++++ .../config/v1/zz_generated.deepcopy.go | 5 ++ 13 files changed, 204 insertions(+), 21 deletions(-) diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index 4d2cbf79d2c..42eb8057284 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -68495,6 +68495,12 @@ func schema_k8sio_kube_scheduler_config_v1_DynamicResourcesArgs(ref common.Refer Ref: ref(metav1.Duration{}.OpenAPIModelName()), }, }, + "bindingTimeout": { + SchemaProps: spec.SchemaProps{ + Description: "BindingTimeout limits how long the PreBind extension point may wait for ResourceClaim device BindingConditions to become satisfied when such conditions are present. While waiting, the scheduler periodically checks device status. If the timeout elapses before all required conditions are true (or any bindingFailureConditions become true), the allocation is cleared and the Pod re-enters scheduling queue. Note that the same or other node may be chosen if feasible; otherwise the Pod is placed in the unschedulable queue and retried based on cluster changes and backoff.\n\nDefaults & feature gates:\n - Defaults to 10 minutes when the DRADeviceBindingConditions feature gate is enabled.\n - Has effect only when BOTH DRADeviceBindingConditions and\n DRAResourceClaimDeviceStatus are enabled; otherwise omit this field.\n - When DRADeviceBindingConditions is disabled, setting this field is considered an error.\n\nValid values:\n - >=1s (non-zero). No upper bound is enforced.\n\nTuning guidance:\n - Lower values reduce time-to-retry when devices aren’t ready but can\n increase churn if drivers typically need longer to report readiness.\n - Review scheduler latency metrics (e.g. PreBind duration in\n `scheduler_framework_extension_point_duration_seconds`) and driver\n readiness behavior before tightening this timeout.", + Ref: ref(metav1.Duration{}.OpenAPIModelName()), + }, + }, }, Required: []string{"filterTimeout"}, }, diff --git a/pkg/scheduler/apis/config/scheme/scheme_test.go b/pkg/scheduler/apis/config/scheme/scheme_test.go index 1c2e366f7c1..2096bcd8ba3 100644 --- a/pkg/scheduler/apis/config/scheme/scheme_test.go +++ b/pkg/scheduler/apis/config/scheme/scheme_test.go @@ -57,6 +57,7 @@ profiles: - name: DynamicResources args: filterTimeout: 10s + bindingTimeout: 30s - name: InterPodAffinity args: hardPodAffinityWeight: 5 @@ -107,7 +108,10 @@ profiles: }, { Name: "DynamicResources", - Args: &config.DynamicResourcesArgs{FilterTimeout: &metav1.Duration{Duration: 10 * time.Second}}, + Args: &config.DynamicResourcesArgs{ + FilterTimeout: &metav1.Duration{Duration: 10 * time.Second}, + BindingTimeout: &metav1.Duration{Duration: 30 * time.Second}, + }, }, { Name: "InterPodAffinity", diff --git a/pkg/scheduler/apis/config/types_pluginargs.go b/pkg/scheduler/apis/config/types_pluginargs.go index fdcfc1b27fb..4cccfa517bb 100644 --- a/pkg/scheduler/apis/config/types_pluginargs.go +++ b/pkg/scheduler/apis/config/types_pluginargs.go @@ -257,6 +257,33 @@ type DynamicResourcesArgs struct { // // Setting it to zero completely disables the timeout. FilterTimeout *metav1.Duration + + // BindingTimeout limits how long the PreBind extension point may wait for + // ResourceClaim device BindingConditions to become satisfied when such + // conditions are present. While waiting, the scheduler periodically checks + // device status. If the timeout elapses before all required conditions are + // true (or any bindingFailureConditions become true), the allocation is + // cleared and the Pod re-enters scheduling queue. Note that the same or other node may be + // chosen if feasible; otherwise the Pod is placed in the unschedulable queue and + // retried based on cluster changes and backoff. + // + // Defaults & feature gates: + // - Defaults to 10 minutes when the DRADeviceBindingConditions feature gate is enabled. + // - Has effect only when BOTH DRADeviceBindingConditions and + // DRAResourceClaimDeviceStatus are enabled; otherwise omit this field. + // - When DRADeviceBindingConditions is disabled, setting this field is considered an error. + // + // Valid values: + // - >=1s (non-zero). No upper bound is enforced. + // + // Tuning guidance: + // - Lower values reduce time-to-retry when devices aren’t ready but can + // increase churn if drivers typically need longer to report readiness. + // - Review scheduler latency metrics (e.g. PreBind duration in + // `scheduler_framework_extension_point_duration_seconds`) and driver + // readiness behavior before tightening this timeout. + BindingTimeout *metav1.Duration } const DynamicResourcesFilterTimeoutDefault = 10 * time.Second +const DynamicResourcesBindingTimeoutDefault = 600 * time.Second diff --git a/pkg/scheduler/apis/config/v1/defaults.go b/pkg/scheduler/apis/config/v1/defaults.go index 9b8017cbf10..225ce267d45 100644 --- a/pkg/scheduler/apis/config/v1/defaults.go +++ b/pkg/scheduler/apis/config/v1/defaults.go @@ -248,4 +248,10 @@ func SetDefaults_DynamicResourcesArgs(obj *configv1.DynamicResourcesArgs) { if obj.FilterTimeout == nil && feature.DefaultFeatureGate.Enabled(features.DRASchedulerFilterTimeout) { obj.FilterTimeout = &metav1.Duration{Duration: configv1.DynamicResourcesFilterTimeoutDefault} } + + if obj.BindingTimeout == nil && + feature.DefaultFeatureGate.Enabled(features.DRADeviceBindingConditions) && + feature.DefaultFeatureGate.Enabled(features.DRAResourceClaimDeviceStatus) { + obj.BindingTimeout = &metav1.Duration{Duration: configv1.DynamicResourcesBindingTimeoutDefault} + } } diff --git a/pkg/scheduler/apis/config/v1/defaults_test.go b/pkg/scheduler/apis/config/v1/defaults_test.go index 9291060c701..edfa6083bfb 100644 --- a/pkg/scheduler/apis/config/v1/defaults_test.go +++ b/pkg/scheduler/apis/config/v1/defaults_test.go @@ -892,6 +892,18 @@ func TestPluginArgsDefaults(t *testing.T) { }, }, }, + { + name: "DynamicResourcesArgs defaults, DRADeviceBindingConditions enabled", + features: map[featuregate.Feature]bool{ + features.DRADeviceBindingConditions: true, + features.DRAResourceClaimDeviceStatus: true, + }, + in: &configv1.DynamicResourcesArgs{}, + want: &configv1.DynamicResourcesArgs{ + FilterTimeout: &metav1.Duration{Duration: 10 * time.Second}, + BindingTimeout: &metav1.Duration{Duration: 600 * time.Second}, + }, + }, } for _, tc := range tests { scheme := runtime.NewScheme() diff --git a/pkg/scheduler/apis/config/v1/zz_generated.conversion.go b/pkg/scheduler/apis/config/v1/zz_generated.conversion.go index 5470b2455bf..63240b7d112 100644 --- a/pkg/scheduler/apis/config/v1/zz_generated.conversion.go +++ b/pkg/scheduler/apis/config/v1/zz_generated.conversion.go @@ -285,6 +285,7 @@ func Convert_config_DefaultPreemptionArgs_To_v1_DefaultPreemptionArgs(in *config func autoConvert_v1_DynamicResourcesArgs_To_config_DynamicResourcesArgs(in *configv1.DynamicResourcesArgs, out *config.DynamicResourcesArgs, s conversion.Scope) error { out.FilterTimeout = (*metav1.Duration)(unsafe.Pointer(in.FilterTimeout)) + out.BindingTimeout = (*metav1.Duration)(unsafe.Pointer(in.BindingTimeout)) return nil } @@ -295,6 +296,7 @@ func Convert_v1_DynamicResourcesArgs_To_config_DynamicResourcesArgs(in *configv1 func autoConvert_config_DynamicResourcesArgs_To_v1_DynamicResourcesArgs(in *config.DynamicResourcesArgs, out *configv1.DynamicResourcesArgs, s conversion.Scope) error { out.FilterTimeout = (*metav1.Duration)(unsafe.Pointer(in.FilterTimeout)) + out.BindingTimeout = (*metav1.Duration)(unsafe.Pointer(in.BindingTimeout)) return nil } diff --git a/pkg/scheduler/apis/config/validation/validation_pluginargs.go b/pkg/scheduler/apis/config/validation/validation_pluginargs.go index 7900b2ea51e..d50881630d6 100644 --- a/pkg/scheduler/apis/config/validation/validation_pluginargs.go +++ b/pkg/scheduler/apis/config/validation/validation_pluginargs.go @@ -19,6 +19,7 @@ package validation import ( "fmt" "strings" + "time" v1 "k8s.io/api/core/v1" metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" @@ -343,5 +344,22 @@ func ValidateDynamicResourcesArgs(path *field.Path, args *config.DynamicResource allErrs = append(allErrs, field.Forbidden(path.Child("filterTimeout"), "DRASchedulingFilterTimeout feature gate is disabled")) } } + + if fts.EnableDRADeviceBindingConditions && fts.EnableDRAResourceClaimDeviceStatus { + if args.BindingTimeout != nil && args.BindingTimeout.Duration < 1*time.Second { + allErrs = append(allErrs, field.Invalid( + path.Child("bindingTimeout"), + args.BindingTimeout, + "must be at least 1 second", + )) + } + } else { + if args.BindingTimeout != nil { + allErrs = append(allErrs, field.Forbidden( + path.Child("bindingTimeout"), + "DRADeviceBindingConditions or DRAResourceClaimDeviceStatus feature gate is disabled", + )) + } + } return allErrs.ToAggregate() } diff --git a/pkg/scheduler/apis/config/validation/validation_pluginargs_test.go b/pkg/scheduler/apis/config/validation/validation_pluginargs_test.go index 9c5a4c43ccf..aec89d5a4a6 100644 --- a/pkg/scheduler/apis/config/validation/validation_pluginargs_test.go +++ b/pkg/scheduler/apis/config/validation/validation_pluginargs_test.go @@ -1158,13 +1158,16 @@ func TestValidateRequestedToCapacityRatioScoringStrategy(t *testing.T) { func TestValidateDynamicResourcesArgs(t *testing.T) { cases := map[string]struct { - args config.DynamicResourcesArgs - wantErrs field.ErrorList - filterTimeoutDisabled bool + args config.DynamicResourcesArgs + wantErrs field.ErrorList + filterTimeoutDisabled bool + bindingConditionsDisabled bool + deviceStatusDisabled bool }{ "valid args (default)": { args: config.DynamicResourcesArgs{ - FilterTimeout: &metav1.Duration{Duration: config.DynamicResourcesFilterTimeoutDefault}, + FilterTimeout: &metav1.Duration{Duration: config.DynamicResourcesFilterTimeoutDefault}, + BindingTimeout: &metav1.Duration{Duration: config.DynamicResourcesBindingTimeoutDefault}, }, }, "valid args (disabled)": { @@ -1195,11 +1198,76 @@ func TestValidateDynamicResourcesArgs(t *testing.T) { }, }, }, + + // BindingTimeout tests + "valid BindingTimeout": { + args: config.DynamicResourcesArgs{ + BindingTimeout: &metav1.Duration{Duration: 30 * time.Second}, + }, + }, + "BindingTimeout < 1s (0s)": { + args: config.DynamicResourcesArgs{ + BindingTimeout: &metav1.Duration{Duration: 0}, + }, + wantErrs: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "bindingTimeout", + Detail: "must be at least 1 second", + }, + }, + }, + "BindingTimeout < 1s (negative)": { + args: config.DynamicResourcesArgs{ + BindingTimeout: &metav1.Duration{Duration: -time.Second}, + }, + wantErrs: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "bindingTimeout", + Detail: "must be at least 1 second", + }, + }, + }, + "BindingTimeout set but DRADeviceBindingConditions disabled": { + args: config.DynamicResourcesArgs{ + BindingTimeout: &metav1.Duration{Duration: time.Second}, + }, + bindingConditionsDisabled: true, + wantErrs: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeForbidden, + Field: "bindingTimeout", + Detail: "requires DRADeviceBindingConditions and DRAResourceClaimDeviceStatus feature gates to be enabled", + }, + }, + }, + "BindingTimeout set but DRAResourceClaimDeviceStatus disabled": { + args: config.DynamicResourcesArgs{ + BindingTimeout: &metav1.Duration{Duration: time.Second}, + }, + deviceStatusDisabled: true, + wantErrs: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeForbidden, + Field: "bindingTimeout", + Detail: "requires DRADeviceBindingConditions and DRAResourceClaimDeviceStatus feature gates to be enabled", + }, + }, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { - err := ValidateDynamicResourcesArgs(nil, &tc.args, schedfeature.Features{EnableDRASchedulerFilterTimeout: !tc.filterTimeoutDisabled}) + err := ValidateDynamicResourcesArgs( + nil, + &tc.args, + schedfeature.Features{ + EnableDRASchedulerFilterTimeout: !tc.filterTimeoutDisabled, + EnableDRADeviceBindingConditions: !tc.bindingConditionsDisabled, + EnableDRAResourceClaimDeviceStatus: !tc.deviceStatusDisabled, + }, + ) if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" { t.Errorf("ValidateDynamicResourcesArgs returned err (-want,+got):\n%s", diff) } diff --git a/pkg/scheduler/apis/config/zz_generated.deepcopy.go b/pkg/scheduler/apis/config/zz_generated.deepcopy.go index d8e9a5db997..2d78d58e587 100644 --- a/pkg/scheduler/apis/config/zz_generated.deepcopy.go +++ b/pkg/scheduler/apis/config/zz_generated.deepcopy.go @@ -61,6 +61,11 @@ func (in *DynamicResourcesArgs) DeepCopyInto(out *DynamicResourcesArgs) { *out = new(v1.Duration) **out = **in } + if in.BindingTimeout != nil { + in, out := &in.BindingTimeout, &out.BindingTimeout + *out = new(v1.Duration) + **out = **in + } return } diff --git a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go index 935f978cd64..9ed029ee52b 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go @@ -73,10 +73,6 @@ const ( // some actual ResourceClaim in the apiserver. specialClaimInMemName = "" - // BindingTimeoutDefaultSeconds is the default timeout for waiting for - // BindingConditions to be ready. - BindingTimeoutDefaultSeconds = 600 - // AssumeExtendedResourceTimeoutDefaultSeconds is the default timeout for waiting // for the extended resource claim to be updated in assumed cache. AssumeExtendedResourceTimeoutDefaultSeconds = 120 @@ -161,13 +157,14 @@ type nodeAllocation struct { // DynamicResources is a plugin that ensures that ResourceClaims are allocated. type DynamicResources struct { - enabled bool - fts feature.Features - filterTimeout time.Duration - fh fwk.Handle - clientset kubernetes.Interface - celCache *cel.Cache - draManager fwk.SharedDRAManager + enabled bool + fts feature.Features + filterTimeout time.Duration + bindingTimeout time.Duration + fh fwk.Handle + clientset kubernetes.Interface + celCache *cel.Cache + draManager fwk.SharedDRAManager } // New initializes a new plugin and returns it. @@ -189,7 +186,10 @@ func New(ctx context.Context, plArgs runtime.Object, fh fwk.Handle, fts feature. enabled: true, fts: fts, filterTimeout: ptr.Deref(args.FilterTimeout, metav1.Duration{}).Duration, - + bindingTimeout: ptr.Deref( + args.BindingTimeout, + metav1.Duration{Duration: config.DynamicResourcesBindingTimeoutDefault}, + ).Duration, fh: fh, clientset: fh.ClientSet(), // This is a LRU cache for compiled CEL expressions. The most @@ -1320,7 +1320,7 @@ func (pl *DynamicResources) PreBind(ctx context.Context, cs fwk.CycleState, pod // We need to wait for the device to be attached to the node. pl.fh.EventRecorder().Eventf(pod, nil, v1.EventTypeNormal, "BindingConditionsPending", "Scheduling", "waiting for binding conditions for device on node %s", nodeName) - err = wait.PollUntilContextTimeout(ctx, 5*time.Second, time.Duration(BindingTimeoutDefaultSeconds)*time.Second, true, + err = wait.PollUntilContextTimeout(ctx, 5*time.Second, pl.bindingTimeout, true, func(ctx context.Context) (bool, error) { return pl.isPodReadyForBinding(state) }) @@ -1627,7 +1627,7 @@ func (pl *DynamicResources) isClaimTimeout(claim *resourceapi.ResourceClaim) boo if deviceRequest.BindingConditions == nil { continue } - if claim.Status.Allocation.AllocationTimestamp.Add(time.Duration(BindingTimeoutDefaultSeconds) * time.Second).Before(time.Now()) { + if claim.Status.Allocation.AllocationTimestamp.Add(pl.bindingTimeout).Before(time.Now()) { return true } } diff --git a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go index 15d1ededbd7..d5b43af5650 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go @@ -1797,7 +1797,10 @@ func TestPlugin(t *testing.T) { "prebind-fail-with-binding-timeout": { enableDRADeviceBindingConditions: true, enableDRAResourceClaimDeviceStatus: true, - pod: podWithClaimName, + args: &config.DynamicResourcesArgs{ + BindingTimeout: &metav1.Duration{Duration: 600 * time.Second}, + }, + pod: podWithClaimName, claims: func() []*resourceapi.ResourceClaim { claim := allocatedClaim.DeepCopy() claim.Status.Allocation = allocationResultWithBindingConditions.DeepCopy() diff --git a/staging/src/k8s.io/kube-scheduler/config/v1/types.go b/staging/src/k8s.io/kube-scheduler/config/v1/types.go index 81e6354db11..8ff68a6ef02 100644 --- a/staging/src/k8s.io/kube-scheduler/config/v1/types.go +++ b/staging/src/k8s.io/kube-scheduler/config/v1/types.go @@ -432,6 +432,33 @@ type DynamicResourcesArgs struct { // // Setting it to zero completely disables the timeout. FilterTimeout *metav1.Duration `json:"filterTimeout"` + + // BindingTimeout limits how long the PreBind extension point may wait for + // ResourceClaim device BindingConditions to become satisfied when such + // conditions are present. While waiting, the scheduler periodically checks + // device status. If the timeout elapses before all required conditions are + // true (or any bindingFailureConditions become true), the allocation is + // cleared and the Pod re-enters scheduling queue. Note that the same or other node may be + // chosen if feasible; otherwise the Pod is placed in the unschedulable queue and + // retried based on cluster changes and backoff. + // + // Defaults & feature gates: + // - Defaults to 10 minutes when the DRADeviceBindingConditions feature gate is enabled. + // - Has effect only when BOTH DRADeviceBindingConditions and + // DRAResourceClaimDeviceStatus are enabled; otherwise omit this field. + // - When DRADeviceBindingConditions is disabled, setting this field is considered an error. + // + // Valid values: + // - >=1s (non-zero). No upper bound is enforced. + // + // Tuning guidance: + // - Lower values reduce time-to-retry when devices aren’t ready but can + // increase churn if drivers typically need longer to report readiness. + // - Review scheduler latency metrics (e.g. PreBind duration in + // `scheduler_framework_extension_point_duration_seconds`) and driver + // readiness behavior before tightening this timeout. + BindingTimeout *metav1.Duration `json:"bindingTimeout,omitempty"` } const DynamicResourcesFilterTimeoutDefault = 10 * time.Second +const DynamicResourcesBindingTimeoutDefault = 600 * time.Second diff --git a/staging/src/k8s.io/kube-scheduler/config/v1/zz_generated.deepcopy.go b/staging/src/k8s.io/kube-scheduler/config/v1/zz_generated.deepcopy.go index 840fb650e36..788a6027907 100644 --- a/staging/src/k8s.io/kube-scheduler/config/v1/zz_generated.deepcopy.go +++ b/staging/src/k8s.io/kube-scheduler/config/v1/zz_generated.deepcopy.go @@ -71,6 +71,11 @@ func (in *DynamicResourcesArgs) DeepCopyInto(out *DynamicResourcesArgs) { *out = new(metav1.Duration) **out = **in } + if in.BindingTimeout != nil { + in, out := &in.BindingTimeout, &out.BindingTimeout + *out = new(metav1.Duration) + **out = **in + } return }