Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes switchover support by ensuring the sidecar is injected into all cluster pods, not just the primary. Previously, sidecars were only injected into the primary pod, causing issues during switchovers when pod specs didn't align between current and target primaries.
- Removes the primary-only sidecar injection restriction in the lifecycle plugin
- Adds logic to ensure only the primary pod performs hibernation operations while all pods monitor activity
- Updates tests to include current primary information and adds new test scenarios for primary detection
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/plugin/lifecycle/lifecycle.go | Removes the check that restricted sidecar injection to primary pods only |
| internal/sidecar/scale_to_zero.go | Adds isPrimary() method and logic to ensure only primary pods hibernate clusters |
| internal/sidecar/scale_to_zero_test.go | Updates existing tests and adds new test cases for primary detection scenarios |
| doc/development.md | Updates documentation to reflect the new behavior of all pods having sidecars |
| README.md | Updates overview to clarify that sidecars are injected into all pods |
| .pre-commit-config.yaml | Adds a local hook to generate manifests during pre-commit checks |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
eminano
added
the
build-images
eminano
temporarily deployed
to
pr-image-build
GitHub Actions
Inactive
eminano
temporarily deployed
to
pr-image-build
GitHub Actions
Inactive
|
🚀 PR Images Built Successfully! Image Tags:
Manifest: Download the These images are for testing purposes and will be automatically cleaned up when the PR is closed. |
eminano
added
build-images
eminano
temporarily deployed
to
pr-image-build
GitHub Actions
Inactive
|
🚀 PR Images Built Successfully! Image Tags:
Manifest: Download the These images are for testing purposes and will be automatically cleaned up when the PR is closed. |
eminano
removed
the
build-images
| if !s.isPrimary(cluster) { | ||
| // reset last active time when it's not the primary to make sure | ||
| // when there's a switchover, the new primary has a clean state | ||
| s.lastActive = time.Time{} |
|
🧹 PR Resources Cleaned Up Since this PR has been merged, the following test resources have been automatically deleted:
Thanks for testing! 🎉 |
3 participants
Until now the sidecar was only injected into the primary, which caused issues during switch over due to the pod spec not being aligned between the current primary and the target primary, resulting in a crash loop backoff.
In order to resolve this, the sidecar container will be now injecting into all cluster pods, but only the primary will hibernate/suspend backups. This allows for a more robust logic, where a switchover is supported and the scale to zero behaviour is seamlessly transitioned to the new primary.
For now, only the primary keeps track of activity, so any switchover/restart will reset the inactivity timeout timer.