Skip to content

feat(orchestrator): include orchestrator namespaces in namespace-inspect#149

Merged
rm3l merged 8 commits into
redhat-developer:mainfrom
rm3l:feat/add_orch_namespaces_to_namespace_inspect
May 10, 2026
Merged

feat(orchestrator): include orchestrator namespaces in namespace-inspect#149
rm3l merged 8 commits into
redhat-developer:mainfrom
rm3l:feat/add_orch_namespaces_to_namespace_inspect

Conversation

@rm3l

@rm3l rm3l commented May 6, 2026

Copy link
Copy Markdown
Member

Description

When orchestrator resources are detected (Serverless operators, Knative namespaces, SonataFlowPlatform CRs, SonataFlow workflows), gather_orchestrator now writes their namespaces to a detected-namespaces.txt file. gather_namespace-inspect reads this file during auto-detection and merges those namespaces into the oc adm inspect command, providing deeper diagnostic coverage of orchestrator infrastructure.

When the user explicitly specifies --namespaces, orchestrator namespaces are not appended — the user's explicit scope is respected.

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

PR acceptance criteria

  • Tests
  • Documentation

How to test changes / Special notes to the reviewer

  1. On a cluster with orchestrator components, run a default must-gather and verify orchestrator namespaces (e.g., openshift-serverless, knative-serving) appear in the namespace-inspect/ output
  2. Run with --namespaces ns1 and verify only ns1 is inspected (no orchestrator namespaces added)
  3. Run with --without-orchestrator and verify detected-namespaces.txt is not created, so namespace-inspect skips orchestrator namespaces
  4. Unit tests: make test — all 157 BATS tests pass

When orchestrator resources are detected (Serverless operators, Knative
namespaces, SonataFlowPlatform CRs, SonataFlow workflows),
gather_orchestrator now writes their namespaces to a
detected-namespaces.txt file. gather_namespace-inspect reads this file
during auto-detection and merges those namespaces into the inspection
list, giving deeper diagnostic coverage of orchestrator infrastructure.

When the user explicitly specifies --namespaces, only those namespaces
are inspected — orchestrator namespaces are not appended, respecting
the user's explicit scope.

Assisted-by: Claude
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label May 6, 2026
Comment thread tests/namespace-inspect.bats Fixed
Comment thread tests/namespace-inspect.bats Fixed
Comment thread tests/orchestrator.bats Fixed
Comment thread tests/orchestrator.bats Fixed
rm3l added 3 commits May 6, 2026 11:25
… merge

Move the empty-namespaces exit check to after orchestrator namespaces
are merged, so clusters with only orchestrator components (and no
direct RHDH deployments) are still inspected.

Assisted-by: Claude
Simplify grep patterns in orchestrator and namespace-inspect tests
to avoid escaped dollar signs inside single quotes, which ShellCheck
flags as unexpanded expressions.

Assisted-by: Claude
@github-actions

Copy link
Copy Markdown

PR images are available (for 1 week):

  1. quay.io/rhdh-community/rhdh-must-gather:pr-149
  2. quay.io/rhdh-community/rhdh-must-gather:pr-149-6f8656db2
  3. quay.io/rhdh-community/rhdh-must-gather:0.0.0-6f8656db2-pr-149

@rm3l rm3l marked this pull request as ready for review May 10, 2026 18:58
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label May 10, 2026
@rhdh-qodo-merge

Copy link
Copy Markdown

Review Summary by Qodo

Include orchestrator namespaces in namespace-inspect inspection

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Orchestrator namespaces automatically included in namespace-inspect
• gather_orchestrator writes detected namespaces to file
• gather_namespace-inspect merges orchestrator namespaces during auto-detection
• Respects explicit --namespaces flag by not appending orchestrator namespaces
• Comprehensive test coverage for orchestrator namespace tracking
Diagram
flowchart LR
  A["gather_orchestrator"] -->|"detects namespaces"| B["detected-namespaces.txt"]
  B -->|"read during auto-detect"| C["gather_namespace-inspect"]
  C -->|"merges with RHDH"| D["namespaces_to_inspect array"]
  D -->|"oc adm inspect"| E["namespace-inspect output"]
  F["--namespaces flag"] -->|"skips merge"| C
