Skip to content

Update VPP and software install status summaries and filters for unified queue #25569

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 22, 2025
Merged

Update VPP and software install status summaries and filters for unified queue #25569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a7d1d51
Update vpp and software install status filters and summaries for unif…
gillespi314 Jan 17, 2025
6895789
Merge branch 'feat-upcoming-activites-queue' into sg-sw-vpp-filters-a…
gillespi314 Jan 20, 2025
9dcf51c
Fix lint
gillespi314 Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions server/datastore/mysql/labels.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,16 +617,32 @@ func (ds *Datastore) applyHostLabelFilters(ctx context.Context, filter fleet.Tea
if opt.SoftwareTitleIDFilter != nil && opt.SoftwareStatusFilter != nil {
// check for software installer metadata
_, err := ds.GetSoftwareInstallerMetadataByTeamAndTitleID(ctx, opt.TeamFilter, *opt.SoftwareTitleIDFilter, false)
if err != nil {
switch {
case fleet.IsNotFound(err):
vppApp, err := ds.GetVPPAppByTeamAndTitleID(ctx, opt.TeamFilter, *opt.SoftwareTitleIDFilter)
if err != nil {
return "", nil, ctxerr.Wrap(ctx, err, "get vpp app by team and title id")
}
vppAppJoin, vppAppParams, err := ds.vppAppJoin(vppApp.VPPAppID, *opt.SoftwareStatusFilter)
if err != nil {
return "", nil, ctxerr.Wrap(ctx, err, "vpp app join")
}
softwareStatusJoin = vppAppJoin
joinParams = append(joinParams, vppAppParams...)

case err != nil:
return "", nil, ctxerr.Wrap(ctx, err, "get software installer metadata by team and title id")

default:
// TODO(uniq): prior code was joining on installer id but based on how list options are parsed [1] it seems like this should be the title id
// [1] https://github.com/fleetdm/fleet/blob/8aecae4d853829cb6e7f828099a4f0953643cf18/server/datastore/mysql/hosts.go#L1088-L1089
installerJoin, installerParams, err := ds.softwareInstallerJoin(*opt.SoftwareTitleIDFilter, *opt.SoftwareStatusFilter)
if err != nil {
return "", nil, ctxerr.Wrap(ctx, err, "software installer join")
}
softwareStatusJoin = installerJoin
joinParams = append(joinParams, installerParams...)
}
// TODO(sarah): are we missing VPP apps here? see ds.applyHostFilters
installerJoin, installerParams, err := ds.softwareInstallerJoin(*opt.SoftwareTitleIDFilter, *opt.SoftwareStatusFilter)
if err != nil {
return "", nil, ctxerr.Wrap(ctx, err, "software installer join")
}
softwareStatusJoin = installerJoin
joinParams = append(joinParams, installerParams...)
}
if softwareStatusJoin != "" {
query += softwareStatusJoin
Expand Down
3 changes: 3 additions & 0 deletions server/datastore/mysql/software.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2209,7 +2209,10 @@ INNER JOIN software_cve scve ON scve.software_id = s.id
hs.host_id = :host_id AND
s.title_id = st.id
) OR `, onlyVulnerableJoin)

// TODO(uniq): refactor vppHostStatusNamedQuery to use the same logic as GetSummaryHostVPPAppInstalls?
status := fmt.Sprintf(`COALESCE(%s, %s)`, "hsi.last_status", vppAppHostStatusNamedQuery("hvsi", "ncr", ""))

if opts.OnlyAvailableForInstall {
// Get software that has a package/VPP installer but was not installed with Fleet
softwareIsInstalledOnHostClause = fmt.Sprintf(` %s IS NULL AND (si.id IS NOT NULL OR vat.adam_id IS NOT NULL) AND %s`, status,
Expand Down
140 changes: 106 additions & 34 deletions server/datastore/mysql/software_installers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,35 +1073,74 @@ WHERE

func (ds *Datastore) GetSummaryHostSoftwareInstalls(ctx context.Context, installerID uint) (*fleet.SoftwareInstallerStatusSummary, error) {
var dest fleet.SoftwareInstallerStatusSummary
// TODO(uniq): do we need to support team filtering or is that not needed because installers are
// associated to teams?

// TODO(mna): must also look in upcoming queue for pending, and most recent
// attempt for an installer might be in upcoming queue...
stmt := `
// TODO(uniq): AFAICT we don't have uniqueness for host_id + title_id in upcoming or
// past activities. In the past the max(id) approach was "good enough" as a proxy for the most
// recent activity since we didn't really need to worry about the order of activities.
// Moving to a time-based approach would be more accurate but would require a more complex and
// potentially slower query.

stmt := `WITH

-- select most recent upcoming activities for each host
upcoming AS (
SELECT
max(ua.id) AS upcoming_id, -- ensure we use only the most recent attempt for each host
host_id
FROM
upcoming_activities ua
JOIN software_install_upcoming_activities siua ON ua.id = siua.upcoming_activity_id
JOIN hosts h ON host_id = h.id
WHERE
activity_type IN('software_install', 'software_uninstall')
AND software_installer_id = :installer_id
GROUP BY
host_id
),

-- select most recent past activities for each host
past AS (
SELECT
max(hsi.id) AS past_id, -- ensure we use only the most recent attempt for each host
host_id
FROM
host_software_installs hsi
JOIN hosts h ON host_id = h.id
WHERE
software_installer_id = :installer_id
AND host_id NOT IN(SELECT host_id FROM upcoming) -- antijoin to exclude hosts with upcoming activities
AND host_deleted_at IS NULL
AND removed = 0
GROUP BY
host_id
)

-- count each status
SELECT
COALESCE(SUM( IF(status = :software_status_pending_install, 1, 0)), 0) AS pending_install,
COALESCE(SUM( IF(status = :software_status_failed_install, 1, 0)), 0) AS failed_install,
COALESCE(SUM( IF(status = :software_status_pending_uninstall, 1, 0)), 0) AS pending_uninstall,
COALESCE(SUM( IF(status = :software_status_failed_uninstall, 1, 0)), 0) AS failed_uninstall,
COALESCE(SUM( IF(status = :software_status_installed, 1, 0)), 0) AS installed
FROM (

-- union most recent past and upcoming activities after joining to get statuses for most recent activities
SELECT
software_installer_id,
past.host_id,
status
FROM
host_software_installs hsi
WHERE
software_installer_id = :installer_id
AND id IN(
SELECT
max(id) -- ensure we use only the most recently created install attempt for each host
FROM host_software_installs
WHERE
software_installer_id = :installer_id
AND host_deleted_at IS NULL
AND removed = 0
GROUP BY
host_id)
) s`
past
JOIN host_software_installs hsi ON hsi.id = past_id
UNION
SELECT
upcoming.host_id,
IF(activity_type = 'software_install', :software_status_pending_install, :software_status_pending_uninstall) AS status
FROM
upcoming
JOIN software_install_upcoming_activities siua ON upcoming_id = siua.upcoming_activity_id
JOIN upcoming_activities ua ON ua.id = upcoming_id) t`

query, args, err := sqlx.Named(stmt, map[string]interface{}{
"installer_id": installerID,
Expand All @@ -1124,16 +1163,43 @@ WHERE
}

func (ds *Datastore) vppAppJoin(appID fleet.VPPAppID, status fleet.SoftwareInstallerStatus) (string, []interface{}, error) {
// Since VPP does not have uninstaller yet, we map the generic pending/failed statuses to the install statuses
switch status {
case fleet.SoftwarePending:
status = fleet.SoftwareInstallPending
case fleet.SoftwareFailed:
status = fleet.SoftwareInstallFailed
default:
// no change
}
// TODO(mna): must join with upcoming queue for pending
// for pending status, we'll join through upcoming_activities
if status == fleet.SoftwarePending || status == fleet.SoftwareInstallPending || status == fleet.SoftwareUninstallPending {
stmt := `JOIN (
SELECT DISTINCT
host_id
FROM
upcoming_activities ua
JOIN vpp_app_upcoming_activities vppua ON ua.id = vppua.upcoming_activity_id
WHERE
%s) hss ON hss.host_id = h.id`

filter := "vppua.adam_id = ? AND vppua.platform = ?"
switch status {
case fleet.SoftwareInstallPending:
filter += " AND ua.activity_type = 'vpp_app_install'"
case fleet.SoftwareUninstallPending:
// TODO: Update this when VPP supports uninstall, for now we map uninstall to install to preserve existing behavior of VPP filters
filter += " AND ua.activity_type = 'vpp_app_install'"
default:
// no change, we're just filtering by app_id and platform so it will pick up any
// activity type that is associated with the app (i.e. both install and uninstall)
}

return fmt.Sprintf(stmt, filter), []interface{}{appID.AdamID, appID.Platform}, nil
}

// TODO: Update this when VPP supports uninstall so that we map for now we map the generic failed status to the install statuses
if status == fleet.SoftwareFailed {
status = fleet.SoftwareInstallFailed // TODO: When VPP supports uninstall this should become STATUS IN ('failed_install', 'failed_uninstall')
}

// TODO(uniq): AFAICT we don't have uniqueness for host_id + title_id in upcoming or
// past activities. In the past the max(id) approach was "good enough" as a proxy for the most
// recent activity since we didn't really need to worry about the order of activities.
// Moving to a time-based approach would be more accurate but would require a more complex and
// potentially slower query.

stmt := fmt.Sprintf(`JOIN (
SELECT
host_id
Expand All @@ -1152,7 +1218,7 @@ WHERE
GROUP BY
host_id, adam_id)
AND (%s) = :status) hss ON hss.host_id = h.id
`, vppAppHostStatusNamedQuery("hvsi", "ncr", ""))
`, vppAppHostStatusNamedQuery("hvsi", "ncr", "")) // TODO(uniq): refactor vppHostStatusNamedQuery to use the same logic as GetSummaryHostVPPAppInstalls?

