From 37470fede3b663f4ec697505b7e8f3c2e2d3c156 Mon Sep 17 00:00:00 2001 From: Scott Gress Date: Fri, 12 Sep 2025 17:48:48 -0500 Subject: [PATCH] Fix marking canceled batch scripts as finished (#32715) for #32711 # Details This PR fixes an issue where batch scripts that are canceled after starting may sometimes continue to be displayed in the "started" list rather than moving to "finished". # Checklist for submitter If some of the following don't apply, delete the relevant line. - [ ] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [X] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [X] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [X] Added/updated automated tests Added new test which fails without the fix, and passes with it. - [X] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [X] QA'd all new/changed functionality manually --- changes/32560-improve-batch-script-sorting | 1 + server/datastore/mysql/scripts.go | 2 +- server/datastore/mysql/scripts_test.go | 28 +++++++++++++++++++++- 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 changes/32560-improve-batch-script-sorting diff --git a/changes/32560-improve-batch-script-sorting b/changes/32560-improve-batch-script-sorting new file mode 100644 index 0000000000..26034127ef --- /dev/null +++ b/changes/32560-improve-batch-script-sorting @@ -0,0 +1 @@ +- Improved the sorting of batch scripts in the Batch Progress UI. Batches in the "started" state now sort by started date, and batches in the "finished" state now sort by the finished date. diff --git a/server/datastore/mysql/scripts.go b/server/datastore/mysql/scripts.go index 772d6bc0ab..8206cd3f53 100644 --- a/server/datastore/mysql/scripts.go +++ b/server/datastore/mysql/scripts.go @@ -2492,7 +2492,7 @@ JOIN ( COUNT(bahr.error) AS num_incompatible, COUNT(IF(hsr.exit_code = 0, 1, NULL)) AS num_ran, COUNT(IF(hsr.exit_code > 0, 1, NULL)) AS num_errored, - COUNT(IF(hsr.canceled = 1 AND hsr.exit_code IS NULL, 1, NULL)) AS num_canceled + COUNT(IF((hsr.canceled = 1 AND hsr.exit_code IS NULL) OR (hsr.host_id IS NULL AND bahr.error is NULL AND ba2.canceled = 1), 1, NULL)) AS num_canceled FROM batch_activities AS ba2 LEFT JOIN batch_activity_host_results AS bahr ON ba2.execution_id = bahr.batch_execution_id diff --git a/server/datastore/mysql/scripts_test.go b/server/datastore/mysql/scripts_test.go index c6bb828937..cee9ee3b4e 100644 --- a/server/datastore/mysql/scripts_test.go +++ b/server/datastore/mysql/scripts_test.go @@ -2458,6 +2458,32 @@ func testMarkActivitiesAsCompleted(t *testing.T, ds *Datastore) { require.NoError(t, err) require.Equal(t, fleet.ScheduledBatchExecutionStarted, batchActivity2.Status) + // Schedule another batch that we will cancel. + execID3, err := ds.BatchExecuteScript(ctx, &user.ID, script.ID, []uint{hostNoScripts.ID, hostWindows.ID, host1.ID, host2.ID, host3.ID}) + require.NoError(t, err) + require.NotEmpty(t, execID3) + + // Update the batch activity status to "started" + ExecAdhocSQL(t, ds, func(tx sqlx.ExtContext) error { + _, err := tx.ExecContext(ctx, "UPDATE batch_activities SET status='started' WHERE execution_id = ?", execID3) + return err + }) + + // Cancel the batch. + err = ds.CancelBatchScript(ctx, execID3) + require.NoError(t, err) + + // First activity should be marked as finished and updated accordingly. + batchActivity, err = ds.GetBatchActivity(ctx, execID3) + require.NoError(t, err) + require.Equal(t, fleet.ScheduledBatchExecutionFinished, batchActivity.Status) + require.Equal(t, uint(5), *batchActivity.NumTargeted) + require.Equal(t, uint(0), *batchActivity.NumRan) + require.Equal(t, uint(0), *batchActivity.NumErrored) + require.Equal(t, uint(2), *batchActivity.NumIncompatible) + require.Equal(t, uint(3), *batchActivity.NumCanceled) + require.Equal(t, uint(0), *batchActivity.NumPending) + // Edge case -- batch activity with no hosts. // In reality this could happen if all the hosts in a batch get deleted. ExecAdhocSQL(t, ds, func(tx sqlx.ExtContext) error { @@ -2470,7 +2496,7 @@ func testMarkActivitiesAsCompleted(t *testing.T, ds *Datastore) { err = ds.MarkActivitiesAsCompleted(ctx) require.NoError(t, err) - // First activity should be marked as finished and updated accordingly. + // Activity should be marked as finished and updated accordingly. batchActivity, err = ds.GetBatchActivity(ctx, "abc123") require.NoError(t, err) require.Equal(t, fleet.ScheduledBatchExecutionFinished, batchActivity.Status)