-
Notifications
You must be signed in to change notification settings - Fork 19
fix: ignore deployment.kubernetes.io/revision annotation #322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ignore deployment.kubernetes.io/revision annotation #322
Conversation
a6c6a6a to
bc86a09
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ryanzhang-oss
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a e2e that place a deployment to verify this works
…k applier Signed-off-by: Wei Weng <Wei.Weng@microsoft.com>
Signed-off-by: Wei Weng <Wei.Weng@microsoft.com>
bc86a09 to
80e9f29
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes issue #319 by ensuring the deployment.kubernetes.io/revision annotation is ignored during resource placement operations. This annotation is automatically set by the Kubernetes deployment controller and should not be considered when comparing resource states during placement.
Key Changes:
- Added logic to filter out the
deployment.kubernetes.io/revisionannotation in resource sanitization - Updated dependencies to include
k8s.io/kubectlpackage for theRevisionAnnotationconstant - Added comprehensive E2E test coverage for deployment placement scenarios
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/controllers/workapplier/apply.go | Added deletion of deployment.RevisionAnnotation in sanitizeManifestObject() |
| pkg/controllers/workapplier/apply_test.go | Added test coverage for revision annotation removal |
| pkg/controllers/placement/resource_selector.go | Added deletion of deployment.RevisionAnnotation in generateRawContent() |
| pkg/controllers/placement/resource_selector_test.go | Added test coverage for revision annotation removal |
| test/e2e/resource_placement_deployment_test.go | New E2E test file for deployment placement scenarios |
| go.mod | Updated k8s.io/metrics version and added k8s.io/kubectl dependency |
| go.sum | Updated checksums for new and upgraded dependencies |
| apis/placement/v1beta1/zz_generated.deepcopy.go | Auto-generated code formatting change (import alias) |
3f97b22 to
80e9f29
Compare
Description of your changes
Fixes #319
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
Special notes for your reviewer