fix(cron): various follow-ups for multiple schedules (#13369)

Signed-off-by: eduardodbr <[email protected]> Signed-off-by: Eduardo Rodrigues <[email protected]> Co-authored-by: Anton Gilgur <[email protected]>
argoproj · Sep 21, 2024 · 2dac126 · 2dac126
1 parent ce7f9bf
commit 2dac126
Show file tree

Hide file tree

Showing 23 changed files with 127 additions and 77 deletions.
diff --git a/api/jsonschema/schema.json b/api/jsonschema/schema.json
diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
diff --git a/cmd/argo/commands/cron/list.go b/cmd/argo/commands/cron/list.go
@@ -66,7 +66,7 @@ func printTable(wfList []wfv1.CronWorkflow, listArgs *listFlags) {
 	if listArgs.allNamespaces {
 		_, _ = fmt.Fprint(w, "NAMESPACE\t")
 	}
-	_, _ = fmt.Fprint(w, "NAME\tAGE\tLAST RUN\tNEXT RUN\tSCHEDULE\tTIMEZONE\tSUSPENDED")
+	_, _ = fmt.Fprint(w, "NAME\tAGE\tLAST RUN\tNEXT RUN\tSCHEDULES\tTIMEZONE\tSUSPENDED")
 	_, _ = fmt.Fprint(w, "\n")
 	for _, cwf := range wfList {
 		if listArgs.allNamespaces {

diff --git a/cmd/argo/commands/cron/util.go b/cmd/argo/commands/cron/util.go
@@ -116,7 +116,7 @@ func getCronWorkflowGet(cwf *wfv1.CronWorkflow) string {
 	out += fmt.Sprintf(fmtStr, "Name:", cwf.ObjectMeta.Name)
 	out += fmt.Sprintf(fmtStr, "Namespace:", cwf.ObjectMeta.Namespace)
 	out += fmt.Sprintf(fmtStr, "Created:", humanize.Timestamp(cwf.ObjectMeta.CreationTimestamp.Time))
-	out += fmt.Sprintf(fmtStr, "Schedule:", cwf.Spec.GetScheduleString())
+	out += fmt.Sprintf(fmtStr, "Schedules:", cwf.Spec.GetScheduleString())
 	out += fmt.Sprintf(fmtStr, "Suspended:", cwf.Spec.Suspend)
 	if cwf.Spec.Timezone != "" {
 		out += fmt.Sprintf(fmtStr, "Timezone:", cwf.Spec.Timezone)

diff --git a/docs/cron-workflows.md b/docs/cron-workflows.md
@@ -43,7 +43,8 @@ You can use `CronWorkflow.spec.workflowMetadata` to add `labels` and `annotation
 
 | Option Name                  | Default Value          | Description |
 |:----------------------------:|:----------------------:|-------------|
-| `schedule`                   | None, must be provided | [Cron schedule](#cron-schedule-syntax) to run `Workflows`. Example: `5 4 * * *` |
+| `schedule`                   | None | [Cron schedule](#cron-schedule-syntax) to run `Workflows`. Example: `5 4 * * *`. Deprecated, use `schedules`. |
+| `schedules`                   | None | v3.6 and after: List of [Cron schedules](#cron-schedule-syntax) to run `Workflows`. Example: `5 4 * * *`, `0 1 * * *`. Either `schedule` or `schedules` must be provided. |
 | `timezone`                   | Machine timezone       | [IANA Timezone](https://en.wikipedia.org/wiki/List_of_tz_database_time_zones) to run `Workflows`. Example: `America/Los_Angeles` |
 | `suspend`                    | `false`                | If `true` Workflow scheduling will not occur. Can be set from the CLI, GitOps, or directly |
 | `concurrencyPolicy`          | `Allow`                | What to do if multiple `Workflows` are scheduled at the same time. `Allow`: allow all, `Replace`: remove all old before scheduling new, `Forbid`: do not allow any new while there are old  |
@@ -150,26 +151,26 @@ $ argo cron create cron.yaml
 Name:                          test-cron-wf
 Namespace:                     argo
 Created:                       Mon Nov 18 10:17:06 -0800 (now)
-Schedule:                      * * * * *
+Schedules:                     * * * * *
 Suspended:                     false
 StartingDeadlineSeconds:       0
 ConcurrencyPolicy:             Forbid
 
 $ argo cron list
-NAME           AGE   LAST RUN   SCHEDULE    SUSPENDED
+NAME           AGE   LAST RUN   SCHEDULES    SUSPENDED
 test-cron-wf   49s   N/A        * * * * *   false
 
 # some time passes
 
 $ argo cron list
-NAME           AGE   LAST RUN   SCHEDULE    SUSPENDED
+NAME           AGE   LAST RUN   SCHEDULES    SUSPENDED
 test-cron-wf   56s   2s         * * * * *   false
 
 $ argo cron get test-cron-wf
 Name:                          test-cron-wf
 Namespace:                     argo
 Created:                       Wed Oct 28 07:19:02 -0600 (23 hours ago)
-Schedule:                      * * * * *
+Schedules:                      * * * * *
 Suspended:                     false
 StartingDeadlineSeconds:       0
 ConcurrencyPolicy:             Replace

diff --git a/docs/fields.md b/docs/fields.md
@@ -390,6 +390,8 @@ CronWorkflow is the definition of a scheduled workflow resource
 
 - [`cron-when.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-when.yaml)
 
+- [`cron-workflow-multiple-schedules.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow-multiple-schedules.yaml)
+
 - [`cron-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow.yaml)
 
 - [`dag-inline-cronworkflow.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/dag-inline-cronworkflow.yaml)
@@ -513,6 +515,8 @@ WorkflowSpec is the specification of a Workflow.
 
 - [`cron-when.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-when.yaml)
 
+- [`cron-workflow-multiple-schedules.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow-multiple-schedules.yaml)
+
 - [`cron-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow.yaml)
 
 - [`custom-metrics.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/custom-metrics.yaml)
@@ -962,6 +966,8 @@ CronWorkflowSpec is the specification of a CronWorkflow
 
 - [`cron-when.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-when.yaml)
 
+- [`cron-workflow-multiple-schedules.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow-multiple-schedules.yaml)
+
 - [`cron-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow.yaml)
 
 - [`custom-metrics.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/custom-metrics.yaml)
@@ -1264,8 +1270,8 @@ CronWorkflowSpec is the specification of a CronWorkflow
 |:----------:|:----------:|---------------|
 |`concurrencyPolicy`|`string`|ConcurrencyPolicy is the K8s-style concurrency policy that will be used|
 |`failedJobsHistoryLimit`|`integer`|FailedJobsHistoryLimit is the number of failed jobs to be kept at a time|
-|`schedule`|`string`|Schedule is a schedule to run the Workflow in Cron format|
-|`schedules`|`Array< string >`|Schedules is a list of schedules to run the Workflow in Cron format|
+|`schedule`|`string`|Schedule is a schedule to run the Workflow in Cron format. Deprecated, use Schedules|
+|`schedules`|`Array< string >`|v3.6 and after: Schedules is a list of schedules to run the Workflow in Cron format|
 |`startingDeadlineSeconds`|`integer`|StartingDeadlineSeconds is the K8s-style deadline that will limit the time a CronWorkflow will be run after its original scheduled time if it is missed.|
 |`stopStrategy`|[`StopStrategy`](#stopstrategy)|v3.6 and after: StopStrategy defines if the CronWorkflow should stop scheduling based on a condition|
 |`successfulJobsHistoryLimit`|`integer`|SuccessfulJobsHistoryLimit is the number of successful jobs to be kept at a time|
@@ -3200,6 +3206,8 @@ SuspendTemplate is a template subtype to suspend a workflow at a predetermined p
 <details markdown>
 <summary>Examples with this field (click to open)</summary>
 
+- [`cron-workflow-multiple-schedules.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow-multiple-schedules.yaml)
+
 - [`cron-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow.yaml)
 
 - [`intermediate-parameters.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/intermediate-parameters.yaml)
@@ -4668,6 +4676,8 @@ ObjectMeta is metadata that all persisted resources must have, which includes al
 
 - [`cron-when.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-when.yaml)
 
+- [`cron-workflow-multiple-schedules.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow-multiple-schedules.yaml)
+
 - [`cron-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow.yaml)
 
 - [`custom-metrics.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/custom-metrics.yaml)
@@ -5294,6 +5304,8 @@ A single application container that you want to run within a pod.
 
 - [`cron-when.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-when.yaml)
 
+- [`cron-workflow-multiple-schedules.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow-multiple-schedules.yaml)
+
 - [`cron-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow.yaml)
 
 - [`custom-metrics.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/custom-metrics.yaml)
@@ -6026,6 +6038,8 @@ PersistentVolumeClaimSpec describes the common attributes of storage devices and
 
 - [`cron-when.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-when.yaml)
 
+- [`cron-workflow-multiple-schedules.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow-multiple-schedules.yaml)
+
 - [`cron-workflow.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/cron-workflow.yaml)
 
 - [`custom-metrics.yaml`](https://github.com/argoproj/argo-workflows/blob/main/examples/custom-metrics.yaml)

diff --git a/examples/cron-workflow-multiple-schedules.yaml b/examples/cron-workflow-multiple-schedules.yaml
@@ -0,0 +1,22 @@
+apiVersion: argoproj.io/v1alpha1
+kind: CronWorkflow
+metadata:
+  name: hello-world-multiple-schedules
+spec:
+  schedules: # v3.6 and after
+    - "*/3 * * * *"
+    - "*/2 * * * *"
+  timezone: "America/Los_Angeles"   # Default to local machine timezone
+  startingDeadlineSeconds: 0
+  concurrencyPolicy: "Replace"      # Default to "Allow"
+  successfulJobsHistoryLimit: 4     # Default 3
+  failedJobsHistoryLimit: 4         # Default 1
+  suspend: false                    # Set to "true" to suspend scheduling
+  workflowSpec:
+    entrypoint: whalesay
+    templates:
+      - name: whalesay
+        container:
+          image: docker/whalesay:latest
+          command: [cowsay]
+          args: ["🕓 hello world. Scheduled on: {{workflow.scheduledTime}}"]
diff --git a/manifests/base/crds/full/argoproj.io_cronworkflows.yaml b/manifests/base/crds/full/argoproj.io_cronworkflows.yaml
diff --git a/pkg/apis/workflow/v1alpha1/cron_workflow_types.go b/pkg/apis/workflow/v1alpha1/cron_workflow_types.go
@@ -44,8 +44,8 @@ const annotationKeyLatestSchedule = workflow.CronWorkflowFullName + "/last-used-
 type CronWorkflowSpec struct {
 	// WorkflowSpec is the spec of the workflow to be run
 	WorkflowSpec WorkflowSpec `json:"workflowSpec" protobuf:"bytes,1,opt,name=workflowSpec,casttype=WorkflowSpec"`
-	// Schedule is a schedule to run the Workflow in Cron format
-	Schedule string `json:"schedule" protobuf:"bytes,2,opt,name=schedule"`
+	// Schedule is a schedule to run the Workflow in Cron format. Deprecated, use Schedules
+	Schedule string `json:"schedule,omitempty" protobuf:"bytes,2,opt,name=schedule"`
 	// ConcurrencyPolicy is the K8s-style concurrency policy that will be used
 	ConcurrencyPolicy ConcurrencyPolicy `json:"concurrencyPolicy,omitempty" protobuf:"bytes,3,opt,name=concurrencyPolicy,casttype=ConcurrencyPolicy"`
 	// Suspend is a flag that will stop new CronWorkflows from running if set to true
@@ -63,7 +63,7 @@ type CronWorkflowSpec struct {
 	WorkflowMetadata *metav1.ObjectMeta `json:"workflowMetadata,omitempty" protobuf:"bytes,9,opt,name=workflowMeta"`
 	// v3.6 and after: StopStrategy defines if the CronWorkflow should stop scheduling based on a condition
 	StopStrategy *StopStrategy `json:"stopStrategy,omitempty" protobuf:"bytes,10,opt,name=stopStrategy"`
-	// Schedules is a list of schedules to run the Workflow in Cron format
+	// v3.6 and after: Schedules is a list of schedules to run the Workflow in Cron format
 	Schedules []string `json:"schedules,omitempty" protobuf:"bytes,11,opt,name=schedules"`
 	// v3.6 and after: When is an expression that determines if a run should be scheduled.
 	When string `json:"when,omitempty" protobuf:"bytes,12,opt,name=when"`
@@ -103,7 +103,7 @@ func (c *CronWorkflow) IsUsingNewSchedule() bool {
 	lastUsedSchedule, exists := c.Annotations[annotationKeyLatestSchedule]
 	// If last-used-schedule does not exist, or if it does not match the current schedule then the CronWorkflow schedule
 	// was just updated
-	return !exists || lastUsedSchedule != c.Spec.GetScheduleString()
+	return !exists || lastUsedSchedule != c.Spec.GetScheduleWithTimezoneString()
 }
 
 func (c *CronWorkflow) SetSchedule(schedule string) {
@@ -131,16 +131,33 @@ func (c *CronWorkflow) GetLatestSchedule() string {
 	return c.Annotations[annotationKeyLatestSchedule]
 }
 
-// GetScheduleString returns the schedule expression with timezone, if available. If multiple
+// GetScheduleString returns the schedule expression without timezone. If multiple
 // expressions are configured it returns a comma separated list of cron expressions
 func (c *CronWorkflowSpec) GetScheduleString() string {
+	return c.getScheduleString(false)
+}
+
+// GetScheduleString returns the schedule expression with timezone, if available. If multiple
+// expressions are configured it returns a comma separated list of cron expressions
+func (c *CronWorkflowSpec) GetScheduleWithTimezoneString() string {
+	return c.getScheduleString(true)
+}
+
+func (c *CronWorkflowSpec) getScheduleString(withTimezone bool) string {
 	var scheduleString string
 	if c.Schedule != "" {
-		scheduleString = c.withTimezone(c.Schedule)
+		if withTimezone {
+			scheduleString = c.withTimezone(c.Schedule)
+		} else {
+			scheduleString = c.Schedule
+		}
 	} else {
 		var sb strings.Builder
 		for i, schedule := range c.Schedules {
-			sb.WriteString(c.withTimezone(schedule))
+			if withTimezone {
+				schedule = c.withTimezone(schedule)
+			}
+			sb.WriteString(schedule)
 			if i != len(c.Schedules)-1 {
 				sb.WriteString(",")
 			}

diff --git a/pkg/apis/workflow/v1alpha1/cron_workflow_types_test.go b/pkg/apis/workflow/v1alpha1/cron_workflow_types_test.go
@@ -29,7 +29,8 @@ func TestCronWorkflowSpec_GetScheduleStrings(t *testing.T) {
 	cwfSpec.Timezone = "America/Los_Angeles"
 	assert.Equal(t, []string{"* * * * *"}, cwfSpec.GetSchedules())
 	assert.Equal(t, []string{"CRON_TZ=America/Los_Angeles * * * * *"}, cwfSpec.GetSchedulesWithTimezone())
-	assert.Equal(t, "CRON_TZ=America/Los_Angeles * * * * *", cwfSpec.GetScheduleString())
+	assert.Equal(t, "* * * * *", cwfSpec.GetScheduleString())
+	assert.Equal(t, "CRON_TZ=America/Los_Angeles * * * * *", cwfSpec.GetScheduleWithTimezoneString())
 
 	cwfSpec = CronWorkflowSpec{
 		Timezone:  "",
@@ -40,5 +41,6 @@ func TestCronWorkflowSpec_GetScheduleStrings(t *testing.T) {
 	cwfSpec.Timezone = "America/Los_Angeles"
 	assert.Equal(t, []string{"* * * * *", "0 * * * *"}, cwfSpec.GetSchedules())
 	assert.Equal(t, []string{"CRON_TZ=America/Los_Angeles * * * * *", "CRON_TZ=America/Los_Angeles 0 * * * *"}, cwfSpec.GetSchedulesWithTimezone())
-	assert.Equal(t, "CRON_TZ=America/Los_Angeles * * * * *,CRON_TZ=America/Los_Angeles 0 * * * *", cwfSpec.GetScheduleString())
+	assert.Equal(t, "* * * * *,0 * * * *", cwfSpec.GetScheduleString())
+	assert.Equal(t, "CRON_TZ=America/Los_Angeles * * * * *,CRON_TZ=America/Los_Angeles 0 * * * *", cwfSpec.GetScheduleWithTimezoneString())
 }
diff --git a/pkg/apis/workflow/v1alpha1/generated.proto b/pkg/apis/workflow/v1alpha1/generated.proto
diff --git a/pkg/apis/workflow/v1alpha1/openapi_generated.go b/pkg/apis/workflow/v1alpha1/openapi_generated.go
diff --git a/sdks/java/client/docs/IoArgoprojWorkflowV1alpha1CronWorkflowSpec.md b/sdks/java/client/docs/IoArgoprojWorkflowV1alpha1CronWorkflowSpec.md