test(bdd): cover control observability install - #564
Open
sbaum1994 wants to merge 4 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds a BDD scenario for installing the observability control profile in a local k3d environment. It adds ServiceMonitor assertions, mocked harness wiring, generated-value checks, and live execution. ChangesObservability control profile
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Godog
participant Helmfile
participant Kubernetes
Godog->>Helmfile: Install local-bdd-observability-control
Helmfile->>Kubernetes: Deploy control profile resources
Godog->>Kubernetes: Verify releases and monitor resources
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/bdd/features/observability-control.feature`:
- Around line 38-40: Update the conflict precheck comment in the topology
bootstrap feature to include the exact required `make destroy` remediation
command, replacing the current ncp-local-cluster command while preserving the
surrounding port-overlap guidance.
- Around line 69-72: Update the observability-control feature’s target allocator
assertion to query .spec.targetAllocator.enabled directly with kubectl jsonpath
and verify the result is true, rather than relying on independent YAML substring
checks. Adjust the corresponding fake-runner command and expected result in
godog_test.go to match the new query.
- Around line 83-88: Strengthen the monitor absence checks in the
observability-control scenario by asserting Kubernetes NotFound error text, not
only exit code 1, for each servicemonitor and podmonitor command. Update the
corresponding fake runner responses to emit the same NotFound output so the
scenario verifies missing resources rather than unrelated command failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 21906d5b-8e2f-490a-8b8c-3f44a39944f8
📒 Files selected for processing (2)
tests/bdd/features/observability-control.featuretests/bdd/godog_test.go
sbaum1994
force-pushed
the
test/bdd-observability-control-pr2
branch
from
July 30, 2026 22:19
70f2bf1 to
3030758
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/bdd/godog_test.go`:
- Around line 466-496: Update TestObservabilityControlFeatureFileWiresToSteps
and its newFakeRunner command map to add explicit successful canned results for
the helm registry login command and all four expected positive servicemonitor
lookups, using the exact generated command strings. Preserve the existing
install-step post-run substring assertion and current negative monitor results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 60a86ead-09ea-4435-994b-e6fb3929e161
📒 Files selected for processing (2)
tests/bdd/features/observability-control.featuretests/bdd/godog_test.go
Add a live Helmfile scenario that provisions ncp-local with the control profile and verifies shared releases, target allocation, control monitors, and compute monitor exclusions. Select function-autoscaler chart 0.2.0 for the current token template and override its unavailable appVersion with the multi-architecture 1.18.10 image for local arm64 validation. Refs: #522, #527 Signed-off-by: Stephanie Baum <sbaum@nvidia.com>
sbaum1994
force-pushed
the
test/bdd-observability-control-pr2
branch
from
July 30, 2026 22:30
3030758 to
101c24e
Compare
Signed-off-by: Stephanie Baum <sbaum@nvidia.com>
Signed-off-by: Stephanie Baum <sbaum@nvidia.com>
kristinapathak
approved these changes
Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Issue #522 requires live coverage for each observability profile. Rendering cannot verify that the
controlprofile installs its shared dependencies, selects the correct monitors, or runs the function autoscaler on local arm64.What changed
ncp-localHelmfile scenario forobservability.profile: control.0.2.0with multi-architecture image1.18.10.kubectl get, plus DSL and wiring tests.Customer Release Notes
Not customer visible.
Plan Summary
Installs the self-managed and observability stacks on
k3d-ncp-local. Defaults are unchanged.Usage
From
tests/bdd:Testing
go test -short ./...: passed../scripts/lint.sh: passed.k3d-ncp-localrun: 32/32 steps passed in 3m07s.Notes
PR #523 supplies the tracked Makefile wrapper. Until it merges, the feature stages
Makefile.distthrough the BDD ledger.References
Related Pull Requests
Dependencies
None. No license or NOTICE changes.
Issues
Relates to #522