Skip to content
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

Switch ResourceManager update cache to JSON Patch #442

Merged
merged 1 commit into from Oct 11, 2023

Conversation

scothis
Copy link
Contributor

@scothis scothis commented Oct 11, 2023

It was previously using JSON Merge Patch, which will recreate structures that are trying to be deleted. JSON Patch will fail to apply the patch if the structure being patched doesn't exist.

For example, if a reconciler defined a volume on a Deployment on create/update a mutating webhook will default the file permissions. If the reconciler later needs to remove that volume, the defaulted file permissions will no longer be re-applied.

It was previously using JSON Merge Patch, which will recreate structures
that are trying to be deleted. JSON Patch will fail to apply the patch
if the structure being patched doesn't exist.

For example, if a reconciler defined a volume on a Deployment on
create/update a mutating webhook will default the file permissions. If
the reconciler later needs to remove that volume, the defaulted file
permissions will no longer be re-applied.

Signed-off-by: Scott Andrews <andrewssc@vmware.com>
@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (7ebda64) 60.72% compared to head (141c439) 60.65%.
Report is 2 commits behind head on main.

❗ Current head 141c439 differs from pull request most recent head eae3574. Consider uploading reports for the commit eae3574 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #442      +/-   ##
==========================================
- Coverage   60.72%   60.65%   -0.07%     
==========================================
  Files          26       26              
  Lines        2518     2524       +6     
==========================================
+ Hits         1529     1531       +2     
- Misses        902      904       +2     
- Partials       87       89       +2     
Files Coverage Δ
reconcilers/resourcemanager.go 76.80% <45.45%> (-2.20%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@scothis scothis merged commit aaaa87e into vmware-labs:main Oct 11, 2023
2 of 4 checks passed
@scothis scothis deleted the rm-patch branch October 11, 2023 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants