diff --git a/internal/store/cronjob.go b/internal/store/cronjob.go index 298cd6de58..9922b99caa 100644 --- a/internal/store/cronjob.go +++ b/internal/store/cronjob.go @@ -31,6 +31,8 @@ import ( "k8s.io/client-go/tools/cache" basemetrics "k8s.io/component-base/metrics" + "k8s.io/klog/v2" + "k8s.io/kube-state-metrics/v2/pkg/metric" generator "k8s.io/kube-state-metrics/v2/pkg/metric_generator" ) @@ -251,8 +253,10 @@ func cronJobMetricFamilies(allowAnnotationsList, allowLabelsList []string) []gen // If the cron job is suspended, don't track the next scheduled time nextScheduledTime, err := getNextScheduledTime(j.Spec.Schedule, j.Status.LastScheduleTime, j.CreationTimestamp, j.Spec.TimeZone) if err != nil { - panic(err) - } else if j.Spec.Suspend == nil || !*j.Spec.Suspend { + klog.ErrorS(err, "Failed to compute next schedule time for cronjob", "namespace", j.Namespace, "cronjob", j.Name, "schedule", j.Spec.Schedule, "timezone", j.Spec.TimeZone) + return &metric.Family{Metrics: ms} + } + if j.Spec.Suspend == nil || !*j.Spec.Suspend { ms = append(ms, &metric.Metric{ LabelKeys: []string{}, LabelValues: []string{}, diff --git a/internal/store/cronjob_test.go b/internal/store/cronjob_test.go index 4a93248621..0d609f2ca3 100644 --- a/internal/store/cronjob_test.go +++ b/internal/store/cronjob_test.go @@ -533,13 +533,16 @@ func TestCronJobStore(t *testing.T) { func TestGetNextScheduledTime(t *testing.T) { testCases := []struct { + Desc string schedule string lastScheduleTime metav1.Time createdTime metav1.Time timeZone string expected time.Time + expectErr bool }{ { + Desc: "valid schedule with UTC timezone", schedule: "0 */6 * * *", lastScheduleTime: metav1.Time{Time: ActiveRunningCronJob1LastScheduleTime}, createdTime: metav1.Time{Time: ActiveRunningCronJob1LastScheduleTime}, @@ -547,18 +550,48 @@ func TestGetNextScheduledTime(t *testing.T) { expected: ActiveRunningCronJob1LastScheduleTime.Add(time.Second*4 + time.Minute*25 + time.Hour), }, { + Desc: "valid schedule with named timezone", schedule: "0 */6 * * *", lastScheduleTime: metav1.Time{Time: ActiveRunningCronJob1LastScheduleTime}, createdTime: metav1.Time{Time: ActiveRunningCronJob1LastScheduleTime}, timeZone: TimeZone, expected: ActiveRunningCronJob1LastScheduleTime.Add(time.Second*4 + time.Minute*25 + time.Hour*5), }, + { + Desc: "invalid timezone returns error", + schedule: "0 */6 * * *", + lastScheduleTime: metav1.Time{Time: ActiveRunningCronJob1LastScheduleTime}, + createdTime: metav1.Time{Time: ActiveRunningCronJob1LastScheduleTime}, + timeZone: "Not/A_Real_Zone", + expectErr: true, + }, + { + Desc: "invalid schedule expression returns error", + schedule: "not a cron expression", + lastScheduleTime: metav1.Time{Time: ActiveRunningCronJob1LastScheduleTime}, + createdTime: metav1.Time{Time: ActiveRunningCronJob1LastScheduleTime}, + timeZone: "UTC", + expectErr: true, + }, } for _, test := range testCases { - actual, _ := getNextScheduledTime(test.schedule, &test.lastScheduleTime, test.createdTime, &test.timeZone) // #nosec G601 + actual, err := getNextScheduledTime(test.schedule, &test.lastScheduleTime, test.createdTime, &test.timeZone) // #nosec G601 + if test.expectErr { + if err == nil { + t.Errorf("%s: expected error, got nil (actual=%v)", test.Desc, actual) + } + if !actual.IsZero() { + t.Errorf("%s: expected zero time on error, got %v", test.Desc, actual) + } + continue + } + if err != nil { + t.Errorf("%s: unexpected error: %v", test.Desc, err) + continue + } if !actual.Equal(test.expected) { - t.Fatalf("%v: expected %v, actual %v", test.schedule, test.expected, actual) + t.Fatalf("%s: expected %v, actual %v", test.Desc, test.expected, actual) } }