feat(nodepool): support timezone for disruption budgets

kwok/charts/crds/karpenter.sh_nodepools.yaml

@@ -127,10 +127,15 @@ spec:
                             description: |-
                               Schedule specifies when a budget begins being active, following
                               the upstream cronjob syntax. If omitted, the budget is always active.
-                              Timezones are not supported.
                               This field is required if Duration is set.
                             pattern: ^(@(annually|yearly|monthly|weekly|daily|midnight|hourly))|((.+)\s(.+)\s(.+)\s(.+)\s(.+))$
                             type: string
+                          timeZone:
+                            description: |-
+                              TimeZone specifies the timezone the Schedule is evaluated against.
+                              If not specified, this will default to the UTC.
+                              See https://en.wikipedia.org/wiki/List_of_tz_database_time_zones for valid time zones.
+                            type: string
                         required:
                           - nodes
                         type: object
@@ -139,6 +144,8 @@ spec:
                       x-kubernetes-validations:
                         - message: '''schedule'' must be set with ''duration'''
                           rule: self.all(x, has(x.schedule) == has(x.duration))
+                        - message: '''timeZone'' can only be set when ''schedule'' is set'
+                          rule: self.all(x, !has(x.timeZone) || has(x.schedule))
                     consolidateAfter:
                       description: |-
                         ConsolidateAfter is the duration the controller will wait

pkg/apis/crds/karpenter.sh_nodepools.yaml

@@ -127,10 +127,15 @@ spec:
                             description: |-
                               Schedule specifies when a budget begins being active, following
                               the upstream cronjob syntax. If omitted, the budget is always active.
-                              Timezones are not supported.
                               This field is required if Duration is set.
                             pattern: ^(@(annually|yearly|monthly|weekly|daily|midnight|hourly))|((.+)\s(.+)\s(.+)\s(.+)\s(.+))$
                             type: string
+                          timeZone:
+                            description: |-
+                              TimeZone specifies the timezone the Schedule is evaluated against.
+                              If not specified, this will default to the UTC.
+                              See https://en.wikipedia.org/wiki/List_of_tz_database_time_zones for valid time zones.
+                            type: string
                         required:
                           - nodes
                         type: object
@@ -139,6 +144,8 @@ spec:
                       x-kubernetes-validations:
                         - message: '''schedule'' must be set with ''duration'''
                           rule: self.all(x, has(x.schedule) == has(x.duration))
+                        - message: '''timeZone'' can only be set when ''schedule'' is set'
+                          rule: self.all(x, !has(x.timeZone) || has(x.schedule))
                     consolidateAfter:
                       description: |-
                         ConsolidateAfter is the duration the controller will wait

pkg/apis/v1/nodepool.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"math"
 	"strconv"
+	"time"
 
 	"github.com/mitchellh/hashstructure/v2"
 	"github.com/robfig/cron/v3"
