Skip to content

Update how deleted hosts show in batch scripts#32290

Merged
sgress454 merged 1 commit intomainfrom
sgress454/32231-handle-deleted-hosts
Aug 27, 2025
Merged

Update how deleted hosts show in batch scripts#32290
sgress454 merged 1 commit intomainfrom
sgress454/32231-handle-deleted-hosts

Conversation

@sgress454
Copy link
Copy Markdown
Contributor

@sgress454 sgress454 commented Aug 25, 2025

for #32231

Details

This PR adjusts the queries for listing batch scripts slightly to count every row in batch_activities matching the filters, regardless of whether any batch_activity_host_results rows exist for it. This handles the edge case of a batch script where all the hosts have been deleted.

Checklist for submitter

If some of the following don't apply, delete the relevant line.

  • Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements)

Testing

  • Added/updated automated tests
    I didn't add tests for this because these tests have already changed quite a bit in Add "batch script host results" API #32174. I can add tests in there when this merges.

  • QA'd all new/changed functionality manually

  • Select a host in Manage Hosts, click Run Script, select a script and do Run Now
  • Delete that host
  • Go to the batch scripts list (Controls -> Scripts -> Batch Progress)
  • Verify that the batch script is still listed.

We don't have clear expectations for what numbers should be displayed for the progress of a batch like this, but this PR at least ensures the batch doesn't disappear.

For unreleased bug fixes in a release candidate, one of:

  • Confirmed that the fix is not expected to adversely impact load test results

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.01%. Comparing base (bf0f74f) to head (f387494).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #32290   +/-   ##
=======================================
  Coverage   64.01%   64.01%           
=======================================
  Files        1985     1985           
  Lines      194371   194378    +7     
  Branches     6462     6462           
=======================================
+ Hits       124424   124434   +10     
+ Misses      60236    60234    -2     
+ Partials     9711     9710    -1     
Flag Coverage Δ
backend 65.28% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@getvictor getvictor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AndreyKizimenko
Copy link
Copy Markdown
Member

@sgress454 once merged, this one will also need to be cherry-picked :)

@sgress454 sgress454 merged commit c920007 into main Aug 27, 2025
39 checks passed
@sgress454 sgress454 deleted the sgress454/32231-handle-deleted-hosts branch August 27, 2025 19:10
sgress454 added a commit that referenced this pull request Aug 27, 2025
for #32231

# Details

This PR adjusts the queries for listing batch scripts slightly to count
_every_ row in `batch_activities` matching the filters, regardless of
whether any `batch_activity_host_results` rows exist for it. This
handles the edge case of a batch script where all the hosts have been
deleted.

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

- [X] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)

## Testing

- [ ] Added/updated automated tests
I didn't add tests for this because these tests have already changed
quite a bit in #32174. I can add
tests in there when this merges.

- [X] QA'd all new/changed functionality manually

* Select a host in Manage Hosts, click Run Script, select a script and
do Run Now
* Delete that host
* Go to the batch scripts list (Controls -> Scripts -> Batch Progress)
* Verify that the batch script is still listed.

We don't have clear expectations for what numbers should be displayed
for the progress of a batch like this, but this PR at least ensures the
batch doesn't disappear.

For unreleased bug fixes in a release candidate, one of:

- [X] Confirmed that the fix is not expected to adversely impact load
test results
sgress454 added a commit that referenced this pull request Aug 27, 2025
> # Cherry pick from main to rc 4.73.0

for #32231

# Details

This PR adjusts the queries for listing batch scripts slightly to count
_every_ row in `batch_activities` matching the filters, regardless of
whether any `batch_activity_host_results` rows exist for it. This
handles the edge case of a batch script where all the hosts have been
deleted.

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

- [X] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)

## Testing

- [ ] Added/updated automated tests I didn't add tests for this because
these tests have already changed quite a bit in
#32174. I can add tests in there
when this merges.

- [X] QA'd all new/changed functionality manually

* Select a host in Manage Hosts, click Run Script, select a script and
do Run Now
* Delete that host
* Go to the batch scripts list (Controls -> Scripts -> Batch Progress)
* Verify that the batch script is still listed.

We don't have clear expectations for what numbers should be displayed
for the progress of a batch like this, but this PR at least ensures the
batch doesn't disappear.

For unreleased bug fixes in a release candidate, one of:

- [X] Confirmed that the fix is not expected to adversely impact load
test results

# 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.

- [ ] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [ ] If paths of existing endpoints are modified without backwards
compatibility, checked the frontend/CLI for any necessary changes

## Testing

- [ ] Added/updated automated tests
- [ ] 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)

- [ ] QA'd all new/changed functionality manually

For unreleased bug fixes in a release candidate, one of:

- [ ] Confirmed that the fix is not expected to adversely impact load
test results
- [ ] Alerted the release DRI if additional load testing is needed

## Database migrations

- [ ] Checked table schema to confirm autoupdate
- [ ] Checked schema for all modified table for columns that will
auto-update timestamps during migration.
- [ ] Confirmed that updating the timestamps is acceptable, and will not
cause unwanted side effects.
- [ ] Ensured the correct collation is explicitly set for character
columns (`COLLATE utf8mb4_unicode_ci`).

## New Fleet configuration settings

- [ ] Setting(s) is/are explicitly excluded from GitOps

If you didn't check the box above, follow this checklist for
GitOps-enabled settings:

- [ ] Verified that the setting is exported via `fleetctl
generate-gitops`
- [ ] Verified the setting is documented in a separate PR to [the GitOps
documentation](https://github.com/fleetdm/fleet/blob/main/docs/Configuration/yaml-files.md#L485)
- [ ] Verified that the setting is cleared on the server if it is not
supplied in a YAML file (or that it is documented as being optional)
- [ ] Verified that any relevant UI is disabled when GitOps mode is
enabled

## fleetd/orbit/Fleet Desktop

- [ ] Verified compatibility with the latest released version of Fleet
(see [Must
rule](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/workflows/fleetd-development-and-release-strategy.md))
- [ ] If the change applies to only one platform, confirmed that
`runtime.GOOS` is used as needed to isolate changes
- [ ] Verified that fleetd runs on macOS, Linux and Windows
- [ ] Verified auto-update works from the released version of component
to the new version (see [tools/tuf/test](../tools/tuf/test/README.md))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants