fix: Assume controller is in UTC when calculating NextScheduledRuntime (

argoproj#4417)

Signed-off-by: Simon Behar <[email protected]>

cmd/argo/commands/cron/get.go

@@ -67,46 +67,46 @@ func printCronWorkflow(wf *wfv1.CronWorkflow, outFmt string) {
  }
 }
 
-func getCronWorkflowGet(wf *wfv1.CronWorkflow) string {
+func getCronWorkflowGet(cwf *wfv1.CronWorkflow) string {
  const fmtStr = "%-30s %v\n"
 
  out := ""
- out += fmt.Sprintf(fmtStr, "Name:", wf.ObjectMeta.Name)
- out += fmt.Sprintf(fmtStr, "Namespace:", wf.ObjectMeta.Namespace)
- out += fmt.Sprintf(fmtStr, "Created:", humanize.Timestamp(wf.ObjectMeta.CreationTimestamp.Time))
- out += fmt.Sprintf(fmtStr, "Schedule:", wf.Spec.Schedule)
- out += fmt.Sprintf(fmtStr, "Suspended:", wf.Spec.Suspend)
- if wf.Spec.Timezone != "" {
- out += fmt.Sprintf(fmtStr, "Timezone:", wf.Spec.Timezone)
+ 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.Schedule)
+ out += fmt.Sprintf(fmtStr, "Suspended:", cwf.Spec.Suspend)
+ if cwf.Spec.Timezone != "" {
+ out += fmt.Sprintf(fmtStr, "Timezone:", cwf.Spec.Timezone)
  }
- if wf.Spec.StartingDeadlineSeconds != nil {
- out += fmt.Sprintf(fmtStr, "StartingDeadlineSeconds:", *wf.Spec.StartingDeadlineSeconds)
+ if cwf.Spec.StartingDeadlineSeconds != nil {
+ out += fmt.Sprintf(fmtStr, "StartingDeadlineSeconds:", *cwf.Spec.StartingDeadlineSeconds)
  }
- if wf.Spec.ConcurrencyPolicy != "" {
- out += fmt.Sprintf(fmtStr, "ConcurrencyPolicy:", wf.Spec.ConcurrencyPolicy)
+ if cwf.Spec.ConcurrencyPolicy != "" {
+ out += fmt.Sprintf(fmtStr, "ConcurrencyPolicy:", cwf.Spec.ConcurrencyPolicy)
  }
- if wf.Status.LastScheduledTime != nil {
- out += fmt.Sprintf(fmtStr, "LastScheduledTime:", humanize.Timestamp(wf.Status.LastScheduledTime.Time))
+ if cwf.Status.LastScheduledTime != nil {
+ out += fmt.Sprintf(fmtStr, "LastScheduledTime:", humanize.Timestamp(cwf.Status.LastScheduledTime.Time))
  }
 
- next, err := wf.GetNextRuntime()
+ next, err := GetNextRuntime(cwf)
  if err == nil {
- out += fmt.Sprintf(fmtStr, "NextScheduledTime:", humanize.Timestamp(next))
+ out += fmt.Sprintf(fmtStr, "NextScheduledTime:", humanize.Timestamp(next)+" (assumes workflow-controller is in UTC)")
  }
 
- if len(wf.Status.Active) > 0 {
+ if len(cwf.Status.Active) > 0 {
  var activeWfNames []string
- for _, activeWf := range wf.Status.Active {
+ for _, activeWf := range cwf.Status.Active {
  activeWfNames = append(activeWfNames, activeWf.Name)
  }
  out += fmt.Sprintf(fmtStr, "Active Workflows:", strings.Join(activeWfNames, ", "))
  }
- if len(wf.Status.Conditions) > 0 {
- out += wf.Status.Conditions.DisplayString(fmtStr, map[wfv1.ConditionType]string{wfv1.ConditionTypeSubmissionError: "✖"})
+ if len(cwf.Status.Conditions) > 0 {
+ out += cwf.Status.Conditions.DisplayString(fmtStr, map[wfv1.ConditionType]string{wfv1.ConditionTypeSubmissionError: "✖"})
  }
- if len(wf.Spec.WorkflowSpec.Arguments.Parameters) > 0 {
+ if len(cwf.Spec.WorkflowSpec.Arguments.Parameters) > 0 {
  out += fmt.Sprintf(fmtStr, "Workflow Parameters:", "")
- for _, param := range wf.Spec.WorkflowSpec.Arguments.Parameters {
+ for _, param := range cwf.Spec.WorkflowSpec.Arguments.Parameters {
  if param.Value == nil {
  continue
  }

cmd/argo/commands/cron/get_test.go

@@ -68,7 +68,7 @@ func TestNextRuntime(t *testing.T) {
  var cronWf v1alpha1.CronWorkflow
  err := yaml.Unmarshal([]byte(invalidCwf), &cronWf)
  if assert.NoError(t, err) {
- next, err := cronWf.GetNextRuntime()
+ next, err := GetNextRuntime(&cronWf)
  if assert.NoError(t, err) {
  assert.LessOrEqual(t, next.Unix(), time.Now().Add(1*time.Minute).Unix())
  assert.Greater(t, next.Unix(), time.Now().Unix())

cmd/argo/commands/cron/list.go

@@ -70,23 +70,23 @@ func printTable(wfList []wfv1.CronWorkflow, listArgs *listFlags) {
  }
  _, _ = fmt.Fprint(w, "NAME\tAGE\tLAST RUN\tNEXT RUN\tSCHEDULE\tSUSPENDED")
  _, _ = fmt.Fprint(w, "\n")
- for _, wf := range wfList {
+ for _, cwf := range wfList {
  if listArgs.allNamespaces {
- _, _ = fmt.Fprintf(w, "%s\t", wf.ObjectMeta.Namespace)
+ _, _ = fmt.Fprintf(w, "%s\t", cwf.ObjectMeta.Namespace)
  }
  var cleanLastScheduledTime string
- if wf.Status.LastScheduledTime != nil {
- cleanLastScheduledTime = humanize.RelativeDurationShort(wf.Status.LastScheduledTime.Time, time.Now())
+ if cwf.Status.LastScheduledTime != nil {
+ cleanLastScheduledTime = humanize.RelativeDurationShort(cwf.Status.LastScheduledTime.Time, time.Now())
  } else {
  cleanLastScheduledTime = "N/A"
  }
  var cleanNextScheduledTime string
- if next, err := wf.GetNextRuntime(); err == nil {
+ if next, err := GetNextRuntime(&cwf); err == nil {
  cleanNextScheduledTime = humanize.RelativeDurationShort(next, time.Now())
  } else {
  cleanNextScheduledTime = "N/A"
  }
- _, _ = fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%t", wf.ObjectMeta.Name, humanize.RelativeDurationShort(wf.ObjectMeta.CreationTimestamp.Time, time.Now()), cleanLastScheduledTime, cleanNextScheduledTime, wf.Spec.Schedule, wf.Spec.Suspend)
+ _, _ = fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%t", cwf.ObjectMeta.Name, humanize.RelativeDurationShort(cwf.ObjectMeta.CreationTimestamp.Time, time.Now()), cleanLastScheduledTime, cleanNextScheduledTime, cwf.Spec.Schedule, cwf.Spec.Suspend)
  _, _ = fmt.Fprintf(w, "\n")
  }
  _ = w.Flush()

cmd/argo/commands/cron/root.go

@@ -7,7 +7,7 @@ import (
 func NewCronWorkflowCommand() *cobra.Command {
  var command = &cobra.Command{
  Use: "cron",
- Short: "manage cron workflows",
+ Short: "manage cron workflows\n\nNextScheduledRun assumes that the workflow-controller uses UTC as its timezone",
  Run: func(cmd *cobra.Command, args []string) {
  cmd.HelpFunc()(cmd, args)
  },

cmd/argo/commands/cron/util.go

@@ -0,0 +1,23 @@
+package cron
+
+import (
+ "time"
+
+ "github.com/robfig/cron/v3"
+
+ "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
+)
+
+// GetNextRuntime returns the next time the workflow should run in local time. It assumes the workflow-controller is in
+// UTC, but nevertheless returns the time in the local timezone.
+func GetNextRuntime(cwf *v1alpha1.CronWorkflow) (time.Time, error) {
+ cronScheduleString := cwf.Spec.Schedule
+ if cwf.Spec.Timezone != "" {
+ cronScheduleString = "CRON_TZ=" + cwf.Spec.Timezone + " " + cronScheduleString
+ }
+ cronSchedule, err := cron.ParseStandard(cronScheduleString)
+ if err != nil {
+ return time.Time{}, err
+ }
+ return cronSchedule.Next(time.Now().UTC()).Local(), nil
+}

docs/cron-workflows.md

@@ -90,15 +90,18 @@ test-cron-wf 56s 2s * * * * * false
 $ argo cron get test-cron-wf
 Name: test-cron-wf
 Namespace: argo
-Created: Mon Nov 18 10:17:06 -0800 (4 minutes ago)
+Created: Wed Oct 28 07:19:02 -0600 (23 hours ago)
 Schedule: * * * * *
 Suspended: false
 StartingDeadlineSeconds: 0
 ConcurrencyPolicy: Replace
-LastScheduledTime: Mon Nov 18 10:21:00 -0800 (51 seconds ago)
+LastScheduledTime: Thu Oct 29 06:51:00 -0600 (11 minutes ago)
+NextScheduledTime: Thu Oct 29 13:03:00 +0000 (32 seconds from now)
 Active Workflows: test-cron-wf-rt4nf
 ```
 
+**Note**: `NextScheduledRun` assumes that the workflow-controller uses UTC as its timezone
+
 ### `kubectl`
 
 Using `kubectl apply -f` and `kubectl get cwf`

pkg/apis/workflow/v1alpha1/cron_workflow_types.go

@@ -1,9 +1,6 @@
 package v1alpha1
 
 import (
- "time"
-
- "github.com/robfig/cron/v3"
  v1 "k8s.io/api/core/v1"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -20,18 +17,6 @@ type CronWorkflow struct {
  Status CronWorkflowStatus `json:"status,omitempty" protobuf:"bytes,3,opt,name=status"`
 }
 
-func (cwf *CronWorkflow) GetNextRuntime() (time.Time, error) {
- cronScheduleString := cwf.Spec.Schedule
- if cwf.Spec.Timezone != "" {
- cronScheduleString = "CRON_TZ=" + cwf.Spec.Timezone + " " + cronScheduleString
- }
- cronSchedule, err := cron.ParseStandard(cronScheduleString)
- if err != nil {
- return time.Time{}, err
- }
- return cronSchedule.Next(time.Now()), nil
-}
-
 // CronWorkflowList is list of CronWorkflow resources
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 type CronWorkflowList struct {

ui/src/app/cron-workflows/components/cron-workflow-summary-panel.tsx

@@ -35,7 +35,10 @@ export const CronWorkflowSummaryPanel = (props: Props) => {
  const statusAttributes = [
  {title: 'Active', value: props.cronWorkflow.status.active ? getCronWorkflowActiveWorkflowList(props.cronWorkflow.status.active) : <i>No Workflows Active</i>},
  {title: 'Last Scheduled Time', value: props.cronWorkflow.status.lastScheduledTime},
- {title: 'Next Scheduled Time', value: getNextScheduledTime(props.cronWorkflow.spec.schedule, props.cronWorkflow.spec.timezone)},
+ {
+ title: 'Next Scheduled Time',
+ value: getNextScheduledTime(props.cronWorkflow.spec.schedule, props.cronWorkflow.spec.timezone) + ' (assumes workflow-controller is in UTC)'
+ },
  {title: 'Conditions', value: <ConditionsPanel conditions={props.cronWorkflow.status.conditions} />}
  ];
  return (
@@ -92,7 +95,7 @@ function getNextScheduledTime(schedule: string, tz: string): string {
  let out = '';
  try {
  out = parser
- .parseExpression(schedule, {tz})
+ .parseExpression(schedule, {utc: !tz, tz})
  .next()
  .toISOString();
  } catch (e) {