@@ -77,6 +78,7 @@ type Disruption struct {
 	// 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))"
+	// +kubebuilder:validation:XValidation:message="'timeZone' can only be set when 'schedule' is set",rule="self.all(x, !has(x.timeZone) || has(x.schedule))"
 	// +kubebuilder:default:={{nodes: "10%"}}
 	// +kubebuilder:validation:MaxItems=50
 	// +optional
@@ -103,7 +105,6 @@ type Budget struct {
 	Nodes string `json:"nodes" hash:"ignore"`
 	// Schedule specifies when a budget begins being active, following
 	// the upstream cronjob syntax. If omitted, the budget is always active.
-	// Timezones are not supported.
 	// This field is required if Duration is set.
 	// +kubebuilder:validation:Pattern:=`^(@(annually|yearly|monthly|weekly|daily|midnight|hourly))|((.+)\s(.+)\s(.+)\s(.+)\s(.+))$`
 	// +optional
@@ -118,6 +119,12 @@ type Budget struct {
 	// +kubebuilder:validation:Type="string"
 	// +optional
 	Duration *metav1.Duration `json:"duration,omitempty" hash:"ignore"`
+	// TimeZone specifies the timezone the Schedule is evaluated against.
+	// If not specified, this will default to the UTC.
+	// See https://en.wikipedia.org/wiki/List_of_tz_database_time_zones for valid time zones.
+	// +kubebuilder:validation:Type="string"
+	// +optional
+	TimeZone *string `json:"timeZone,omitempty" hash:"ignore"`
 }
 
 type ConsolidationPolicy string
@@ -354,16 +361,24 @@ func (in *Budget) IsActive(c clock.Clock) (bool, error) {
 	if in.Schedule == nil && in.Duration == nil {
 		return true, nil
 	}
-	schedule, err := cron.ParseStandard(fmt.Sprintf("TZ=UTC %s", lo.FromPtr(in.Schedule)))
+	tz := "UTC"
+	if in.TimeZone != nil && *in.TimeZone != "" {
+		tz = *in.TimeZone
+	}
+	loc, err := time.LoadLocation(tz)
+	if err != nil {
+		return false, fmt.Errorf("invalid time zone %q: %w", tz, err)
+	}
+	schedule, err := cron.ParseStandard(fmt.Sprintf("TZ=%s %s", tz, lo.FromPtr(in.Schedule)))
 	if err != nil {
 		// Should only occur if there's a discrepancy
 		// with the validation regex and the cron package.
 		return false, fmt.Errorf("invariant violated, invalid cron %s", schedule)
 	}
 	// Walk back in time for the duration associated with the schedule
-	checkPoint := c.Now().UTC().Add(-lo.FromPtr(in.Duration).Duration)
+	checkPoint := c.Now().In(loc).Add(-lo.FromPtr(in.Duration).Duration)
 	nextHit := schedule.Next(checkPoint)
-	return !nextHit.After(c.Now().UTC()), nil
+	return !nextHit.After(c.Now().In(loc)), nil
 }
 
 func GetIntStrFromValue(str string) intstr.IntOrString {

pkg/apis/v1/nodepool_budgets_test.go

@@ -149,7 +149,6 @@ var _ = Describe("Budgets", func() {
 		})
 
 		It("should get the minimum budget for each reason", func() {
-
 			nodePool.Spec.Disruption.Budgets = append(nodePool.Spec.Disruption.Budgets,
 				[]Budget{
 					{
@@ -173,7 +172,6 @@ var _ = Describe("Budgets", func() {
 			Expect(err).To(BeNil())
 			Expect(underutilizedAllowedDisruption).To(Equal(10))
 		})
-
 	})
 
 	Context("AllowedDisruptions", func() {
@@ -209,7 +207,7 @@ var _ = Describe("Budgets", func() {
 	})
 
 	Context("IsActive", func() {
-		It("should always consider a schedule and time in UTC", func() {
+		It("should consider a schedule and time in UTC when no timezone is specified", func() {
 			// Set the time to start of June 2000 in a time zone 1 hour ahead of UTC
 			fakeClock = clock.NewFakeClock(time.Date(2000, time.June, 0, 0, 0, 0, 0, time.FixedZone("fake-zone", 3600)))
 			budgets[0].Schedule = lo.ToPtr("@daily")
@@ -237,6 +235,29 @@ var _ = Describe("Budgets", func() {
 			Expect(err).To(Succeed())
 			Expect(active).To(BeFalse())
 		})
+		It("should reutn that a schedule is active with a timezone", func() {
+			// Set the time to the middle of the year of 2000 as KST, the best year ever
+			fakeClock = clock.NewFakeClock(time.Date(2000, time.June, 15, 12, 30, 30, 0, time.FixedZone("Asia/Seoul", 9*3600)))
+			budgets[0].TimeZone = lo.ToPtr("Asia/Seoul")
+			active, err := budgets[0].IsActive(fakeClock)
+			Expect(err).To(Succeed())
+			Expect(active).To(BeTrue())
+		})
+		It("should return that a schedule is inactive with a timezone", func() {
+			// Set the time to the middle of the year of 2000 as KST, the best year ever
+			fakeClock = clock.NewFakeClock(time.Date(2000, time.June, 15, 12, 30, 30, 0, time.FixedZone("Asia/Seoul", 9*3600)))
+			budgets[0].Schedule = lo.ToPtr("@yearly")
+			budgets[0].TimeZone = lo.ToPtr("Asia/Seoul")
+			active, err := budgets[0].IsActive(fakeClock)
+			Expect(err).To(Succeed())
+			Expect(active).To(BeFalse())
+		})
+		It("should return an error if the timezone is invalid", func() {
+			budgets[0].TimeZone = lo.ToPtr("Invalid/Timezone")
+			_, err := budgets[0].IsActive(fakeClock)
+			Expect(err).ToNot(Succeed())
+			Expect(err.Error()).To(ContainSubstring("invalid time zone"))
+		})
 		It("should return that a schedule is active when the schedule hit is in the middle of the duration", func() {
 			// Set the date to the start of the year 1000, the best year ever
 			fakeClock = clock.NewFakeClock(time.Date(1000, time.January, 1, 12, 0, 0, 0, time.UTC))