Loading

Grey Divider

File Changes

1. collection-scripts/gather_orchestrator ✨ Enhancement +19/-0

Track orchestrator namespaces for namespace-inspect

• Initialize ORCH_NAMESPACES_FILE to track detected orchestrator namespaces
• Append namespace to file when OpenShift Serverless, Serverless Logic, Knative Serving/Eventing
 detected
• Append SonataFlowPlatform and SonataFlow workflow namespaces to tracking file
• Deduplicate namespaces using sort -u and log final list

collection-scripts/gather_orchestrator


2. collection-scripts/gather_namespace-inspect ✨ Enhancement +28/-6

Merge orchestrator namespaces into inspection scope

• Move empty-namespaces exit check after orchestrator namespace merge
• Read detected-namespaces.txt from orchestrator directory during auto-detection
• Merge orchestrator namespaces into inspection list with deduplication logic
• Skip orchestrator namespace merge when user explicitly specifies --namespaces

collection-scripts/gather_namespace-inspect


3. tests/orchestrator.bats 🧪 Tests +50/-0

Test orchestrator namespace file tracking

• Add tests for ORCH_NAMESPACES_FILE initialization and writing
• Verify namespace tracking for Serverless, Serverless Logic, Knative components
• Test SonataFlowPlatform and SonataFlow workflow namespace tracking
• Validate namespace deduplication using sort -u

tests/orchestrator.bats


View more (1)
4. tests/namespace-inspect.bats 🧪 Tests +68/-0

Add namespace-inspect integration tests

• New test file for gather_namespace-inspect script validation
• Verify script reads orchestrator detected-namespaces.txt file
• Test orchestrator namespace addition and deduplication logic
• Validate auto-detection behavior and empty file handling
• Confirm exit message when no namespaces found

tests/namespace-inspect.bats


Grey Divider

Qodo Logo

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented May 10, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Dedup before normalization ✓ Resolved 🐞 Bug ☼ Reliability
Description
In gather_namespace-inspect, orchestrator namespaces are deduplicated before whitespace
normalization, but the script trims whitespace afterwards; this can allow duplicates (e.g., "ns" vs
" ns") to survive and be passed multiple times to oc adm inspect. This can also inflate computed
timeouts because they scale with the raw array length.
Code

collection-scripts/gather_namespace-inspect[R96-107]

+        while IFS= read -r orch_ns; do
+            [[ -z "$orch_ns" ]] && continue
+            already_included=false
+            for existing_ns in "${namespaces_to_inspect[@]:-}"; do
+                if [[ "$existing_ns" == "$orch_ns" ]]; then
+                    already_included=true
+                    break
+                fi
+            done
+            if [[ "$already_included" != "true" ]]; then
+                namespaces_to_inspect+=("$orch_ns")
+                log_info "Added orchestrator namespace: $orch_ns"
Evidence
The orchestrator merge loop compares raw strings and appends them, and only later the script runs an
unconditional whitespace-trim across the entire namespaces_to_inspect array; because trimming
happens after the dedup comparison, two values that normalize to the same namespace can both remain
in the array and be used to build the oc adm inspect args and timeout multiplier.

