fix!: consolidate cronworkflow variables

Issues #13474 and #12305 both introduced expressions into CronWorkflows as new features for 3.6. Motivation The expressions for `when` include the prefix `cronworkflow.`. Those for `stopStrategy.condition` do not. Unify both of these under `cronworkflow.` and allow both sets of variables to be used in both expressions. This simplifies the documentation and is less surprising. stopExpression's `condition` is mostly not a field name used, so in agreement with @agilgur5 I've renamed it to `expression`. Modifications Renamed `stopStrategy.condition` to `stopStrategy.expression` Created a single function to generate all the variables for both expressions and use that instead of the two disparate mechanisms. Modified some code comments to be godoc compliant. Verification Existing tests still pass with appropriate amendments to the test cases Note This is a breaking change vs 3.6-rc1 and rc2 for users of stopStrategy only. Signed-off-by: Alan Clucas <[email protected]>
argoproj · Oct 4, 2024 · 84afcd9 · 84afcd9
1 parent 9f83ae2
commit 84afcd9
Show file tree

Hide file tree

Showing 15 changed files with 808 additions and 795 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/docs/cron-workflows.md b/docs/cron-workflows.md
@@ -39,7 +39,7 @@ In the above example it would be similar to `test-cron-wf-tj6fe`.
 
 You can use `CronWorkflow.spec.workflowMetadata` to add `labels` and `annotations`.
 
-### `CronWorkflow` Options
+### `CronWorkflow` Spec
 
 | Option Name                  | Default Value          | Description |
 |:----------------------------:|:----------------------:|-------------|
@@ -51,7 +51,8 @@ You can use `CronWorkflow.spec.workflowMetadata` to add `labels` and `annotation
 | `startingDeadlineSeconds`    | `0`                    | Seconds after [the last scheduled time](#crash-recovery) during which a missed `Workflow` will still be run. |
 | `successfulJobsHistoryLimit` | `3`                    | Number of successful `Workflows` to persist |
 | `failedJobsHistoryLimit`     | `1`                    | Number of failed `Workflows` to persist |
-| `stopStrategy`               | `nil`                  | v3.6 and after: defines if the CronWorkflow should stop scheduling based on a condition |
+| `stopStrategy`               | `nil`                  | v3.6 and after: defines if the CronWorkflow should stop scheduling based on an expression |
+| `stopStrategy.expression`    | None                   | v3.6 and after: an expression which if present must evaluate to false for the workflow to be created |
 
 ### Cron Schedule Syntax
 
@@ -120,14 +121,14 @@ For example, if you want to stop scheduling new workflows after one success:
 
 ```yaml
 stopStrategy:
-  condition: "succeeded >= 1"
+  expression: "cronworkflow.succeeded >= 1"
 ```
 
 You can also stop scheduling new workflows after three failures with:
 
 ```yaml
 stopStrategy:
-  condition: "failed >= 3"
+  expression: "cronworkflow.failed >= 3"
 ```
 
 <!-- markdownlint-disable MD046 -- this is indented due to the admonition, not a code block -->

diff --git a/docs/variables.md b/docs/variables.md
@@ -197,6 +197,8 @@ Only available for `successCondition`
 | `cronworkflow.annotations.<NAME>` | CronWorkflow annotations (`string`) |
 | `cronworkflow.annotations.json` | CronWorkflow annotations as a JSON string (`string`) |
 | `cronworkflow.lastScheduledTime` | The time since this workflow was last scheduled, value is nil on first run (`*time.Time`) |
+| `cronworkflow.failed` | Counts how many times child workflows failed |
+| `cronworkflow.succeeded` | Counts how many times child workflows succeeded |
 
 ### `RetryStrategy`
 
@@ -290,17 +292,6 @@ For `Template`-level metrics:
 | `workflow.status` | Workflow status. One of: `Succeeded`, `Failed`, `Error` |
 | `workflow.failures` | A list of JSON objects containing information about nodes that failed or errored during execution. Available fields: `displayName`, `message`, `templateName`, `phase`, `podName`, and `finishedAt`. |
 
-### `stopStrategy`
-
-> v3.6 and after
-
-When using the `condition` field within the [`stopStrategy` of a `CronWorkflow`](cron-workflows.md#automatically-stopping-a-cronworkflow), special variables are available.
-
-| Variable | Description|
-|----------|------------|
-| `failed` | Counts how many times child workflows failed |
-| `succeeded` | Counts how many times child workflows succeeded |
-
 ### Knowing where you are
 
 The idea with creating a `WorkflowTemplate` is that they are reusable bits of code you will use in many actual Workflows. Sometimes it is useful to know which workflow you are part of.

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
@@ -69,11 +69,12 @@ type CronWorkflowSpec struct {
 	When string `json:"when,omitempty" protobuf:"bytes,12,opt,name=when"`
 }
 
-// v3.6 and after: StopStrategy defines if the CronWorkflow should stop scheduling based on a condition
+// StopStrategy defines if the CronWorkflow should stop scheduling based on an expression. v3.6 and after
 type StopStrategy struct {
-	// v3.6 and after: Condition is an expression that stops scheduling workflows when true. Use the
-	// variables `failed` or `succeeded` to access the number of failed or successful child workflows.
-	Condition string `json:"condition" protobuf:"bytes,1,opt,name=condition"`
+	// Expression is an expression that stops scheduling workflows when true. Use the
+	// variables `cronworkflow.failed` or `cronworkflow.succeeded` to access the number of failed or successful child workflows.
+	// v3.6 and after
+	Expression string `json:"expression" protobuf:"bytes,1,opt,name=expression"`
 }
 
 // CronWorkflowStatus is the status of a CronWorkflow