fix(e2e): update ground control URL to match service name#341
Open
maishivamhoo123 wants to merge 2 commits intocontainer-registry:mainfrom
Open
fix(e2e): update ground control URL to match service name#341maishivamhoo123 wants to merge 2 commits intocontainer-registry:mainfrom
maishivamhoo123 wants to merge 2 commits intocontainer-registry:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughAdded environment variable-based crash simulation during ZTR registration to support end-to-end testing. Includes conditional sleep in registration process and new E2E test suite orchestrating token generation, satellite pause, crash simulation, and recovery verification. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as E2E Test Suite
participant Sat as Satellite<br/>(ZTR Process)
participant Env as Environment<br/>(E2E_SIMULATE_ZTR_CRASH)
participant Logs as Verification<br/>(Log Analysis)
Test->>Sat: generate-token (register satellite)
Sat->>Sat: obtain registration token
Test->>Sat: start-satellite-ztr-paused (with env var set)
Sat->>Env: check E2E_SIMULATE_ZTR_CRASH
Env-->>Sat: "true"
Sat->>Sat: log warning & sleep 10s
Note over Sat: Satellite paused during ZTR
Test->>Sat: crash-and-restart-ztr (kill container)
Sat-->>Test: container stopped
Test->>Sat: restart satellite (env var unset)
Sat->>Env: check E2E_SIMULATE_ZTR_CRASH
Env-->>Sat: not set
Sat->>Sat: continue registration normally
Sat->>Sat: complete ZTR registration
Test->>Logs: verify-ztr-recovery (inspect logs)
Logs-->>Test: confirm successful recovery
Test->>Test: cleanup-crash-ztr
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codacy's Analysis Summary0 new issue (≤ 0 issue) Review Pull Request in Codacy →
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/state/registration_process.go`:
- Around line 155-161: The registerSatellite function currently reads
E2E_SIMULATE_ZTR_CRASH and calls time.Sleep(10 * time.Second) which both ships a
test hook into production and blocks ignoring ctx cancellation; replace the
blocking sleep with a context-aware wait (use select on time.After and ctx.Done
or a context-aware helper like sleepWithContext(ctx, 10*time.Second)) so the
function returns promptly on ctx cancellation, and remove the env-driven test
hook from the main file by either moving the hook into a separate file guarded
with a //go:build e2e build tag or by adding an injectable sleepFn parameter
(default no-op in production) that tests can override; reference
registerSatellite, the E2E_SIMULATE_ZTR_CRASH check, and time.Sleep when making
these changes.
In `@taskfiles/e2e.yml`:
- Line 1047: The shared temp file /tmp/e2e_satellite_token causes stale-token
cross-test interference between the crash-recovery and ZTR-crash flows; change
the ZTR-crash-specific tasks to use a unique token path (e.g.
/tmp/e2e_ztr_crash_token): update the producer task generate-token (the
invocation that echoes TOKEN) used for ZTR crash and the consumer tasks
crash-and-restart-ztr and any related steps (crash-and-restart-satellite or
test-crash-ztr) to read/write the new path, and ensure any CI task names that
refer to the old path (test-crash-ztr, cleanup-crash-recovery references) are
updated accordingly so only the ZTR-crash flow uses /tmp/e2e_ztr_crash_token.
- Around line 1093-1106: The verify-ztr-recovery task is grepping for log
strings that don't exist (it looks for "registration successful" and "starting
replication"); update the verify-ztr-recovery target in taskfiles/e2e.yml (the
verify-ztr-recovery cmds block) to use real signals of recovery: search logs for
the existing "replicating" message from state_process.go and/or verify the
satellite container is running with no recent ERROR/PANIC entries (e.g., check
`docker inspect` for .State.Running and grep logs for -i "error|panic" absence)
since the registration completion only closes z.Done and emits no success
message. Ensure the new check combines a positive indicator ("replicating" or
running state) and a lack of errors so the task exits 0 on true recovery and 1
otherwise.
- Around line 1058-1066: In the docker run lines for the
start-satellite-ztr-paused and crash-and-restart-ztr tasks, the environment
variable passed is GROUND_CONTROL_TOKEN=$TOKEN but the satellite binary reads
TOKEN; change the env var name to TOKEN and quote the value to preserve
spacing/characters (i.e., replace -e GROUND_CONTROL_TOKEN=$TOKEN with -e
TOKEN="$TOKEN" in both affected docker run commands) so the satellite receives
the token and ZTR registration can proceed.
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
1 participant
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.
Description
This PR Fixed the Crash during SPIFFE ZTR (x509pop)
Summary by cubic
Updates e2e to use the correct Ground Control host (gc) and adds a crash-recovery test for SPIFFE ZTR (x509pop). Fixes registration failures and confirms the satellite recovers after a forced crash.
Bug Fixes
New Features
Written for commit 5fb46f6. Summary will update on new commits.
Summary by CodeRabbit
Release Notes