Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 18 Packages Projects Releases Wiki Activity

Fix false-success reporting for failed software installs (#49515)

Browse Source 
**Related issue:** Resolves #49475

Makes a non-zero install-script exit code a terminal failure so an
install that failed but whose post-install script exited 0 is no longer
reported as installed, in both the Go status computation and the
`host_software_installs` `status`/`execution_status` generated columns.

# Checklist for submitter

- [x] Changes file added for user-visible changes in `changes/`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.

## Testing

- [x] Added/updated automated tests
- [x] QA'd all new/changed functionality manually

## Database migrations

- [x] Checked schema for all modified table for columns that will
auto-update timestamps during migration. Redefining the
`status`/`execution_status` generated columns rebuilds the table, but
`ON UPDATE CURRENT_TIMESTAMP` is not triggered by `ALTER TABLE`, so
`updated_at` is preserved.
- [x] Confirmed that updating the timestamps is acceptable, and will not
cause unwanted side effects.
- [x] Ensured the correct collation is explicitly set for character
columns (`COLLATE utf8mb4_unicode_ci`).


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Installations that fail during the install script are now correctly
reported as failed, even if the post-install script succeeds.
* Install and execution status reporting is now consistent about which
script exit code takes precedence.
* Pending, successful, failed, canceled, and uninstall outcomes continue
to be reported correctly.
* **Tests**
* Added regression/unit test coverage for install-status and
execution-status precedence across mixed install/post-install exit code
scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Carlo
2026-07-17 17:59:14 -04:00
committed by GitHub
parent 13224b0660
commit 8cd9503267
6 changed files with 201 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changes/49475-failed-installs-reported-successful
+1
View File
@@ -0,0 +1 @@
- Fixed a bug where a failed software install was reported as successfully installed when the install script exited with an error but a post-install script exited successfully.
server/datastore/mysql/migrations/tables/20260717152653_FixInstallStatusPrecedence.go
+71
View File
@@ -0,0 +1,71 @@
package tables
import (
"database/sql"
"fmt"
)
func init() {
MigrationClient.AddMigration(Up_20260717152653, Down_20260717152653)
}
// Up_20260717152653 corrects the precedence of the generated status and
// execution_status columns on host_software_installs. Previously a post-install
// script that exited 0 reported the install as "installed" even when the install
// script itself exited non-zero. Because fleetd runs the post-install script
// regardless of the install script's outcome, that masked failed installs as
// successful. A non-zero install-script exit code is now terminal (failed_install)
// and evaluated before the post-install script result.
//
// Both columns are changed in one ALTER TABLE: each ALTER TABLE implicitly
// commits, so separate statements could leave status on the new definition while
// execution_status kept the old one if the second failed. A single statement also
// rebuilds the table once rather than twice.
func Up_20260717152653(tx *sql.Tx) error {
if _, err := tx.Exec(`
ALTER TABLE host_software_installs
MODIFY COLUMN ` + "`status`" + ` ENUM('pending_install','failed_install','installed','pending_uninstall','failed_uninstall','canceled_install','canceled_uninstall')
GENERATED ALWAYS AS (
CASE
WHEN removed = 1 THEN NULL
WHEN canceled = 1 AND uninstall = 0 THEN 'canceled_install'
WHEN canceled = 1 AND uninstall = 1 THEN 'canceled_uninstall'
WHEN install_script_exit_code IS NOT NULL AND install_script_exit_code != 0 THEN 'failed_install'
WHEN post_install_script_exit_code IS NOT NULL AND post_install_script_exit_code = 0 THEN 'installed'
WHEN post_install_script_exit_code IS NOT NULL AND post_install_script_exit_code != 0 THEN 'failed_install'
WHEN install_script_exit_code IS NOT NULL AND install_script_exit_code = 0 THEN 'installed'
WHEN pre_install_query_output IS NOT NULL AND pre_install_query_output = '' THEN 'failed_install'
WHEN host_id IS NOT NULL AND uninstall = 0 THEN 'pending_install'
WHEN uninstall_script_exit_code IS NOT NULL AND uninstall_script_exit_code != 0 THEN 'failed_uninstall'
WHEN uninstall_script_exit_code IS NOT NULL AND uninstall_script_exit_code = 0 THEN NULL
WHEN host_id IS NOT NULL AND uninstall = 1 THEN 'pending_uninstall'
ELSE NULL
END
) STORED,
MODIFY COLUMN ` + "`execution_status`" + ` ENUM('pending_install','failed_install','installed','pending_uninstall','failed_uninstall','canceled_install','canceled_uninstall')
GENERATED ALWAYS AS (
CASE
WHEN canceled = 1 AND uninstall = 0 THEN 'canceled_install'
WHEN canceled = 1 AND uninstall = 1 THEN 'canceled_uninstall'
WHEN install_script_exit_code IS NOT NULL AND install_script_exit_code != 0 THEN 'failed_install'
WHEN post_install_script_exit_code IS NOT NULL AND post_install_script_exit_code = 0 THEN 'installed'
WHEN post_install_script_exit_code IS NOT NULL AND post_install_script_exit_code != 0 THEN 'failed_install'
WHEN install_script_exit_code IS NOT NULL AND install_script_exit_code = 0 THEN 'installed'
WHEN pre_install_query_output IS NOT NULL AND pre_install_query_output = '' THEN 'failed_install'
WHEN host_id IS NOT NULL AND uninstall = 0 THEN 'pending_install'
WHEN uninstall_script_exit_code IS NOT NULL AND uninstall_script_exit_code != 0 THEN 'failed_uninstall'
WHEN uninstall_script_exit_code IS NOT NULL AND uninstall_script_exit_code = 0 THEN NULL
WHEN host_id IS NOT NULL AND uninstall = 1 THEN 'pending_uninstall'
ELSE NULL
END
) VIRTUAL
`); err != nil {
return fmt.Errorf("fixing install status precedence generated columns: %w", err)
}
return nil
}
func Down_20260717152653(tx *sql.Tx) error {
return nil
}
server/datastore/mysql/migrations/tables/20260717152653_FixInstallStatusPrecedence_test.go
+60
View File
@@ -0,0 +1,60 @@
package tables
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestUp_20260717152653(t *testing.T) {
db := applyUpToPrev(t)
// A failed install: the install script exited non-zero, but the post-install
// script (which fleetd runs regardless) exited 0.
const failedExecID = "failed-install-post-success"
_, err := db.Exec(`
INSERT INTO host_software_installs
(execution_id, host_id, install_script_exit_code, post_install_script_exit_code)
VALUES (?, 1, 1, 0)`, failedExecID)
require.NoError(t, err)
// A genuine success: both the install and post-install scripts exited 0.
const successExecID = "install-and-post-success"
_, err = db.Exec(`
INSERT INTO host_software_installs
(execution_id, host_id, install_script_exit_code, post_install_script_exit_code)
VALUES (?, 1, 0, 0)`, successExecID)
require.NoError(t, err)
// Before the migration, the buggy precedence reports the failed install as installed.
var before string
require.NoError(t, db.QueryRow(`SELECT status FROM host_software_installs WHERE execution_id = ?`, failedExecID).Scan(&before))
require.Equal(t, "installed", before)
applyNext(t, db)
// After the migration, a non-zero install-script exit code is terminal for both
// generated columns, regardless of the post-install script result. The STORED
// column is recomputed for the existing row by the table rebuild.
assertStatus := func(execID, wantStatus, wantExecStatus string) {
t.Helper()
var status, execStatus string
require.NoError(t, db.QueryRow(`SELECT status FROM host_software_installs WHERE execution_id = ?`, execID).Scan(&status))
require.NoError(t, db.QueryRow(`SELECT execution_status FROM host_software_installs WHERE execution_id = ?`, execID).Scan(&execStatus))
require.Equal(t, wantStatus, status)
require.Equal(t, wantExecStatus, execStatus)
}
assertStatus(failedExecID, "failed_install", "failed_install")
// Regression: a genuine success is still reported as installed.
assertStatus(successExecID, "installed", "installed")
// Regression: install succeeded but post-install failed is still a failure.
const postFailedExecID = "install-success-post-failed"
_, err = db.Exec(`
INSERT INTO host_software_installs
(execution_id, host_id, install_script_exit_code, post_install_script_exit_code)
VALUES (?, 1, 0, 1)`, postFailedExecID)
require.NoError(t, err)
assertStatus(postFailedExecID, "failed_install", "failed_install")
}
server/datastore/mysql/schema.sql
+4 -4
View File
File diff suppressed because one or more lines are too long
server/fleet/software_installer.go
+8 -5
View File
@@ -1142,19 +1142,22 @@ type HostSoftwareInstallResultPayload struct {
RetriesRemaining uint `json:"retries_remaining,omitempty"`
}
// Status returns the status computed from the result payload. It should match the logic
// found in the database-computed status (see
// softwareInstallerHostStatusNamedQuery in mysql/software.go).
// Status returns the status computed from the result payload. It must match the
// precedence of the database-computed status and execution_status generated
// columns on host_software_installs (see schema.sql). A non-zero install-script
// exit code is a terminal failure: the post-install script runs regardless of
// the install script's outcome, so its exit code must not be allowed to report a
// failed install as installed.
func (h *HostSoftwareInstallResultPayload) Status() SoftwareInstallerStatus {
switch {
case h.InstallScriptExitCode != nil && *h.InstallScriptExitCode != 0:
return SoftwareInstallFailed
case h.PostInstallScriptExitCode != nil && *h.PostInstallScriptExitCode == 0:
return SoftwareInstalled
case h.PostInstallScriptExitCode != nil && *h.PostInstallScriptExitCode != 0:
return SoftwareInstallFailed
case h.InstallScriptExitCode != nil && *h.InstallScriptExitCode == 0:
return SoftwareInstalled
case h.InstallScriptExitCode != nil && *h.InstallScriptExitCode != 0:
return SoftwareInstallFailed
case h.PreInstallConditionOutput != nil && *h.PreInstallConditionOutput == "":
return SoftwareInstallFailed
default:
server/fleet/software_installer_test.go
+57
View File
@@ -341,3 +341,60 @@ func TestIconChangesDedupPrefersPopulatedRow(t *testing.T) {
require.Empty(t, changes.IconsToUpload)
require.Empty(t, changes.IconsToUpdate)
}
func TestHostSoftwareInstallResultPayloadStatus(t *testing.T) {
cases := []struct {
name string
payload HostSoftwareInstallResultPayload
want SoftwareInstallerStatus
}{
{
// fleetd runs the post-install script regardless of the install
// script's outcome, so a succeeding post-install must not mask a
// failed install.
name: "install failed, post-install succeeded",
payload: HostSoftwareInstallResultPayload{InstallScriptExitCode: new(1), PostInstallScriptExitCode: new(0)},
want: SoftwareInstallFailed,
},
{
name: "install failed, no post-install",
payload: HostSoftwareInstallResultPayload{InstallScriptExitCode: new(1)},
want: SoftwareInstallFailed,
},
{
name: "install and post-install succeeded",
payload: HostSoftwareInstallResultPayload{InstallScriptExitCode: new(0), PostInstallScriptExitCode: new(0)},
want: SoftwareInstalled,
},
{
name: "install succeeded, post-install failed",
payload: HostSoftwareInstallResultPayload{InstallScriptExitCode: new(0), PostInstallScriptExitCode: new(1)},
want: SoftwareInstallFailed,
},
{
name: "install succeeded, no post-install",
payload: HostSoftwareInstallResultPayload{InstallScriptExitCode: new(0)},
want: SoftwareInstalled,
},
{
name: "scripts disabled is a failure",
payload: HostSoftwareInstallResultPayload{InstallScriptExitCode: new(ExitCodeScriptsDisabled)},
want: SoftwareInstallFailed,
},
{
name: "empty pre-install condition is a failure",
payload: HostSoftwareInstallResultPayload{PreInstallConditionOutput: new("")},
want: SoftwareInstallFailed,
},
{
name: "nothing reported yet is pending",
payload: HostSoftwareInstallResultPayload{},
want: SoftwareInstallPending,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
require.Equal(t, tc.want, tc.payload.Status())
})
}
}