only register beholder with chip router if the latter is running#22263
Merged
only register beholder with chip router if the latter is running#22263
Conversation
Contributor
|
I see you updated files related to
|
Contributor
|
✅ No conflicts with other open PRs targeting |
|
|
product-security-plaid-production
Bot
requested review from
george-dorin and
simsonraj
Contributor
There was a problem hiding this comment.
Pull request overview
Risk Rating: MEDIUM
This PR changes how Beholder interacts with the local ChIP router: it now reads router connection details from persisted local CRE state, skips router registration when the router is unavailable, and tries to preserve prior Beholder state across env start failures/interruptions. In the broader codebase, these changes sit in the local CRE lifecycle and test helper path, so they affect both developer workflows and system-test setup.
Changes:
- Switched chiprouter client initialization to load router endpoints from persisted local CRE state instead of
CTF_CONFIGS. - Made Beholder startup register with the router only when
chiprouter.EnsureStartedsucceeds. - Added restoration of persisted Beholder state during
env startpanic/signal recovery and ensured the state directory exists before rewritingchip_ingress.toml.
Scrupulous human review areas:
- Error handling around
chiprouter.EnsureStartedin Beholder startup, especially which failures should be ignored versus surfaced. - Recovery/cleanup flows in
env start, because they now recreate only part of the deleted Beholder state after startup interruption/failure. - Interactions between persisted local state, router subscriber IDs, and
env beholder stop.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
system-tests/lib/cre/chiprouter/router.go |
Changes chiprouter client bootstrap to read from persisted local CRE state. |
core/scripts/cre/environment/environment/environment.go |
Threads persisted Beholder state through env start recovery/signal handling. |
core/scripts/cre/environment/environment/beholder.go |
Removes mandatory router startup check, delays conditional router registration, and updates Beholder state restoration. |
mchain0
approved these changes
May 5, 2026
|
skudasov
approved these changes
May 5, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
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
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.
No description provided.