Skip to content

fix: PagerBug: Hibernating filter in Devtron Apps list shows incomplete results (pager #2974) - #7048

Closed
devtronpageragent[bot] wants to merge 2 commits into
mainfrom
agent/pager-2974
Closed

devtronpageragent[bot] wants to merge 2 commits into
mainfrom
agent/pager-2974

Conversation

@devtronpageragent

@devtronpageragent devtronpageragent Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Agent-authored draft fix for pager issue #2974

This pull request was written by an automated agent and reviewed by nobody. Devtron's test suites do not cover this code (make test-unit runs only ./pkg/pipeline; dashboard CI never runs tests), so you are the gate. Read the causal chain below and decide whether it holds.

Tracking issue: https://github.com/devtron-labs/sprint-tasks/issues/2974

Fixes devtron-labs/sprint-tasks#2974

Root cause

AppListingServiceImpl.FetchAppsByEnvironmentV2 special-cases the HIBERNATING app-status filter: when the request contains it, the whole AppStatuses list is blanked out before being handed to the repository (fetchAppListingRequest.AppStatuses = []string{}), so the SQL runs LIMIT/OFFSET and count(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, and appSize is overwritten with len(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.go returns nil) and commented out in enterprise, while HIBERNATING is in fact persisted in app_status.status and is filterable in SQL like any other status.

From symptom to defect

  1. The Devtron Apps list sends the selected App Status filter to the backend as appStatuses, together with offset and size for pagination.
    dashboard/src/components/app/list/appList.modal.ts · getDevtronAppListPayload
    appStatuses: appStatus, alongside offset, and size: pageSize, in the returned payload; the Hibernating option's DTO value is HIBERNATING (src/components/app/list-new/AppListType.ts:52).
  2. The UI renders exactly the rows the server returns and takes the pager's total row count straight from the server's appCount — it does no client-side status filtering.
    dashboard/src/components/app/list/DevtronAppListContainer.tsx · getRows
    const totalCount = data?.result?.appCount ?? 0 and totalRows: totalCount at lines 174 and 193.
  3. The POST /orchestrator/app/list (and /list/v2) handler decodes the request and returns the service's second return value verbatim as AppCount.
    devtron-enterprise/api/restHandler/app/appList/AppListingRestHandler.go · AppListingRestHandlerImpl.FetchAppsByEnvironmentV2
    envContainers, appsCount, err := handler.appListingService.FetchAppsByEnvironmentV2(...) (line 412) then AppCount: appsCount (line 450); the route is bound in api/router/app/appList/AppListingRouter.go:28-34.
  4. Because the request contains HIBERNATING, the service empties the status filter before building the repository filter, so the paginated query is run against all apps.
    devtron-enterprise/pkg/app/AppListingService.go · AppListingServiceImpl.FetchAppsByEnvironmentV2
    Pre-fix lines 528-536: isFilteredOnHibernatingStatus := impl.isFilteredOnHibernatingStatus(...) then fetchAppListingRequest.AppStatuses = []string{}, and the filter struct is built from that now-empty field (AppStatuses: fetchAppListingRequest.AppStatuses).
  5. The repository computes both the page of app ids and the grand total from that same (now status-less) filter, so the page is a page of all apps and appsSize is the count of all apps.
    devtron-enterprise/internal/sql/repository/AppListingRepository.go · AppListingRepositoryImpl.FetchAppsByEnvironmentV2
    appsSize = appIdCountDtos[0].TotalCount (line 342) where the query comes from GetAppIdsQueryWithPaginationForAppNameSearch(appListingFilter), which appends LIMIT ? OFFSET ? and a count(distinct(a.id)) sub-select over the same join (helper/AppListingRepositoryQueryBuilder.go:246-260).
  6. The status predicate that was skipped is an ordinary SQL predicate on the joined app_status table, i.e. it could have been applied before pagination.
    devtron-enterprise/internal/sql/repository/helper/AppListingRepositoryQueryBuilder.go · buildAppListingWhereCondition
    whereCondition += " and aps.status IN (?) " (line ~330) over LEFT JOIN app_status aps on aps.app_id = a.id and p.environment_id = aps.env_id (CommonJoinSubQuery, line 178).
  7. HIBERNATING is a value actually stored in app_status.status, so the SQL predicate above matches hibernating apps — the in-memory pass was not needed to produce it.
    devtron-enterprise/pkg/appStatus/AppStatusService.go · AppStatusServiceImpl.UpdateStatusWithAppIdEnvId / GetHibernationStatus
    if status == HealthStatusSuspended || status == HealthStatusHibernating { status = HealthStatusHibernatingFilter } where HealthStatusHibernatingFilter string = "HIBERNATING"; written from pkg/app/AppService.go:253,299 via ComputeAppstatus.
  8. The in-memory filter then discards most of the returned page and replaces the total count with the size of the surviving slice — the reported symptom.
    devtron-enterprise/pkg/app/AppListingService.go · AppListingServiceImpl.FetchAppsByEnvironmentV2 (post-filter)
    Pre-fix lines 598-607: if slices.Contains(appStatusesFilter, container.AppStatus) … appSize = len(filteredContainers).
  9. The on-the-fly status enrichment the in-memory filter existed to serve does not run in this code path, so no status can differ from what the DB predicate would have matched.
    devtron-enterprise/pkg/app/AppListingService_ent.go · updateAppStatusForHelmTypePipelines
    The 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 is return nil (devtron/pkg/app/AppListingService_ent.go:7-9).

Explanations considered and rejected

  • The dashboard filters or paginates client-side and drops hibernating rows. — ruled out by dashboard/src/components/app/list/DevtronAppListContainer.tsx:174,193 takes totalRows directly from result.appCount and maps every returned container into a row; appList.modal.ts:153 forwards appStatuses unchanged to the server. There is no status predicate on the client.
  • HIBERNATING is simply not persisted in app_status, so the filter is legitimately sparse and the real bug is in status writing. — ruled out by pkg/appStatus/AppStatusService.go maps both Suspended and Hibernated to the literal HIBERNATING before persisting, and GetHibernationStatus maps Healthy→HIBERNATING for stop-type releases (pkg/app/AppService.go:253,299). Independently, the existing in-memory filter compares against container.AppStatus, which is aps.status selected 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.
  • RBAC scoping (fetchAppListingRequest.AppIds = validAppIds for non-super-admins) truncates the list. — ruled out by That list is applied as a.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.
  • The defect is in the Helm/installed-apps listing (pkg/appStore/.../InstalledAppRepository.go also has an appStatuses filter). — ruled out by The ticket is about the Devtron Apps tab, which posts to /orchestrator/app/list → AppListingRestHandler.FetchAppsByEnvironmentV2 (router/app/appList/AppListingRouter.go:28-34); that path queries app with a.app_type = CustomApp (buildAppListingWhereCondition line 288) and never touches the app-store repository.
  • The count is wrong because the total-count sub-query double-counts or mis-binds parameters (the builder appends queryParams twice). — ruled out by The duplication at GetAppIdsQueryWithPaginationForAppNameSearch:271 is deliberate — the same join/where string is embedded twice (once in the count sub-select, once in the outer FROM) and go-pg binds positionally, so the parameter list must be doubled. That code is shared by every filter combination and would corrupt counts for all statuses, not just Hibernating.

What this PR changes in devtron

In pkg/app/AppListingService.go, removed the HIBERNATING special case from FetchAppsByEnvironmentV2: the requested AppStatuses are no longer blanked out before the repository call, and the in-memory post-filter that overwrote appSize is gone. HIBERNATING is now filtered in SQL like every other app status, so LIMIT/OFFSET and count(distinct a.id) are computed over the filtered set. Deleted the now-unused isFilteredOnHibernatingStatus helper and its only-use import (bean2 "client/argocdServer/bean").

Risk: Behaviour change visible to any consumer of POST /orchestrator/app/list with appStatuses containing 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 (their app_status is 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 an app_status row it will now be absent rather than accidentally present — see unknowns. slices is still used elsewhere in the file and stays imported; bean2 was used only by the deleted helper.

  • pkg/app/AppListingService.go

Verified

  • Build: not run. This change has not been compiled. Not compile-verified. This environment has no Go toolchain and the checkouts exclude vendor/, and the task instructions explicitly forbid running go build/go test here. I checked by hand that the two removed identifiers (isFilteredOnHibernatingStatus, appStatusesFilter) have no remaining references in either repo, that bean2 has no other use in the edited file, and that slices is still referenced (lines 516, 543) so its import must stay.
  • Tests: none run.
  • bean2. appears nowhere else in devtron/pkg/app/AppListingService.go, so dropping the import is required and safe (grep over the file).
  • isFilteredOnHibernatingStatus and appStatusesFilter have zero remaining references across devtron and devtron-enterprise (grep over both trees).
  • slices.Contains is still used at lines 516 and 543 of the edited file, so golang.org/x/exp/slices remains a live import.
  • helper.AppListingFilter has an AppStatuses []string field and buildAppListingWhereCondition turns it into aps.status IN (?), so passing the unmodified list through is a supported code path — it is what every non-HIBERNATING status already does.
  • The function's signature, its return statement and the surrounding function boundaries are unchanged (re-read lines 434-501 after editing).

NOT verified

Read this section before the diff.

  • That the package compiles — no Go toolchain here. CI on the draft PR is the first compile.
  • That the HIBERNATING rows returned by SQL are the same set the old in-memory pass would have produced on an unpaginated listing. I argued this from the fact that container.AppStatus is populated from aps.status and the on-the-fly enrichment is a no-op in OSS, but I could not run it against a database.
  • Query performance of aps.status IN ('HIBERNATING') on a large app_status table.

Open assumptions:

  • Whether every application the customer sees as hibernating in the data export has a corresponding app_status row with status HIBERNATING. 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 an app_status row written, they will still be missing. The export's own source of truth was not available to me and should be compared against select count(*) from app_status where status = 'HIBERNATING'.
  • Query cost on a large installation. The HIBERNATING listing now pays the same 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 on app_status.status (no migrations inspected, no database available).
  • Whether any consumer other than the Devtron Apps list relies on the old truncating behaviour of 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

  1. On a build of this branch, open Devtron Apps → Filter → App Status → Hibernating in an environment with more hibernating apps than one page.
  2. Confirm the total count in the UI now matches 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).
  3. Page through the result set and confirm every page is full and no app repeats or is skipped; compare the union of all pages against the data export attached to the ticket.
  4. Regression: apply App Status = Healthy alone, and Hibernating + Healthy together; both should return counts consistent with app_status and paginate normally.
  5. Regression: apply Hibernating + "NOT DEPLOYED" together. This combination now returns never-deployed apps as well (it previously returned none of them); confirm with product that this is the wanted behaviour.
  6. Regression: repeat the Hibernating filter as a non-super-admin user with access to a subset of apps, and confirm the list and count are restricted to that subset (the RBAC AppIds predicate is applied in the same query).

🤖 Generated by the agentic pager-duty fix engine. Draft, agent-authored, do not merge without review.

…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
@devtronpageragent devtronpageragent Bot added the agent-authored Opened by the pager-duty fix agent. Draft; never merge without review. label Sep 22, 2026
@devtronpageragent

Copy link
Copy Markdown
Author

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-code-review

Copy link
Copy Markdown

Bito Automatic Review Skipped - Draft PR

Bito didn't auto-review because this pull request is in draft status.
No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change draft PR review settings here, or contact your Bito workspace admin at shivam@devtron.ai.

@github-actions

Copy link
Copy Markdown

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
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown

Follow-up commit: handle Helm apps whose AppStatus isn't in DB

Per feedback from @Shivam Nagar in #product-engg: the previous commit's SQL-only fix is correct for ArgoCD/GitOps pipelines (status persisted in app_status), but Helm pipelines don't have their status persisted there — it's resolved on the fly at listing time (AppListingServiceImpl.updateAppStatusForHelmTypePipelines). A pure aps.status IN (...) predicate would silently exclude such Helm apps (and undercount them) whenever HIBERNATING is requested, since their app_status row doesn't exist (aps.status IS NULL).

Verified in this OSS repo: AppStatusServiceImpl.UpdateStatusWithAppIdEnvId is only reached from GitOps/ArgoCD application-watch events (AppService.UpdateDeploymentStatusForGitOpsPipelines) and from the Argo-only branch of ResourceTreeService.FetchResourceTree (util.IsAcdApp(...) guard) — so a pure-Helm CD pipeline never gets an app_status row written through the normal flow. AppListingService_ent.go#updateAppStatusForHelmTypePipelines is currently a no-op stub in this OSS fork (return nil), i.e. the on-the-fly fetch itself isn't implemented here yet — this fix preserves the code path/contract for when it is (or in the enterprise fork), rather than silently dropping it.

What changed (2nd commit):

  • internal/sql/repository/helper/AppListingRepositoryQueryBuilder.go — buildAppListingWhereCondition: when HIBERNATING is among the requested AppStatuses, the predicate becomes (aps.status IN (?) or (p.deployment_app_type = 'helm' and aps.status IS NULL)), so Helm rows with no persisted status aren't excluded from the SQL query/pagination/count outright. Every other status still matches purely against the persisted column, unchanged.
  • pkg/app/AppListingService.go — FetchAppsByEnvironmentV2: before the on-the-fly Helm status fetch, records which returned rows had AppStatus == "" (i.e. no persisted status). After the on-the-fly fetch runs, reconcileAppsWithUnresolvedStatus rechecks only those rows against the requested filter and drops the ones that don't match, decrementing appSize by exactly the number dropped. Rows whose status was already persisted (and already correctly filtered+paginated in SQL) are left untouched.
  • Added unit tests for both the query-builder predicate (AppListingRepositoryQueryBuilder_app_status_test.go) and the reconciliation logic (AppListingService_hibernating_filter_test.go).

Known trade-off (documented, not hidden): admitting all aps.status IS NULL Helm rows into the paginated SQL window before the post-fetch check means a page can still come back with fewer than Size matching rows if many non-hibernating, not-yet-synced Helm apps land on that page and get dropped afterward. This mirrors the pre-existing behavior of the original (pre-draft) code for this subset, but scoped now to only the unresolved-status rows instead of the entire result set — so it no longer corrupts pagination/count for the ArgoCD/DB-backed majority case, which was the actual reported bug. A fully correct fix for the Helm case (matching on-the-fly status pre-pagination) would require persisting Helm status to app_status or a live-status cache, which is out of scope here.

Build (go build ./internal/sql/repository/helper/... ./pkg/app/) and the package's existing + new unit tests pass locally with -mod=mod (vendor/ in this checkout is missing an unrelated Azure SDK dependency, so go build needed -mod=mod; this is unrelated to this change).

@github-actions

Copy link
Copy Markdown

Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n

@sonarqubecloud

Copy link
Copy Markdown

@Shivam-nagar23

Shivam-nagar23 commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Closing — superseded by a smaller fix, raised on both main and hotfix-v0.46.0 in both repos:

Repo Base PR
devtron-enterprise main devtron-labs/devtron-enterprise#3420
devtron-enterprise hotfix-v0.46.0 devtron-labs/devtron-enterprise#3421
devtron (OSS) main #7049
devtron (OSS) hotfix-v0.46.0 #7050

The root cause identified here was correct and an independent investigation reached the same place: FetchAppsByEnvironmentV2 blanks the whole AppStatuses filter when HIBERNATING is requested, so LIMIT/OFFSET and count(distinct a.id) run over the unfiltered app set and the status filter is re-applied in memory to a single page. Confirmed by querying app_status directly in the reporting environment: the filter returns a tiny fraction of the hibernating apps that exist, and reports that fraction as the total.

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, updateAppStatusForHelmTypePipelines re-enabled over that set, then merge/sort/paginate in Go. Reasons that was not taken:

  1. It removes pagination from the query for exactly the customers who have many apps — the reporting client is that profile.
  2. It re-enables the gRPC fan-out that was deliberately removed in Jul 2025 for slowness (62335a019d, "removed app status for helm type pipeline"), now over up to 100k containers — one GetAppStatusV2 call per helm pipeline, per list request.
  3. In OSS the helm cohort can never match anything (updateAppStatusForHelmTypePipelines is a return nil stub there), so the 100k fetch and merge are pure cost for zero benefit.
  4. The Go merge diverges from the SQL it replaces — it sorts LastDeployedTime as a string where SQL orders by MAX(pco.id), and lowercases app names where Postgres uses collation. Page boundaries would differ between the hibernating-filtered list and every other list.
  5. The cohort predicate dc.deployment_app_type = 'helm' AND aps.status IS NULL doesn't match how deployment type is actually resolved (GetDeploymentAppTypeForCDInBulk, with a gitops-configured fallback), and dc is a LEFT JOIN that may have no row.

The replacement is a pure deletion (11 insertions, 31 deletions). HIBERNATING is an ordinary value persisted in app_status.status since Jun 2024, and the in-memory pass compared against that very same column — so the predicate was always expressible in SQL, and buildAppListingWhereCondition already emits aps.status IN (?) for any status list, NOT DEPLOYED combination included. Only when the filter is applied moves; per-row semantics are unchanged.

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 app_status from the status-sync worker, after which the SQL predicate covers them for free. That is recorded as a TODO in the code and needs its own ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-authored Opened by the pager-duty fix agent. Draft; never merge without review. PR:Ready-to-Review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants