chore: change naming of budget.crontab and budget.maxUnavailable (#850)

designs/disruption-controls.md

@@ -19,11 +19,11 @@ spec: # This is not a complete NodePool Spec.
     expireAfter: 10m || Never # Equivalent to v1alpha5 TTLSecondsUntilExpired
     budgets:
     # On Weekdays during business hours, don't do any deprovisioning.
-    - crontab: "0 9 * * mon-fri"
+    - schedule: "0 9 * * mon-fri"
       duration: 8h
-      maxUnavailable: 0
+      nodes: 0
     # Every other time, only allow 10 nodes to be deprovisioned simultaneously
-    - maxUnavailable: 10
+    - nodes: 10
 ```
 
 ## Code Definition
@@ -32,7 +32,7 @@ spec: # This is not a complete NodePool Spec.
 type Disruption struct {
     {...}
     // Budgets is a list of Budgets.
-    // If there are multiple active budgets, the most restrictive budget's maxUnavailable is respected.
+    // If there are multiple active budgets, the most restrictive budget's value is respected.
     Budgets []Budget `json:"budgets,omitempty" hash:"ignore"`
     // ConsolidateAfter is a nillable duration, parsed as a metav1.Duration.
     // Users can use "Never" to disable Consolidation.
@@ -50,27 +50,27 @@ type Disruption struct {
 // number of Node Claims that can be terminated at a time.
 // Unless specified, a budget is always active.
 type Budget struct {
-    // MaxUnavailable dictates how many NodeClaims owned by this NodePool
+    // Nodes dictates how many NodeClaims owned by this NodePool
     // can be terminating at once.
     // This only respects and considers NodeClaims with the karpenter.sh/disruption taint.
-    MaxUnavailable intstr.IntOrString `json:"maxUnavailable" hash:"ignore"`
-    // Crontab specifies when a budget begins being active.
-    // Crontab uses the same syntax as a Cronjob.
+    Nodes intstr.IntOrString `json:"nodes" hash:"ignore"`
+    // Schedule specifies when a budget begins being active.
+    // Schedule uses the same syntax as a Cronjob.
     // And can support a TZ.
     // "Minute Hour DayOfMonth Month DayOfWeek"
     // This is required if Duration is set.
-    Crontab *string `json:"crontab,omitempty" hash:"ignore"`
-    // Duration determines how long a Budget is active since each Crontab hit.
-    // This is required if Crontab is set.
+    Schedule *string `json:"schedule,omitempty" hash:"ignore"`
+    // Duration determines how long a Budget is active since each schedule hit.
+    // This is required if schedule is set.
     Duration *metav1.Duration `json:"duration,omitempty" hash:"ignore"`
 }
 ```
 
 ## Validation/Defaults
 
-For each `Budget`, `MaxUnavailable` is required, and must be non-negative. Users can disable scale down for a NodePool by setting this to `0`. Users must either omit both `Crontab` and `Duration` or set both of them, since `Crontab` and `Duration` are inherently linked. Omitting these two fields will be equivalent to an always active `Budget`. Users cannot define a seconds value in `Duration`, since the smallest denomination of time in upstream Crontabs are minutes. Note that `MaxUnavailable` will only refer to nodes with the `karpenter.sh/disruption` taint set.
+For each `Budget`, `Nodes` is required, and must be non-negative. Users can disable scale down for a NodePool by setting this to `0`. Users must either omit both `Schedule` and `Duration` or set both of them, since `Schedule` and `Duration` are inherently linked. Omitting these two fields will be equivalent to an always active `Budget`. Users cannot define a seconds nodes in `Duration`, since the smallest denomination of time in upstream Schedules are minutes. Note that `Nodes` will only refer to nodes with the `karpenter.sh/disruption` taint set.
 
-- Omitting the field `Budgets` will cause the field to be defaulted to one `Budget` with `MaxUnavailable: 10%`.
+- Omitting the field `Budgets` will cause the field to be defaulted to one `Budget` with `Nodes: 10%`.
 - `ConsolidationPolicy` will be defaulted to `WhenUnderutilized`, with a `consolidateAfter` value of `15s`, which is the same value for Consolidation in v1alpha5.
 
 ```yaml
@@ -84,9 +84,9 @@ spec:
     consolidateAfter: 15s
     expireAfter: 30d
     budgets:
-    - maxUnavailable: 10%
+    - nodes: 10%
 ```
-Karpenter will not persist a default for the `Crontab` and `Duration` fields.
+Karpenter will not persist a default for the `Schedule` and `Duration` fields.
 
 ## API Choices
 

pkg/apis/crds/karpenter.sh_nodepools.yaml

@@ -48,32 +48,32 @@ spec:
                   properties:
                     budgets:
                       default:
-                        - maxUnavailable: 10%
-                      description: Budgets is a list of Budgets. If there are multiple active budgets, Karpenter uses the most restrictive maxUnavailable. If left undefined, this will default to one budget with a maxUnavailable to 10%.
+                        - nodes: 10%
+                      description: Budgets is a list of Budgets. If there are multiple active budgets, Karpenter uses the most restrictive value. If left undefined, this will default to one budget with a value to 10%.
                       items:
                         description: Budget defines when Karpenter will restrict the number of Node Claims that can be terminating simultaneously.
                         properties:
-                          crontab:
-                            description: Crontab specifies when a budget begins being active, using the upstream cronjob syntax. If omitted, the budget is always active. Currently timezones are not supported. This is required if Duration is set.
-                            pattern: ^(@(annually|yearly|monthly|weekly|daily|midnight|hourly))|((.+)\s(.+)\s(.+)\s(.+)\s(.+))$
-                            type: string
                           duration:
-                            description: Duration determines how long a Budget is active since each Crontab hit. Only minutes and hours are accepted, as cron does not work in seconds. If omitted, the budget is always active. This is required if Crontab is set. This regex has an optional 0s at the end since the duration.String() always adds a 0s at the end.
+                            description: Duration determines how long a Budget is active since each Schedule hit. Only minutes and hours are accepted, as cron does not work in seconds. If omitted, the budget is always active. This is required if Schedule is set. This regex has an optional 0s at the end since the duration.String() always adds a 0s at the end.
                             pattern: ^([0-9]+(m|h)+(0s)?)$
                             type: string
-                          maxUnavailable:
+                          nodes:
                             default: 10%
-                            description: 'MaxUnavailable dictates how many NodeClaims owned by this NodePool can be terminating at once. It must be set. This only considers NodeClaims with the karpenter.sh/disruption taint. We can''t use an intstr.IntOrString since kubebuilder doesn''t support pattern checking for int values for IntOrString values. Ref: https://github.com/kubernetes-sigs/controller-tools/blob/55efe4be40394a288216dab63156b0a64fb82929/pkg/crd/markers/validation.go#L379-L388'
+                            description: 'Nodes dictates how many NodeClaims owned by this NodePool can be terminating at once. It must be set. This only considers NodeClaims with the karpenter.sh/disruption taint. We can''t use an intstr.IntOrString since kubebuilder doesn''t support pattern checking for int nodes for IntOrString nodes. Ref: https://github.com/kubernetes-sigs/controller-tools/blob/55efe4be40394a288216dab63156b0a64fb82929/pkg/crd/markers/validation.go#L379-L388'
                             pattern: ^((100|[0-9]{1,2})%|[0-9]+)$
                             type: string
+                          schedule:
+                            description: Schedule specifies when a budget begins being active, using the upstream cronjob syntax. If omitted, the budget is always active. Currently timezones are not supported. This is required if Duration is set.
+                            pattern: ^(@(annually|yearly|monthly|weekly|daily|midnight|hourly))|((.+)\s(.+)\s(.+)\s(.+)\s(.+))$
+                            type: string
                         required:
-                          - maxUnavailable
+                          - nodes
                         type: object
                       maxItems: 50
                       type: array
                       x-kubernetes-validations:
-                        - message: '''crontab'' must be set with ''duration'''
-                          rule: '!self.all(x, (has(x.crontab) && !has(x.duration)) || (!has(x.crontab) && has(x.duration)))'
+                        - message: '''schedule'' must be set with ''duration'''
+                          rule: '!self.all(x, (has(x.schedule) && !has(x.duration)) || (!has(x.schedule) && has(x.duration)))'
                     consolidateAfter:
                       description: ConsolidateAfter is the duration the controller will wait before attempting to terminate nodes that are underutilized. Refer to ConsolidationPolicy for how underutilization is considered.
                       pattern: ^(([0-9]+(s|m|h))+)|(Never)$

pkg/apis/v1beta1/nodepool.go

@@ -89,10 +89,10 @@ type Disruption struct {
 	ExpireAfter NillableDuration `json:"expireAfter"`
 	// Budgets is a list of Budgets.
 	// If there are multiple active budgets, Karpenter uses
-	// the most restrictive maxUnavailable. If left undefined,
-	// this will default to one budget with a maxUnavailable to 10%.
-	// +kubebuilder:validation:XValidation:message="'crontab' must be set with 'duration'",rule="!self.all(x, (has(x.crontab) && !has(x.duration)) || (!has(x.crontab) && has(x.duration)))"
-	// +kubebuilder:default:={{maxUnavailable: "10%"}}
+	// the most restrictive value. If left undefined,
+	// this will default to one budget with a value to 10%.
+	// +kubebuilder:validation:XValidation:message="'schedule' must be set with 'duration'",rule="!self.all(x, (has(x.schedule) && !has(x.duration)) || (!has(x.schedule) && has(x.duration)))"
+	// +kubebuilder:default:={{nodes: "10%"}}
 	// +kubebuilder:validation:MaxItems=50
 	// +optional
 	Budgets []Budget `json:"budgets,omitempty" hash:"ignore"`
@@ -101,26 +101,26 @@ type Disruption struct {
 // Budget defines when Karpenter will restrict the
 // number of Node Claims that can be terminating simultaneously.
 type Budget struct {
-	// MaxUnavailable dictates how many NodeClaims owned by this NodePool
+	// Nodes dictates how many NodeClaims owned by this NodePool
 	// can be terminating at once. It must be set.
 	// This only considers NodeClaims with the karpenter.sh/disruption taint.
 	// We can't use an intstr.IntOrString since kubebuilder doesn't support pattern
-	// checking for int values for IntOrString values.
+	// checking for int nodes for IntOrString nodes.
 	// Ref: https://github.com/kubernetes-sigs/controller-tools/blob/55efe4be40394a288216dab63156b0a64fb82929/pkg/crd/markers/validation.go#L379-L388
 	// +kubebuilder:validation:Pattern:="^((100|[0-9]{1,2})%|[0-9]+)$"
 	// +kubebuilder:default:="10%"
-	MaxUnavailable string `json:"maxUnavailable" hash:"ignore"`
-	// Crontab specifies when a budget begins being active,
+	Nodes string `json:"nodes" hash:"ignore"`
+	// Schedule specifies when a budget begins being active,
 	// using the upstream cronjob syntax. If omitted, the budget is always active.
 	// Currently timezones are not supported.
 	// This is required if Duration is set.
 	// +kubebuilder:validation:Pattern:=`^(@(annually|yearly|monthly|weekly|daily|midnight|hourly))|((.+)\s(.+)\s(.+)\s(.+)\s(.+))$`
 	// +optional
-	Crontab *string `json:"crontab,omitempty" hash:"ignore"`
-	// Duration determines how long a Budget is active since each Crontab hit.
+	Schedule *string `json:"schedule,omitempty" hash:"ignore"`
+	// Duration determines how long a Budget is active since each Schedule hit.
 	// Only minutes and hours are accepted, as cron does not work in seconds.
 	// If omitted, the budget is always active.
-	// This is required if Crontab is set.
+	// This is required if Schedule is set.
 	// This regex has an optional 0s at the end since the duration.String() always adds
 	// a 0s at the end.
 	// +kubebuilder:validation:Pattern=`^([0-9]+(m|h)+(0s)?)$`
@@ -232,7 +232,7 @@ func (in *NodePool) GetAllowedDisruptions(ctx context.Context, c clock.Clock, nu
 
 // GetAllowedDisruptions returns an intstr.IntOrString that can be used a comparison
 // for calculating if a disruption action is allowed. It returns an error if the
-// crontab is invalid. This returns MAXINT if the value is unbounded.
+// schedule is invalid. This returns MAXINT if the value is unbounded.
 func (in *Budget) GetAllowedDisruptions(c clock.Clock, numNodes int) (int, error) {
 	active, err := in.IsActive(c)
 	if err != nil {
@@ -243,10 +243,10 @@ func (in *Budget) GetAllowedDisruptions(c clock.Clock, numNodes int) (int, error
 	}
 	var val intstr.IntOrString
 	// If err is nil, we treat it as an int.
-	if intVal, err := strconv.Atoi(in.MaxUnavailable); err == nil {
+	if intVal, err := strconv.Atoi(in.Nodes); err == nil {
 		val = intstr.FromInt(intVal)
 	} else {
-		val = intstr.FromString(in.MaxUnavailable)
+		val = intstr.FromString(in.Nodes)
 	}
 	res, err := intstr.GetScaledValueFromIntOrPercent(lo.ToPtr(val), numNodes, false)
 	if err != nil {
@@ -257,20 +257,20 @@ func (in *Budget) GetAllowedDisruptions(c clock.Clock, numNodes int) (int, error
 }
 
 // IsActive takes a clock as input and returns if a budget is active.
-// It walks back in time the time.Duration associated with the crontab,
+// It walks back in time the time.Duration associated with the schedule,
 // and checks if the next time the schedule will hit is before the current time.
-// If the last crontab hit is exactly the duration in the past, this means the
-// schedule is active, as any more crontab hits in between would only extend this
-// window. This ensures that any previous crontab hits for a schedule are considered.
+// If the last schedule hit is exactly the duration in the past, this means the
+// schedule is active, as any more schedule hits in between would only extend this
+// window. This ensures that any previous schedule hits for a schedule are considered.
 func (in *Budget) IsActive(c clock.Clock) (bool, error) {
-	if in.Crontab == nil && in.Duration == nil {
+	if in.Schedule == nil && in.Duration == nil {
 		return true, nil
 	}
-	schedule, err := cron.ParseStandard(lo.FromPtr(in.Crontab))
+	schedule, err := cron.ParseStandard(lo.FromPtr(in.Schedule))
 	if err != nil {
-		return false, fmt.Errorf("parsing crontab, %w", err)
+		return false, fmt.Errorf("parsing schedule, %w", err)
 	}
-	// Walk back in time for the duration associated with the crontab
+	// Walk back in time for the duration associated with the schedule
 	checkPoint := c.Now().Add(-lo.FromPtr(in.Duration).Duration)
 	nextHit := schedule.Next(checkPoint)
 	return !nextHit.After(c.Now()), nil