Skip to content

Commit bc7dd6b

Browse files
alien2003claude
andcommitted
feat: add cronWorkflowDefaults to controller config. Fixes #12627
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: alien2003 <alien2003@protonmail.ch>
1 parent 8bf2a9b commit bc7dd6b

11 files changed

Lines changed: 240 additions & 3 deletions
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
Component: CronWorkflows
2+
Issues: 12627
3+
Description: Add `cronWorkflowDefaults` to the controller ConfigMap so default `CronWorkflow` spec values can be set controller-wide
4+
Author: [alien2003](https://github.com/alien2003)
5+
6+
`cronWorkflowDefaults` mirrors the existing `workflowDefaults`, but for `CronWorkflow` spec fields such as `concurrencyPolicy`, `startingDeadlineSeconds`, `successfulJobsHistoryLimit`, `failedJobsHistoryLimit` and `timezone`.
7+
Use it to enforce settings across every `CronWorkflow` a controller manages, for example capping the number of retained child Workflows for cost control without editing each `CronWorkflow`.
8+
9+
A value set on the `CronWorkflow` itself always takes precedence over the default.
10+
Defaults are applied to the controller's in-memory copy during reconciliation and are never written back to the resource, so they do not conflict with GitOps tools that own the `CronWorkflow` manifest.
11+
12+
```yaml
13+
data:
14+
cronWorkflowDefaults: |
15+
concurrencyPolicy: "Replace"
16+
successfulJobsHistoryLimit: 4
17+
failedJobsHistoryLimit: 4
18+
timezone: "America/New_York"
19+
```

config/config.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ type Config struct {
8686
// WorkflowDefaults are values that will apply to all Workflows from this controller, unless overridden on the Workflow-level
8787
WorkflowDefaults *wfv1.Workflow `json:"workflowDefaults,omitempty"`
8888

89+
// CronWorkflowDefaults are values that will apply to all CronWorkflows from this controller, unless overridden on the CronWorkflow-level
90+
CronWorkflowDefaults *wfv1.CronWorkflowSpec `json:"cronWorkflowDefaults,omitempty"`
91+
8992
// PodSpecLogStrategy enables the logging of podspec on controller log.
9093
PodSpecLogStrategy PodSpecLogStrategy `json:"podSpecLogStrategy,omitzero"`
9194

docs/default-workflow-specs.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,29 @@ data:
5555
parallelism: 3
5656
5757
```
58+
59+
## Setting Default CronWorkflow Values
60+
61+
> v4.1 and after
62+
63+
Default `CronWorkflow` spec values can be specified under the `cronWorkflowDefaults` key in the `workflow-controller-configmap`.
64+
These apply to all `CronWorkflows` executed from the controller, such as `concurrencyPolicy`, `startingDeadlineSeconds`, `successfulJobsHistoryLimit`, `failedJobsHistoryLimit` and `timezone`.
65+
If a `CronWorkflow` sets a value that also has a default, the `CronWorkflow`'s value takes precedence.
66+
See the [Field Reference](./fields.md#cronworkflowspec) for the full list of `CronWorkflowSpec` fields.
67+
68+
Defaults are applied to the controller's in-memory copy during reconciliation and are never written back to the `CronWorkflow`, so they do not conflict with GitOps tools that manage the resource.
69+
70+
For example, to keep at most four successful and four failed child Workflows for every `CronWorkflow`:
71+
72+
```yaml
73+
apiVersion: v1
74+
kind: ConfigMap
75+
metadata:
76+
name: workflow-controller-configmap
77+
data:
78+
cronWorkflowDefaults: |
79+
concurrencyPolicy: "Replace"
80+
successfulJobsHistoryLimit: 4
81+
failedJobsHistoryLimit: 4
82+
timezone: "America/New_York"
83+
```

docs/workflow-controller-configmap.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ Config contains the root of the configuration settings for the workflow controll
8686
| `Links` | `Array<`[`Link`](fields.md#link)`>` | Links to related apps. |
8787
| `Columns` | `Array<`[`Column`](fields.md#column)`>` | Columns are custom columns that will be exposed in the Workflow List View. |
8888
| `WorkflowDefaults` | [`wfv1.Workflow`](fields.md#workflow) | WorkflowDefaults are values that will apply to all Workflows from this controller, unless overridden on the Workflow-level |
89+
| `CronWorkflowDefaults` | [`wfv1.CronWorkflowSpec`](fields.md#cronworkflowspec) | CronWorkflowDefaults are values that will apply to all CronWorkflows from this controller, unless overridden on the CronWorkflow-level |
8990
| `PodSpecLogStrategy` | [`PodSpecLogStrategy`](#podspeclogstrategy) | PodSpecLogStrategy enables the logging of podspec on controller log. |
9091
| `PodGCGracePeriodSeconds` | `int64` | PodGCGracePeriodSeconds specifies the duration in seconds before a terminating pod is forcefully killed. Value must be non-negative integer. A zero value indicates that the pod will be forcefully terminated immediately. Defaults to the Kubernetes default of 30 seconds. |
9192
| `PodGCDeleteDelayDuration` | [`metav1.Duration`](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#duration-v1-meta) | PodGCDeleteDelayDuration specifies the duration before pods in the GC queue get deleted. Value must be non-negative. A zero value indicates that the pods will be deleted immediately. Defaults to 5 seconds. |

docs/workflow-controller-configmap.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,14 @@ data:
438438
secondsAfterSuccess: 5
439439
parallelism: 3
440440
441+
# Default values that will apply to all CronWorkflows from this controller, unless overridden on the CronWorkflow-level
442+
# See more: docs/default-workflow-specs.md
443+
cronWorkflowDefaults: |
444+
concurrencyPolicy: "Replace"
445+
successfulJobsHistoryLimit: 4
446+
failedJobsHistoryLimit: 4
447+
timezone: "America/New_York"
448+
441449
# SSO Configuration for the Argo server.
442450
# You must also start argo server with `--auth-mode sso`.
443451
# https://argo-workflows.readthedocs.io/en/latest/argo-server-auth-mode/

workflow/controller/controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ func (wfc *WorkflowController) runPodController(ctx context.Context, podGCWorker
278278
func (wfc *WorkflowController) runCronController(ctx context.Context, cronWorkflowWorkers int) {
279279
defer runtimeutil.HandleCrashWithContext(ctx, runtimeutil.PanicHandlers...)
280280

281-
cronController := cron.NewCronController(ctx, wfc.wfclientset, wfc.dynamicInterface, wfc.namespace, wfc.GetManagedNamespace(), wfc.Config.InstanceID, wfc.metrics, wfc.eventRecorderManager, cronWorkflowWorkers, wfc.wftmplInformer, wfc.cwftmplInformer, wfc.Config.WorkflowDefaults)
281+
cronController := cron.NewCronController(ctx, wfc.wfclientset, wfc.dynamicInterface, wfc.namespace, wfc.GetManagedNamespace(), wfc.Config.InstanceID, wfc.metrics, wfc.eventRecorderManager, cronWorkflowWorkers, wfc.wftmplInformer, wfc.cwftmplInformer, wfc.Config.WorkflowDefaults, wfc.Config.CronWorkflowDefaults)
282282
cronController.Run(ctx)
283283
}
284284

workflow/cron/controller.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ type Controller struct {
4949
wftmplInformer wfextvv1alpha1.WorkflowTemplateInformer
5050
cwftmplInformer wfextvv1alpha1.ClusterWorkflowTemplateInformer
5151
wfDefaults *v1alpha1.Workflow
52+
cronWfDefaults *v1alpha1.CronWorkflowSpec
5253
cronWfQueue workqueue.TypedRateLimitingInterface[string]
5354
dynamicInterface dynamic.Interface
5455
metrics *metrics.Metrics
@@ -76,6 +77,7 @@ func init() {
7677
// NewCronController creates a new cron controller
7778
func NewCronController(ctx context.Context, wfclientset versioned.Interface, dynamicInterface dynamic.Interface, namespace string, managedNamespace string, instanceID string, metrics *metrics.Metrics,
7879
eventRecorderManager events.EventRecorderManager, cronWorkflowWorkers int, wftmplInformer wfextvv1alpha1.WorkflowTemplateInformer, cwftmplInformer wfextvv1alpha1.ClusterWorkflowTemplateInformer, wfDefaults *v1alpha1.Workflow,
80+
cronWfDefaults *v1alpha1.CronWorkflowSpec,
7981
) *Controller {
8082
ctx, logger := logging.RequireLoggerFromContext(ctx).WithField("component", "cron").InContext(ctx)
8183

@@ -89,6 +91,7 @@ func NewCronController(ctx context.Context, wfclientset versioned.Interface, dyn
8991
dynamicInterface: dynamicInterface,
9092
cronWfQueue: metrics.RateLimiterWithBusyWorkers(ctx, workqueue.DefaultTypedControllerRateLimiter[string](), "cron_wf_queue"),
9193
wfDefaults: wfDefaults,
94+
cronWfDefaults: cronWfDefaults,
9295
metrics: metrics,
9396
eventRecorderManager: eventRecorderManager,
9497
wftmplInformer: wftmplInformer,
@@ -179,7 +182,7 @@ func (cc *Controller) processNextCronItem(ctx context.Context) bool {
179182
}
180183
ctx = wfctx.InjectObjectMeta(ctx, &cronWf.ObjectMeta)
181184

182-
cronWorkflowOperationCtx := newCronWfOperationCtx(ctx, cronWf, cc.wfClientset, cc.metrics, cc.wftmplInformer, cc.cwftmplInformer, cc.wfDefaults)
185+
cronWorkflowOperationCtx := newCronWfOperationCtx(ctx, cronWf, cc.wfClientset, cc.metrics, cc.wftmplInformer, cc.cwftmplInformer, cc.wfDefaults, cc.cronWfDefaults)
183186

184187
err = cronWorkflowOperationCtx.validateCronWorkflow(ctx)
185188
if err != nil {
@@ -297,7 +300,7 @@ func (cc *Controller) syncCronWorkflow(ctx context.Context, cronWf *v1alpha1.Cro
297300
cc.keyLock.Lock(key)
298301
defer cc.keyLock.Unlock(key)
299302

300-
cwoc := newCronWfOperationCtx(ctx, cronWf, cc.wfClientset, cc.metrics, cc.wftmplInformer, cc.cwftmplInformer, cc.wfDefaults)
303+
cwoc := newCronWfOperationCtx(ctx, cronWf, cc.wfClientset, cc.metrics, cc.wftmplInformer, cc.cwftmplInformer, cc.wfDefaults, cc.cronWfDefaults)
301304
err := cwoc.enforceHistoryLimit(ctx, workflows)
302305
if err != nil {
303306
return err

workflow/cron/operator.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ type cronWfOperationCtx struct {
4646
wfClientset versioned.Interface
4747
wfClient typed.WorkflowInterface
4848
wfDefaults *v1alpha1.Workflow
49+
cronWfDefaults *v1alpha1.CronWorkflowSpec
4950
cronWfIf typed.CronWorkflowInterface
5051
wftmplInformer wfextvv1alpha1.WorkflowTemplateInformer
5152
cwftmplInformer wfextvv1alpha1.ClusterWorkflowTemplateInformer
@@ -57,16 +58,33 @@ type cronWfOperationCtx struct {
5758
ctx context.Context
5859
}
5960

61+
// applyCronWorkflowDefaults merges controller-level CronWorkflow defaults into the in-memory
62+
// CronWorkflow spec, leaving any field the CronWorkflow already sets untouched. It only mutates the
63+
// working copy: the reconcile loop never persists the spec, so defaults don't cause GitOps drift.
64+
// The defaults are deep-copied because MergeToCronWorkflowSpec temporarily mutates its patch argument
65+
// and cronWfDefaults is shared across concurrent workers.
66+
func applyCronWorkflowDefaults(ctx context.Context, cronWf *v1alpha1.CronWorkflow, cronWfDefaults *v1alpha1.CronWorkflowSpec) {
67+
if cronWfDefaults == nil {
68+
return
69+
}
70+
if err := util.MergeToCronWorkflowSpec(cronWfDefaults.DeepCopy(), &cronWf.Spec); err != nil {
71+
logging.RequireLoggerFromContext(ctx).WithError(err).Error(ctx, "Failed to apply CronWorkflow defaults")
72+
}
73+
}
74+
6075
func newCronWfOperationCtx(ctx context.Context, cronWorkflow *v1alpha1.CronWorkflow, wfClientset versioned.Interface,
6176
metrics *metrics.Metrics, wftmplInformer wfextvv1alpha1.WorkflowTemplateInformer,
6277
cwftmplInformer wfextvv1alpha1.ClusterWorkflowTemplateInformer, wfDefaults *v1alpha1.Workflow,
78+
cronWfDefaults *v1alpha1.CronWorkflowSpec,
6379
) *cronWfOperationCtx {
6480
log := logging.RequireLoggerFromContext(ctx)
81+
applyCronWorkflowDefaults(ctx, cronWorkflow, cronWfDefaults)
6582
return &cronWfOperationCtx{
6683
cronWf: cronWorkflow,
6784
wfClientset: wfClientset,
6885
wfClient: wfClientset.ArgoprojV1alpha1().Workflows(cronWorkflow.Namespace),
6986
wfDefaults: wfDefaults,
87+
cronWfDefaults: cronWfDefaults,
7088
cronWfIf: wfClientset.ArgoprojV1alpha1().CronWorkflows(cronWorkflow.Namespace),
7189
wftmplInformer: wftmplInformer,
7290
cwftmplInformer: cwftmplInformer,

workflow/cron/operator_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,3 +815,46 @@ func TestEvaluateWhenUnresolvedOutside(t *testing.T) {
815815
require.NoError(t, err)
816816
assert.True(t, result)
817817
}
818+
819+
func TestApplyCronWorkflowDefaults(t *testing.T) {
820+
ctx := logging.TestContext(t.Context())
821+
822+
newCronWf := func() *v1alpha1.CronWorkflow {
823+
cronWf := &v1alpha1.CronWorkflow{}
824+
v1alpha1.MustUnmarshal([]byte(scheduledWf), cronWf)
825+
return cronWf
826+
}
827+
828+
t.Run("DefaultsFillUnsetFields", func(t *testing.T) {
829+
cronWf := newCronWf() // sets startingDeadlineSeconds: 30, no concurrencyPolicy
830+
successLimit := int32(4)
831+
defaults := &v1alpha1.CronWorkflowSpec{
832+
ConcurrencyPolicy: v1alpha1.ReplaceConcurrent,
833+
SuccessfulJobsHistoryLimit: &successLimit,
834+
}
835+
applyCronWorkflowDefaults(ctx, cronWf, defaults)
836+
assert.Equal(t, v1alpha1.ReplaceConcurrent, cronWf.Spec.ConcurrencyPolicy)
837+
require.NotNil(t, cronWf.Spec.SuccessfulJobsHistoryLimit)
838+
assert.Equal(t, int32(4), *cronWf.Spec.SuccessfulJobsHistoryLimit)
839+
// fields set on the CronWorkflow itself are preserved
840+
require.NotNil(t, cronWf.Spec.StartingDeadlineSeconds)
841+
assert.Equal(t, int64(30), *cronWf.Spec.StartingDeadlineSeconds)
842+
assert.Equal(t, []string{"* * * * *"}, cronWf.Spec.Schedules)
843+
})
844+
845+
t.Run("CronWorkflowOverridesDefaults", func(t *testing.T) {
846+
cronWf := newCronWf()
847+
deadline := int64(120)
848+
defaults := &v1alpha1.CronWorkflowSpec{StartingDeadlineSeconds: &deadline}
849+
applyCronWorkflowDefaults(ctx, cronWf, defaults)
850+
require.NotNil(t, cronWf.Spec.StartingDeadlineSeconds)
851+
assert.Equal(t, int64(30), *cronWf.Spec.StartingDeadlineSeconds)
852+
})
853+
854+
t.Run("NilDefaultsIsNoOp", func(t *testing.T) {
855+
cronWf := newCronWf()
856+
applyCronWorkflowDefaults(ctx, cronWf, nil)
857+
assert.Empty(t, cronWf.Spec.ConcurrencyPolicy)
858+
assert.Equal(t, []string{"* * * * *"}, cronWf.Spec.Schedules)
859+
})
860+
}

workflow/util/merge.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,70 @@ func MergeTo(patch, target *wfv1.Workflow) error {
191191
return nil
192192
}
193193

194+
// MergeToCronWorkflowSpec merges the "patch" CronWorkflowSpec into the "target" CronWorkflowSpec.
195+
// If the target defines a field, that value takes precedence over the patch. This mirrors MergeTo,
196+
// including the manual handling of the embedded WorkflowSpec's hooks and labelsFrom, which strategic
197+
// merge does not handle.
198+
func MergeToCronWorkflowSpec(patch, target *wfv1.CronWorkflowSpec) error {
199+
if target == nil || patch == nil {
200+
return nil
201+
}
202+
203+
// Temporarily remove hooks and labelsFrom as they don't merge
204+
patchHooks := patch.WorkflowSpec.Hooks
205+
patch.WorkflowSpec.Hooks = nil
206+
var patchLabelsFrom map[string]wfv1.LabelValueFrom
207+
if patch.WorkflowSpec.WorkflowMetadata != nil {
208+
patchLabelsFrom = patch.WorkflowSpec.WorkflowMetadata.LabelsFrom
209+
patch.WorkflowSpec.WorkflowMetadata.LabelsFrom = nil
210+
}
211+
212+
patchBytes, err := json.Marshal(patch)
213+
patch.WorkflowSpec.Hooks = patchHooks
214+
if len(patchLabelsFrom) != 0 {
215+
patch.WorkflowSpec.WorkflowMetadata.LabelsFrom = patchLabelsFrom
216+
}
217+
if err != nil {
218+
return err
219+
}
220+
221+
targetBytes, err := json.Marshal(target)
222+
if err != nil {
223+
return err
224+
}
225+
226+
mergedBytes, err := strategicpatch.StrategicMergePatch(patchBytes, targetBytes, wfv1.CronWorkflowSpec{})
227+
if err != nil {
228+
return err
229+
}
230+
231+
*target = wfv1.CronWorkflowSpec{}
232+
if err = json.Unmarshal(mergedBytes, target); err != nil {
233+
return err
234+
}
235+
236+
if len(patchHooks) != 0 && target.WorkflowSpec.Hooks == nil {
237+
target.WorkflowSpec.Hooks = make(wfv1.LifecycleHooks)
238+
}
239+
for name, hook := range patchHooks {
240+
// If the patch hook doesn't exist in target
241+
if _, ok := target.WorkflowSpec.Hooks[name]; !ok {
242+
target.WorkflowSpec.Hooks[name] = hook
243+
}
244+
}
245+
246+
if len(patchLabelsFrom) != 0 && target.WorkflowSpec.WorkflowMetadata.LabelsFrom == nil {
247+
target.WorkflowSpec.WorkflowMetadata.LabelsFrom = make(map[string]wfv1.LabelValueFrom)
248+
}
249+
for key, val := range patchLabelsFrom {
250+
// If the patch labelFrom doesn't exist in target
251+
if _, ok := target.WorkflowSpec.WorkflowMetadata.LabelsFrom[key]; !ok {
252+
target.WorkflowSpec.WorkflowMetadata.LabelsFrom[key] = val
253+
}
254+
}
255+
return nil
256+
}
257+
194258
// mergeMap will merge all element from right map to left map if it is not present in left.
195259
func mergeMap(from, to map[string]string) {
196260
for key, val := range from {

0 commit comments

Comments
 (0)