Give more flexibility to requeueing parameters in the config api (#1701)

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
kubernetes-sigs · Feb 7, 2024 · 90fa327 · 90fa327
1 parent 39ca969
commit 90fa327
Show file tree

Hide file tree

Showing 9 changed files with 110 additions and 45 deletions.
diff --git a/apis/config/v1beta1/configuration_types.go b/apis/config/v1beta1/configuration_types.go
@@ -197,9 +197,9 @@ type WaitForPodsReady struct {
 	// and defaults to true otherwise.
 	BlockAdmission *bool `json:"blockAdmission,omitempty"`
 
-	// RequeuingTimestamp defines the timestamp used for requeuing a Workload
-	// that was evicted due to Pod readiness. Defaults to Eviction.
-	RequeuingTimestamp *RequeuingTimestamp `json:"requeuingTimestamp,omitempty"`
+	// RequeuingStrategy defines the strategy for requeuing a Workload.
+	// +optional
+	RequeuingStrategy *RequeuingStrategy `json:"requeuingStrategy,omitempty"`
 }
 
 type MultiKueue struct {
@@ -217,6 +217,13 @@ type MultiKueue struct {
 	Origin *string `json:"origin,omitempty"`
 }
 
+type RequeuingStrategy struct {
+	// Timestamp defines the timestamp used for requeuing a Workload
+	// that was evicted due to Pod readiness. Defaults to Eviction.
+	// +optional
+	Timestamp *RequeuingTimestamp `json:"timestamp,omitempty"`
+}
+
 type RequeuingTimestamp string
 
 const (
@@ -228,7 +235,6 @@ const (
 )
 
 type InternalCertManagement struct {
-
 	// Enable controls whether to enable internal cert management or not.
 	// Defaults to true. If you want to use a third-party management, e.g. cert-manager,
 	// set it to false. See the user guide for more information.

diff --git a/apis/config/v1beta1/defaults.go b/apis/config/v1beta1/defaults.go
@@ -120,8 +120,10 @@ func SetDefaults_Configuration(cfg *Configuration) {
 			}
 			cfg.WaitForPodsReady.BlockAdmission = &defaultBlockAdmission
 		}
-		if cfg.WaitForPodsReady.RequeuingTimestamp == nil {
-			cfg.WaitForPodsReady.RequeuingTimestamp = ptr.To(EvictionTimestamp)
+		if cfg.WaitForPodsReady.RequeuingStrategy == nil || cfg.WaitForPodsReady.RequeuingStrategy.Timestamp == nil {
+			cfg.WaitForPodsReady.RequeuingStrategy = &RequeuingStrategy{
+				Timestamp: ptr.To(EvictionTimestamp),
+			}
 		}
 	}
 	if cfg.Integrations == nil {

diff --git a/apis/config/v1beta1/defaults_test.go b/apis/config/v1beta1/defaults_test.go
@@ -359,10 +359,12 @@ func TestSetDefaults_Configuration(t *testing.T) {
 			},
 			want: &Configuration{
 				WaitForPodsReady: &WaitForPodsReady{
-					Enable:             true,
-					BlockAdmission:     ptr.To(true),
-					Timeout:            &podsReadyTimeoutTimeout,
-					RequeuingTimestamp: ptr.To(EvictionTimestamp),
+					Enable:         true,
+					BlockAdmission: ptr.To(true),
+					Timeout:        &podsReadyTimeoutTimeout,
+					RequeuingStrategy: &RequeuingStrategy{
+						Timestamp: ptr.To(EvictionTimestamp),
+					},
 				},
 				Namespace:         ptr.To(DefaultNamespace),
 				ControllerManager: defaultCtrlManagerConfigurationSpec,
@@ -386,10 +388,12 @@ func TestSetDefaults_Configuration(t *testing.T) {
 			},
 			want: &Configuration{
 				WaitForPodsReady: &WaitForPodsReady{
-					Enable:             false,
-					BlockAdmission:     ptr.To(false),
-					Timeout:            &podsReadyTimeoutTimeout,
-					RequeuingTimestamp: ptr.To(EvictionTimestamp),
+					Enable:         false,
+					BlockAdmission: ptr.To(false),
+					Timeout:        &podsReadyTimeoutTimeout,
+					RequeuingStrategy: &RequeuingStrategy{
+						Timestamp: ptr.To(EvictionTimestamp),
+					},
 				},
 				Namespace:         ptr.To(DefaultNamespace),
 				ControllerManager: defaultCtrlManagerConfigurationSpec,
@@ -405,20 +409,24 @@ func TestSetDefaults_Configuration(t *testing.T) {
 		"respecting provided waitForPodsReady values": {
 			original: &Configuration{
 				WaitForPodsReady: &WaitForPodsReady{
-					Enable:             true,
-					Timeout:            &podsReadyTimeoutOverwrite,
-					RequeuingTimestamp: ptr.To(CreationTimestamp),
+					Enable:  true,
+					Timeout: &podsReadyTimeoutOverwrite,
+					RequeuingStrategy: &RequeuingStrategy{
+						Timestamp: ptr.To(CreationTimestamp),
+					},
 				},
 				InternalCertManagement: &InternalCertManagement{
 					Enable: ptr.To(false),
 				},
 			},
 			want: &Configuration{
 				WaitForPodsReady: &WaitForPodsReady{
-					Enable:             true,
-					BlockAdmission:     ptr.To(true),
-					Timeout:            &podsReadyTimeoutOverwrite,
-					RequeuingTimestamp: ptr.To(CreationTimestamp),
+					Enable:         true,
+					BlockAdmission: ptr.To(true),
+					Timeout:        &podsReadyTimeoutOverwrite,
+					RequeuingStrategy: &RequeuingStrategy{
+						Timestamp: ptr.To(CreationTimestamp),
+					},
 				},
 				Namespace:         ptr.To(DefaultNamespace),
 				ControllerManager: defaultCtrlManagerConfigurationSpec,

diff --git a/apis/config/v1beta1/zz_generated.deepcopy.go b/apis/config/v1beta1/zz_generated.deepcopy.go
diff --git a/cmd/kueue/main.go b/cmd/kueue/main.go
@@ -344,8 +344,9 @@ func waitForPodsReady(cfg *configapi.Configuration) bool {
 }
 
 func podsReadyRequeuingTimestamp(cfg *configapi.Configuration) configapi.RequeuingTimestamp {
-	if cfg.WaitForPodsReady != nil && cfg.WaitForPodsReady.RequeuingTimestamp != nil {
-		return *cfg.WaitForPodsReady.RequeuingTimestamp
+	if cfg.WaitForPodsReady != nil && cfg.WaitForPodsReady.RequeuingStrategy != nil &&
+		cfg.WaitForPodsReady.RequeuingStrategy.Timestamp != nil {
+		return *cfg.WaitForPodsReady.RequeuingStrategy.Timestamp
 	}
 	return configapi.EvictionTimestamp
 }

diff --git a/keps/1282-pods-ready-requeue-strategy/README.md b/keps/1282-pods-ready-requeue-strategy/README.md
@@ -124,18 +124,18 @@ Possible settings:
 ```go
 type WaitForPodsReady struct {
 	...
-	// requeuingStrategy defines the strategy for requeuing a Workload
+	// RequeuingStrategy defines the strategy for requeuing a Workload
 	// +optional
 	RequeuingStrategy *RequeuingStrategy `json:"requeuingStrategy,omitempty"`
 }
 
 type RequeuingStrategy struct {
-	// timestamp defines the timestamp used for requeuing a Workload
+	// Timestamp defines the timestamp used for requeuing a Workload
 	// that was evicted due to Pod readiness. Defaults to Eviction.
 	// +optional
 	Timestamp *RequeuingTimestamp `json:"timestamp,omitempty"`
 
-	// backoffLimitCount defines the maximum number of requeuing retries.
+	// BackoffLimitCount defines the maximum number of requeuing retries.
 	// When the number is reached, the workload is deactivated (`.spec.activate`=`false`).
 	//
 	// Defaults to null. 
@@ -146,10 +146,10 @@ type RequeuingStrategy struct {
 type RequeuingTimestamp string
 
 const (
-	// creationTimestamp timestamp (from Workload .metadata.creationTimestamp).
+	// CreationTimestamp timestamp (from Workload .metadata.creationTimestamp).
 	CreationTimestamp RequeuingTimestamp = "Creation"
 
-	// evictionTimestamp timestamp (from Workload .status.conditions).
+	// EvictionTimestamp timestamp (from Workload .status.conditions).
 	EvictionTimestamp RequeuingTimestamp = "Eviction"
 )
 ```

diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
@@ -539,10 +539,12 @@ multiKueue:
 				ManageJobsWithoutQueueName: false,
 				InternalCertManagement:     enableDefaultInternalCertManagement,
 				WaitForPodsReady: &configapi.WaitForPodsReady{
-					Enable:             true,
-					BlockAdmission:     ptr.To(true),
-					Timeout:            &metav1.Duration{Duration: 5 * time.Minute},
-					RequeuingTimestamp: ptr.To(configapi.EvictionTimestamp),
+					Enable:         true,
+					BlockAdmission: ptr.To(true),
+					Timeout:        &metav1.Duration{Duration: 5 * time.Minute},
+					RequeuingStrategy: &configapi.RequeuingStrategy{
+						Timestamp: ptr.To(configapi.EvictionTimestamp),
+					},
 				},
 				ClientConnection: defaultClientConnection,
 				Integrations:     defaultIntegrations,

diff --git a/site/content/en/docs/reference/kueue-config.v1beta1.md b/site/content/en/docs/reference/kueue-config.v1beta1.md
@@ -580,13 +580,38 @@ Defaults to 5.</p>
 </tbody>
 </table>
 
+## `RequeuingStrategy`     {#RequeuingStrategy}
+
+
+**Appears in:**
+
+- [WaitForPodsReady](#WaitForPodsReady)
+
+
+
+<table class="table">
+<thead><tr><th width="30%">Field</th><th>Description</th></tr></thead>
+<tbody>
+
+
+<tr><td><code>timestamp</code><br/>
+<a href="#RequeuingTimestamp"><code>RequeuingTimestamp</code></a>
+</td>
+<td>
+   <p>Timestamp defines the timestamp used for requeuing a Workload
+that was evicted due to Pod readiness. Defaults to Eviction.</p>
+</td>
+</tr>
+</tbody>
+</table>
+
 ## `RequeuingTimestamp`     {#RequeuingTimestamp}
 
 (Alias of `string`)
 
 **Appears in:**
 
-- [WaitForPodsReady](#WaitForPodsReady)
+- [RequeuingStrategy](#RequeuingStrategy)
 
 
 
@@ -634,12 +659,11 @@ until the jobs reach the PodsReady=true condition. It defaults to false if Enabl
 and defaults to true otherwise.</p>
 </td>
 </tr>
-<tr><td><code>requeuingTimestamp</code> <B>[Required]</B><br/>
-<a href="#RequeuingTimestamp"><code>RequeuingTimestamp</code></a>
+<tr><td><code>requeuingStrategy</code><br/>
+<a href="#RequeuingStrategy"><code>RequeuingStrategy</code></a>
 </td>
 <td>
-   <p>RequeuingTimestamp defines the timestamp used for requeuing a Workload
-that was evicted due to Pod readiness. Defaults to Eviction.</p>
+   <p>RequeuingStrategy defines the strategy for requeuing a Workload.</p>
 </td>
 </tr>
 </tbody>

diff --git a/test/integration/scheduler/podsready/suite_test.go b/test/integration/scheduler/podsready/suite_test.go
@@ -60,10 +60,12 @@ func TestSchedulerWithWaitForPodsReady(t *testing.T) {
 func managerAndSchedulerSetupWithTimeoutAdmission(mgr manager.Manager, ctx context.Context, value time.Duration, blockAdmission bool, requeuingTimestamp config.RequeuingTimestamp) {
 	cfg := &config.Configuration{
 		WaitForPodsReady: &config.WaitForPodsReady{
-			Enable:             true,
-			BlockAdmission:     &blockAdmission,
-			Timeout:            &metav1.Duration{Duration: value},
-			RequeuingTimestamp: ptr.To(requeuingTimestamp),
+			Enable:         true,
+			BlockAdmission: &blockAdmission,
+			Timeout:        &metav1.Duration{Duration: value},
+			RequeuingStrategy: &config.RequeuingStrategy{
+				Timestamp: ptr.To(requeuingTimestamp),
+			},
 		},
 	}
 	mgr.GetScheme().Default(cfg)