From fc27a1d5678486aaf38ecbc73979c73e123cd1dc Mon Sep 17 00:00:00 2001 From: Kacper Rzetelski Date: Tue, 19 Mar 2024 23:56:59 +0100 Subject: [PATCH 1/6] Extend manager task spec with cron and timezone API fields --- pkg/api/scylla/v1/types_cluster.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/api/scylla/v1/types_cluster.go b/pkg/api/scylla/v1/types_cluster.go index 57e663345f7..e59f3ef5c64 100644 --- a/pkg/api/scylla/v1/types_cluster.go +++ b/pkg/api/scylla/v1/types_cluster.go @@ -329,6 +329,14 @@ type SchedulerTaskSpec struct { // +kubebuilder:default:=3 // +optional NumRetries *int64 `json:"numRetries,omitempty"` + + // cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s]. + // +optional + Cron *string `json:"cron,omitempty"` + + // timezone specifies the timezone of cron field. + // +optional + Timezone *string `json:"timezone,omitempty"` } type RepairTaskSpec struct { From 97d0643f1981539dac0817333b4388b6ea1caf06 Mon Sep 17 00:00:00 2001 From: Kacper Rzetelski Date: Tue, 19 Mar 2024 23:58:20 +0100 Subject: [PATCH 2/6] Support cron and timezone API fields in manager task conversion funcs --- pkg/controller/manager/types_old.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkg/controller/manager/types_old.go b/pkg/controller/manager/types_old.go index d5ea4e513ff..f7e52177aca 100644 --- a/pkg/controller/manager/types_old.go +++ b/pkg/controller/manager/types_old.go @@ -79,6 +79,8 @@ func (r *RepairTask) FromManager(t *managerclient.TaskListItem) error { r.Interval = pointer.Ptr(t.Schedule.Interval) r.StartDate = pointer.Ptr(t.Schedule.StartDate.String()) r.NumRetries = pointer.Ptr(t.Schedule.NumRetries) + r.Cron = pointer.Ptr(t.Schedule.Cron) + r.Timezone = pointer.Ptr(t.Schedule.Timezone) props := t.Properties.(map[string]interface{}) if err := mapstructure.Decode(props, r); err != nil { @@ -153,6 +155,8 @@ func (b *BackupTask) FromManager(t *managerclient.TaskListItem) error { b.Interval = pointer.Ptr(t.Schedule.Interval) b.StartDate = pointer.Ptr(t.Schedule.StartDate.String()) b.NumRetries = pointer.Ptr(t.Schedule.NumRetries) + b.Cron = pointer.Ptr(t.Schedule.Cron) + b.Timezone = pointer.Ptr(t.Schedule.Timezone) props := t.Properties.(map[string]interface{}) if err := mapstructure.Decode(props, b); err != nil { @@ -193,6 +197,14 @@ func schedulerTaskSpecToManager(schedulerTaskSpec *scyllav1.SchedulerTaskSpec) ( schedule.NumRetries = *schedulerTaskSpec.NumRetries } + if schedulerTaskSpec.Cron != nil { + schedule.Cron = *schedulerTaskSpec.Cron + } + + if schedulerTaskSpec.Timezone != nil { + schedule.Timezone = *schedulerTaskSpec.Timezone + } + return schedule, nil } From b186a6665083a914573479571c3d444d269f679d Mon Sep 17 00:00:00 2001 From: Kacper Rzetelski Date: Tue, 19 Mar 2024 20:42:08 +0100 Subject: [PATCH 3/6] Extend manager task converstion unit tests with cron and timezone API fields --- pkg/controller/manager/types_old_test.go | 25 +++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/pkg/controller/manager/types_old_test.go b/pkg/controller/manager/types_old_test.go index b35ffc3ca41..2114d99bab3 100644 --- a/pkg/controller/manager/types_old_test.go +++ b/pkg/controller/manager/types_old_test.go @@ -38,6 +38,8 @@ func TestRepairTask_ToManager(t *testing.T) { StartDate: pointer.Ptr(validDate), NumRetries: pointer.Ptr[int64](3), Interval: pointer.Ptr("7d"), + Cron: pointer.Ptr("0 23 * * SAT"), + Timezone: pointer.Ptr("CET"), }, DC: []string{"us-east1"}, FailFast: false, @@ -67,9 +69,11 @@ func TestRepairTask_ToManager(t *testing.T) { "host": "10.0.0.1", }, Schedule: &managerclient.Schedule{ + Cron: "0 23 * * SAT", Interval: "7d", NumRetries: 3, StartDate: validDateTime, + Timezone: "CET", }, Type: "repair", }, @@ -84,6 +88,8 @@ func TestRepairTask_ToManager(t *testing.T) { StartDate: pointer.Ptr(validDate), NumRetries: pointer.Ptr[int64](3), Interval: pointer.Ptr("7d"), + Cron: pointer.Ptr("0 23 * * SAT"), + Timezone: pointer.Ptr("CET"), }, DC: []string{"us-east1"}, FailFast: true, @@ -115,8 +121,10 @@ func TestRepairTask_ToManager(t *testing.T) { }, Schedule: &managerclient.Schedule{ Interval: "7d", + Cron: "0 23 * * SAT", NumRetries: 0, StartDate: validDateTime, + Timezone: "CET", }, Type: "repair", }, @@ -153,12 +161,12 @@ func TestRepairTask_FromManager(t *testing.T) { name: "fields and properties are propagated", managerTask: &managerclient.TaskListItem{ ClusterID: "cluster_id", + Name: "repair_task_name", Enabled: true, ErrorCount: 1, ID: "repair_task_id", LastError: validDateTime, LastSuccess: validDateTime, - Name: "repair_task_name", NextActivation: validDateTime, Properties: map[string]interface{}{ "dc": []string{ @@ -176,9 +184,10 @@ func TestRepairTask_FromManager(t *testing.T) { Retry: 1, Schedule: &managerclient.Schedule{ Interval: "7d", + Cron: "0 23 * * SAT", NumRetries: 3, - RetryWait: "10m", StartDate: validDateTime, + Timezone: "CET", }, Status: managerclient.TaskStatusRunning, SuccessCount: 0, @@ -192,6 +201,8 @@ func TestRepairTask_FromManager(t *testing.T) { StartDate: pointer.Ptr(validDate), NumRetries: pointer.Ptr[int64](3), Interval: pointer.Ptr("7d"), + Cron: pointer.Ptr("0 23 * * SAT"), + Timezone: pointer.Ptr("CET"), }, DC: []string{"us-east1"}, FailFast: true, @@ -242,6 +253,8 @@ func TestBackupTask_ToManager(t *testing.T) { StartDate: pointer.Ptr(validDate), NumRetries: pointer.Ptr[int64](3), Interval: pointer.Ptr("7d"), + Cron: pointer.Ptr("0 23 * * SAT"), + Timezone: pointer.Ptr("CET"), }, DC: []string{"us-east1"}, Keyspace: []string{"test"}, @@ -291,8 +304,10 @@ func TestBackupTask_ToManager(t *testing.T) { }, Schedule: &managerclient.Schedule{ Interval: "7d", + Cron: "0 23 * * SAT", NumRetries: 3, StartDate: validDateTime, + Timezone: "CET", }, Type: "backup", }, @@ -356,13 +371,15 @@ func TestBackupTask_FromManager(t *testing.T) { "10", "us-east1:100", }, + "purge_only": false, }, Retry: 1, Schedule: &managerclient.Schedule{ Interval: "7d", + Cron: "0 23 * * SAT", NumRetries: 3, - RetryWait: "10m", StartDate: validDateTime, + Timezone: "CET", }, Status: managerclient.TaskStatusRunning, SuccessCount: 0, @@ -376,6 +393,8 @@ func TestBackupTask_FromManager(t *testing.T) { StartDate: pointer.Ptr(validDate), NumRetries: pointer.Ptr[int64](3), Interval: pointer.Ptr("7d"), + Cron: pointer.Ptr("0 23 * * SAT"), + Timezone: pointer.Ptr("CET"), }, DC: []string{"us-east1"}, Keyspace: []string{"test"}, From d216bf22f1dc6f6709802275d52ddb64ad28c7a2 Mon Sep 17 00:00:00 2001 From: Kacper Rzetelski Date: Wed, 27 Mar 2024 12:02:50 +0100 Subject: [PATCH 4/6] Validate cron, timezone, and mutual exclusivity of non-zero interval and cron --- .../scylla/validation/cluster_validation.go | 56 +++ .../validation/cluster_validation_test.go | 452 ++++++++++++++++++ 2 files changed, 508 insertions(+) diff --git a/pkg/api/scylla/validation/cluster_validation.go b/pkg/api/scylla/validation/cluster_validation.go index 4f08f35a53c..b068ab2b138 100644 --- a/pkg/api/scylla/validation/cluster_validation.go +++ b/pkg/api/scylla/validation/cluster_validation.go @@ -4,12 +4,16 @@ import ( "fmt" "reflect" "strconv" + "strings" + "time" + "github.com/robfig/cron/v3" "github.com/scylladb/go-set/strset" scyllav1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1" "github.com/scylladb/scylla-operator/pkg/helpers/slices" "github.com/scylladb/scylla-operator/pkg/pointer" "github.com/scylladb/scylla-operator/pkg/semver" + "github.com/scylladb/scylla-operator/pkg/util/duration" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/util/sets" apimachineryutilvalidation "k8s.io/apimachinery/pkg/util/validation" @@ -34,6 +38,8 @@ var ( scyllav1.BroadcastAddressTypeServiceClusterIP, scyllav1.BroadcastAddressTypeServiceLoadBalancerIngress, } + + schedulerTaskSpecCronParseOptions = cron.SecondOptional | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor ) func ValidateScyllaCluster(c *scyllav1.ScyllaCluster) field.ErrorList { @@ -180,6 +186,8 @@ func ValidateScyllaClusterSpec(spec *scyllav1.ScyllaClusterSpec, fldPath *field. allErrs = append(allErrs, field.Duplicate(fldPath.Child("backups").Index(i).Child("name"), b.Name)) } managerTaskNames.Add(b.Name) + + allErrs = append(allErrs, ValidateBackupTaskSpec(&b, fldPath.Child("backups").Index(i))...) } if spec.GenericUpgrade != nil { @@ -221,6 +229,54 @@ func ValidateRepairTaskSpec(repairTaskSpec *scyllav1.RepairTaskSpec, fldPath *fi allErrs = append(allErrs, field.Invalid(fldPath.Child("intensity"), repairTaskSpec.Intensity, "must be a float")) } + allErrs = append(allErrs, ValidateSchedulerTaskSpec(&repairTaskSpec.SchedulerTaskSpec, fldPath)...) + + return allErrs +} + +func ValidateBackupTaskSpec(backupTaskSpec *scyllav1.BackupTaskSpec, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + allErrs = append(allErrs, ValidateSchedulerTaskSpec(&backupTaskSpec.SchedulerTaskSpec, fldPath)...) + + return allErrs +} + +func ValidateSchedulerTaskSpec(schedulerTaskSpec *scyllav1.SchedulerTaskSpec, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if schedulerTaskSpec.Cron != nil { + _, err := cron.NewParser(schedulerTaskSpecCronParseOptions).Parse(*schedulerTaskSpec.Cron) + if err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("cron"), schedulerTaskSpec.Cron, err.Error())) + } + + if strings.Contains(*schedulerTaskSpec.Cron, "TZ") { + allErrs = append(allErrs, field.Invalid(fldPath.Child("cron"), schedulerTaskSpec.Cron, "can't use TZ or CRON_TZ in cron, use timezone instead")) + } + } + + if schedulerTaskSpec.Timezone != nil { + _, err := time.LoadLocation(*schedulerTaskSpec.Timezone) + if err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("timezone"), schedulerTaskSpec.Timezone, err.Error())) + } + } + + // We can only validate interval when cron is non-nil for backwards compatibility. + if schedulerTaskSpec.Interval != nil && schedulerTaskSpec.Cron != nil { + intervalDuration, err := duration.ParseDuration(*schedulerTaskSpec.Interval) + if err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("interval"), schedulerTaskSpec.Interval, "valid units are d, h, m, s")) + } else if intervalDuration != 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("interval"), "can't be non-zero when cron is specified")) + } + } + + if schedulerTaskSpec.Timezone != nil && schedulerTaskSpec.Cron == nil { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("timezone"), "can't be set when cron is not specified")) + } + return allErrs } diff --git a/pkg/api/scylla/validation/cluster_validation_test.go b/pkg/api/scylla/validation/cluster_validation_test.go index 5941acd94d8..0e2af178d0b 100644 --- a/pkg/api/scylla/validation/cluster_validation_test.go +++ b/pkg/api/scylla/validation/cluster_validation_test.go @@ -87,6 +87,458 @@ func TestValidateScyllaCluster(t *testing.T) { }, expectedErrorString: `spec.backups[0].name: Duplicate value: "task-name"`, }, + { + name: "invalid cron in manager repair task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Repairs = append(cluster.Spec.Repairs, v1.RepairTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Cron: pointer.Ptr("invalid"), + }, + Intensity: "1", + }) + return cluster + }(), + expectedErrorList: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "spec.repairs[0].cron", + BadValue: pointer.Ptr("invalid"), + Detail: "expected 5 to 6 fields, found 1: [invalid]", + }, + }, + expectedErrorString: `spec.repairs[0].cron: Invalid value: "invalid": expected 5 to 6 fields, found 1: [invalid]`, + }, + { + name: "unallowed TZ in cron in manager repair task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Repairs = append(cluster.Spec.Repairs, v1.RepairTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Cron: pointer.Ptr("TZ=Europe/Warsaw 0 23 * * SAT"), + }, + Intensity: "1", + }) + return cluster + }(), + expectedErrorList: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "spec.repairs[0].cron", + BadValue: pointer.Ptr("TZ=Europe/Warsaw 0 23 * * SAT"), + Detail: "can't use TZ or CRON_TZ in cron, use timezone instead", + }, + }, + expectedErrorString: `spec.repairs[0].cron: Invalid value: "TZ=Europe/Warsaw 0 23 * * SAT": can't use TZ or CRON_TZ in cron, use timezone instead`, + }, + { + name: "unallowed TZ in cron in manager backup task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Backups = append(cluster.Spec.Backups, v1.BackupTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Cron: pointer.Ptr("TZ=Europe/Warsaw 0 23 * * SAT"), + }, + }) + return cluster + }(), + expectedErrorList: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "spec.backups[0].cron", + BadValue: pointer.Ptr("TZ=Europe/Warsaw 0 23 * * SAT"), + Detail: "can't use TZ or CRON_TZ in cron, use timezone instead", + }, + }, + expectedErrorString: `spec.backups[0].cron: Invalid value: "TZ=Europe/Warsaw 0 23 * * SAT": can't use TZ or CRON_TZ in cron, use timezone instead`, + }, + { + name: "unallowed CRON_TZ in cron in manager repair task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Repairs = append(cluster.Spec.Repairs, v1.RepairTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Cron: pointer.Ptr("CRON_TZ=Europe/Warsaw 0 23 * * SAT"), + }, + Intensity: "1", + }) + return cluster + }(), + expectedErrorList: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "spec.repairs[0].cron", + BadValue: pointer.Ptr("CRON_TZ=Europe/Warsaw 0 23 * * SAT"), + Detail: "can't use TZ or CRON_TZ in cron, use timezone instead", + }, + }, + expectedErrorString: `spec.repairs[0].cron: Invalid value: "CRON_TZ=Europe/Warsaw 0 23 * * SAT": can't use TZ or CRON_TZ in cron, use timezone instead`, + }, + { + name: "unallowed CRON_TZ in cron in manager backup task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Backups = append(cluster.Spec.Backups, v1.BackupTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Cron: pointer.Ptr("CRON_TZ=Europe/Warsaw 0 23 * * SAT"), + }, + }) + return cluster + }(), + expectedErrorList: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "spec.backups[0].cron", + BadValue: pointer.Ptr("CRON_TZ=Europe/Warsaw 0 23 * * SAT"), + Detail: "can't use TZ or CRON_TZ in cron, use timezone instead", + }, + }, + expectedErrorString: `spec.backups[0].cron: Invalid value: "CRON_TZ=Europe/Warsaw 0 23 * * SAT": can't use TZ or CRON_TZ in cron, use timezone instead`, + }, + { + name: "invalid timezone in manager repair task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Repairs = append(cluster.Spec.Repairs, v1.RepairTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Cron: pointer.Ptr("0 23 * * SAT"), + Timezone: pointer.Ptr("invalid"), + }, + Intensity: "1", + }) + return cluster + }(), + expectedErrorList: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "spec.repairs[0].timezone", + BadValue: pointer.Ptr("invalid"), + Detail: "unknown time zone invalid", + }, + }, + expectedErrorString: `spec.repairs[0].timezone: Invalid value: "invalid": unknown time zone invalid`, + }, + { + name: "invalid timezone in manager backup task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Backups = append(cluster.Spec.Backups, v1.BackupTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Cron: pointer.Ptr("0 23 * * SAT"), + Timezone: pointer.Ptr("invalid"), + }, + }) + return cluster + }(), + expectedErrorList: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "spec.backups[0].timezone", + BadValue: pointer.Ptr("invalid"), + Detail: "unknown time zone invalid", + }, + }, + expectedErrorString: `spec.backups[0].timezone: Invalid value: "invalid": unknown time zone invalid`, + }, + { + name: "cron and timezone in manager repair task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Repairs = append(cluster.Spec.Repairs, v1.RepairTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Cron: pointer.Ptr("0 23 * * SAT"), + Timezone: pointer.Ptr("Europe/Warsaw"), + }, + Intensity: "1", + }) + return cluster + }(), + expectedErrorList: field.ErrorList{}, + expectedErrorString: "", + }, + { + name: "cron and timezone in manager backup task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Backups = append(cluster.Spec.Backups, v1.BackupTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Cron: pointer.Ptr("0 23 * * SAT"), + Timezone: pointer.Ptr("Europe/Warsaw"), + }, + }) + return cluster + }(), + expectedErrorList: field.ErrorList{}, + expectedErrorString: "", + }, + { + name: "nil cron and invalid interval in manager repair task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Repairs = append(cluster.Spec.Repairs, v1.RepairTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Interval: pointer.Ptr("invalid"), + Cron: nil, + }, + Intensity: "1", + }) + return cluster + }(), + expectedErrorList: field.ErrorList{}, + expectedErrorString: "", + }, + { + name: "nil cron and invalid interval in manager backup task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Backups = append(cluster.Spec.Backups, v1.BackupTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Interval: pointer.Ptr("invalid"), + Cron: nil, + }, + }) + return cluster + }(), + expectedErrorList: field.ErrorList{}, + expectedErrorString: "", + }, + { + name: "nil cron and non-zero interval in manager repair task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Repairs = append(cluster.Spec.Repairs, v1.RepairTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Interval: pointer.Ptr("7d"), + Cron: nil, + }, + Intensity: "1", + }) + return cluster + }(), + expectedErrorList: field.ErrorList{}, + expectedErrorString: "", + }, + { + name: "nil cron and non-zero interval in manager backup task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Backups = append(cluster.Spec.Backups, v1.BackupTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Interval: pointer.Ptr("7d"), + Cron: nil, + }, + }) + return cluster + }(), + expectedErrorList: field.ErrorList{}, + expectedErrorString: "", + }, + { + name: "cron and invalid interval in manager repair task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Repairs = append(cluster.Spec.Repairs, v1.RepairTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Interval: pointer.Ptr("invalid"), + Cron: pointer.Ptr("0 23 * * SAT"), + }, + Intensity: "1", + }) + return cluster + }(), + expectedErrorList: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "spec.repairs[0].interval", + BadValue: pointer.Ptr("invalid"), + Detail: "valid units are d, h, m, s", + }, + }, + expectedErrorString: `spec.repairs[0].interval: Invalid value: "invalid": valid units are d, h, m, s`, + }, + { + name: "cron and invalid interval in manager backup task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Backups = append(cluster.Spec.Backups, v1.BackupTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Interval: pointer.Ptr("invalid"), + Cron: pointer.Ptr("0 23 * * SAT"), + }, + }) + return cluster + }(), + expectedErrorList: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "spec.backups[0].interval", + BadValue: pointer.Ptr("invalid"), + Detail: "valid units are d, h, m, s", + }, + }, + expectedErrorString: `spec.backups[0].interval: Invalid value: "invalid": valid units are d, h, m, s`, + }, + { + name: "cron and zero interval in manager repair task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Repairs = append(cluster.Spec.Repairs, []v1.RepairTaskSpec{ + { + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Interval: pointer.Ptr("0"), + Cron: pointer.Ptr("0 23 * * SAT"), + }, + Intensity: "1", + }, + { + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "other-task-name", + Interval: pointer.Ptr("0d"), + Cron: pointer.Ptr("0 23 * * SAT"), + }, + Intensity: "1", + }, + }...) + return cluster + }(), + expectedErrorList: field.ErrorList{}, + expectedErrorString: "", + }, + { + name: "cron and zero interval in manager backup task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Backups = append(cluster.Spec.Backups, []v1.BackupTaskSpec{ + { + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Interval: pointer.Ptr("0"), + Cron: pointer.Ptr("0 23 * * SAT"), + }, + }, + { + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "other-task-name", + Interval: pointer.Ptr("0d"), + Cron: pointer.Ptr("0 23 * * SAT"), + }, + }, + }...) + return cluster + }(), + expectedErrorList: field.ErrorList{}, + expectedErrorString: "", + }, + { + name: "cron and non-zero interval in manager repair task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Repairs = append(cluster.Spec.Repairs, v1.RepairTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Interval: pointer.Ptr("7d"), + Cron: pointer.Ptr("0 23 * * SAT"), + }, + Intensity: "1", + }) + return cluster + }(), + expectedErrorList: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeForbidden, + Field: "spec.repairs[0].interval", + BadValue: "", + Detail: "can't be non-zero when cron is specified", + }, + }, + expectedErrorString: `spec.repairs[0].interval: Forbidden: can't be non-zero when cron is specified`, + }, + { + name: "cron and non-zero interval in manager backup task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Backups = append(cluster.Spec.Backups, v1.BackupTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Interval: pointer.Ptr("7d"), + Cron: pointer.Ptr("0 23 * * SAT"), + }, + }) + return cluster + }(), + expectedErrorList: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeForbidden, + Field: "spec.backups[0].interval", + BadValue: "", + Detail: "can't be non-zero when cron is specified", + }, + }, + expectedErrorString: `spec.backups[0].interval: Forbidden: can't be non-zero when cron is specified`, + }, + { + name: "timezone and nil cron in manager repair task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Repairs = append(cluster.Spec.Repairs, v1.RepairTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Cron: nil, + Timezone: pointer.Ptr("UTC"), + }, + Intensity: "1", + }) + return cluster + }(), + expectedErrorList: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeForbidden, + Field: "spec.repairs[0].timezone", + BadValue: "", + Detail: "can't be set when cron is not specified", + }, + }, + expectedErrorString: `spec.repairs[0].timezone: Forbidden: can't be set when cron is not specified`, + }, + { + name: "timezone and nil cron in manager backup task spec", + cluster: func() *v1.ScyllaCluster { + cluster := validCluster.DeepCopy() + cluster.Spec.Backups = append(cluster.Spec.Backups, v1.BackupTaskSpec{ + SchedulerTaskSpec: v1.SchedulerTaskSpec{ + Name: "task-name", + Cron: nil, + Timezone: pointer.Ptr("UTC"), + }, + }) + return cluster + }(), + expectedErrorList: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeForbidden, + Field: "spec.backups[0].timezone", + BadValue: "", + Detail: "can't be set when cron is not specified", + }, + }, + expectedErrorString: `spec.backups[0].timezone: Forbidden: can't be set when cron is not specified`, + }, { name: "when CQL ingress is provided, domains must not be empty", cluster: func() *v1.ScyllaCluster { From fc81c7445d0715e2571c68e0a69acabcfc52d5c9 Mon Sep 17 00:00:00 2001 From: Kacper Rzetelski Date: Wed, 27 Mar 2024 13:14:16 +0100 Subject: [PATCH 5/6] Update generated --- deploy/operator.yaml | 24 +++++++++++++++++++ .../scylla.scylladb.com/scyllaclusters.rst | 24 +++++++++++++++++++ helm/scylla-manager/values.schema.json | 16 +++++++++++++ helm/scylla/values.schema.json | 16 +++++++++++++ .../scylla.scylladb.com_scyllaclusters.yaml | 24 +++++++++++++++++++ pkg/api/scylla/v1/zz_generated.deepcopy.go | 10 ++++++++ 6 files changed, 114 insertions(+) diff --git a/deploy/operator.yaml b/deploy/operator.yaml index d4a3adad3e4..3252d742af9 100644 --- a/deploy/operator.yaml +++ b/deploy/operator.yaml @@ -1146,6 +1146,9 @@ spec: description: backups specifies backup tasks in Scylla Manager. When Scylla Manager is not installed, these will be ignored. items: properties: + cron: + description: cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s]. + type: string dc: description: dc is a list of datacenter glob patterns, e.g. 'dc1,!otherdc*' used to specify the DCs to include or exclude from backup. items: @@ -1190,6 +1193,9 @@ spec: startDate: description: startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s. type: string + timezone: + description: timezone specifies the timezone of cron field. + type: string uploadParallel: description: 'uploadParallel is a list of upload parallelism limits in the format [:]. The : part is optional and allows for specifying different limits in selected datacenters. If The : part is not set the limit is global (e.g. ''dc1:2,5'') the runs are parallel in n nodes (2 in dc1) and n nodes in all the other datacenters.' items: @@ -3102,6 +3108,9 @@ spec: description: repairs specify repair tasks in Scylla Manager. When Scylla Manager is not installed, these will be ignored. items: properties: + cron: + description: cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s]. + type: string dc: description: dc is a list of datacenter glob patterns, e.g. 'dc1', '!otherdc*' used to specify the DCs to include or exclude from backup. items: @@ -3145,6 +3154,9 @@ spec: startDate: description: startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s. type: string + timezone: + description: timezone specifies the timezone of cron field. + type: string type: object type: array repository: @@ -3174,6 +3186,9 @@ spec: description: backups reflects status of backup tasks. items: properties: + cron: + description: cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s]. + type: string dc: description: dc is a list of datacenter glob patterns, e.g. 'dc1,!otherdc*' used to specify the DCs to include or exclude from backup. items: @@ -3224,6 +3239,9 @@ spec: startDate: description: startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s. type: string + timezone: + description: timezone specifies the timezone of cron field. + type: string uploadParallel: description: 'uploadParallel is a list of upload parallelism limits in the format [:]. The : part is optional and allows for specifying different limits in selected datacenters. If The : part is not set the limit is global (e.g. ''dc1:2,5'') the runs are parallel in n nodes (2 in dc1) and n nodes in all the other datacenters.' items: @@ -3345,6 +3363,9 @@ spec: description: repairs reflects status of repair tasks. items: properties: + cron: + description: cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s]. + type: string dc: description: dc is a list of datacenter glob patterns, e.g. 'dc1', '!otherdc*' used to specify the DCs to include or exclude from backup. items: @@ -3394,6 +3415,9 @@ spec: startDate: description: startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s. type: string + timezone: + description: timezone specifies the timezone of cron field. + type: string type: object type: array upgrade: diff --git a/docs/source/api-reference/groups/scylla.scylladb.com/scyllaclusters.rst b/docs/source/api-reference/groups/scylla.scylladb.com/scyllaclusters.rst index 67e3e627814..3128ca29037 100755 --- a/docs/source/api-reference/groups/scylla.scylladb.com/scyllaclusters.rst +++ b/docs/source/api-reference/groups/scylla.scylladb.com/scyllaclusters.rst @@ -290,6 +290,9 @@ object * - Property - Type - Description + * - cron + - string + - cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s]. * - dc - array (string) - dc is a list of datacenter glob patterns, e.g. 'dc1,!otherdc*' used to specify the DCs to include or exclude from backup. @@ -320,6 +323,9 @@ object * - startDate - string - startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s. + * - timezone + - string + - timezone specifies the timezone of cron field. * - uploadParallel - array (string) - uploadParallel is a list of upload parallelism limits in the format [:]. The : part is optional and allows for specifying different limits in selected datacenters. If The : part is not set the limit is global (e.g. 'dc1:2,5') the runs are parallel in n nodes (2 in dc1) and n nodes in all the other datacenters. @@ -4661,6 +4667,9 @@ object * - Property - Type - Description + * - cron + - string + - cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s]. * - dc - array (string) - dc is a list of datacenter glob patterns, e.g. 'dc1', '!otherdc*' used to specify the DCs to include or exclude from backup. @@ -4694,6 +4703,9 @@ object * - startDate - string - startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s. + * - timezone + - string + - timezone specifies the timezone of cron field. .. _api-scylla.scylladb.com-scyllaclusters-v1-.status: @@ -4771,6 +4783,9 @@ object * - Property - Type - Description + * - cron + - string + - cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s]. * - dc - array (string) - dc is a list of datacenter glob patterns, e.g. 'dc1,!otherdc*' used to specify the DCs to include or exclude from backup. @@ -4807,6 +4822,9 @@ object * - startDate - string - startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s. + * - timezone + - string + - timezone specifies the timezone of cron field. * - uploadParallel - array (string) - uploadParallel is a list of upload parallelism limits in the format [:]. The : part is optional and allows for specifying different limits in selected datacenters. If The : part is not set the limit is global (e.g. 'dc1:2,5') the runs are parallel in n nodes (2 in dc1) and n nodes in all the other datacenters. @@ -4888,6 +4906,9 @@ object * - Property - Type - Description + * - cron + - string + - cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s]. * - dc - array (string) - dc is a list of datacenter glob patterns, e.g. 'dc1', '!otherdc*' used to specify the DCs to include or exclude from backup. @@ -4927,6 +4948,9 @@ object * - startDate - string - startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s. + * - timezone + - string + - timezone specifies the timezone of cron field. .. _api-scylla.scylladb.com-scyllaclusters-v1-.status.upgrade: diff --git a/helm/scylla-manager/values.schema.json b/helm/scylla-manager/values.schema.json index 8992cc323d9..6586a3f6b82 100644 --- a/helm/scylla-manager/values.schema.json +++ b/helm/scylla-manager/values.schema.json @@ -201,6 +201,10 @@ "description": "backups specifies backup tasks in Scylla Manager. When Scylla Manager is not installed, these will be ignored.", "items": { "properties": { + "cron": { + "description": "cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s].", + "type": "string" + }, "dc": { "description": "dc is a list of datacenter glob patterns, e.g. 'dc1,!otherdc*' used to specify the DCs to include or exclude from backup.", "items": { @@ -260,6 +264,10 @@ "description": "startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s.", "type": "string" }, + "timezone": { + "description": "timezone specifies the timezone of cron field.", + "type": "string" + }, "uploadParallel": { "description": "uploadParallel is a list of upload parallelism limits in the format [:]. The : part is optional and allows for specifying different limits in selected datacenters. If The : part is not set the limit is global (e.g. 'dc1:2,5') the runs are parallel in n nodes (2 in dc1) and n nodes in all the other datacenters.", "items": { @@ -537,6 +545,10 @@ "description": "repairs specify repair tasks in Scylla Manager. When Scylla Manager is not installed, these will be ignored.", "items": { "properties": { + "cron": { + "description": "cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s].", + "type": "string" + }, "dc": { "description": "dc is a list of datacenter glob patterns, e.g. 'dc1', '!otherdc*' used to specify the DCs to include or exclude from backup.", "items": { @@ -592,6 +604,10 @@ "startDate": { "description": "startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s.", "type": "string" + }, + "timezone": { + "description": "timezone specifies the timezone of cron field.", + "type": "string" } }, "type": "object" diff --git a/helm/scylla/values.schema.json b/helm/scylla/values.schema.json index 70c2a470917..8ad3285bb64 100644 --- a/helm/scylla/values.schema.json +++ b/helm/scylla/values.schema.json @@ -90,6 +90,10 @@ "description": "backups specifies backup tasks in Scylla Manager. When Scylla Manager is not installed, these will be ignored.", "items": { "properties": { + "cron": { + "description": "cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s].", + "type": "string" + }, "dc": { "description": "dc is a list of datacenter glob patterns, e.g. 'dc1,!otherdc*' used to specify the DCs to include or exclude from backup.", "items": { @@ -149,6 +153,10 @@ "description": "startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s.", "type": "string" }, + "timezone": { + "description": "timezone specifies the timezone of cron field.", + "type": "string" + }, "uploadParallel": { "description": "uploadParallel is a list of upload parallelism limits in the format [:]. The : part is optional and allows for specifying different limits in selected datacenters. If The : part is not set the limit is global (e.g. 'dc1:2,5') the runs are parallel in n nodes (2 in dc1) and n nodes in all the other datacenters.", "items": { @@ -426,6 +434,10 @@ "description": "repairs specify repair tasks in Scylla Manager. When Scylla Manager is not installed, these will be ignored.", "items": { "properties": { + "cron": { + "description": "cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s].", + "type": "string" + }, "dc": { "description": "dc is a list of datacenter glob patterns, e.g. 'dc1', '!otherdc*' used to specify the DCs to include or exclude from backup.", "items": { @@ -481,6 +493,10 @@ "startDate": { "description": "startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s.", "type": "string" + }, + "timezone": { + "description": "timezone specifies the timezone of cron field.", + "type": "string" } }, "type": "object" diff --git a/pkg/api/scylla/v1/scylla.scylladb.com_scyllaclusters.yaml b/pkg/api/scylla/v1/scylla.scylladb.com_scyllaclusters.yaml index 19463a4047b..7889732bdfe 100644 --- a/pkg/api/scylla/v1/scylla.scylladb.com_scyllaclusters.yaml +++ b/pkg/api/scylla/v1/scylla.scylladb.com_scyllaclusters.yaml @@ -118,6 +118,9 @@ spec: description: backups specifies backup tasks in Scylla Manager. When Scylla Manager is not installed, these will be ignored. items: properties: + cron: + description: cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s]. + type: string dc: description: dc is a list of datacenter glob patterns, e.g. 'dc1,!otherdc*' used to specify the DCs to include or exclude from backup. items: @@ -162,6 +165,9 @@ spec: startDate: description: startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s. type: string + timezone: + description: timezone specifies the timezone of cron field. + type: string uploadParallel: description: 'uploadParallel is a list of upload parallelism limits in the format [:]. The : part is optional and allows for specifying different limits in selected datacenters. If The : part is not set the limit is global (e.g. ''dc1:2,5'') the runs are parallel in n nodes (2 in dc1) and n nodes in all the other datacenters.' items: @@ -2074,6 +2080,9 @@ spec: description: repairs specify repair tasks in Scylla Manager. When Scylla Manager is not installed, these will be ignored. items: properties: + cron: + description: cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s]. + type: string dc: description: dc is a list of datacenter glob patterns, e.g. 'dc1', '!otherdc*' used to specify the DCs to include or exclude from backup. items: @@ -2117,6 +2126,9 @@ spec: startDate: description: startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s. type: string + timezone: + description: timezone specifies the timezone of cron field. + type: string type: object type: array repository: @@ -2146,6 +2158,9 @@ spec: description: backups reflects status of backup tasks. items: properties: + cron: + description: cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s]. + type: string dc: description: dc is a list of datacenter glob patterns, e.g. 'dc1,!otherdc*' used to specify the DCs to include or exclude from backup. items: @@ -2196,6 +2211,9 @@ spec: startDate: description: startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s. type: string + timezone: + description: timezone specifies the timezone of cron field. + type: string uploadParallel: description: 'uploadParallel is a list of upload parallelism limits in the format [:]. The : part is optional and allows for specifying different limits in selected datacenters. If The : part is not set the limit is global (e.g. ''dc1:2,5'') the runs are parallel in n nodes (2 in dc1) and n nodes in all the other datacenters.' items: @@ -2317,6 +2335,9 @@ spec: description: repairs reflects status of repair tasks. items: properties: + cron: + description: cron specifies the task schedule as a cron expression. It supports an extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every X[h|m|s]. + type: string dc: description: dc is a list of datacenter glob patterns, e.g. 'dc1', '!otherdc*' used to specify the DCs to include or exclude from backup. items: @@ -2366,6 +2387,9 @@ spec: startDate: description: startDate specifies the task start date expressed in the RFC3339 format or now[+duration], e.g. now+3d2h10m, valid units are d, h, m, s. type: string + timezone: + description: timezone specifies the timezone of cron field. + type: string type: object type: array upgrade: diff --git a/pkg/api/scylla/v1/zz_generated.deepcopy.go b/pkg/api/scylla/v1/zz_generated.deepcopy.go index ef79479668e..847e4a0a03d 100644 --- a/pkg/api/scylla/v1/zz_generated.deepcopy.go +++ b/pkg/api/scylla/v1/zz_generated.deepcopy.go @@ -609,6 +609,16 @@ func (in *SchedulerTaskSpec) DeepCopyInto(out *SchedulerTaskSpec) { *out = new(int64) **out = **in } + if in.Cron != nil { + in, out := &in.Cron, &out.Cron + *out = new(string) + **out = **in + } + if in.Timezone != nil { + in, out := &in.Timezone, &out.Timezone + *out = new(string) + **out = **in + } return } From 7ea90bae594da3591cffac1567476f6493c50ff2 Mon Sep 17 00:00:00 2001 From: Kacper Rzetelski Date: Thu, 18 Apr 2024 14:26:27 +0200 Subject: [PATCH 6/6] Update dependencies --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 8aae48daff4..ccb6110a08e 100644 --- a/go.mod +++ b/go.mod @@ -31,6 +31,7 @@ require ( github.com/onsi/gomega v1.32.0 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.19.0 + github.com/robfig/cron/v3 v3.0.1 github.com/scylladb/go-set v1.0.2 github.com/scylladb/gocqlx/v2 v2.8.0 github.com/scylladb/scylla-manager/v3 v3.2.7 @@ -174,7 +175,6 @@ require ( github.com/rclone/rclone v1.66.0 // indirect github.com/rfjakob/eme v1.1.2 // indirect github.com/rivo/uniseg v0.4.7 // indirect - github.com/robfig/cron/v3 v3.0.1 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/scylladb/go-log v0.0.7 // indirect github.com/scylladb/go-reflectx v1.0.1 // indirect