feat(helm): collect dependency workloads from multi-Deployment releases [Intelligent Assistant OKP] by maysunfaisal · Pull Request #361 · redhat-developer/rhdh-must-gather

maysunfaisal · 2026-09-09T21:46:57Z

Description

Native RHDH Helm releases can contain multiple Deployments, such as the main RHDH workload and Intelligent Assistant's standalone OKP workload.

Previously, gather_helm treated all Deployment names as a single value, resulting in an invalid Kubernetes resource name and incomplete diagnostic collection.

This change:

Iterates over every Deployment rendered by a native Helm release.
Identifies the Deployment containing backstage-backend as the primary RHDH workload.
Collects additional Deployments under:
helm/releases/ns=<namespace>/<release>/dependencies/<deployment>/
Collects Deployment details, rollout history, pods, and current/previous per-container logs for dependency workloads.
Skips Backstage-specific application metadata and heap dumps for dependency workloads.
Documents the resulting directory structure and adds regression tests.

LCORE remains covered through the existing per-container collection because it runs as a sidecar in the main RHDH pod. OKP is collected as a separate dependency Deployment.

Which issue(s) does this PR fix or relate to

https://redhat.atlassian.net/browse/RHIDP-16868
(raised in redhat-developer/rhdh-chart#500 (comment))

PR acceptance criteria

Tests
Documentation

How to test changes / Special notes to the reviewer

Automated validation

pre-commit run --all-files
make test

The Linux BATS test suite passes all 164 tests. Pre-commit hooks pass without generated-file drift.

Test against an OpenShift deployment

Use a native Helm deployment of RHDH with Intelligent Assistant and OKP enabled:

make run-local \
  BASE_COLLECTION_PATH=./out \
  OPTS="--namespaces <namespace> --without-operator --without-orchestrator --without-platform --without-route --without-ingress --without-namespace-inspect"

Inspect the collected files:

find ./out/helm/releases/ns=<namespace>/<release> -type f | sort

Expected results:

The main RHDH Deployment is collected under deployment/.
LCORE logs are collected under the main pod's container=lightspeed-core/ directory.
The OKP Deployment is collected under dependencies/<okp-deployment>/.
OKP Deployment details, pods, rollout history, and container logs are present.
Backstage-specific application data and heap dumps are not collected for OKP.

This was validated on OpenShift using an RHDH Helm release containing:

Main RHDH Deployment with backstage-backend and lightspeed-core
Separate OKP Deployment with the okp container

github-actions · 2026-09-09T22:16:37Z

PR images are available (for 1 week):

quay.io/rhdh-community/rhdh-must-gather:pr-361
quay.io/rhdh-community/rhdh-must-gather:pr-361-5a6ba8245

maysunfaisal · 2026-09-10T18:10:39Z

@rm3l Could you please take a look at this PR? Thank you.

rm3l · 2026-09-15T11:19:36Z

@rm3l Could you please take a look at this PR? Thank you.

Thanks @maysunfaisal. Note that we are currently rewriting the must-gather to Golang in #356
The PR is still under review but should hopefully be merged soon.
So it'll be better if your PR is recreated after that refactoring PR is merged. Thanks.

rm3l

@rm3l Could you please take a look at this PR? Thank you.

Thanks @maysunfaisal. Note that we are currently rewriting the must-gather to Golang in #356 The PR is still under review but should hopefully be merged soon. So it'll be better if your PR is recreated after that refactoring PR is merged. Thanks.

FYI, the rewrite PR has just been merged. Please recreate this PR in Golang. Thanks.

maysunfaisal · 2026-09-15T23:08:16Z

@rm3l PTAL I have made teh changes in golang and verified with a rhdh-chart deployment while working on https://redhat.atlassian.net/browse/RHIDP-16949

github-actions · 2026-09-15T23:18:52Z

PR images are available (for 1 week):

quay.io/rhdh-community/rhdh-must-gather:pr-361
quay.io/rhdh-community/rhdh-must-gather:pr-361-6131a3f8d

rm3l · 2026-09-16T09:19:06Z

/agentic_review

rhdh-qodo-merge · 2026-09-16T09:24:34Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. A dependency becomes primary 🐞 Bug ≡ Correctness ⭐ New

Description

selectPrimaryDeployment returns the alphabetically first Deployment when none contains
backstage-backend, so an unrelated workload is assigned to deployment/ with SkipAppData
disabled. This occurs when the main Deployment is absent or its lookup fails while a dependency
remains, causing dependency process collection and primary-path output despite the documented
dependency policy.

Code

internal/collector/helm.go[628]
+	return sorted[0]

Relevance

●●● Strong
Fallback violates the PR’s primary/dependency policy and can enable Backstage-only collection on
unrelated workloads.
PR-#255

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Failed Deployment lookups are skipped before selection, and the selector explicitly falls back to
the first remaining name. The caller treats that pointer as primary and disables SkipAppData,
while CollectWorkload uses that flag to enable process and heap-dump collection.

internal/collector/helm.go[209-228]
internal/collector/helm.go[612-628]
internal/collector/workload.go[107-123]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When no fetched Deployment contains `backstage-backend`, primary selection falls back to an unrelated dependency. The caller then stores that dependency as the primary workload and enables Backstage-specific collection for it.

## Fix Focus Areas
- internal/collector/helm.go[218-238]
- internal/collector/helm.go[612-628]

## Recommended Fix
Return `nil` when no Deployment contains `backstage-backend`. In that case, retain the warning and collect every fetched Deployment under `dependencies/<deployment>` with `SkipAppData` enabled rather than designating an arbitrary primary.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Primary deployments overwrite diagnostics~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

collectReleaseData assigns every Deployment containing backstage-backend to the same
releaseDir/deployment directory. When a release contains multiple matching Deployments, later
collections overwrite workload metadata and rollout history while leaving mixed pod logs and
application data from earlier collections.

Code

internal/collector/helm.go[R215-217]

+			outDir := filepath.Join(releaseDir, "deployment")
+			skipAppData := false
+			if !deploymentHasContainer(dep, backstageContainer) {

Relevance

●●● Strong
Multiple primary deployments share fixed output paths, deterministically overwriting metadata and
rollout-history artifacts.
PR-#255

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new loop independently classifies every rendered Deployment, and each matching item receives the
same output path. CollectWorkload writes fixed metadata and rollout-history file names into that
path while pod-specific directories can accumulate, producing a collection whose files describe
different Deployments.

internal/collector/helm.go[207-230]
internal/collector/workload.go[47-54]
internal/collector/workload.go[78-124]
README.md[341-347]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Every Deployment containing `backstage-backend` is currently treated as primary and written to the same output directory, causing diagnostic files to be overwritten or mixed when more than one Deployment matches.

## Fix Focus Areas
- internal/collector/helm.go[207-230]
- internal/collector/helm_test.go[91-158]

## Recommended Fix
Resolve all rendered Deployments first and select exactly one deterministic primary Deployment before collecting them. Route only that Deployment to `deployment/`, route every remaining Deployment to its own `dependencies/<name>/` directory, define a fallback when none contains `backstage-backend`, and add tests covering multiple and zero primary candidates.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Cross-namespace dependencies are missed~~ ✗ Dismissed 🐞 Bug ≡ Correctness

Description

extractWorkloadNames discards metadata.namespace, and the new loop resolves every extracted name
through Deployments(ns) where ns is the Helm release namespace. A release rendering a dependency
into another namespace therefore encounters a failed lookup and continues without collecting that
workload or its diagnostics.

Code

internal/collector/helm.go[209]

+			dep, err := cfg.Client.Clientset.AppsV1().Deployments(ns).Get(ctx, deployName, metav1.GetOptions{})

Relevance

●●● Strong
Cross-namespace manifests cannot be retrieved when lookups always use the Helm release namespace.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Release processing passes the release namespace into collectReleaseData, while the extractor
retains only names and the added lookup always uses that release namespace. The error branch
immediately continues, and the same namespace would otherwise be propagated into WorkloadRef, so
an explicitly namespaced rendered Deployment cannot reach collection.

internal/collector/helm.go[126-135]
internal/collector/helm.go[207-229]
internal/collector/helm.go[567-583]
internal/collector/workload.go[41-52]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Rendered Deployment namespaces are discarded, so dependencies explicitly targeting a namespace other than the Helm release namespace are looked up in the wrong place and skipped.

## Fix Focus Areas
- internal/collector/helm.go[207-229]
- internal/collector/helm.go[567-590]
- internal/collector/helm_test.go[91-135]

## Recommended Fix
Return each Deployment name together with its rendered namespace, defaulting an omitted namespace to the release namespace. Use the resolved namespace for the live lookup, `WorkloadRef`, logging, and processed-workload key, honor configured namespace scope, and add extraction and collection tests for explicit cross-namespace manifests.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

✅ Compliance rules (platform): 7 rules

✅ Cross-repo context — repo relationships

Explored: repo: redhat-developer/rhdh-chart (sha: e6e1319d)

Explored: repo: redhat-developer/rhdh (sha: b8140abe)

Review mode: ⚖️ Balanced: This push changes runtime Helm workload discovery and collection behavior across multiple related paths, creating real correctness risk but not enough independent logic to warrant redundant extended review.

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 32d9deb

Results up to commit 6131a3f ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Cross-namespace dependencies are missed~~ ✗ Dismissed 🐞 Bug ≡ Correctness

Description

extractWorkloadNames discards metadata.namespace, and the new loop resolves every extracted name
through Deployments(ns) where ns is the Helm release namespace. A release rendering a dependency
into another namespace therefore encounters a failed lookup and continues without collecting that
workload or its diagnostics.

Code

internal/collector/helm.go[209]

+			dep, err := cfg.Client.Clientset.AppsV1().Deployments(ns).Get(ctx, deployName, metav1.GetOptions{})

Relevance

●●● Strong
Cross-namespace manifests cannot be retrieved when lookups always use the Helm release namespace.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Release processing passes the release namespace into collectReleaseData, while the extractor
retains only names and the added lookup always uses that release namespace. The error branch
immediately continues, and the same namespace would otherwise be propagated into WorkloadRef, so
an explicitly namespaced rendered Deployment cannot reach collection.

internal/collector/helm.go[126-135]
internal/collector/helm.go[207-229]
internal/collector/helm.go[567-583]
internal/collector/workload.go[41-52]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Rendered Deployment namespaces are discarded, so dependencies explicitly targeting a namespace other than the Helm release namespace are looked up in the wrong place and skipped.

## Fix Focus Areas
- internal/collector/helm.go[207-229]
- internal/collector/helm.go[567-590]
- internal/collector/helm_test.go[91-135]

## Recommended Fix
Return each Deployment name together with its rendered namespace, defaulting an omitted namespace to the release namespace. Use the resolved namespace for the live lookup, `WorkloadRef`, logging, and processed-workload key, honor configured namespace scope, and add extraction and collection tests for explicit cross-namespace manifests.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Primary deployments overwrite diagnostics~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

collectReleaseData assigns every Deployment containing backstage-backend to the same
releaseDir/deployment directory. When a release contains multiple matching Deployments, later
collections overwrite workload metadata and rollout history while leaving mixed pod logs and
application data from earlier collections.

Code

internal/collector/helm.go[R215-217]

+			outDir := filepath.Join(releaseDir, "deployment")
+			skipAppData := false
+			if !deploymentHasContainer(dep, backstageContainer) {

Relevance

●●● Strong
Multiple primary deployments share fixed output paths, deterministically overwriting metadata and
rollout-history artifacts.
PR-#255

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new loop independently classifies every rendered Deployment, and each matching item receives the
same output path. CollectWorkload writes fixed metadata and rollout-history file names into that
path while pod-specific directories can accumulate, producing a collection whose files describe
different Deployments.

internal/collector/helm.go[207-230]
internal/collector/workload.go[47-54]
internal/collector/workload.go[78-124]
README.md[341-347]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Every Deployment containing `backstage-backend` is currently treated as primary and written to the same output directory, causing diagnostic files to be overwritten or mixed when more than one Deployment matches.

## Fix Focus Areas
- internal/collector/helm.go[207-230]
- internal/collector/helm_test.go[91-158]

## Recommended Fix
Resolve all rendered Deployments first and select exactly one deterministic primary Deployment before collecting them. Route only that Deployment to `deployment/`, route every remaining Deployment to its own `dependencies/<name>/` directory, define a fallback when none contains `backstage-backend`, and add tests covering multiple and zero primary candidates.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

rm3l

Looks good, thanks. Worth addressing Qodo's comment: #361 (comment)

Native Helm releases can render multiple Deployments. Collecting only the first misses dependencies such as the Intelligent Assistant OKP workload. Collect every rendered Deployment, keep the one containing backstage-backend as the primary workload, and gather other Deployments under dependencies without Backstage-specific process or heap data. Document the layout and cover detection behavior in Go tests. Assisted-by: Claude Co-authored-by: Codex <noreply@openai.com>

github-actions · 2026-09-16T23:48:09Z

PR images are available (for 1 week):

quay.io/rhdh-community/rhdh-must-gather:pr-361
quay.io/rhdh-community/rhdh-must-gather:pr-361-32d9deb6b

maysunfaisal · 2026-09-16T23:49:41Z

@rm3l addressed qodo review, PTAL, thank you!

rm3l · 2026-09-17T20:37:51Z

/agentic_review

rhdh-qodo-merge · 2026-09-17T20:41:33Z

Code review by qodo was updated up to the latest commit 32d9deb

openshift-ci Bot requested review from OpinionatedHeron and rm3l September 9, 2026 21:47

github-advanced-security Bot found potential problems Sep 9, 2026

View reviewed changes

Comment thread tests/helm.bats Fixed

Comment thread tests/helm.bats Fixed

maysunfaisal changed the title ~~feat(helm): collect dependency workloads from native releases~~ feat(helm): collect dependency workloads from multi-Deployment releases [Intelligent Assistant OKP] Sep 9, 2026

maysunfaisal force-pushed the collect-intelligent-assistant-workloads-helm branch 2 times, most recently from fd1359f to 5a6ba82 Compare September 9, 2026 22:02

rm3l requested changes Sep 15, 2026

View reviewed changes

maysunfaisal force-pushed the collect-intelligent-assistant-workloads-helm branch from 5a6ba82 to 6131a3f Compare September 15, 2026 23:07

maysunfaisal requested a review from rm3l September 15, 2026 23:09

rhdh-qodo-merge Bot reviewed Sep 16, 2026

View reviewed changes

Comment thread internal/collector/helm.go Outdated

Comment thread internal/collector/helm.go

rm3l reviewed Sep 16, 2026

View reviewed changes

maysunfaisal force-pushed the collect-intelligent-assistant-workloads-helm branch from 6131a3f to 32d9deb Compare September 16, 2026 23:35

rhdh-qodo-merge Bot reviewed Sep 17, 2026

View reviewed changes

Comment thread internal/collector/helm.go

rm3l approved these changes Sep 17, 2026

View reviewed changes

rm3l merged commit 6fcc783 into redhat-developer:main Sep 17, 2026
9 checks passed

Conversation

maysunfaisal commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Which issue(s) does this PR fix or relate to

PR acceptance criteria

How to test changes / Special notes to the reviewer

Automated validation

Test against an OpenShift deployment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

maysunfaisal commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rm3l commented Sep 15, 2026

Uh oh!

rm3l left a comment

Choose a reason for hiding this comment

Uh oh!

maysunfaisal commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 15, 2026

Uh oh!

rm3l commented Sep 16, 2026

Uh oh!

rhdh-qodo-merge Bot commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

rm3l left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 16, 2026

Uh oh!

maysunfaisal commented Sep 16, 2026

Uh oh!

rm3l commented Sep 17, 2026

Uh oh!

Uh oh!

rhdh-qodo-merge Bot commented Sep 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

maysunfaisal commented Sep 9, 2026 •

edited

Loading

maysunfaisal commented Sep 10, 2026 •

edited

Loading

maysunfaisal commented Sep 15, 2026 •

edited

Loading

rhdh-qodo-merge Bot commented Sep 16, 2026 •

edited

Loading