diff --git a/internal/temporalcli/commands.schedule.go b/internal/temporalcli/commands.schedule.go index 5deadb8ec..7e6dd6d72 100644 --- a/internal/temporalcli/commands.schedule.go +++ b/internal/temporalcli/commands.schedule.go @@ -221,31 +221,82 @@ func toIntervalSpec(str string) (client.ScheduleIntervalSpec, error) { } func (c *ScheduleConfigurationOptions) toScheduleSpec(spec *client.ScheduleSpec) error { - spec.CronExpressions = c.Cron - // Skip not supported - spec.Jitter = c.Jitter.Duration() - spec.TimeZoneName = c.TimeZone - spec.StartAt = c.StartTime.Time() - spec.EndAt = c.EndTime.Time() + return c.toScheduleSpecForUpdate(spec, nil) +} - var err error - for _, calPbStr := range c.Calendar { - var calPb schedpb.CalendarSpec - if err = protojson.Unmarshal([]byte(calPbStr), &calPb); err != nil { - return fmt.Errorf("failed to parse json calendar spec: %w", err) +// toScheduleSpecForUpdate applies CLI options to spec, preserving existing spec +// fields when the user did not pass the corresponding flags (fully incremental update). +// If existing is nil, behaves like toScheduleSpec (no preservation). +func (c *ScheduleConfigurationOptions) toScheduleSpecForUpdate(spec *client.ScheduleSpec, existing *client.ScheduleSpec) error { + // Jitter, TimeZoneName, StartAt, EndAt: preserve from existing when flag not set + if existing != nil && c.FlagSet != nil { + if !c.FlagSet.Changed("jitter") { + spec.Jitter = existing.Jitter + } else { + spec.Jitter = c.Jitter.Duration() } - cron, err := toCronString(&calPb) - if err != nil { - return err + if !c.FlagSet.Changed("time-zone") { + spec.TimeZoneName = existing.TimeZoneName + } else { + spec.TimeZoneName = c.TimeZone + } + if !c.FlagSet.Changed("start-time") { + spec.StartAt = existing.StartAt + } else { + spec.StartAt = c.StartTime.Time() + } + if !c.FlagSet.Changed("end-time") { + spec.EndAt = existing.EndAt + } else { + spec.EndAt = c.EndTime.Time() + } + } else { + spec.Jitter = c.Jitter.Duration() + spec.TimeZoneName = c.TimeZone + spec.StartAt = c.StartTime.Time() + spec.EndAt = c.EndTime.Time() + } + + // Cron/calendar: preserve from existing when not provided by CLI + if len(c.Cron) == 0 && len(c.Calendar) == 0 { + if existing != nil { + spec.CronExpressions = append([]string(nil), existing.CronExpressions...) + spec.Calendars = append([]client.ScheduleCalendarSpec(nil), existing.Calendars...) + } else { + spec.CronExpressions = nil + spec.Calendars = nil + } + } else { + spec.CronExpressions = append([]string(nil), c.Cron...) + var err error + for _, calPbStr := range c.Calendar { + var calPb schedpb.CalendarSpec + if err = protojson.Unmarshal([]byte(calPbStr), &calPb); err != nil { + return fmt.Errorf("failed to parse json calendar spec: %w", err) + } + cron, err := toCronString(&calPb) + if err != nil { + return err + } + spec.CronExpressions = append(spec.CronExpressions, cron) } - spec.CronExpressions = append(spec.CronExpressions, cron) } - for _, intStr := range c.Interval { - int, err := toIntervalSpec(intStr) - if err != nil { - return err + + // Intervals: preserve from existing when not provided by CLI + if len(c.Interval) == 0 { + if existing != nil { + spec.Intervals = append([]client.ScheduleIntervalSpec(nil), existing.Intervals...) + } else { + spec.Intervals = nil + } + } else { + for _, intStr := range c.Interval { + intv, err := toIntervalSpec(intStr) + if err != nil { + return err + } + spec.Intervals = append(spec.Intervals, intv) } - spec.Intervals = append(spec.Intervals, int) } return nil @@ -540,13 +591,18 @@ func (c *TemporalScheduleUpdateCommand) run(cctx *CommandContext, args []string) newSchedule.State.RemainingActions = c.RemainingActions } - if err = c.toScheduleSpec(newSchedule.Spec); err != nil { + sch := cl.ScheduleClient().GetHandle(cctx, c.ScheduleId) + description, err := sch.Describe(cctx) + if err != nil { + return err + } + + if err = c.toScheduleSpecForUpdate(newSchedule.Spec, description.Schedule.Spec); err != nil { return err } else if newSchedule.Action, err = toScheduleAction(&c.SharedWorkflowStartOptions, &c.PayloadInputOptions); err != nil { return err } - sch := cl.ScheduleClient().GetHandle(cctx, c.ScheduleId) return sch.Update(cctx, client.ScheduleUpdateOptions{ DoUpdate: func(u client.ScheduleUpdateInput) (*client.ScheduleUpdate, error) { // replace whole schedule diff --git a/internal/temporalcli/commands.schedule_test.go b/internal/temporalcli/commands.schedule_test.go index d828e55ea..edf9b04a9 100644 --- a/internal/temporalcli/commands.schedule_test.go +++ b/internal/temporalcli/commands.schedule_test.go @@ -9,6 +9,7 @@ import ( "io" "math/rand" "regexp" + "strings" "time" "github.com/stretchr/testify/assert" @@ -510,6 +511,105 @@ func (s *SharedServerSuite) TestSchedule_Update() { }, 10*time.Second, 100*time.Millisecond) } +func (s *SharedServerSuite) TestSchedule_Update_PreservesCronWhenNotProvided() { + schedId, schedWfId, res := s.createSchedule("--cron", "0 9 * * *") + s.NoError(res.Err) + + // Update with only non-spec flags (no --cron). Existing cron should be preserved. + res = s.Execute( + "schedule", "update", + "--address", s.Address(), + "-s", schedId, + "--task-queue", "SomeOtherTq", + "--type", "SomeOtherWf", + "--workflow-id", schedWfId, + "--overlap-policy", "AllowAll", + ) + s.NoError(res.Err) + + s.Eventually(func() bool { + res = s.Execute( + "schedule", "describe", + "--address", s.Address(), + "-s", schedId, + "-o", "json", + ) + s.NoError(res.Err) + var j struct { + Schedule struct { + Spec struct { + CronExpressions []string `json:"cronExpressions"` + } `json:"spec"` + Action struct { + StartWorkflow struct { + WorkflowType struct { + Name string `json:"name"` + } `json:"workflowType"` + TaskQueue struct { + Name string `json:"name"` + } `json:"taskQueue"` + } `json:"startWorkflow"` + } `json:"action"` + } `json:"schedule"` + Info struct { + FutureActionTimes []string `json:"futureActionTimes"` + } `json:"info"` + } + s.NoError(json.Unmarshal(res.Stdout.Bytes(), &j)) + // Cron was preserved: either cronExpressions contains our cron, or futureActionTimes + // are at 09:00 UTC (server may return structuredCalendar instead of cronExpressions). + cronPreserved := (len(j.Schedule.Spec.CronExpressions) == 1 && j.Schedule.Spec.CronExpressions[0] == "0 9 * * *") || + (len(j.Info.FutureActionTimes) > 0 && (strings.Contains(j.Info.FutureActionTimes[0], "T09:00") || strings.Contains(j.Info.FutureActionTimes[0], " 09:00"))) + return cronPreserved && + j.Schedule.Action.StartWorkflow.WorkflowType.Name == "SomeOtherWf" && + j.Schedule.Action.StartWorkflow.TaskQueue.Name == "SomeOtherTq" + }, 10*time.Second, 100*time.Millisecond) +} + +func (s *SharedServerSuite) TestSchedule_Update_PreservesJitterWhenNotProvided() { + schedId, schedWfId, res := s.createSchedule("--interval", "10d", "--jitter", "1m") + s.NoError(res.Err) + + // Update with only non-spec flags (no --jitter). Existing jitter should be preserved. + res = s.Execute( + "schedule", "update", + "--address", s.Address(), + "-s", schedId, + "--task-queue", "SomeOtherTq", + "--type", "SomeOtherWf", + "--workflow-id", schedWfId, + ) + s.NoError(res.Err) + + s.Eventually(func() bool { + res = s.Execute( + "schedule", "describe", + "--address", s.Address(), + "-s", schedId, + "-o", "json", + ) + s.NoError(res.Err) + var j struct { + Schedule struct { + Spec struct { + Jitter string `json:"jitter"` + } `json:"spec"` + Action struct { + StartWorkflow struct { + WorkflowType struct { + Name string `json:"name"` + } `json:"workflowType"` + } `json:"startWorkflow"` + } `json:"action"` + } `json:"schedule"` + } + s.NoError(json.Unmarshal(res.Stdout.Bytes(), &j)) + // Jitter 1m is stored as 60s by the server + return (j.Schedule.Spec.Jitter == "60s" || j.Schedule.Spec.Jitter == "1m") && + j.Schedule.Action.StartWorkflow.WorkflowType.Name == "SomeOtherWf" + }, 10*time.Second, 100*time.Millisecond) +} + func (s *SharedServerSuite) TestSchedule_Memo_Update() { schedId, schedWfId, res := s.createSchedule("--memo", "bar=1") s.NoError(res.Err)