From 21c6733c1b22089334fb8f602f3dd4d9d4760e15 Mon Sep 17 00:00:00 2001
From: gillespi314 <73313222+gillespi314@users.noreply.github.com>
Date: Fri, 3 Mar 2023 12:14:10 -0600
Subject: [PATCH] Release schedule lock when triggered run spans schedule
 interval (#10240)

---
 changes/bugfix-trigger-release-lock      |  1 +
 server/service/schedule/schedule.go      |  1 -
 server/service/schedule/schedule_test.go | 63 ++++++++++++++++++++++++
 server/service/schedule/testing_utils.go |  1 +
 4 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100644 changes/bugfix-trigger-release-lock

diff --git a/changes/bugfix-trigger-release-lock b/changes/bugfix-trigger-release-lock
new file mode 100644
index 0000000000..3260663486
--- /dev/null
+++ b/changes/bugfix-trigger-release-lock
@@ -0,0 +1 @@
+- Fixed a bug where `fleetctl trigger` doesn't release the schedule lock when the triggered run spans the regularly scheduled interval. This can prevent a second Fleet instance from using `fleetctl trigger` until the lock expires. This issue occurs infrequently under normal use. When it does occur, it resolves on its own in time; however, it may last up one full interval.
diff --git a/server/service/schedule/schedule.go b/server/service/schedule/schedule.go
index 00d205469a..63138b02aa 100644
--- a/server/service/schedule/schedule.go
+++ b/server/service/schedule/schedule.go
@@ -225,7 +225,6 @@ func (s *Schedule) Start() {
 					s.setIntervalStartedAt(newStart)
 					schedTicker.Reset(s.getRemainingInterval(newStart))
 					level.Debug(s.logger).Log("waiting", fmt.Sprintf("triggered run spanned schedule interval, new wait %v", s.getRemainingInterval(newStart)))
-					continue
 				}
 
 				cancelHold()
diff --git a/server/service/schedule/schedule_test.go b/server/service/schedule/schedule_test.go
index c1b05e2023..f620265a35 100644
--- a/server/service/schedule/schedule_test.go
+++ b/server/service/schedule/schedule_test.go
@@ -446,6 +446,69 @@ func TestScheduleHoldLock(t *testing.T) {
 	}
 }
 
+func TestTriggerReleaseLock(t *testing.T) {
+	ctx, cancelFn := context.WithCancel(context.Background())
+	defer cancelFn()
+
+	name := "test_trigger_release_lock"
+	instanceID := "test_instance"
+	schedInterval := 2 * time.Second
+	jobRuntime := 2200 * time.Millisecond
+
+	locker := SetupMockLocker(name, instanceID, time.Now().Truncate(1*time.Second))
+	err := locker.AddChannels(t, "unlocked")
+	require.NoError(t, err)
+	seedStats := fleet.CronStats{
+		ID:        1,
+		StatsType: fleet.CronStatsTypeScheduled,
+		Name:      name,
+		Instance:  instanceID,
+		CreatedAt: time.Now().Truncate(1 * time.Second),
+		UpdatedAt: time.Now().Truncate(1 * time.Second),
+
+		Status: fleet.CronStatsStatusCompleted,
+	}
+	statsStore := SetUpMockStatsStore(name, seedStats)
+
+	jobsRun := uint32(0)
+	s := New(
+		ctx, name, instanceID, schedInterval, locker, statsStore,
+		WithJob("test_job", func(ctx context.Context) error {
+			time.Sleep(jobRuntime)
+			atomic.AddUint32(&jobsRun, 1)
+			return nil
+		}),
+	)
+	s.Start()
+
+	<-time.After(1 * time.Second)
+	_, err = s.Trigger()
+	require.NoError(t, err)
+
+	select {
+	case <-time.After(4 * schedInterval):
+		t.Errorf("timeout")
+		t.FailNow()
+	case <-locker.Unlocked:
+		stats, err := statsStore.GetLatestCronStats(ctx, name)
+		require.NoError(t, err)
+		require.Len(t, stats, 2)
+
+		statsByType := make(map[fleet.CronStatsType]fleet.CronStats)
+		for _, s := range stats {
+			statsByType[s.StatsType] = s
+		}
+		require.Len(t, statsByType, 2)
+		require.Contains(t, statsByType, fleet.CronStatsTypeTriggered)
+		require.Contains(t, statsByType, fleet.CronStatsTypeScheduled)
+
+		require.Equal(t, fleet.CronStatsStatusCompleted, statsByType[fleet.CronStatsTypeTriggered].Status)
+		require.Equal(t, seedStats, statsByType[fleet.CronStatsTypeScheduled])
+	}
+
+	require.True(t, locker.expiresAt.Before(time.Now()))
+}
+
 func TestMultipleScheduleInstancesConfigChangesDS(t *testing.T) {
 	ctx, cancelFunc := context.WithCancel(context.Background())
 	defer cancelFunc()
diff --git a/server/service/schedule/testing_utils.go b/server/service/schedule/testing_utils.go
index 59f9e21ae2..059408555d 100644
--- a/server/service/schedule/testing_utils.go
+++ b/server/service/schedule/testing_utils.go
@@ -88,6 +88,7 @@ func (ml *MockLock) Unlock(ctx context.Context, name string, owner string) error
 	if ml.Unlocked != nil {
 		ml.Unlocked <- struct{}{}
 	}
+	ml.expiresAt = time.Now()
 	return nil
 }