-
Notifications
You must be signed in to change notification settings - Fork 271
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: calculate SSA diffs with smd.merge.Updater #467
Conversation
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Codecov ReportBase: 55.40% // Head: 55.46% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #467 +/- ##
==========================================
+ Coverage 55.40% 55.46% +0.06%
==========================================
Files 41 41
Lines 4480 4504 +24
==========================================
+ Hits 2482 2498 +16
- Misses 1807 1813 +6
- Partials 191 193 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
k8s Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
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.
lgtm! Gonna ask @alexmt for a review, mostly for awareness.
Leo spoke with Alex about this change at ArgoCon, so I'm comfortable merging it. |
This PR introduces a different strategy to calculate diffs while using server-side apply.
The new approach will delegate all diff calculation logic to the
merge.Updater.Apply
function in thestructured-merge-diff
k8s library.The previous implementation was naive and was missing many edge cases already addressed by
merge.Updater.Apply
.However,
merge.Updater.Apply
has a hard to solve dependencies requirements. In order to provide the necessary dependencies, this PR borrows code from the k8s api-server which is properly documented in thepkg.diff.internal.fieldmanager
package (seedoc.go
).Fix argoproj/argo-cd#10627