From e0700728b8f9efef05e48b32f748976a0ccad202 Mon Sep 17 00:00:00 2001 From: Carlo <1778532+cdcme@users.noreply.github.com> Date: Mon, 23 Feb 2026 08:48:53 -0500 Subject: [PATCH] Add retries for software installs (#39827) Fixes #34068 Adds automatic retries (up to 3 attempts) for failed software installs from host details, self-service, and setup experience across all installer types. --- changes/34068-software-install-retries | 1 + ee/server/service/software_installers.go | 11 + ee/server/service/software_installers_test.go | 8 + server/datastore/mysql/activities.go | 21 +- server/datastore/mysql/software_installers.go | 66 ++++- server/datastore/mysql/software_test.go | 97 ++++++ server/datastore/mysql/vpp.go | 3 +- server/fleet/datastore.go | 3 + server/fleet/software_installer.go | 9 + server/mock/datastore_mock.go | 12 + server/service/apple_mdm.go | 33 +-- server/service/integration_enterprise_test.go | 278 ++++++++++++++++++ server/service/integration_mdm_test.go | 19 +- .../service/integration_vpp_install_test.go | 134 +++++---- server/service/orbit.go | 91 +++++- server/service/orbit_test.go | 176 +++++++++++ 16 files changed, 874 insertions(+), 88 deletions(-) create mode 100644 changes/34068-software-install-retries diff --git a/changes/34068-software-install-retries b/changes/34068-software-install-retries new file mode 100644 index 0000000000..6270fc949b --- /dev/null +++ b/changes/34068-software-install-retries @@ -0,0 +1 @@ +- Added automatic retries for failed software, excluding VPP app installs. diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index 9658aaa288..ce4225ed5e 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -1443,8 +1443,14 @@ func (svc *Service) installSoftwareTitleUsingInstaller(ctx context.Context, host } } + // Reset old attempts so the new install starts fresh at attempt 1. + if err := svc.ds.ResetNonPolicyInstallAttempts(ctx, host.ID, installer.InstallerID); err != nil { + return ctxerr.Wrap(ctx, err, "reset install attempts before new install") + } + _, err := svc.ds.InsertSoftwareInstallRequest(ctx, host.ID, installer.InstallerID, fleet.HostSoftwareInstallOptions{ SelfService: false, + WithRetries: true, }) return ctxerr.Wrap(ctx, err, "inserting software install request") } @@ -2702,8 +2708,13 @@ func (svc *Service) SelfServiceInstallSoftwareTitle(ctx context.Context, host *f } } + if err := svc.ds.ResetNonPolicyInstallAttempts(ctx, host.ID, installer.InstallerID); err != nil { + return ctxerr.Wrap(ctx, err, "reset install attempts before self-service install") + } + _, err = svc.ds.InsertSoftwareInstallRequest(ctx, host.ID, installer.InstallerID, fleet.HostSoftwareInstallOptions{ SelfService: true, + WithRetries: true, }) return ctxerr.Wrap(ctx, err, "inserting self-service software install request") } diff --git a/ee/server/service/software_installers_test.go b/ee/server/service/software_installers_test.go index 97f66cbf74..d6388705b5 100644 --- a/ee/server/service/software_installers_test.go +++ b/ee/server/service/software_installers_test.go @@ -128,6 +128,9 @@ func TestInstallUninstallAuth(t *testing.T) { ds.GetHostLastInstallDataFunc = func(ctx context.Context, hostID uint, installerID uint) (*fleet.HostLastInstallData, error) { return nil, nil } + ds.ResetNonPolicyInstallAttemptsFunc = func(ctx context.Context, hostID uint, softwareInstallerID uint) error { + return nil + } ds.InsertSoftwareInstallRequestFunc = func(ctx context.Context, hostID uint, softwareInstallerID uint, opts fleet.HostSoftwareInstallOptions) (string, error, ) { @@ -667,6 +670,11 @@ func TestInstallShScriptOnDarwin(t *testing.T) { return nil, nil } + // Reset retry attempts (no-op for test) + ds.ResetNonPolicyInstallAttemptsFunc = func(ctx context.Context, hostID uint, softwareInstallerID uint) error { + return nil + } + // Capture that install request was inserted ds.InsertSoftwareInstallRequestFunc = func(ctx context.Context, hostID uint, softwareInstallerID uint, opts fleet.HostSoftwareInstallOptions) (string, error) { return "install-uuid", nil diff --git a/server/datastore/mysql/activities.go b/server/datastore/mysql/activities.go index d6dfbb5754..8de6763017 100644 --- a/server/datastore/mysql/activities.go +++ b/server/datastore/mysql/activities.go @@ -1263,7 +1263,7 @@ func (ds *Datastore) activateNextSoftwareInstallActivity(ctx context.Context, tx const insStmt = ` INSERT INTO host_software_installs (execution_id, host_id, software_installer_id, user_id, self_service, - policy_id, installer_filename, version, software_title_id, software_title_name) + policy_id, installer_filename, version, software_title_id, software_title_name, attempt_number) SELECT ua.execution_id, ua.host_id, @@ -1274,7 +1274,24 @@ SELECT COALESCE(si.filename, ua.payload->>'$.installer_filename', '[deleted installer]'), COALESCE(si.version, ua.payload->>'$.version', 'unknown'), COALESCE(si.title_id, siua.software_title_id), - COALESCE(st.name, ua.payload->>'$.software_title_name', '[deleted title]') + COALESCE(st.name, ua.payload->>'$.software_title_name', '[deleted title]'), + -- Compute the attempt number for this activation. Each retry creates a + -- new upcoming_activity (via InsertSoftwareInstallRequest), so when that + -- new activity activates, COUNT(*) of previous completed attempts gives + -- the number of prior tries. +1 makes this the next attempt in sequence: + -- first install = 1, first retry = 2, second retry = 3, etc. + CASE + WHEN siua.policy_id IS NULL AND COALESCE(ua.payload->'$.with_retries', 0) = 1 THEN ( + SELECT COUNT(*) + 1 + FROM host_software_installs hsi2 + WHERE hsi2.host_id = ua.host_id + AND hsi2.software_installer_id = siua.software_installer_id + AND hsi2.policy_id IS NULL + AND hsi2.removed = 0 AND hsi2.canceled = 0 AND hsi2.host_deleted_at IS NULL + AND (hsi2.attempt_number > 0 OR hsi2.attempt_number IS NULL) + ) + ELSE NULL + END FROM upcoming_activities ua INNER JOIN software_install_upcoming_activities siua diff --git a/server/datastore/mysql/software_installers.go b/server/datastore/mysql/software_installers.go index 633f8c469c..88bef321ab 100644 --- a/server/datastore/mysql/software_installers.go +++ b/server/datastore/mysql/software_installers.go @@ -767,6 +767,66 @@ func (ds *Datastore) resetInstallerPolicyAutomationAttempts(ctx context.Context, return nil } +// ResetNonPolicyInstallAttempts resets all attempt numbers for non-policy +// software installer executions for a host and cancels any pending retry +// installs. This is called before user-initiated installs to ensure a fresh +// retry sequence and to allow the new install to proceed without being +// blocked by a pending retry. +// +// Cancellation uses cancelHostUpcomingActivity to handle edge cases like +// marking setup experience entries as failed and activating the next +// upcoming activity. +func (ds *Datastore) ResetNonPolicyInstallAttempts(ctx context.Context, hostID, softwareInstallerID uint) error { + return ds.withTx(ctx, func(tx sqlx.ExtContext) error { + // Reset attempt_number for old installs. Setting attempt_number to 0 + // marks these records as superseded by a new install request. The + // filters (attempt_number > 0 OR attempt_number IS NULL) throughout + // the codebase skip these records when counting attempts. + _, err := tx.ExecContext(ctx, ` + UPDATE host_software_installs + SET attempt_number = 0 + WHERE host_id = ? + AND software_installer_id = ? + AND policy_id IS NULL + AND (attempt_number > 0 OR attempt_number IS NULL) + `, hostID, softwareInstallerID) + if err != nil { + return ctxerr.Wrap(ctx, err, "reset non-policy install attempts") + } + + // Find activated pending retry installs to cancel. Only cancel + // activities that have been activated (activated_at IS NOT NULL); + // non-activated upcoming activities should be left unchanged. + var executionIDs []string + if err := sqlx.SelectContext(ctx, tx, &executionIDs, ` + SELECT ua.execution_id + FROM upcoming_activities ua + INNER JOIN software_install_upcoming_activities siua + ON ua.id = siua.upcoming_activity_id + WHERE ua.host_id = ? + AND siua.software_installer_id = ? + AND siua.policy_id IS NULL + AND ua.activity_type = 'software_install' + AND ua.activated_at IS NOT NULL + `, hostID, softwareInstallerID); err != nil { + return ctxerr.Wrap(ctx, err, "query pending non-policy install retries") + } + + // Use cancelHostUpcomingActivity for each pending retry. This + // handles setup experience cleanup, marks host_software_installs + // as canceled, and activates the next upcoming activity. + // The returned ActivityDetails is discarded because this is an + // internal reset for a new install, not a user-initiated cancel. + for _, execID := range executionIDs { + if _, err := ds.cancelHostUpcomingActivity(ctx, tx, hostID, execID); err != nil { + return ctxerr.Wrap(ctx, err, "cancel pending non-policy install retry") + } + } + + return nil + }) +} + func (ds *Datastore) ValidateOrbitSoftwareInstallerAccess(ctx context.Context, hostID uint, installerID uint) (bool, error) { // NOTE: this is ok to only look in host_software_installs (and ignore // upcoming_activities), because orbit should not be able to get the @@ -1188,6 +1248,7 @@ VALUES 'version', ?, 'software_title_name', ?, 'source', ?, + 'with_retries', ?, 'user', (SELECT JSON_OBJECT('name', name, 'email', email, 'gravatar_url', gravatar_url) FROM users WHERE id = ?) ) )` @@ -1228,7 +1289,9 @@ VALUES } var userID *uint - if ctxUser := authz.UserFromContext(ctx); ctxUser != nil && opts.PolicyID == nil { + if opts.UserID != nil { + userID = opts.UserID + } else if ctxUser := authz.UserFromContext(ctx); ctxUser != nil && opts.PolicyID == nil { userID = &ctxUser.ID } execID := uuid.NewString() @@ -1245,6 +1308,7 @@ VALUES installerDetails.Version, installerDetails.TitleName, installerDetails.Source, + opts.WithRetries, userID, ) if err != nil { diff --git a/server/datastore/mysql/software_test.go b/server/datastore/mysql/software_test.go index 78baed09b1..d51f006094 100644 --- a/server/datastore/mysql/software_test.go +++ b/server/datastore/mysql/software_test.go @@ -110,6 +110,7 @@ func TestSoftware(t *testing.T) { {"ListHostSoftwareInHouseApps", testListHostSoftwareInHouseApps}, {"ListHostSoftwareAndroidVPPAppMatching", testListHostSoftwareAndroidVPPAppMatching}, {"CountHostSoftwareInstallAttempts", testCountHostSoftwareInstallAttempts}, + {"ResetNonPolicyInstallAttempts", testResetNonPolicyInstallAttempts}, {"ListSoftwareVersionsSearchByTitleName", testListSoftwareVersionsSearchByTitleName}, {"ListSoftwareInventoryDeletedHost", testListSoftwareInventoryDeletedHost}, {"ListHostSoftwareShPackageForDarwin", testListHostSoftwareShPackageForDarwin}, @@ -11172,6 +11173,102 @@ func testCountHostSoftwareInstallAttempts(t *testing.T, ds *Datastore) { require.Equal(t, 0, count) } +func testResetNonPolicyInstallAttempts(t *testing.T, ds *Datastore) { + ctx := context.Background() + + host := test.NewHost(t, ds, "host-reset-1", "10.0.0.20", "hostReset1Key", "hostReset1UUID", time.Now()) + user := test.NewUser(t, ds, "ResetUser", "reset@example.com", true) + + // Create a software installer + tfr, err := fleet.NewTempFileReader(strings.NewReader("reset content"), t.TempDir) + require.NoError(t, err) + installerID, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + InstallScript: "echo installing", + InstallerFile: tfr, + StorageID: "storage-reset-1", + Filename: "reset-installer.pkg", + Title: "ResetSoftware", + Version: "1.0", + Source: "apps", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + }) + require.NoError(t, err) + + // Insert non-policy installs with attempt numbers + install1UUID, err := ds.InsertSoftwareInstallRequest(ctx, host.ID, installerID, fleet.HostSoftwareInstallOptions{}) + require.NoError(t, err) + _, err = ds.SetHostSoftwareInstallResult(ctx, &fleet.HostSoftwareInstallResultPayload{ + HostID: host.ID, + InstallUUID: install1UUID, + InstallScriptExitCode: ptr.Int(1), + InstallScriptOutput: ptr.String("failed"), + }, ptr.Int(1)) + require.NoError(t, err) + + install2UUID, err := ds.InsertSoftwareInstallRequest(ctx, host.ID, installerID, fleet.HostSoftwareInstallOptions{}) + require.NoError(t, err) + _, err = ds.SetHostSoftwareInstallResult(ctx, &fleet.HostSoftwareInstallResultPayload{ + HostID: host.ID, + InstallUUID: install2UUID, + InstallScriptExitCode: ptr.Int(1), + InstallScriptOutput: ptr.String("failed again"), + }, ptr.Int(2)) + require.NoError(t, err) + + // Also insert a policy install to ensure it's NOT reset + policy, err := ds.NewGlobalPolicy(ctx, &user.ID, fleet.PolicyPayload{ + Name: "reset-test-policy", + Query: "SELECT 1;", + }) + require.NoError(t, err) + + install3UUID, err := ds.InsertSoftwareInstallRequest(ctx, host.ID, installerID, fleet.HostSoftwareInstallOptions{ + PolicyID: &policy.ID, + }) + require.NoError(t, err) + _, err = ds.SetHostSoftwareInstallResult(ctx, &fleet.HostSoftwareInstallResultPayload{ + HostID: host.ID, + InstallUUID: install3UUID, + InstallScriptExitCode: ptr.Int(1), + InstallScriptOutput: ptr.String("policy install failed"), + }, ptr.Int(1)) + require.NoError(t, err) + + // Helper to count non-policy install attempts (where attempt_number > 0 or IS NULL) + countNonPolicyAttempts := func() int { + var count int + err := sqlx.GetContext(ctx, ds.reader(ctx), &count, ` + SELECT COUNT(*) FROM host_software_installs + WHERE host_id = ? AND software_installer_id = ? AND policy_id IS NULL + AND removed = 0 AND canceled = 0 AND host_deleted_at IS NULL + AND (attempt_number > 0 OR attempt_number IS NULL) + `, host.ID, installerID) + require.NoError(t, err) + return count + } + + // Verify non-policy count before reset + require.Equal(t, 2, countNonPolicyAttempts()) + + // Verify policy count before reset + policyCount, err := ds.CountHostSoftwareInstallAttempts(ctx, host.ID, installerID, policy.ID) + require.NoError(t, err) + require.Equal(t, 1, policyCount) + + // Reset non-policy attempts + err = ds.ResetNonPolicyInstallAttempts(ctx, host.ID, installerID) + require.NoError(t, err) + + // Non-policy count should be 0 (all reset to attempt_number=0) + require.Equal(t, 0, countNonPolicyAttempts()) + + // Policy count should be unchanged + policyCount, err = ds.CountHostSoftwareInstallAttempts(ctx, host.ID, installerID, policy.ID) + require.NoError(t, err) + require.Equal(t, 1, policyCount) +} + // testListSoftwareVersionsSearchByTitleName tests that searching software versions // by a software title name finds all software entries under that title, even when // individual software entry names differ from the title name. diff --git a/server/datastore/mysql/vpp.go b/server/datastore/mysql/vpp.go index a0b8877af2..6558f4fab1 100644 --- a/server/datastore/mysql/vpp.go +++ b/server/datastore/mysql/vpp.go @@ -1991,7 +1991,8 @@ SELECT ncr.updated_at AS ack_at, ncr.status AS install_command_status, va.bundle_identifier AS bundle_identifier, - va.latest_version AS expected_version + va.latest_version AS expected_version, + hvsi.retry_count AS retry_count FROM nano_command_results ncr JOIN host_vpp_software_installs hvsi ON hvsi.command_uuid = ncr.command_uuid JOIN vpp_apps va ON va.adam_id = hvsi.adam_id AND va.platform = hvsi.platform diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index c8660f51e7..333aa9c031 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -906,6 +906,9 @@ type Datastore interface { // CountHostSoftwareInstallAttempts counts how many install attempts exist for a specific // host, software installer, and policy combination. Used to calculate attempt_number. CountHostSoftwareInstallAttempts(ctx context.Context, hostID, softwareInstallerID, policyID uint) (int, error) + // ResetNonPolicyInstallAttempts resets the attempt_number for all non-policy install attempts + // for a given host and software installer so that a new install starts fresh. + ResetNonPolicyInstallAttempts(ctx context.Context, hostID, softwareInstallerID uint) error // CountHostScriptAttempts counts how many script execution attempts exist for a specific // host, script, and policy combination. Used to calculate attempt_number. CountHostScriptAttempts(ctx context.Context, hostID, scriptID, policyID uint) (int, error) diff --git a/server/fleet/software_installer.go b/server/fleet/software_installer.go index 0d642110c1..150640edc5 100644 --- a/server/fleet/software_installer.go +++ b/server/fleet/software_installer.go @@ -1070,6 +1070,9 @@ type SoftwareScopeLabel struct { TitleID uint `db:"title_id" json:"-"` // not rendered in JSON, used to store the associated title ID (may be the empty value in some cases) } +// Max total attempts (including initial) for a non-policy software install. +const MaxSoftwareInstallAttempts = 3 + // HostSoftwareInstallOptions contains options that apply to a software or VPP // app install request. type HostSoftwareInstallOptions struct { @@ -1079,6 +1082,12 @@ type HostSoftwareInstallOptions struct { // ForScheduledUpdates means the install request is for iOS/iPadOS // scheduled updates, which means it was Fleet-initiated. ForScheduledUpdates bool + // UserID is an explicit user ID for retries (overrides context user when set). + UserID *uint + // WithRetries indicates the install should be retried on failure (up to + // MaxSoftwareInstallAttempts total). Set by host details, self-service, + // and setup experience install paths. + WithRetries bool } // IsFleetInitiated returns true if the software install is initiated by Fleet. diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index af49214fe3..fcd77dd051 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -703,6 +703,8 @@ type IsPolicyFailingFunc func(ctx context.Context, policyID uint, hostID uint) ( type CountHostSoftwareInstallAttemptsFunc func(ctx context.Context, hostID uint, softwareInstallerID uint, policyID uint) (int, error) +type ResetNonPolicyInstallAttemptsFunc func(ctx context.Context, hostID uint, softwareInstallerID uint) error + type CountHostScriptAttemptsFunc func(ctx context.Context, hostID uint, scriptID uint, policyID uint) (int, error) type IncrementPolicyViolationDaysFunc func(ctx context.Context) error @@ -2796,6 +2798,9 @@ type DataStore struct { CountHostSoftwareInstallAttemptsFunc CountHostSoftwareInstallAttemptsFunc CountHostSoftwareInstallAttemptsFuncInvoked bool + ResetNonPolicyInstallAttemptsFunc ResetNonPolicyInstallAttemptsFunc + ResetNonPolicyInstallAttemptsFuncInvoked bool + CountHostScriptAttemptsFunc CountHostScriptAttemptsFunc CountHostScriptAttemptsFuncInvoked bool @@ -6787,6 +6792,13 @@ func (s *DataStore) CountHostSoftwareInstallAttempts(ctx context.Context, hostID return s.CountHostSoftwareInstallAttemptsFunc(ctx, hostID, softwareInstallerID, policyID) } +func (s *DataStore) ResetNonPolicyInstallAttempts(ctx context.Context, hostID uint, softwareInstallerID uint) error { + s.mu.Lock() + s.ResetNonPolicyInstallAttemptsFuncInvoked = true + s.mu.Unlock() + return s.ResetNonPolicyInstallAttemptsFunc(ctx, hostID, softwareInstallerID) +} + func (s *DataStore) CountHostScriptAttempts(ctx context.Context, hostID uint, scriptID uint, policyID uint) (int, error) { s.mu.Lock() s.CountHostScriptAttemptsFuncInvoked = true diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index cc34ae54dc..ffd81ec6ea 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -3776,25 +3776,22 @@ func (svc *MDMAppleCheckinAndCommandService) CommandAndReportResults(r *mdm.Requ if cmdResult.Status == fleet.MDMAppleStatusError || cmdResult.Status == fleet.MDMAppleStatusCommandFormatError { - for _, errorChain := range cmdResult.ErrorChain { - if errorChain.ErrorCode != apple_mdm.VPPLicenseNotFound { - // We only want to retry on license not found errors - continue - } - - // Fetch the host vpp install info - vppInstall, err := svc.ds.GetHostVPPInstallByCommandUUID(r.Context, cmdResult.CommandUUID) - if err != nil { - return nil, ctxerr.Wrap(r.Context, err, "fetching host vpp install by command uuid") - } - if vppInstall.RetryCount < 3 { - // Requeue the app for installation - if err := svc.ds.RetryVPPInstall(r.Context, vppInstall); err != nil { - return nil, ctxerr.Wrap(r.Context, err, "retrying VPP install for host") - } - level.Info(svc.logger).Log("msg", "re-queued VPP app installation due to missing license", "host_id", vppInstall.HostID, "command_uuid", cmdResult.CommandUUID, "retry_count", vppInstall.RetryCount+1) - return nil, nil + // Retry VPP install on any MDM error (up to MaxSoftwareInstallAttempts). + // N.b., VPP uses 0-based retry_count, so this comparison gives + // MaxSoftwareInstallAttempts retries (not attempts). This pre-dates + // the non-policy retry feature and is intentionally left as-is. + vppInstall, err := svc.ds.GetHostVPPInstallByCommandUUID(r.Context, cmdResult.CommandUUID) + if err != nil { + return nil, ctxerr.Wrap(r.Context, err, "fetching host vpp install by command uuid") + } + if vppInstall != nil && vppInstall.RetryCount < fleet.MaxSoftwareInstallAttempts { + if err := svc.ds.RetryVPPInstall(r.Context, vppInstall); err != nil { + return nil, ctxerr.Wrap(r.Context, err, "retrying VPP install for host") } + level.Info(svc.logger).Log("msg", "re-queued VPP app installation", + "host_id", vppInstall.HostID, "command_uuid", cmdResult.CommandUUID, + "retry_count", vppInstall.RetryCount+1, "error_status", cmdResult.Status) + return nil, nil } // this might be a setup experience VPP install, so we'll try to update setup experience status diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 1a28844297..9b92dd8af9 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -14522,6 +14522,26 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerHostRequests() { s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", h3.ID), nil, http.StatusOK, &hostRespFailed) require.False(t, hostRespFailed.Host.RefetchRequested, "RefetchRequested should be false after failed software install") + // Exhaust automatic retries for h3 so it reaches terminal "failed" state. + // Server-side retries queue up to MaxSoftwareInstallAttempts attempts. + for attempt := 2; attempt <= fleet.MaxSoftwareInstallAttempts; attempt++ { + getHostSoftwareResp = getHostSoftwareResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", h3.ID), nil, http.StatusOK, &getHostSoftwareResp) + require.Len(t, getHostSoftwareResp.Software, 1) + require.NotNil(t, getHostSoftwareResp.Software[0].SoftwarePackage) + require.NotNil(t, getHostSoftwareResp.Software[0].SoftwarePackage.LastInstall) + retryUUID := getHostSoftwareResp.Software[0].SoftwarePackage.LastInstall.InstallUUID + require.NotEqual(t, installUUID3, retryUUID, "retry should have a new install UUID (attempt %d)", attempt) + s.Do("POST", "/api/fleet/orbit/software_install/result", json.RawMessage(fmt.Sprintf(`{ + "orbit_node_key": %q, + "install_uuid": %q, + "pre_install_condition_output": "ok", + "install_script_exit_code": 1, + "install_script_output": "retry %d failed" + }`, *h3.OrbitNodeKey, retryUUID, attempt)), http.StatusNoContent) + installUUID3 = retryUUID + } + s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/%d/install", h4.ID, titleID), nil, http.StatusAccepted, &resp) s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", h4.ID), nil, http.StatusOK, &getHostSoftwareResp) require.Len(t, getHostSoftwareResp.Software, 1) @@ -14537,6 +14557,23 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerHostRequests() { s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", h4.ID), nil, http.StatusOK, &hostRespPreInstallFailed) require.False(t, hostRespPreInstallFailed.Host.RefetchRequested, "RefetchRequested should be false after failed pre-install condition") + // Exhaust automatic retries for h4 so it reaches terminal "failed" state. + for attempt := 2; attempt <= fleet.MaxSoftwareInstallAttempts; attempt++ { + getHostSoftwareResp = getHostSoftwareResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", h4.ID), nil, http.StatusOK, &getHostSoftwareResp) + require.Len(t, getHostSoftwareResp.Software, 1) + require.NotNil(t, getHostSoftwareResp.Software[0].SoftwarePackage) + require.NotNil(t, getHostSoftwareResp.Software[0].SoftwarePackage.LastInstall) + retryUUID := getHostSoftwareResp.Software[0].SoftwarePackage.LastInstall.InstallUUID + require.NotEqual(t, installUUID4a, retryUUID, "retry should have a new install UUID (attempt %d)", attempt) + s.Do("POST", "/api/fleet/orbit/software_install/result", json.RawMessage(fmt.Sprintf(`{ + "orbit_node_key": %q, + "install_uuid": %q, + "pre_install_condition_output": "" + }`, *h4.OrbitNodeKey, retryUUID)), http.StatusNoContent) + installUUID4a = retryUUID + } + s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/%d/install", h4.ID, titleID), nil, http.StatusAccepted, &resp) s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", h4.ID), nil, http.StatusOK, &getHostSoftwareResp) require.Len(t, getHostSoftwareResp.Software, 1) @@ -17923,6 +17960,247 @@ func (s *integrationEnterpriseTestSuite) TestPolicyAutomationSoftwareInstallRetr require.Equal(t, 1, attemptCounts.NullCount, "should have exactly 1 row with attempt_number IS NULL (pending)") } +func (s *integrationEnterpriseTestSuite) TestNonPolicySoftwareInstallRetries() { + t := s.T() + ctx := context.Background() + + team, err := s.ds.NewTeam(ctx, &fleet.Team{Name: t.Name()}) + require.NoError(t, err) + + host, err := s.ds.NewHost(ctx, &fleet.Host{ + DetailUpdatedAt: time.Now(), + LabelUpdatedAt: time.Now(), + PolicyUpdatedAt: time.Now(), + SeenTime: time.Now().Add(-1 * time.Minute), + OsqueryHostID: ptr.String(t.Name()), + NodeKey: ptr.String(t.Name()), + UUID: uuid.New().String(), + Hostname: fmt.Sprintf("%s.local", t.Name()), + Platform: "darwin", + TeamID: &team.ID, + }) + require.NoError(t, err) + orbitKey := setOrbitEnrollment(t, host, s.ds) + host.OrbitNodeKey = &orbitKey + + pkgPayload := &fleet.UploadSoftwareInstallerPayload{ + InstallScript: "install script", + Filename: "dummy_installer.pkg", + TeamID: &team.ID, + } + s.uploadSoftwareInstaller(t, pkgPayload, http.StatusOK, "") + + var resp listSoftwareTitlesResponse + s.DoJSON("GET", "/api/latest/fleet/software/titles", listSoftwareTitlesRequest{}, + http.StatusOK, &resp, "query", "DummyApp", "team_id", fmt.Sprintf("%d", team.ID)) + require.Len(t, resp.SoftwareTitles, 1) + require.NotNil(t, resp.SoftwareTitles[0].SoftwarePackage) + softwareTitleID := resp.SoftwareTitles[0].ID + + var installerID uint + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, &installerID, + `SELECT id FROM software_installers WHERE global_or_team_id = ? AND filename = ?`, + team.ID, "dummy_installer.pkg") + }) + require.NotZero(t, installerID) + + getPendingInstall := func() *fleet.HostSoftwareInstallerResult { + var results []*fleet.HostSoftwareInstallerResult + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.SelectContext(ctx, q, &results, ` + SELECT + id, + execution_id, + host_id, + software_installer_id, + self_service, + install_script_exit_code, + attempt_number + FROM host_software_installs + WHERE host_id = ? AND install_script_exit_code IS NULL AND policy_id IS NULL + ORDER BY id ASC + `, host.ID) + }) + if len(results) == 0 { + return nil + } + return results[0] + } + submitInstallResult := func(installUUID string, exitCode int) { + s.Do("POST", "/api/fleet/orbit/software_install/result", json.RawMessage( + fmt.Sprintf(`{ + "orbit_node_key": %q, + "install_uuid": %q, + "install_script_exit_code": %d, + "install_script_output": "install output" + }`, *host.OrbitNodeKey, installUUID, exitCode), + ), http.StatusNoContent) + } + getInstallResults := func() []*fleet.HostSoftwareInstallerResult { + var results []*fleet.HostSoftwareInstallerResult + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.SelectContext(ctx, q, &results, ` + SELECT id, execution_id, host_id, software_installer_id, self_service, + install_script_exit_code, attempt_number + FROM host_software_installs + WHERE host_id = ? AND software_installer_id = ? AND policy_id IS NULL + ORDER BY id ASC + `, host.ID, installerID) + }) + return results + } + countActivities := func() int { + var count int + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, &count, ` + SELECT COUNT(*) + FROM activities + WHERE activity_type = 'installed_software' + AND JSON_EXTRACT(details, '$.host_id') = ? + AND JSON_EXTRACT(details, '$.status') = 'failed_install' + `, host.ID) + }) + return count + } + + // Trigger install from host details (admin-initiated, non-policy) + s.Do("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/%d/install", host.ID, softwareTitleID), + nil, http.StatusAccepted) + + // Wait for install to be queued + require.EventuallyWithT(t, func(t *assert.CollectT) { + pendingInstall := getPendingInstall() + assert.NotNil(t, pendingInstall, "install should be queued") + }, 5*time.Second, 100*time.Millisecond) + + // Fail the first attempt + pendingInstall := getPendingInstall() + require.NotNil(t, pendingInstall) + installUUID1 := pendingInstall.InstallUUID + submitInstallResult(installUUID1, 1) + + // Verify attempt 1 and retry queued + require.EventuallyWithT(t, func(t *assert.CollectT) { + results := getInstallResults() + assert.GreaterOrEqual(t, len(results), 2, "should have completed attempt and pending retry") + if len(results) >= 2 { + assert.NotNil(t, results[0].AttemptNumber) + assert.Equal(t, 1, *results[0].AttemptNumber) + assert.NotNil(t, results[0].InstallScriptExitCode) + assert.Nil(t, results[1].InstallScriptExitCode) + } + }, 5*time.Second, 100*time.Millisecond) + + // Activity created for each failure + require.EventuallyWithT(t, func(t *assert.CollectT) { + assert.Equal(t, 1, countActivities(), "activity should be created for attempt 1") + }, 5*time.Second, 100*time.Millisecond) + + // Wait for pending retry + require.EventuallyWithT(t, func(t *assert.CollectT) { + pendingInstall := getPendingInstall() + assert.NotNil(t, pendingInstall) + if pendingInstall != nil { + assert.NotEqual(t, installUUID1, pendingInstall.InstallUUID) + } + }, 5*time.Second, 100*time.Millisecond) + + // Fail attempt 2 + pendingInstall = getPendingInstall() + require.NotNil(t, pendingInstall) + installUUID2 := pendingInstall.InstallUUID + submitInstallResult(installUUID2, 1) + + // Verify attempt 2 and retry queued + require.EventuallyWithT(t, func(t *assert.CollectT) { + results := getInstallResults() + assert.GreaterOrEqual(t, len(results), 3) + if len(results) >= 3 { + assert.NotNil(t, results[1].AttemptNumber) + assert.Equal(t, 2, *results[1].AttemptNumber) + assert.Nil(t, results[2].InstallScriptExitCode) + } + }, 5*time.Second, 100*time.Millisecond) + require.Equal(t, 2, countActivities()) + + // Wait for second retry + require.EventuallyWithT(t, func(t *assert.CollectT) { + pendingInstall := getPendingInstall() + assert.NotNil(t, pendingInstall) + }, 5*time.Second, 100*time.Millisecond) + + // Fail attempt 3 (final) + pendingInstall = getPendingInstall() + require.NotNil(t, pendingInstall) + installUUID3 := pendingInstall.InstallUUID + submitInstallResult(installUUID3, 1) + + // Verify attempt 3 is final — no more retries + results := getInstallResults() + require.Len(t, results, 3, "should have exactly 3 completed attempts") + require.NotNil(t, results[2].AttemptNumber) + require.Equal(t, 3, *results[2].AttemptNumber) + require.NotNil(t, results[2].InstallScriptExitCode) + + // All 3 attempts create activities + require.EventuallyWithT(t, func(t *assert.CollectT) { + assert.Equal(t, 3, countActivities(), "activity should be created for each attempt") + }, 5*time.Second, 100*time.Millisecond) + + // No more retries + time.Sleep(2 * time.Second) + pendingInstall = getPendingInstall() + require.Nil(t, pendingInstall) + + // Verify exactly 3 attempts + results = getInstallResults() + require.Len(t, results, 3) + require.Equal(t, 1, *results[0].AttemptNumber) + require.Equal(t, 2, *results[1].AttemptNumber) + require.Equal(t, 3, *results[2].AttemptNumber) + + // Manual re-install resets retry count + s.Do("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/%d/install", host.ID, softwareTitleID), + nil, http.StatusAccepted) + + require.EventuallyWithT(t, func(t *assert.CollectT) { + pendingInstall := getPendingInstall() + assert.NotNil(t, pendingInstall) + }, 5*time.Second, 100*time.Millisecond) + + // Fail the new first attempt + pendingInstall = getPendingInstall() + require.NotNil(t, pendingInstall) + submitInstallResult(pendingInstall.InstallUUID, 1) + + // Verify retry is queued (fresh sequence starts at attempt 1) + require.EventuallyWithT(t, func(t *assert.CollectT) { + results := getInstallResults() + // Find the last completed result + for i := len(results) - 1; i >= 0; i-- { + if results[i].InstallScriptExitCode != nil { + assert.NotNil(t, results[i].AttemptNumber) + assert.Equal(t, 1, *results[i].AttemptNumber, "fresh sequence should start at attempt 1") + break + } + } + // Should have a pending retry + pendingInstall := getPendingInstall() + assert.NotNil(t, pendingInstall, "retry should be queued for fresh sequence") + }, 5*time.Second, 100*time.Millisecond) + + // Succeed the retry + pendingInstall = getPendingInstall() + require.NotNil(t, pendingInstall) + submitInstallResult(pendingInstall.InstallUUID, 0) // exit code 0 = success + + // No more retries after success + time.Sleep(2 * time.Second) + pendingInstall = getPendingInstall() + require.Nil(t, pendingInstall) +} + func (s *integrationEnterpriseTestSuite) TestPolicyAutomationsSoftwareInstallersLabelScoping() { t := s.T() ctx := context.Background() diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index aa3e87dc5b..b6d2f8eda1 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -13696,8 +13696,12 @@ func (s *integrationMDMTestSuite) TestVPPApps() { fmt.Sprint(team.ID), "software_title_id", fmt.Sprint(errTitleID)) require.Equal(t, 1, countResp.Count) - // Simulate failed installation on the host - errorOnInstallApplicationCommand(1234) + // Simulate failed installation on the host, exhaust retries (MaxSoftwareInstallAttempts = 3) + // First error triggers retry 1, second triggers retry 2, third triggers retry 3, + // fourth exhausts retries and marks as failed. + for range fleet.MaxSoftwareInstallAttempts + 1 { + errorOnInstallApplicationCommand(1234) + } listResp = listHostsResponse{} s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusOK, &listResp, "software_status", "failed", "team_id", fmt.Sprint(team.ID), @@ -19055,6 +19059,17 @@ func (s *integrationMDMTestSuite) TestCancelUpcomingActivity() { "orbit_node_key": %q, "install_uuid": %q, "pre_install_condition_output": "ok", "install_script_exit_code": 1, "install_script_output": "fail" }`, *mdmHost.OrbitNodeKey, hostActivitiesResp.Activities[1].UUID)), http.StatusNoContent) + // Exhaust automatic retries for the failed software install. + // Server-side retries queue up to MaxSoftwareInstallAttempts attempts. + for attempt := 2; attempt <= fleet.MaxSoftwareInstallAttempts; attempt++ { + hostActivitiesResp = listHostUpcomingActivitiesResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/activities/upcoming", mdmHost.ID), nil, http.StatusOK, &hostActivitiesResp) + require.Len(t, hostActivitiesResp.Activities, 1, "should have pending retry (attempt %d)", attempt) + s.Do("POST", "/api/fleet/orbit/software_install/result", json.RawMessage(fmt.Sprintf(`{ + "orbit_node_key": %q, "install_uuid": %q, "pre_install_condition_output": "ok", "install_script_exit_code": 1, "install_script_output": "fail" + }`, *mdmHost.OrbitNodeKey, hostActivitiesResp.Activities[0].UUID)), http.StatusNoContent) + } + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/activities/upcoming", mdmHost.ID), nil, http.StatusOK, &hostActivitiesResp) require.Len(t, hostActivitiesResp.Activities, 0) diff --git a/server/service/integration_vpp_install_test.go b/server/service/integration_vpp_install_test.go index 29be1d92b5..1e2ad20a9d 100644 --- a/server/service/integration_vpp_install_test.go +++ b/server/service/integration_vpp_install_test.go @@ -239,13 +239,35 @@ func (s *integrationMDMTestSuite) TestVPPAppInstallVerification() { processVPPInstallOnClient := func(mdmClient *mdmtest.TestAppleMDMClient, opts vppInstallOpts) string { var installCmdUUID string + app, ok := expectedAppsByBundleID[opts.bundleID] + require.Truef(t, ok, "unexpected bundle ID: %s", opts.bundleID) + + ackInstalledAppList := func(cmd *mdm.Command) (*mdm.Command, error) { + return mdmClient.AcknowledgeInstalledApplicationList( + mdmClient.UUID, + cmd.CommandUUID, + []fleet.Software{ + { + Name: "RandomApp", + BundleIdentifier: "com.example.randomapp", + Version: "9.9.9", + Installed: false, + }, + { + Name: app.Name, + BundleIdentifier: app.BundleIdentifier, + Version: app.LatestVersion, + Installed: opts.appInstallVerified, + }, + }, + ) + } + // Process the InstallApplication command s.runWorker() cmd, err := mdmClient.Idle() require.NoError(t, err) - app, ok := expectedAppsByBundleID[opts.bundleID] - require.Truef(t, ok, "unexpected bundle ID: %s", opts.bundleID) for cmd != nil { var fullCmd micromdm.CommandPayload switch cmd.Command.RequestType { @@ -253,7 +275,6 @@ func (s *integrationMDMTestSuite) TestVPPAppInstallVerification() { require.NoError(t, plist.Unmarshal(cmd.Raw, &fullCmd)) installCmdUUID = cmd.CommandUUID if opts.failOnInstall { - t.Logf("Failed command UUID: %s", installCmdUUID) cmd, err = mdmClient.Err(cmd.CommandUUID, []mdm.ErrorChain{{ErrorCode: 1234}}) require.NoError(t, err) continue @@ -265,24 +286,7 @@ func (s *integrationMDMTestSuite) TestVPPAppInstallVerification() { // If we are polling to verify the install, we should get an // InstalledApplicationList command instead of an InstallApplication command. require.NoError(t, plist.Unmarshal(cmd.Raw, &fullCmd)) - _, err = mdmClient.AcknowledgeInstalledApplicationList( - mdmClient.UUID, - cmd.CommandUUID, - []fleet.Software{ - { - Name: "RandomApp", - BundleIdentifier: "com.example.randomapp", - Version: "9.9.9", - Installed: false, - }, - { - Name: app.Name, - BundleIdentifier: app.BundleIdentifier, - Version: app.LatestVersion, - Installed: opts.appInstallVerified, - }, - }, - ) + _, err = ackInstalledAppList(cmd) require.NoError(t, err) return "" default: @@ -301,38 +305,57 @@ func (s *integrationMDMTestSuite) TestVPPAppInstallVerification() { }) } - // Process the verification command (InstalledApplicationList) - s.runWorker() - // Check that there is now a verify command in flight - checkCommandsInFlight(1) - cmd, err = mdmClient.Idle() - require.NoError(t, err) - for cmd != nil { - var fullCmd micromdm.CommandPayload - switch cmd.Command.RequestType { - case "InstalledApplicationList": - require.NoError(t, plist.Unmarshal(cmd.Raw, &fullCmd)) - cmd, err = mdmClient.AcknowledgeInstalledApplicationList( - mdmClient.UUID, - cmd.CommandUUID, - []fleet.Software{ - { - Name: "RandomApp", - BundleIdentifier: "com.example.randomapp", - Version: "9.9.9", - Installed: false, - }, - { - Name: app.Name, - BundleIdentifier: app.BundleIdentifier, - Version: app.LatestVersion, - Installed: opts.appInstallVerified, - }, - }, - ) + // Process the verification command (InstalledApplicationList). + // When verification times out and retries are available, the handler + // re-enqueues InstallApplication. We loop through the full retry cycle + // until retries are exhausted or install succeeds. + for attempt := range fleet.MaxSoftwareInstallAttempts + 1 { + s.runWorker() + if attempt == 0 { + checkCommandsInFlight(1) + } + cmd, err = mdmClient.Idle() + require.NoError(t, err) + for cmd != nil { + var fullCmd micromdm.CommandPayload + switch cmd.Command.RequestType { + case "InstalledApplicationList": + require.NoError(t, plist.Unmarshal(cmd.Raw, &fullCmd)) + cmd, err = ackInstalledAppList(cmd) + require.NoError(t, err) + default: + require.Fail(t, "unexpected MDM command on client", cmd.Command.RequestType) + } + } + + if !opts.appInstallTimeout { + break + } + + // After acking the InstalledApplicationList, the handler runs and + // may retry (enqueue a new InstallApplication). Check for it. + cmd, err = mdmClient.Idle() + require.NoError(t, err) + if cmd == nil { + // No retry — retries exhausted or install verified + break + } + + require.Equal(t, "InstallApplication", cmd.Command.RequestType) + installCmdUUID = cmd.CommandUUID + cmd, err = mdmClient.Acknowledge(cmd.CommandUUID) + require.NoError(t, err) + // Backdate the ack for the next verify timeout + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(context.Background(), "UPDATE nano_command_results SET updated_at = ? WHERE command_uuid = ?", time.Now().Add(-11*time.Minute), installCmdUUID) + return err + }) + // The Acknowledge response may include the InstalledApplicationList + // command (sent by the server after acking InstallApplication). + // Drain it — the outer loop's Idle() will re-fetch it. + for cmd != nil { + cmd, err = mdmClient.NotNow(cmd.CommandUUID) require.NoError(t, err) - default: - require.Fail(t, "unexpected MDM command on client", cmd.Command.RequestType) } } @@ -375,14 +398,17 @@ func (s *integrationMDMTestSuite) TestVPPAppInstallVerification() { fmt.Sprint(team.ID), "software_title_id", fmt.Sprint(errTitleID)) require.Equal(t, 1, countResp.Count) - // Simulate failed installation on the host + // Simulate failed installation on the host — exhaust retries (MaxSoftwareInstallAttempts = 3) opts := vppInstallOpts{ failOnInstall: true, appInstallVerified: false, appInstallTimeout: false, bundleID: addedApp.BundleIdentifier, } - failedCmdUUID := processVPPInstallOnClient(mdmDevice, opts) + var failedCmdUUID string + for range fleet.MaxSoftwareInstallAttempts + 1 { + failedCmdUUID = processVPPInstallOnClient(mdmDevice, opts) + } // We should have cleared out upcoming_activies since the install failed mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { diff --git a/server/service/orbit.go b/server/service/orbit.go index fee1ac7bd4..681c89f2ce 100644 --- a/server/service/orbit.go +++ b/server/service/orbit.go @@ -1531,14 +1531,24 @@ func (svc *Service) SaveHostSoftwareInstallResult(ctx context.Context, result *f return nil } - // Calculate attempt_number for policy automation retries by counting existing attempts - attemptNumber, err := svc.getPolicyAutomationSoftwareInstallerAttemptNumber(ctx, host, result.InstallUUID) + // Calculate attempt_number for retries by counting existing attempts + attemptNumber, err := svc.getSoftwareInstallerAttemptNumber(ctx, host, result.InstallUUID) if err != nil { return err } + // Check if a non-policy install failure will be retried so we can skip + // updating setup experience status during intermediate retries. + willRetryNonPolicyOnFailure := false + if attemptNumber != nil && *attemptNumber < fleet.MaxSoftwareInstallAttempts && result.Status() == fleet.SoftwareInstallFailed { + currentInstall, checkErr := svc.ds.GetSoftwareInstallResults(ctx, result.InstallUUID) + if checkErr == nil && currentInstall != nil && currentInstall.PolicyID == nil { + willRetryNonPolicyOnFailure = true + } + } + var fromSetupExperience bool - if fleet.IsSetupExperienceSupported(host.Platform) { + if fleet.IsSetupExperienceSupported(host.Platform) && !willRetryNonPolicyOnFailure { // This might be a setup experience software install result, so we attempt to update the // "Setup experience" status for that item. hostUUID, err := fleet.HostUUIDForSetupExperience(host) @@ -1624,6 +1634,33 @@ func (svc *Service) SaveHostSoftwareInstallResult(ctx context.Context, result *f } } + // Non-policy install retry (host details, self-service, setup experience). + // Errors here are logged but do not abort the handler. The primary + // action, recording the install result from the device, must succeed + // regardless of whether a retry can be scheduled. If retry scheduling + // fails, the install is marked as failed (no retry) and the admin can + // manually re-trigger. + if hsi.PolicyID == nil && status == fleet.SoftwareInstallFailed { + shouldRetry, retryErr := svc.shouldRetrySoftwareInstall(ctx, hsi) + if retryErr != nil { + level.Error(svc.logger).Log( + "msg", "failed to check if software install should retry", + "host_id", host.ID, + "install_uuid", result.InstallUUID, + "err", retryErr, + ) + } else if shouldRetry { + if retryErr := svc.retrySoftwareInstall(ctx, host, hsi, fromSetupExperience); retryErr != nil { + level.Error(svc.logger).Log( + "msg", "failed to queue software install retry", + "host_id", host.ID, + "install_uuid", result.InstallUUID, + "err", retryErr, + ) + } + } + } + if shouldCreateActivity { if err := svc.NewActivity( ctx, @@ -1694,6 +1731,32 @@ func (svc *Service) retryPolicyAutomationSoftwareInstall(ctx context.Context, ho return err } +// shouldRetrySoftwareInstall checks if a failed non-policy software install should be retried. +func (svc *Service) shouldRetrySoftwareInstall(ctx context.Context, hsi *fleet.HostSoftwareInstallerResult) (bool, error) { + if hsi.AttemptNumber == nil { + return false, nil + } + return *hsi.AttemptNumber < fleet.MaxSoftwareInstallAttempts, nil +} + +// retrySoftwareInstall queues a retry for a non-policy software install. +func (svc *Service) retrySoftwareInstall(ctx context.Context, host *fleet.Host, hsi *fleet.HostSoftwareInstallerResult, fromSetupExperience bool) error { + level.Info(svc.logger).Log( + "msg", "queuing software install retry", + "host_id", host.ID, + "software_installer_id", *hsi.SoftwareInstallerID, + "self_service", hsi.SelfService, + "current_attempt", *hsi.AttemptNumber, + ) + _, err := svc.ds.InsertSoftwareInstallRequest(ctx, host.ID, *hsi.SoftwareInstallerID, fleet.HostSoftwareInstallOptions{ + SelfService: hsi.SelfService, + UserID: hsi.UserID, + ForSetupExperience: fromSetupExperience, + WithRetries: true, + }) + return err +} + // shouldRetryPolicyAutomationScript checks if a failed policy automation script should be retried. // Returns true if retry should be queued func (svc *Service) shouldRetryPolicyAutomationScript(ctx context.Context, host *fleet.Host, hsr *fleet.HostScriptResult) (bool, error) { @@ -1757,24 +1820,32 @@ func (svc *Service) getPolicyAutomationScriptAttemptNumber(ctx context.Context, return nil, nil // nil for manual runs } -// getPolicyAutomationSoftwareInstallerAttemptNumber calculates the attempt number for a policy automation software install. -// Returns nil for manual/self-service installs (not triggered by policy automation). -func (svc *Service) getPolicyAutomationSoftwareInstallerAttemptNumber(ctx context.Context, host *fleet.Host, installUUID string) (*int, error) { +// getSoftwareInstallerAttemptNumber calculates the attempt number for a software install. +// Returns nil for installs that don't have a software_installer_id. +func (svc *Service) getSoftwareInstallerAttemptNumber(ctx context.Context, host *fleet.Host, installUUID string) (*int, error) { currentInstall, err := svc.ds.GetSoftwareInstallResults(ctx, installUUID) if err != nil && !fleet.IsNotFound(err) { return nil, ctxerr.Wrap(ctx, err, "get current install info for attempt number calculation") } - // Only calculate attempt_number for policy automation installs - if currentInstall != nil && currentInstall.PolicyID != nil && currentInstall.SoftwareInstallerID != nil { + if currentInstall == nil || currentInstall.SoftwareInstallerID == nil { + return nil, nil + } + + // Policy automation installs + if currentInstall.PolicyID != nil { count, err := svc.ds.CountHostSoftwareInstallAttempts(ctx, host.ID, *currentInstall.SoftwareInstallerID, *currentInstall.PolicyID) if err != nil { - return nil, ctxerr.Wrap(ctx, err, "count previous install attempts") + return nil, ctxerr.Wrap(ctx, err, "count previous policy install attempts") } return &count, nil } - return nil, nil // nil for manual/self-service installs + // Non-policy installs (host details, self-service, setup experience): + // attempt_number is set at activation time for retry-eligible installs + // (those created with WithRetries=true). If nil, this install was not + // created with retry support. + return currentInstall.AttemptNumber, nil } ///////////////////////////////////////////////////////////////////////////////// diff --git a/server/service/orbit_test.go b/server/service/orbit_test.go index 5055fdee34..2075c8d212 100644 --- a/server/service/orbit_test.go +++ b/server/service/orbit_test.go @@ -13,6 +13,7 @@ import ( "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/mdm" "github.com/fleetdm/fleet/v4/server/mock" + logging "github.com/fleetdm/fleet/v4/server/platform/logging" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/fleetdm/fleet/v4/server/test" "github.com/stretchr/testify/require" @@ -696,3 +697,178 @@ func TestGetSoftwareInstallDetails(t *testing.T) { require.Nil(t, d2) }) } + +func TestShouldRetrySoftwareInstall(t *testing.T) { + svc := &Service{ + logger: logging.NewNopLogger(), + } + ctx := context.Background() + + t.Run("nil attempt number returns false", func(t *testing.T) { + hsi := &fleet.HostSoftwareInstallerResult{ + AttemptNumber: nil, + } + shouldRetry, err := svc.shouldRetrySoftwareInstall(ctx, hsi) + require.NoError(t, err) + require.False(t, shouldRetry) + }) + + t.Run("attempt below max returns true", func(t *testing.T) { + for _, attempt := range []int{1, 2} { + hsi := &fleet.HostSoftwareInstallerResult{ + AttemptNumber: ptr.Int(attempt), + } + shouldRetry, err := svc.shouldRetrySoftwareInstall(ctx, hsi) + require.NoError(t, err) + require.True(t, shouldRetry, "attempt %d should retry", attempt) + } + }) + + t.Run("attempt at max returns false", func(t *testing.T) { + hsi := &fleet.HostSoftwareInstallerResult{ + AttemptNumber: ptr.Int(fleet.MaxSoftwareInstallAttempts), + } + shouldRetry, err := svc.shouldRetrySoftwareInstall(ctx, hsi) + require.NoError(t, err) + require.False(t, shouldRetry) + }) + + t.Run("attempt above max returns false", func(t *testing.T) { + hsi := &fleet.HostSoftwareInstallerResult{ + AttemptNumber: ptr.Int(fleet.MaxSoftwareInstallAttempts + 1), + } + shouldRetry, err := svc.shouldRetrySoftwareInstall(ctx, hsi) + require.NoError(t, err) + require.False(t, shouldRetry) + }) +} + +func TestRetrySoftwareInstall(t *testing.T) { + ds := new(mock.Store) + svc := &Service{ + ds: ds, + logger: logging.NewNopLogger(), + } + ctx := context.Background() + + installerID := uint(42) + userID := uint(7) + host := &fleet.Host{ID: 1} + hsi := &fleet.HostSoftwareInstallerResult{ + SoftwareInstallerID: &installerID, + SelfService: true, + UserID: &userID, + AttemptNumber: ptr.Int(1), + } + + var capturedOpts fleet.HostSoftwareInstallOptions + ds.InsertSoftwareInstallRequestFunc = func(ctx context.Context, hostID uint, softwareInstallerID uint, opts fleet.HostSoftwareInstallOptions) (string, error) { + require.Equal(t, host.ID, hostID) + require.Equal(t, installerID, softwareInstallerID) + capturedOpts = opts + return "new-uuid", nil + } + + t.Run("preserves self-service and user ID", func(t *testing.T) { + err := svc.retrySoftwareInstall(ctx, host, hsi, false) + require.NoError(t, err) + require.True(t, ds.InsertSoftwareInstallRequestFuncInvoked) + require.True(t, capturedOpts.SelfService) + require.NotNil(t, capturedOpts.UserID) + require.Equal(t, userID, *capturedOpts.UserID) + require.False(t, capturedOpts.ForSetupExperience) + require.True(t, capturedOpts.WithRetries) + }) + + t.Run("passes setup experience flag", func(t *testing.T) { + ds.InsertSoftwareInstallRequestFuncInvoked = false + err := svc.retrySoftwareInstall(ctx, host, hsi, true) + require.NoError(t, err) + require.True(t, ds.InsertSoftwareInstallRequestFuncInvoked) + require.True(t, capturedOpts.ForSetupExperience) + }) +} + +func TestGetSoftwareInstallerAttemptNumber(t *testing.T) { + ds := new(mock.Store) + svc := &Service{ + ds: ds, + logger: logging.NewNopLogger(), + } + ctx := context.Background() + host := &fleet.Host{ID: 1} + + t.Run("returns nil when install not found", func(t *testing.T) { + ds.GetSoftwareInstallResultsFunc = func(ctx context.Context, installUUID string) (*fleet.HostSoftwareInstallerResult, error) { + return nil, newNotFoundError() + } + result, err := svc.getSoftwareInstallerAttemptNumber(ctx, host, "uuid-1") + require.NoError(t, err) + require.Nil(t, result) + }) + + t.Run("returns nil when software installer ID is nil", func(t *testing.T) { + ds.GetSoftwareInstallResultsFunc = func(ctx context.Context, installUUID string) (*fleet.HostSoftwareInstallerResult, error) { + return &fleet.HostSoftwareInstallerResult{SoftwareInstallerID: nil}, nil + } + result, err := svc.getSoftwareInstallerAttemptNumber(ctx, host, "uuid-1") + require.NoError(t, err) + require.Nil(t, result) + }) + + t.Run("counts policy install attempts", func(t *testing.T) { + policyID := uint(10) + installerID := uint(20) + ds.GetSoftwareInstallResultsFunc = func(ctx context.Context, installUUID string) (*fleet.HostSoftwareInstallerResult, error) { + return &fleet.HostSoftwareInstallerResult{ + SoftwareInstallerID: &installerID, + PolicyID: &policyID, + }, nil + } + ds.CountHostSoftwareInstallAttemptsFunc = func(ctx context.Context, hostID, siID, polID uint) (int, error) { + require.Equal(t, host.ID, hostID) + require.Equal(t, installerID, siID) + require.Equal(t, policyID, polID) + return 2, nil + } + result, err := svc.getSoftwareInstallerAttemptNumber(ctx, host, "uuid-1") + require.NoError(t, err) + require.NotNil(t, result) + require.Equal(t, 2, *result) + require.True(t, ds.CountHostSoftwareInstallAttemptsFuncInvoked) + }) + + t.Run("returns attempt number from install for non-policy retry-eligible install", func(t *testing.T) { + installerID := uint(20) + attemptNum := 2 + ds.GetSoftwareInstallResultsFunc = func(ctx context.Context, installUUID string) (*fleet.HostSoftwareInstallerResult, error) { + return &fleet.HostSoftwareInstallerResult{ + SoftwareInstallerID: &installerID, + PolicyID: nil, // non-policy install + AttemptNumber: &attemptNum, + }, nil + } + ds.CountHostSoftwareInstallAttemptsFuncInvoked = false + result, err := svc.getSoftwareInstallerAttemptNumber(ctx, host, "uuid-1") + require.NoError(t, err) + require.NotNil(t, result) + require.Equal(t, 2, *result) + require.False(t, ds.CountHostSoftwareInstallAttemptsFuncInvoked) + }) + + t.Run("returns nil for non-policy install without retry support", func(t *testing.T) { + installerID := uint(20) + ds.GetSoftwareInstallResultsFunc = func(ctx context.Context, installUUID string) (*fleet.HostSoftwareInstallerResult, error) { + return &fleet.HostSoftwareInstallerResult{ + SoftwareInstallerID: &installerID, + PolicyID: nil, // non-policy install + AttemptNumber: nil, // not created with WithRetries + }, nil + } + ds.CountHostSoftwareInstallAttemptsFuncInvoked = false + result, err := svc.getSoftwareInstallerAttemptNumber(ctx, host, "uuid-1") + require.NoError(t, err) + require.Nil(t, result) + require.False(t, ds.CountHostSoftwareInstallAttemptsFuncInvoked) + }) +}