Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear pending (un)installs when installers are deleted #23427

Merged
merged 9 commits into from
Oct 31, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ const DELETE_SW_INSTALLED_DURING_SETUP_ERROR_MSG =
interface IDeleteSoftwareModalProps {
softwareId: number;
teamId: number;
softwarePackageName?: string;
onExit: () => void;
onSuccess: () => void;
}

const DeleteSoftwareModal = ({
softwareId,
teamId,
softwarePackageName,
onExit,
onSuccess,
}: IDeleteSoftwareModalProps) => {
Expand All @@ -51,7 +53,23 @@ const DeleteSoftwareModal = ({
return (
<Modal className={baseClass} title="Delete software" onExit={onExit}>
<>
<p>Software won&apos;t be uninstalled from existing hosts.</p>
<p>
Software won&apos;t be uninstalled from existing hosts, but any
pending pending installs and uninstalls{" "}
{softwarePackageName ? (
<>
for <b> {softwarePackageName}</b>{" "}
</>
) : (
""
)}
will be canceled.
</p>
<p>
Installs or uninstalls currently running on a host will still
complete, but results won’t appear in Fleet.
</p>
<p>You cannot undo this action.</p>
<div className="modal-cta-wrap">
<Button variant="alert" onClick={onDeleteSoftware}>
Delete
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@ const SoftwarePackageCard = ({
{showDeleteModal && (
<DeleteSoftwareModal
softwareId={softwareId}
softwarePackageName={softwarePackage?.name}
teamId={teamId}
onExit={() => setShowDeleteModal(false)}
onSuccess={onDeleteSuccess}
Expand Down
4 changes: 2 additions & 2 deletions server/datastore/mysql/activities.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,12 +261,12 @@ func (ds *Datastore) ListHostUpcomingActivities(ctx context.Context, hostID uint
`SELECT
COUNT(*) c
FROM host_software_installs hsi
WHERE hsi.host_id = :host_id AND
WHERE hsi.host_id = :host_id AND hsi.software_installer_id IS NOT NULL AND
hsi.status = :software_status_install_pending`,
`SELECT
COUNT(*) c
FROM host_software_installs hsi
WHERE hsi.host_id = :host_id AND
WHERE hsi.host_id = :host_id AND hsi.software_installer_id IS NOT NULL AND
Copy link
Member

Choose a reason for hiding this comment

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

This is the actual fix for the bug reported by QA wolf right? Should the other changes be linked to different issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Underlying cause for the QAWolf bug is that we now need to manually clean up pending installs when installers get deleted, rather than the pending installs getting deleted automatically due to the ON DELETE CASCADE that was previously on host_software_installs.software_installer_id. This fix is belt-and-suspenders so the apparent problem goes away in QAWolf's environment without either a migration or a manual DB query, but without the larger cleanup we'd have invisible pending (un)installs when an installer is deleted, and the install part is a regression from v4.58.0.

Copy link
Member Author

Choose a reason for hiding this comment

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

Basically, #23350 exists because the implementation of #21654 was incomplete, so this is unreleased whichever way we slice this.

hsi.status = :software_status_uninstall_pending`,
`
SELECT
Expand Down
23 changes: 22 additions & 1 deletion server/datastore/mysql/activities_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,10 +417,23 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) {
UserID: u.ID,
})
require.NoError(t, err)
sw3, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{
InstallScript: "install to delete",
InstallerFile: installer,
StorageID: uuid.NewString(),
Filename: "todelete.pkg",
Title: "todelete",
Source: "apps",
Version: "0.0.3",
UserID: u.ID,
})
require.NoError(t, err)
sw1Meta, err := ds.GetSoftwareInstallerMetadataByID(ctx, sw1)
require.NoError(t, err)
sw2Meta, err := ds.GetSoftwareInstallerMetadataByID(ctx, sw2)
require.NoError(t, err)
sw3Meta, err := ds.GetSoftwareInstallerMetadataByID(ctx, sw3)
require.NoError(t, err)

// insert a VPP app
vppCommand1, vppCommand2 := "vpp-command-1", "vpp-command-2"
Expand Down Expand Up @@ -523,7 +536,15 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) {
h2SelfService, err := ds.InsertSoftwareInstallRequest(noUserCtx, h2.ID, sw1Meta.InstallerID, true, nil)
require.NoError(t, err)

// nothing for h3
// create pending install and uninstall requests for h3 that will be deleted
_, err = ds.InsertSoftwareInstallRequest(ctx, h3.ID, sw3Meta.InstallerID, false, nil)
require.NoError(t, err)
err = ds.InsertSoftwareUninstallRequest(ctx, "uninstallRun", h3.ID, sw3Meta.InstallerID)
require.NoError(t, err)

// delete installer (should clear pending requests)
err = ds.DeleteSoftwareInstaller(ctx, sw3Meta.InstallerID)
require.NoError(t, err)

// force-set the order of the created_at timestamps
endTime := SetOrderedCreatedAtTimestamps(t, ds, time.Now(), "host_script_results", "execution_id", h1A, h1B)
Expand Down
145 changes: 118 additions & 27 deletions server/datastore/mysql/software_installers.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,38 +414,45 @@ var (
)

func (ds *Datastore) DeleteSoftwareInstaller(ctx context.Context, id uint) error {
// allow delete only if install_during_setup is false
res, err := ds.writer(ctx).ExecContext(ctx, `DELETE FROM software_installers WHERE id = ? AND install_during_setup = 0`, id)
if err != nil {
if isMySQLForeignKey(err) {
// Check if the software installer is referenced by a policy automation.
var count int
if err := sqlx.GetContext(ctx, ds.reader(ctx), &count, `SELECT COUNT(*) FROM policies WHERE software_installer_id = ?`, id); err != nil {
return ctxerr.Wrapf(ctx, err, "getting reference from policies")
}
if count > 0 {
return errDeleteInstallerWithAssociatedPolicy
}
return ds.withTx(ctx, func(tx sqlx.ExtContext) error {
err := ds.runInstallerUpdateSideEffectsInTransaction(ctx, tx, id, true, true)
if err != nil {
return ctxerr.Wrap(ctx, err, "clean up related installs and uninstalls")
}
return ctxerr.Wrap(ctx, err, "delete software installer")
}

rows, _ := res.RowsAffected()
if rows == 0 {
// could be that the software installer does not exist, or it is installed
// during setup, do additional check.
var installDuringSetup bool
if err := sqlx.GetContext(ctx, ds.reader(ctx), &installDuringSetup,
`SELECT install_during_setup FROM software_installers WHERE id = ?`, id); err != nil && !errors.Is(err, sql.ErrNoRows) {
return ctxerr.Wrap(ctx, err, "check if software installer is installed during setup")
// allow delete only if install_during_setup is false
res, err := tx.ExecContext(ctx, `DELETE FROM software_installers WHERE id = ? AND install_during_setup = 0`, id)
if err != nil {
if isMySQLForeignKey(err) {
// Check if the software installer is referenced by a policy automation.
var count int
if err := sqlx.GetContext(ctx, tx, &count, `SELECT COUNT(*) FROM policies WHERE software_installer_id = ?`, id); err != nil {
return ctxerr.Wrapf(ctx, err, "getting reference from policies")
}
if count > 0 {
return errDeleteInstallerWithAssociatedPolicy
}
}
return ctxerr.Wrap(ctx, err, "delete software installer")
}
if installDuringSetup {
return errDeleteInstallerInstalledDuringSetup

rows, _ := res.RowsAffected()
if rows == 0 {
// could be that the software installer does not exist, or it is installed
// during setup, do additional check.
var installDuringSetup bool
if err := sqlx.GetContext(ctx, tx, &installDuringSetup,
`SELECT install_during_setup FROM software_installers WHERE id = ?`, id); err != nil && !errors.Is(err, sql.ErrNoRows) {
return ctxerr.Wrap(ctx, err, "check if software installer is installed during setup")
}
if installDuringSetup {
return errDeleteInstallerInstalledDuringSetup
}
return notFound("SoftwareInstaller").WithID(id)
}
return notFound("SoftwareInstaller").WithID(id)
}

return nil
return nil
})
}

func (ds *Datastore) InsertSoftwareInstallRequest(ctx context.Context, hostID uint, softwareInstallerID uint, selfService bool, policyID *uint) (string, error) {
Expand Down Expand Up @@ -847,13 +854,59 @@ WHERE
team_id = ?
`

const deleteAllPendingUninstallScriptExecutions = `
DELETE FROM host_script_results WHERE execution_id IN (
SELECT execution_id FROM host_software_installs WHERE status = 'pending_uninstall'
AND software_installer_id IN (
SELECT id FROM software_installers WHERE global_or_team_id = ?
)
)
`
const deleteAllPendingSoftwareInstalls = `
DELETE FROM host_software_installs
WHERE status IN('pending_install', 'pending_uninstall')
AND software_installer_id IN (
SELECT id FROM software_installers WHERE global_or_team_id = ?
)
`
const markAllSoftwareInstallsAsRemoved = `
UPDATE host_software_installs SET removed = TRUE
WHERE status IS NOT NULL AND host_deleted_at IS NULL
AND software_installer_id IN (
SELECT id FROM software_installers WHERE global_or_team_id = ?
)
`

const deleteAllInstallersInTeam = `
DELETE FROM
software_installers
WHERE
global_or_team_id = ?
`

const deletePendingUninstallScriptExecutionsNotInList = `
DELETE FROM host_script_results WHERE execution_id IN (
SELECT execution_id FROM host_software_installs WHERE status = 'pending_uninstall'
AND software_installer_id IN (
SELECT id FROM software_installers WHERE global_or_team_id = ? AND title_id NOT IN (?)
)
)
`
const deletePendingSoftwareInstallsNotInList = `
DELETE FROM host_software_installs
WHERE status IN('pending_install', 'pending_uninstall')
AND software_installer_id IN (
SELECT id FROM software_installers WHERE global_or_team_id = ? AND title_id NOT IN (?)
)
`
const markSoftwareInstallsNotInListAsRemoved = `
UPDATE host_software_installs SET removed = TRUE
WHERE status IS NOT NULL AND host_deleted_at IS NULL
AND software_installer_id IN (
SELECT id FROM software_installers WHERE global_or_team_id = ? AND title_id NOT IN (?)
)
`

const unsetInstallersNotInListFromPolicies = `
UPDATE
policies
Expand Down Expand Up @@ -963,9 +1016,23 @@ ON DUPLICATE KEY UPDATE
if _, err := tx.ExecContext(ctx, unsetAllInstallersFromPolicies, globalOrTeamID); err != nil {
return ctxerr.Wrap(ctx, err, "unset all obsolete installers in policies")
}

if _, err := tx.ExecContext(ctx, deleteAllPendingUninstallScriptExecutions, globalOrTeamID); err != nil {
return ctxerr.Wrap(ctx, err, "delete all pending uninstall script executions")
}

if _, err := tx.ExecContext(ctx, deleteAllPendingSoftwareInstalls, globalOrTeamID); err != nil {
return ctxerr.Wrap(ctx, err, "delete all pending host software install records")
}

if _, err := tx.ExecContext(ctx, markAllSoftwareInstallsAsRemoved, globalOrTeamID); err != nil {
return ctxerr.Wrap(ctx, err, "mark all host software installs as removed")
}

if _, err := tx.ExecContext(ctx, deleteAllInstallersInTeam, globalOrTeamID); err != nil {
return ctxerr.Wrap(ctx, err, "delete obsolete software installers")
}

return nil
}

Expand Down Expand Up @@ -1010,6 +1077,30 @@ ON DUPLICATE KEY UPDATE
}
}

stmt, args, err = sqlx.In(deletePendingUninstallScriptExecutionsNotInList, globalOrTeamID, titleIDs)
if err != nil {
return ctxerr.Wrap(ctx, err, "build statement to delete pending uninstall script executions")
}
if _, err := tx.ExecContext(ctx, stmt, args...); err != nil {
return ctxerr.Wrap(ctx, err, "delete obsolete pending uninstall script executions")
}

stmt, args, err = sqlx.In(deletePendingSoftwareInstallsNotInList, globalOrTeamID, titleIDs)
if err != nil {
return ctxerr.Wrap(ctx, err, "build statement to delete pending software installs")
}
if _, err := tx.ExecContext(ctx, stmt, args...); err != nil {
return ctxerr.Wrap(ctx, err, "delete obsolete pending host software install records")
}

stmt, args, err = sqlx.In(markSoftwareInstallsNotInListAsRemoved, globalOrTeamID, titleIDs)
if err != nil {
return ctxerr.Wrap(ctx, err, "build statement to mark obsolete host software installs as removed")
}
if _, err := tx.ExecContext(ctx, stmt, args...); err != nil {
return ctxerr.Wrap(ctx, err, "mark obsolete host software installs as removed")
}

stmt, args, err = sqlx.In(deleteInstallersNotInList, globalOrTeamID, titleIDs)
if err != nil {
return ctxerr.Wrap(ctx, err, "build statement to delete obsolete installers")
Expand Down
12 changes: 7 additions & 5 deletions server/datastore/mysql/software_installers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,21 +548,23 @@ func testGetSoftwareInstallResult(t *testing.T, ds *Datastore) {
require.NoError(t, err)
require.Equal(t, swFilename, res.SoftwarePackage)

// delete installer to confirm that we can still access the install record
// delete installer to confirm that we can still access the install record (unless pending)
err = ds.DeleteSoftwareInstaller(ctx, installerID)
require.NoError(t, err)

if tc.expectedStatus == fleet.SoftwareInstallPending { // expect pending to be deleted
_, err = ds.GetSoftwareInstallResults(ctx, installUUID)
require.Error(t, err, notFound("HostSoftwareInstallerResult"))
return
}

ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
// ensure version is not changed, though we don't expose it yet
var version string
err := sqlx.GetContext(ctx, q, &version, `SELECT "version" FROM host_software_installs WHERE execution_id = ?`, installUUID)
require.NoError(t, err)
require.Equal(t, "1.11", version)

// let's also set the removed flag to ensure that the status we're pulling doesn't change
_, err = q.ExecContext(ctx, `UPDATE host_software_installs SET removed = 1 WHERE execution_id = ?`, installUUID)
require.NoError(t, err)

return nil
})

Expand Down
52 changes: 52 additions & 0 deletions server/service/integration_enterprise_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11174,6 +11174,26 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersSideEffec
orbitKey := setOrbitEnrollment(t, h, s.ds)
h.OrbitNodeKey = &orbitKey

// create another host that doesn't have fleetd installed
h2, err := s.ds.NewHost(context.Background(), &fleet.Host{
DetailUpdatedAt: time.Now(),
LabelUpdatedAt: time.Now(),
PolicyUpdatedAt: time.Now(),
SeenTime: time.Now().Add(-1 * time.Minute),
OsqueryHostID: ptr.String(t.Name() + uuid.New().String()),
NodeKey: ptr.String(t.Name() + uuid.New().String()),
Hostname: fmt.Sprintf("%sbar.local", t.Name()),
Platform: "linux",
})
require.NoError(t, err)
err = s.ds.AddHostsToTeam(context.Background(), &tm.ID, []uint{h2.ID})
require.NoError(t, err)
h2.TeamID = &tm.ID

// host installs fleetd
orbitKey2 := setOrbitEnrollment(t, h2, s.ds)
h2.OrbitNodeKey = &orbitKey2

// install software
installResp := installSoftwareResponse{}
s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/%d/install", h.ID, titlesResp.SoftwareTitles[0].ID), nil, http.StatusAccepted, &installResp)
Expand Down Expand Up @@ -11297,6 +11317,38 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersSideEffec
installDetailsResp := getSoftwareInstallResultsResponse{}
s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/install/%s/results", installUUID), nil, http.StatusOK, &installDetailsResp)
require.Equal(t, fleet.SoftwareInstalled, installDetailsResp.Results.Status)

// queue another install before we delete the installer via batch
pendingResp := installSoftwareResponse{}
s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/%d/install", h.ID, titlesResp.SoftwareTitles[0].ID), nil, http.StatusAccepted, &pendingResp)

// install should show as pending
s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", h.ID), nil, http.StatusOK, &afterPreinstallHostResp)
require.Equal(t, fleet.SoftwareInstallPending, *afterPreinstallHostResp.Software[0].Status)
installUUID = afterPreinstallHostResp.Software[0].SoftwarePackage.LastInstall.InstallUUID

// queue an uninstall on another host
uninstallResp := installSoftwareResponse{}
s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/%d/uninstall", h2.ID, titlesResp.SoftwareTitles[0].ID), nil, http.StatusAccepted, &uninstallResp)

// uninstall should show as pending
s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", h2.ID), nil, http.StatusOK, &afterPreinstallHostResp)
require.Equal(t, fleet.SoftwareUninstallPending, *afterPreinstallHostResp.Software[0].Status)

// delete all installers
s.DoJSON("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: []fleet.SoftwareInstallerPayload{}}, http.StatusOK, &batchResponse, "team_name", tm.Name)
packages = waitBatchSetSoftwareInstallersCompleted(t, s, tm.Name, batchResponse.RequestUUID)
require.Len(t, packages, 0)

// software should no longer exist on either host
s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", h.ID), nil, http.StatusOK, &afterPreinstallHostResp)
require.Len(t, afterPreinstallHostResp.Software, 0)
s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", h2.ID), nil, http.StatusOK, &afterPreinstallHostResp)
require.Len(t, afterPreinstallHostResp.Software, 0)

// pending install record should not exist
installDetailsResp = getSoftwareInstallResultsResponse{}
s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/install/%s/results", installUUID), nil, http.StatusNotFound, &installDetailsResp)
}

func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallersWithPoliciesAssociated() {
Expand Down
Loading