-
Notifications
You must be signed in to change notification settings - Fork 880
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: rs conflict with fallback to patch #3559
Merged
zachaller
merged 46 commits into
argoproj:master
from
zachaller:fix-rs-conflict-with-fallback-to-patch
Jun 7, 2024
Merged
Changes from all commits
Commits
Show all changes
46 commits
Select commit
Hold shift + click to select a range
7943d8d
fix: fallback to patch on scale conflict
zachaller 650e86d
typo
zachaller ae05623
cleanup commented out code
zachaller 29fe254
Trigger Build
zachaller 8bfb37d
only patch rollouts manged fields
zachaller 7526220
lint
zachaller 3822105
fix flaky test
zachaller b30b93d
fix flaky test
zachaller 5e2bda5
reduce patch size
zachaller 0dd919e
get some logs
zachaller 90d5ada
cleanup
zachaller aa8f677
improve tests
zachaller 332ef46
Trigger Build
zachaller a73bfa5
add env var to log diff
zachaller 23f6883
remove expirment rs patch
zachaller a6297b6
imporve logs
zachaller a913d1c
use correct variable
zachaller 401c8c8
change env var
zachaller 0e6db13
fix flaky e2e
zachaller 3810f1d
fix flaky e2e
zachaller 2384b7e
fix flaky e2e
zachaller 89ed30f
remove not found rollouts
zachaller 61a56bb
update replica count
zachaller d9a6655
lint
zachaller faaafc6
refactor cleanup
zachaller 2aeaf63
keep track of UpdatedReplicas on sync
zachaller 7411720
some hpa tests and log changes
zachaller f4a2e27
remove update to UpdatedReplicas
zachaller bec20f2
add more test
zachaller a9e11ef
fix test
zachaller 1a6fe3b
undo change
zachaller 8454637
add comment to flaky tests
zachaller 2b73c42
cleanup Makefile
zachaller 5f09e84
Merge branch 'master' of github.com:argoproj/argo-rollouts into fix-r…
zachaller 66b2e59
remove test
zachaller cce7ac4
use labels
zachaller 1272d22
remove make file change
zachaller bbcc80e
add label to test
zachaller e1bc540
review changes
zachaller 4ebd960
change to TODO
zachaller fed308b
fix test
zachaller 046fde9
Merge branch 'master' of github.com:argoproj/argo-rollouts into fix-r…
zachaller e39bb6e
add extra logging for tests
zachaller 697a55f
Trigger Build
zachaller e52400d
add ignore to codecov
zachaller ec9d9b0
we always generate patch because we are comparing against emtpy obj
zachaller File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,3 +19,5 @@ ignore: | |
- 'pkg/client/.*' | ||
- 'vendor/.*' | ||
- '**/mocks/*' | ||
- 'hack/gen-crd-spec/main.go' | ||
- 'hack/gen-docs/main.go' |
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
Oops, something went wrong.
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.
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 should probably add several test cases to validate the new
updateReplicaSetFallbackToPatch
methodThere 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.
There are two test that cover most the happy paths to lock in the behavior on the two conflict points here. I don't think it's worth going down a lot of the error paths like incorrect json marshal's etc.
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.
codecovs percentages don't always make sense to me, here is the coverage of that function https://app.codecov.io/gh/argoproj/argo-rollouts/pull/3559?src=pr&el=tree&filepath=rollout%2Fcontroller.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=argoproj#6b8a01c38fada35656503847a6f4a00d-R951
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.
It seems that the coverage is comparing the wrong base from the link above.