return sqlx.Named(stmt, map[string]interface{}{
"status": status,
Expand All @@ -1168,15 +1234,14 @@ WHERE
}

func (ds *Datastore) softwareInstallerJoin(titleID uint, status fleet.SoftwareInstallerStatus) (string, []interface{}, error) {
level.Info(ds.logger).Log("msg", "software installer join", "title_id", titleID, "status", status)
// for pending status, we'll join through upcoming_activities
if status == fleet.SoftwarePending || status == fleet.SoftwareInstallPending || status == fleet.SoftwareUninstallPending {
stmt := `JOIN (
SELECT DISTINCT
host_id
FROM
software_install_upcoming_activities siua
JOIN upcoming_activities ua ON ua.id = siua.upcoming_activity_id
upcoming_activities ua
JOIN software_install_upcoming_activities siua ON ua.id = siua.upcoming_activity_id
WHERE
%s) hss ON hss.host_id = h.id`

Expand All @@ -1200,6 +1265,12 @@ WHERE
statusFilter = "hsi.status IN (:installFailed, :uninstallFailed)"
}

// TODO(uniq): AFAICT we don't have uniqueness for host_id + title_id in upcoming or
// past activities. In the past the max(id) approach was "good enough" as a proxy for the most
// recent activity since we didn't really need to worry about the order of activities.
// Moving to a time-based approach would be more accurate but would require a more complex and
// potentially slower query.

stmt := fmt.Sprintf(`JOIN (
SELECT
host_id
Expand All @@ -1212,7 +1283,8 @@ WHERE
max(id) -- ensure we use only the most recent install attempt for each host
FROM host_software_installs
WHERE
software_title_id = :title_id
host_id = hsi.host_id
Copy link
Member

Choose a reason for hiding this comment

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

That was missing, was that a bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it may have been just potential inefficiency in the prior subquery rather than a bug since the subquery is still grouping by hsi.host_id and being then joined on hosts.id. This change seemed to improve the EXPLAIN when I was testing locally.

AND software_title_id = :title_id
AND removed = 0
GROUP BY
host_id, software_title_id)
Expand All @@ -1223,7 +1295,7 @@ WHERE
"status": status,
"installFailed": fleet.SoftwareInstallFailed,
"uninstallFailed": fleet.SoftwareUninstallFailed,
// TODO(sarah): prior code was joining based on installer id bug based on how list options are parsed [1] it seems like this should be the title id
// TODO(uniq): prior code was joining based on installer id but based on how list options are parsed [1] it seems like this should be the title id
// [1] https://github.com/fleetdm/fleet/blob/8aecae4d853829cb6e7f828099a4f0953643cf18/server/datastore/mysql/hosts.go#L1088-L1089
"title_id": titleID,
})
Expand Down
Loading
Loading