Repository navigation
fix: PagerBug: Hibernating filter in Devtron Apps list shows incomplete results (pager #2974) - #7048
devtronpageragent[bot] wants to merge 2 commits into
Conversation
…te results Automated fix for pager issue devtron-labs/sprint-tasks#2974. Root cause and verification plan are in the pull request body. Refs: https://github.com/devtron-labs/sprint-tasks/issues/2974
|
Paired with https://github.com/devtron-labs/devtron-enterprise/pull/3418 . These repositories are a hard fork at the same module path, so this fix needs both diffs — merging one without the other leaves the other half broken. |
|
Bito Automatic Review Skipped - Draft PR |
|
Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n |
The draft fix for #2974 filtered+paginated HIBERNATING purely via the db-side aps.status predicate. That is correct for ArgoCD/GitOps pipelines, whose status is persisted to app_status, but Helm pipelines never get a row written there through the normal sync path (AppStatusServiceImpl.UpdateStatusWithAppIdEnvId is only reached from GitOps events) - their status is instead resolved on the fly at listing time. A pure SQL predicate on aps.status would silently exclude such Helm apps from a Hibernating search, and from the total count, since aps.status is NULL for them. buildAppListingWhereCondition now lets rows with no persisted app_status through (scoped to deployment_app_type = 'helm') instead of excluding them outright, only when HIBERNATING is requested; every other status still matches purely against the persisted column. FetchAppsByEnvironmentV2 records which returned rows had no persisted status before the on-the-fly Helm status fetch runs, then reconcileAppsWithUnresolvedStatus rechecks just those rows against the requested filter afterwards and decrements appSize by exactly the number dropped - the sql-computed count/pagination for the persisted-status (Argo) rows is left untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ee9oPaW5Yx1Sx7Ysmjb2W
Follow-up commit: handle Helm apps whose AppStatus isn't in DBPer feedback from @Shivam Nagar in #product-engg: the previous commit's SQL-only fix is correct for ArgoCD/GitOps pipelines (status persisted in Verified in this OSS repo: What changed (2nd commit):
Known trade-off (documented, not hidden): admitting all Build ( |
|
Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n |
|
|
Closing — superseded by a smaller fix, raised on both
The root cause identified here was correct and an independent investigation reached the same place: The fix is what differs. This PR keeps the two-source design — an unpaginated fetch capped at 100k rows whenever the Hibernating filter is on,
The replacement is a pure deletion (11 insertions, 31 deletions). On the helm gap this PR aimed to close: it is real but small — the overwhelming majority of hibernated app-environments are GitOps pipelines, which the SQL predicate covers — and it is pre-existing rather than a regression, since the in-memory pass read the same empty column. The right fix is to persist helm-pipeline status into |



Agent-authored draft fix for pager issue #2974
Tracking issue: https://github.com/devtron-labs/sprint-tasks/issues/2974
Fixes devtron-labs/sprint-tasks#2974
Root cause
AppListingServiceImpl.FetchAppsByEnvironmentV2special-cases theHIBERNATINGapp-status filter: when the request contains it, the wholeAppStatuseslist is blanked out before being handed to the repository (fetchAppListingRequest.AppStatuses = []string{}), so the SQL runs LIMIT/OFFSET andcount(distinct a.id)over the unfiltered app set. The status filter is then re-applied in memory, but only to the one page of rows that came back, andappSizeis overwritten withlen(filteredContainers). The result is a handful of hibernating apps (only those that happened to land on the requested page of the unfiltered listing) and a total count equal to that handful, so the UI offers no further pages. The in-memory pass was a workaround for app statuses fetched on the fly for Helm pipelines, but that fetch is a no-op in OSS (AppListingService_ent.goreturns nil) and commented out in enterprise, whileHIBERNATINGis in fact persisted inapp_status.statusand is filterable in SQL like any other status.From symptom to defect
appStatuses, together withoffsetandsizefor pagination.dashboard/src/components/app/list/appList.modal.ts·getDevtronAppListPayloadappStatuses: appStatus,alongsideoffset,andsize: pageSize,in the returned payload; the Hibernating option's DTO value isHIBERNATING(src/components/app/list-new/AppListType.ts:52).appCount— it does no client-side status filtering.dashboard/src/components/app/list/DevtronAppListContainer.tsx·getRowsconst totalCount = data?.result?.appCount ?? 0andtotalRows: totalCountat lines 174 and 193.AppCount.devtron-enterprise/api/restHandler/app/appList/AppListingRestHandler.go·AppListingRestHandlerImpl.FetchAppsByEnvironmentV2envContainers, appsCount, err := handler.appListingService.FetchAppsByEnvironmentV2(...)(line 412) thenAppCount: appsCount(line 450); the route is bound in api/router/app/appList/AppListingRouter.go:28-34.devtron-enterprise/pkg/app/AppListingService.go·AppListingServiceImpl.FetchAppsByEnvironmentV2Pre-fix lines 528-536:
isFilteredOnHibernatingStatus := impl.isFilteredOnHibernatingStatus(...)thenfetchAppListingRequest.AppStatuses = []string{}, and the filter struct is built from that now-empty field (AppStatuses: fetchAppListingRequest.AppStatuses).appsSizeis the count of all apps.devtron-enterprise/internal/sql/repository/AppListingRepository.go·AppListingRepositoryImpl.FetchAppsByEnvironmentV2appsSize = appIdCountDtos[0].TotalCount(line 342) where the query comes fromGetAppIdsQueryWithPaginationForAppNameSearch(appListingFilter), which appendsLIMIT ? OFFSET ?and acount(distinct(a.id))sub-select over the same join (helper/AppListingRepositoryQueryBuilder.go:246-260).devtron-enterprise/internal/sql/repository/helper/AppListingRepositoryQueryBuilder.go·buildAppListingWhereConditionwhereCondition += " and aps.status IN (?) "(line ~330) overLEFT JOIN app_status aps on aps.app_id = a.id and p.environment_id = aps.env_id(CommonJoinSubQuery, line 178).devtron-enterprise/pkg/appStatus/AppStatusService.go·AppStatusServiceImpl.UpdateStatusWithAppIdEnvId / GetHibernationStatusif status == HealthStatusSuspended || status == HealthStatusHibernating { status = HealthStatusHibernatingFilter }whereHealthStatusHibernatingFilter string = "HIBERNATING"; written from pkg/app/AppService.go:253,299 viaComputeAppstatus.devtron-enterprise/pkg/app/AppListingService.go·AppListingServiceImpl.FetchAppsByEnvironmentV2 (post-filter)Pre-fix lines 598-607:
if slices.Contains(appStatusesFilter, container.AppStatus)…appSize = len(filteredContainers).devtron-enterprise/pkg/app/AppListingService_ent.go·updateAppStatusForHelmTypePipelinesThe call site is commented out in FetchAppsByEnvironmentV2:
//err = impl.updateAppStatusForHelmTypePipelines(envContainers)with the note "Removed the below logic call due to slowness at zupee"; in OSS the same method isreturn nil(devtron/pkg/app/AppListingService_ent.go:7-9).Explanations considered and rejected
totalRowsdirectly fromresult.appCountand maps every returned container into a row; appList.modal.ts:153 forwardsappStatusesunchanged to the server. There is no status predicate on the client.SuspendedandHibernatedto the literalHIBERNATINGbefore persisting, andGetHibernationStatusmaps Healthy→HIBERNATING for stop-type releases (pkg/app/AppService.go:253,299). Independently, the existing in-memory filter compares againstcontainer.AppStatus, which isaps.statusselected by the query (query builder:aps.status as app_status) — so the rows the UI does show today prove the value is in the column.fetchAppListingRequest.AppIds = validAppIdsfor non-super-admins) truncates the list. — ruled out by That list is applied asa.id IN (?)inside the same paginated query for every filter combination (buildAppListingWhereCondition, AppIds branch), so it would shrink Healthy/Degraded listings identically. The ticket's symptom is specific to selecting Hibernating, and the HIBERNATING branch is the only status-dependent code in the path.appwitha.app_type = CustomApp(buildAppListingWhereCondition line 288) and never touches the app-store repository.What this PR changes in
devtronIn
pkg/app/AppListingService.go, removed the HIBERNATING special case fromFetchAppsByEnvironmentV2: the requestedAppStatusesare no longer blanked out before the repository call, and the in-memory post-filter that overwroteappSizeis gone.HIBERNATINGis now filtered in SQL like every other app status, so LIMIT/OFFSET andcount(distinct a.id)are computed over the filtered set. Deleted the now-unusedisFilteredOnHibernatingStatushelper and its only-use import (bean2 "client/argocdServer/bean").Risk: Behaviour change visible to any consumer of POST /orchestrator/app/list with
appStatusescontaining HIBERNATING: the response now contains a full page of hibernating apps and a real total count instead of a truncated slice. Combining HIBERNATING with "NOT DEPLOYED" also changes: previously the NOT-DEPLOYED apps were silently dropped by the in-memory pass (theirapp_statusis empty); now the query builder's dedicated NOT DEPLOYED branch applies, so those apps appear. That is the intended semantics but it is a user-visible difference. If any hibernating app lacks anapp_statusrow it will now be absent rather than accidentally present — see unknowns.slicesis still used elsewhere in the file and stays imported;bean2was used only by the deleted helper.pkg/app/AppListingService.goVerified
isFilteredOnHibernatingStatus,appStatusesFilter) have no remaining references in either repo, thatbean2has no other use in the edited file, and thatslicesis still referenced (lines 516, 543) so its import must stay.bean2.appears nowhere else in devtron/pkg/app/AppListingService.go, so dropping the import is required and safe (grep over the file).isFilteredOnHibernatingStatusandappStatusesFilterhave zero remaining references across devtron and devtron-enterprise (grep over both trees).slices.Containsis still used at lines 516 and 543 of the edited file, sogolang.org/x/exp/slicesremains a live import.helper.AppListingFilterhas anAppStatuses []stringfield andbuildAppListingWhereConditionturns it intoaps.status IN (?), so passing the unmodified list through is a supported code path — it is what every non-HIBERNATING status already does.NOT verified
Read this section before the diff.
container.AppStatusis populated fromaps.statusand the on-the-fly enrichment is a no-op in OSS, but I could not run it against a database.aps.status IN ('HIBERNATING')on a largeapp_statustable.Open assumptions:
app_statusrow with statusHIBERNATING. This fix removes the pagination truncation, so the UI will now return everything the SQL predicate matches — but if some hibernated apps (e.g. non-GitOps/Helm-type pipelines whose status was previously computed on the fly, a path that is disabled in both repos) never get anapp_statusrow written, they will still be missing. The export's own source of truth was not available to me and should be compared againstselect count(*) from app_status where status = 'HIBERNATING'.aps.status IN (...)predicate that Healthy/Degraded already pay, so it is not a new query shape, but I could not check for an index onapp_status.status(no migrations inspected, no database available).FetchAppsByEnvironmentV2. Both routes bound to it are the app-list endpoints, and no other caller of the service method exists in either repo, but I did not audit the enterprise-only callers beyond grep.Verification plan for QA
select count(distinct a.id) from app a left join pipeline p on a.id=p.app_id and p.deleted=false left join app_status aps on aps.app_id=a.id and aps.env_id=p.environment_id where a.active=true and a.app_type=0 and aps.status='HIBERNATING'(add the team/env predicates if filters are set).app_statusand paginate normally.AppIdspredicate is applied in the same query).🤖 Generated by the agentic pager-duty fix engine. Draft,
agent-authored, do not merge without review.