collection-scripts/gather_namespace-inspect[91-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
`gather_namespace-inspect` deduplicates orchestrator namespaces before normalizing them, but later trims whitespace from all namespaces. This ordering can allow duplicates that normalize to the same value (or whitespace-only lines) to leak into `namespaces_to_inspect`, causing redundant `oc adm inspect` targets and inflated timeouts.

### Issue Context
- Orchestrator namespaces are read from `orchestrator/detected-namespaces.txt`.
- The script currently trims whitespace only after merging namespaces.

### Fix Focus Areas
- collection-scripts/gather_namespace-inspect[91-123]

### Suggested fix approach
- Normalize `orch_ns` immediately after reading (e.g., `orch_ns=$(echo "$orch_ns" | xargs)`), then skip if empty.
- Normalize `existing_ns` similarly (or ensure the base list is normalized earlier) before comparing.
- After the final trimming loop, consider removing empty entries and re-deduplicating the array once (e.g., via `printf '%s\n' ... | sort -u`).

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



Advisory comments

2. Grep-only integration tests ✓ Resolved 🐞 Bug ☼ Reliability
Description
The newly added BATS tests for namespace-inspect/orchestrator mostly assert string presence via grep
rather than executing the scripts with controlled inputs, so they can pass even if the runtime
integration is broken. This reduces the chance of catching regressions in the new
orchestrator-namespace merge behavior.
Code

tests/namespace-inspect.bats[R36-57]

+@test "gather_namespace-inspect reads orchestrator detected-namespaces.txt" {
+    run grep -q 'orchestrator/detected-namespaces.txt' "${SCRIPTS_DIR}/gather_namespace-inspect"
+    [ "$status" -eq 0 ]
+}
+
+@test "gather_namespace-inspect adds orchestrator namespaces to inspection" {
+    run grep -q 'Adding orchestrator-related namespaces' "${SCRIPTS_DIR}/gather_namespace-inspect"
+    [ "$status" -eq 0 ]
+}
+
+@test "gather_namespace-inspect deduplicates orchestrator namespaces" {
+    run grep -q 'already_included=true' "${SCRIPTS_DIR}/gather_namespace-inspect"
+    [ "$status" -eq 0 ]
+}
+
+@test "gather_namespace-inspect handles empty orchestrator namespaces file" {
+    # The -s test returns false for empty files, so the block is skipped
+    run grep -q 'orch_ns_file' "${SCRIPTS_DIR}/gather_namespace-inspect"
+    [ "$status" -eq 0 ]
+    run grep -- '-s' "${SCRIPTS_DIR}/gather_namespace-inspect"
+    [ "$status" -eq 0 ]
+}
Evidence
The new tests primarily use grep -q against the script source to confirm the code exists, but do
not run gather_namespace-inspect with a prepared orchestrator/detected-namespaces.txt and mocked
oc/helm/kubectl to validate the behavior (e.g., that namespaces are actually merged/deduped
and not merged when RHDH_TARGET_NAMESPACES is set).

tests/namespace-inspect.bats[36-68]

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

### Issue description
Current tests for the new orchestrator namespace behavior mostly grep for strings in the script, which doesn’t validate runtime behavior.

### Issue Context
You can test behavior without a real cluster by:
- Creating a temp `BASE_COLLECTION_PATH`
- Writing a fake `orchestrator/detected-namespaces.txt`
- Mocking `oc`, `kubectl`, `helm`, and `jq` in PATH (or as exported functions) to return deterministic outputs

### Fix Focus Areas
- tests/namespace-inspect.bats[36-68]

### Suggested test additions
- Test auto-detect mode:
 - Create `${BASE_COLLECTION_PATH}/orchestrator/detected-namespaces.txt` with duplicates and whitespace variants.
 - Mock `oc adm inspect` to capture args to a file.
 - Assert the final args include orchestrator namespaces exactly once.
- Test explicit namespaces mode:
 - Set `RHDH_TARGET_NAMESPACES=ns1` and ensure orchestrator namespaces are not appended.
- Test empty file:
 - Ensure empty detected file does not change inspected namespaces.

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


3. Empty namespaces file artifact ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
gather_orchestrator always creates/truncates orchestrator/detected-namespaces.txt at startup, even
when no orchestrator components are detected, leaving an empty file in the output bundle. This can
be misleading because the presence of the file does not imply any orchestrator namespaces were
actually detected.
Code

collection-scripts/gather_orchestrator[R21-24]

+# File to track orchestrator-related namespaces for gather_namespace-inspect
+ORCH_NAMESPACES_FILE="$orchestrator_dir/detected-namespaces.txt"
+: > "$ORCH_NAMESPACES_FILE"
+
Evidence
The script truncates/creates the detected-namespaces file unconditionally, and later only sorts/logs
if it is non-empty; therefore clusters without orchestrator components still get an empty
detected-namespaces.txt file whenever the orchestrator collector runs.

collection-scripts/gather_orchestrator[15-24]
collection-scripts/gather_orchestrator[438-442]

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

### Issue description
`gather_orchestrator` always creates/truncates `detected-namespaces.txt` even when nothing orchestrator-related is detected, producing an empty output artifact that can confuse users/tools.

### Issue Context
`gather_namespace-inspect` already uses `[[ -s file ]]` so it only consumes non-empty files.

### Fix Focus Areas
- collection-scripts/gather_orchestrator[15-24]
- collection-scripts/gather_orchestrator[438-442]

### Suggested fix approach
- Don’t create/truncate the file at startup.
- Instead, create it lazily on the first write (e.g., `echo ... >> "$ORCH_NAMESPACES_FILE"` after ensuring the directory exists), or create it only if `orchestrator_detected=true` at the end.
- Optionally remove the file at the end if it exists but is empty.

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


Grey Divider

Qodo Logo

@github-actions

Copy link
Copy Markdown

PR images are available (for 1 week):

  1. quay.io/rhdh-community/rhdh-must-gather:pr-149
  2. quay.io/rhdh-community/rhdh-must-gather:pr-149-6f8656db2
  3. quay.io/rhdh-community/rhdh-must-gather:0.0.0-6f8656db2-pr-149

rm3l added 4 commits May 10, 2026 21:49
Orchestrator namespaces read from detected-namespaces.txt were
deduplicated before whitespace trimming, allowing entries like
" foo " and "foo" to both appear in the final list. This caused
redundant oc adm inspect targets and inflated timeouts.

Normalize each namespace (trim whitespace) immediately upon reading,
both for orchestrator entries and existing entries during comparison.
Replace the post-merge trim-only loop with a full cleanup pass that
trims, removes empty entries, and deduplicates the entire array.

Assisted-by: Claude
The script unconditionally truncated detected-namespaces.txt at
startup, leaving an empty file when no orchestrator components were
found. This can confuse users and downstream tools inspecting the
output artifacts.

Remove the eager truncation (`: > "$ORCH_NAMESPACES_FILE"`). All
writes already use append (>>), so the file is created naturally on
the first detected namespace. gather_namespace-inspect already
guards with `[[ -s file ]]`, so file absence is handled correctly.

Assisted-by: Claude
…or tests

The previous tests only grepped for strings in the script source,
which validated code presence but not runtime behavior. Replace them
with tests that actually execute the script against mock binaries
(oc, kubectl, helm, jq) and verify the resulting oc adm inspect
arguments.

New tests cover:
- Deduplication of orchestrator namespaces with whitespace variants
- Explicit --namespaces mode ignoring orchestrator namespaces
- Empty detected-namespaces.txt producing no extra namespaces
- Absent detected-namespaces.txt handled gracefully
- Merging auto-detected and orchestrator namespaces without duplicates
- Whitespace-only lines in detected-namespaces.txt being ignored

Assisted-by: Claude
@github-actions

Copy link
Copy Markdown

PR images are available (for 1 week):

  1. quay.io/rhdh-community/rhdh-must-gather:pr-149
  2. quay.io/rhdh-community/rhdh-must-gather:pr-149-4dd9bf09e
  3. quay.io/rhdh-community/rhdh-must-gather:0.0.0-4dd9bf09e-pr-149

@rm3l rm3l merged commit 81be148 into redhat-developer:main May 10, 2026
7 checks passed
@rm3l rm3l deleted the feat/add_orch_namespaces_to_namespace_inspect branch May 10, 2026 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants