From 523fb1663d3436126a8aac6a327fc614d9991ae0 Mon Sep 17 00:00:00 2001 From: Joaquim Rocha Date: Wed, 18 Aug 2021 12:53:17 +0100 Subject: [PATCH 1/4] groups: Replace GetGroupInstancesStats's query by goqu --- backend/pkg/api/groups.go | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/backend/pkg/api/groups.go b/backend/pkg/api/groups.go index c1646e8de..304f4f621 100644 --- a/backend/pkg/api/groups.go +++ b/backend/pkg/api/groups.go @@ -556,22 +556,24 @@ func (api *API) GetGroupInstancesStats(groupID, duration string) (*InstancesStat if err != nil { return nil, err } - query := fmt.Sprintf(` - SELECT - count(*) total, - sum(case when status IS NULL then 1 else 0 end) undefined, - sum(case when status = %d then 1 else 0 end) error, - sum(case when status = %d then 1 else 0 end) update_granted, - sum(case when status = %d then 1 else 0 end) complete, - sum(case when status = %d then 1 else 0 end) installed, - sum(case when status = %d then 1 else 0 end) downloaded, - sum(case when status = %d then 1 else 0 end) downloading, - sum(case when status = %d then 1 else 0 end) onhold - FROM instance_application - WHERE group_id=$1 AND last_check_for_updates > now() at time zone 'utc' - interval '%s' AND %s`, - InstanceStatusError, InstanceStatusUpdateGranted, InstanceStatusComplete, InstanceStatusInstalled, - InstanceStatusDownloaded, InstanceStatusDownloading, InstanceStatusOnHold, durationString, ignoreFakeInstanceCondition("instance_id")) - err = api.db.QueryRowx(query, groupID).StructScan(&instancesStats) + + query, _, err := goqu.From("instance_application").Select( + goqu.COUNT("*").As("total"), + goqu.COALESCE(goqu.SUM(goqu.L("case when status IS NULL then 1 else 0 end")), 0).As("undefined"), + goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusError)), 0).As("error"), + goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusUpdateGranted)), 0).As("update_granted"), + goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusComplete)), 0).As("complete"), + goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusInstalled)), 0).As("installed"), + goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusDownloaded)), 0).As("downloaded"), + goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusDownloading)), 0).As("downloading"), + goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusOnHold)), 0).As("onhold"), + ).Where(goqu.C("group_id").Eq(groupID), goqu.L("last_check_for_updates > now() at time zone 'utc' - interval ?", durationString), + goqu.L(ignoreFakeInstanceCondition("instance_id")), + ).ToSQL() + if err != nil { + return nil, err + } + err = api.db.QueryRowx(query).StructScan(&instancesStats) if err != nil { return nil, err } From 534ad25b78e2cf74d1a6c374bd07ea8c1de9acf9 Mon Sep 17 00:00:00 2001 From: Joaquim Rocha Date: Wed, 18 Aug 2021 18:20:13 +0100 Subject: [PATCH 2/4] backend: Change how the groups' stats are calculated The groups' stats were being computed without taking into account the version of the group, so the information could be mistakenly interpreted by users (e.g. having instances in an "update complete" state but that being for an older version because the instances stopped querying for updates for a while). So this patch changes the query that performs the stats in order to show exactly how many instances are up to date or updating to the current version. --- backend/pkg/api/groups.go | 51 +++++++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/backend/pkg/api/groups.go b/backend/pkg/api/groups.go index 304f4f621..ab5a6c6dc 100644 --- a/backend/pkg/api/groups.go +++ b/backend/pkg/api/groups.go @@ -136,6 +136,8 @@ type InstancesStatusStats struct { Downloaded null.Int `db:"downloaded" json:"downloaded"` Downloading null.Int `db:"downloading" json:"downloading"` OnHold null.Int `db:"onhold" json:"onhold"` + OtherVersions null.Int `db:"other_versions" json:"other_versions"` + TimedOut null.Int `db:"timed_out" json:"timed_out"` } // UpdatesStats represents a set of statistics about the status of the updates @@ -557,19 +559,42 @@ func (api *API) GetGroupInstancesStats(groupID, duration string) (*InstancesStat return nil, err } - query, _, err := goqu.From("instance_application").Select( - goqu.COUNT("*").As("total"), - goqu.COALESCE(goqu.SUM(goqu.L("case when status IS NULL then 1 else 0 end")), 0).As("undefined"), - goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusError)), 0).As("error"), - goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusUpdateGranted)), 0).As("update_granted"), - goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusComplete)), 0).As("complete"), - goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusInstalled)), 0).As("installed"), - goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusDownloaded)), 0).As("downloaded"), - goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusDownloading)), 0).As("downloading"), - goqu.COALESCE(goqu.SUM(goqu.L("case when status = ? then 1 else 0 end", InstanceStatusOnHold)), 0).As("onhold"), - ).Where(goqu.C("group_id").Eq(groupID), goqu.L("last_check_for_updates > now() at time zone 'utc' - interval ?", durationString), - goqu.L(ignoreFakeInstanceCondition("instance_id")), - ).ToSQL() + packageVersion := "" + group, err := api.GetGroup(groupID) + + if err == nil { + packageVersion = "" + if group.Channel != nil && group.Channel.Package != nil { + packageVersion = group.Channel.Package.Version + } + } + + query := "" + // If we have no package assigned, then we cannot thoroughly report on the status for the group's version, + // so we send out just the total + if packageVersion == "" { + query, _, err = goqu.From("instance_application").Select( + goqu.COUNT("*").As("total"), + ).Where(goqu.C("group_id").Eq(groupID), goqu.L("last_check_for_updates > now() at time zone 'utc' - interval ?", durationString), + goqu.L(ignoreFakeInstanceCondition("instance_id")), + ).ToSQL() + } else { + query, _, err = goqu.From("instance_application").Select( + goqu.COUNT("*").As("total"), + goqu.COALESCE(goqu.SUM(goqu.L("case when (update_in_progress = 'false' or now() at time zone 'utc' - last_update_granted_ts < interval ?) and version != ? and status IS NULL then 1 else 0 end", group.PolicyUpdateTimeout, packageVersion)), 0).As("undefined"), + goqu.COALESCE(goqu.SUM(goqu.L("case when (update_in_progress = 'false' or now() at time zone 'utc' - last_update_granted_ts < interval ?) and last_update_version = ? and status = ? then 1 else 0 end", group.PolicyUpdateTimeout, packageVersion, InstanceStatusError)), 0).As("error"), + goqu.COALESCE(goqu.SUM(goqu.L("case when (update_in_progress = 'false' or now() at time zone 'utc' - last_update_granted_ts < interval ?) and last_update_version = ? and status = ? then 1 else 0 end", group.PolicyUpdateTimeout, packageVersion, InstanceStatusUpdateGranted)), 0).As("update_granted"), + goqu.COALESCE(goqu.SUM(goqu.L("case when (update_in_progress = 'false' or now() at time zone 'utc' - last_update_granted_ts < interval ?) and (version = ? and status IS NULL) or (version = ? and status = ?) then 1 else 0 end", group.PolicyUpdateTimeout, packageVersion, packageVersion, InstanceStatusComplete)), 0).As("complete"), + goqu.COALESCE(goqu.SUM(goqu.L("case when (update_in_progress = 'false' or now() at time zone 'utc' - last_update_granted_ts < interval ?) and last_update_version = ? and status = ? then 1 else 0 end", group.PolicyUpdateTimeout, packageVersion, InstanceStatusInstalled)), 0).As("installed"), + goqu.COALESCE(goqu.SUM(goqu.L("case when (update_in_progress = 'false' or now() at time zone 'utc' - last_update_granted_ts < interval ?) and last_update_version = ? and status = ? then 1 else 0 end", group.PolicyUpdateTimeout, packageVersion, InstanceStatusDownloaded)), 0).As("downloaded"), + goqu.COALESCE(goqu.SUM(goqu.L("case when (update_in_progress = 'false' or now() at time zone 'utc' - last_update_granted_ts < interval ?) and last_update_version = ? and status = ? then 1 else 0 end", group.PolicyUpdateTimeout, packageVersion, InstanceStatusDownloading)), 0).As("downloading"), + goqu.COALESCE(goqu.SUM(goqu.L("case when (update_in_progress = 'false' or now() at time zone 'utc' - last_update_granted_ts < interval ?) and last_update_version = ? and status = ? then 1 else 0 end", group.PolicyUpdateTimeout, packageVersion, InstanceStatusOnHold)), 0).As("onhold"), + goqu.COALESCE(goqu.SUM(goqu.L("case when (update_in_progress = 'false' or now() at time zone 'utc' - last_update_granted_ts < interval ?) and last_update_version != ? and version != ? and status IS NOT NULL then 1 else 0 end", group.PolicyUpdateTimeout, packageVersion, packageVersion)), 0).As("other_versions"), + goqu.COALESCE(goqu.SUM(goqu.L("case when update_in_progress = 'true' and now() at time zone 'utc' - last_update_granted_ts > interval ? then 1 else 0 end", group.PolicyUpdateTimeout)), 0).As("timed_out"), + ).Where(goqu.C("group_id").Eq(groupID), goqu.L("last_check_for_updates > now() at time zone 'utc' - interval ?", durationString), + goqu.L(ignoreFakeInstanceCondition("instance_id")), + ).ToSQL() + } if err != nil { return nil, err } From c2aa5a71b3869ca8a351a59f0963df34fbacb689 Mon Sep 17 00:00:00 2001 From: Joaquim Rocha Date: Wed, 18 Aug 2021 18:24:43 +0100 Subject: [PATCH 3/4] frontend: Update circular charts to the match update progress The update progress report has been changed to be more accurate, so this patch also updates the group's charts to reflect that change. --- .../src/js/components/Instances/Charts.tsx | 34 +++++++++++-------- .../src/js/components/Instances/StatusDefs.ts | 30 +++++++++++----- frontend/src/js/utils/helpers.ts | 20 +++++++++++ 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/frontend/src/js/components/Instances/Charts.tsx b/frontend/src/js/components/Instances/Charts.tsx index bc314c8e3..bdedf6824 100644 --- a/frontend/src/js/components/Instances/Charts.tsx +++ b/frontend/src/js/components/Instances/Charts.tsx @@ -209,35 +209,39 @@ export default function InstanceStatusArea(props: InstanceStatusAreaProps) { count: [{ key: 'complete' }], }, { - status: 'InstanceStatusDownloaded', - count: [{ key: 'downloaded' }], + status: 'InstanceStatusNotUpdating', + count: [{ key: 'other_versions', label: t('instances|InstanceStatusOtherVersions') }], }, { - status: 'InstanceStatusOther', - count: [ - { key: 'onhold', label: t('instances|InstanceStatusOnHold') }, - { key: 'undefined', label: t('instances|InstanceStatusUndefined') }, - ], + status: 'InstanceStatusOnHold', + count: [{ key: 'onhold' }], }, { - status: 'InstanceStatusInstalled', - count: [{ key: 'installed' }], - }, - { - status: 'InstanceStatusDownloading', + status: 'InstanceStatusUpdating', count: [ - { key: 'downloading', label: t('instances|InstanceStatusDownloading') }, { key: 'update_granted', label: t('instances|InstanceStatusUpdateGranted') }, + { key: 'downloading', label: t('instances|InstanceStatusDownloading') }, + { key: 'downloaded', label: t('instances|InstanceStatusDownloaded') }, + { key: 'installed', label: t('instances|InstanceStatusInstalled') }, ], }, { status: 'InstanceStatusError', count: [{ key: 'error' }], }, + { + status: 'InstanceStatusTimedOut', + count: [{ key: 'timed_out' }], + }, ]; - statusDefs['InstanceStatusOther'] = { ...statusDefs['InstanceStatusUndefined'] }; - statusDefs['InstanceStatusOther'].label = t('instances|Other'); + statusDefs['InstanceStatusNotUpdating'] = { ...statusDefs['InstanceStatusUndefined'] }; + statusDefs['InstanceStatusNotUpdating'].label = t('instances|Not updating'); + + statusDefs['InstanceStatusUpdating'] = { + ...statusDefs['InstanceStatusDownloading'], + label: t('instances|Updating'), + }; const totalInstances = instanceStats ? instanceStats.total : 0; diff --git a/frontend/src/js/components/Instances/StatusDefs.ts b/frontend/src/js/components/Instances/StatusDefs.ts index 754009500..de8684fb7 100644 --- a/frontend/src/js/components/Instances/StatusDefs.ts +++ b/frontend/src/js/components/Instances/StatusDefs.ts @@ -1,11 +1,11 @@ import alertCircleOutline from '@iconify/icons-mdi/alert-circle-outline'; import checkCircleOutline from '@iconify/icons-mdi/check-circle-outline'; -import downloadCircleOutline from '@iconify/icons-mdi/download-circle-outline'; -import helpCircleOutline from '@iconify/icons-mdi/help-circle-outline'; +import clockAlertOutline from '@iconify/icons-mdi/clock-alert-outline'; +import closeCircleOutline from '@iconify/icons-mdi/close-circle-outline'; import packageVariantClosed from '@iconify/icons-mdi/package-variant-closed'; import pauseCircle from '@iconify/icons-mdi/pause-circle'; import playCircle from '@iconify/icons-mdi/play-circle'; -import progressDownload from '@iconify/icons-mdi/progress-download'; +import refresh from '@iconify/icons-mdi/refresh'; import { Theme } from '@material-ui/core'; import { useTranslation } from 'react-i18next'; @@ -21,7 +21,7 @@ function makeStatusDefs(theme: Theme): { return { InstanceStatusComplete: { - label: t('instances|Complete'), + label: t('instances|Up to date'), color: 'rgba(15,15,15,1)', icon: checkCircleOutline, queryValue: '4', @@ -29,12 +29,12 @@ function makeStatusDefs(theme: Theme): { InstanceStatusDownloaded: { label: t('instances|Downloaded'), color: 'rgba(40,95,43,1)', - icon: downloadCircleOutline, + icon: refresh, queryValue: '6', }, InstanceStatusOnHold: { label: t('instances|On Hold'), - color: theme.palette.grey['400'], + color: 'rgb(89, 89, 89)', icon: pauseCircle, queryValue: '8', }, @@ -47,19 +47,19 @@ function makeStatusDefs(theme: Theme): { InstanceStatusDownloading: { label: t('instances|Downloading'), color: 'rgba(17,40,141,1)', - icon: progressDownload, + icon: refresh, queryValue: '7', }, InstanceStatusError: { label: t('instances|Error'), color: 'rgba(164,45,36,1)', - icon: alertCircleOutline, + icon: closeCircleOutline, queryValue: '3', }, InstanceStatusUndefined: { label: t('instances|Unknown'), color: 'rgb(89, 89, 89)', - icon: helpCircleOutline, + icon: alertCircleOutline, queryValue: '1', }, InstanceStatusUpdateGranted: { @@ -68,6 +68,18 @@ function makeStatusDefs(theme: Theme): { icon: playCircle, queryValue: '2', }, + InstanceStatusTimedOut: { + label: t('instances|Timed out'), + color: 'rgba(164,45,36,1)', + icon: clockAlertOutline, + queryValue: '8', + }, + InstanceStatusOtherVersions: { + label: t('instances|Not updating to this version'), + color: 'rgb(89, 89, 89)', + icon: alertCircleOutline, + queryValue: '', + }, }; } diff --git a/frontend/src/js/utils/helpers.ts b/frontend/src/js/utils/helpers.ts index a0851f22e..49c514fc4 100644 --- a/frontend/src/js/utils/helpers.ts +++ b/frontend/src/js/utils/helpers.ts @@ -230,6 +230,26 @@ export function getInstanceStatus(statusID: number, version?: string) { explanation: 'There was an update pending for the instance but it was put on hold because of the rollout policy', }, + 9: { + type: 'InstanceStatusTimedOut', + className: 'danger', + spinning: false, + icon: 'glyphicon glyphicon-remove', + description: 'Error updating', + bgColor: 'rgba(244, 67, 54, 0.1)', + textColor: '#F44336', + status: 'Error', + explanation: 'The instance reported an error while updating to version ' + version, + }, + 10: { + type: 'InstanceStatusOtherVersions', + className: '', + spinning: false, + icon: '', + description: '', + status: 'Undefined', + explanation: '', + }, }; const statusDetails = statusID ? status[statusID] : status[1]; From e83f23a532124b0baf1af3744d952ba395f240b0 Mon Sep 17 00:00:00 2001 From: Joaquim Rocha Date: Wed, 18 Aug 2021 18:37:51 +0100 Subject: [PATCH 4/4] frontend: Add a notice to the group's charts area if there's no version Because without a version, the group's report is not accurate. --- .../src/js/components/Groups/ItemExtended.tsx | 1 + .../src/js/components/Instances/Charts.tsx | 72 +++++++++++-------- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/frontend/src/js/components/Groups/ItemExtended.tsx b/frontend/src/js/components/Groups/ItemExtended.tsx index 4e492381f..d201768fa 100644 --- a/frontend/src/js/components/Groups/ItemExtended.tsx +++ b/frontend/src/js/components/Groups/ItemExtended.tsx @@ -264,6 +264,7 @@ function ItemExtended(props: { - {instanceStateCount.map(({ status, count }, i) => { - // Sort the data entries so the smaller amounts are shown first. - count.sort((obj1, obj2) => { - const stats1 = instanceStats[obj1.key]; - const stats2 = instanceStats[obj2.key]; - if (stats1 === stats2) return 0; - if (stats1 < stats2) return -1; - return 1; - }); + {!groupHasVersion ? ( + + + It's not possible to get an accurate report as the group has no channel/version + assigned to it. + + + ) : ( + instanceStateCount.map(({ status, count }, i) => { + // Sort the data entries so the smaller amounts are shown first. + count.sort((obj1, obj2) => { + const stats1 = instanceStats[obj1.key]; + const stats2 = instanceStats[obj2.key]; + if (stats1 === stats2) return 0; + if (stats1 < stats2) return -1; + return 1; + }); - return ( - - { - const statusLabel = statusDefs[label].label; - return { - value: instanceStats[key] / instanceStats.total, - color: statusDefs[label].color, - description: t('{{statusLabel}}: {{stat, number}} instances', { - statusLabel: statusLabel, - stat: instanceStats[key], - }), - }; - })} - width={140} - height={140} - {...statusDefs[status]} - /> - - ); - })} + return ( + + { + const statusLabel = statusDefs[label].label; + return { + value: instanceStats[key] / instanceStats.total, + color: statusDefs[label].color, + description: t('{{statusLabel}}: {{stat, number}} instances', { + statusLabel: statusLabel, + stat: instanceStats[key], + }), + }; + })} + width={140} + height={140} + {...statusDefs[status]} + /> + + ); + }) + )} ) : (