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.

Sign up for GitHub

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

Jump to bottom

refactor: server side apply over create / update #3075

Merged
merged 42 commits into from
Nov 6, 2024
Merged

refactor: server side apply over create / update #3075

merged 42 commits into from
Nov 6, 2024

Conversation

AustinAbro321
Copy link
Contributor

Description

There are several situations where we create, check if the resource already existed, and if it does then we update. It is simpler in this situation to just use server side apply

Related Issue

Fixes #2626

Checklist before merging

@AustinAbro321
changing create/updates to use ssa
466e27a 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@netlify Netlify
Copy link

netlify bot commented Oct 7, 2024 •
edited
Loading

✅ Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit 20f1069
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/672a5d3b544adb0008a78936

@AustinAbro321
WIP, more to do but tests passing
3e2fd88 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
merge
b978890 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
tests passing
bdb756d 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
patch-> apply
9d1a35d 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
remove patches
9f757e9 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
tests passing
0219569 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
merge
0b17664 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
fix debug
21d8c04 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@codecov Codecov
Copy link

codecov bot commented Oct 17, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 77.89474% with 42 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/pkg/cluster/zarf.go 0.00% 19 Missing ⚠️
src/pkg/packager/remove.go 0.00% 9 Missing ⚠️
src/internal/packager/helm/post-render.go 0.00% 8 Missing ⚠️
src/pkg/packager/deploy.go 0.00% 4 Missing ⚠️
src/pkg/cluster/state.go 85.71% 2 Missing ⚠️
Files with missing lines Coverage Δ
src/pkg/cluster/cluster.go 0.00% <ø> (ø)
src/pkg/cluster/injector.go 66.35% <100.00%> (-3.65%) ⬇️
src/pkg/cluster/namespace.go 16.66% <100.00%> (-22.47%) ⬇️
src/pkg/cluster/secrets.go 63.06% <100.00%> (-3.61%) ⬇️
src/pkg/cluster/state.go 60.33% <85.71%> (-0.65%) ⬇️
src/pkg/packager/deploy.go 5.41% <0.00%> (+0.10%) ⬆️
src/internal/packager/helm/post-render.go 12.85% <0.00%> (+1.56%) ⬆️
src/pkg/packager/remove.go 0.00% <0.00%> (ø)
src/pkg/cluster/zarf.go 14.83% <0.00%> (+2.46%) ⬆️

... and 2 files with indirect coverage changes

@AustinAbro321
health checks
4516b5f 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
pointer
905711d 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
using correct svc again
71eb153 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
ssa in zarf
7c38c13 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
add field manager
00868f4 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
adopt existing resources
b121ded 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
comment
e83f6e6 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
delete comment
570b244 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
add comment
5736f56 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
make injection pod and service easier to read
ca48559 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
injector
52643f7 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
remove redundant test
72f07c9 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
fix post render logic
5cae3e5 
Signed-off-by: Austin Abro <austinabro321@gmail.com>
@AustinAbro321
fix post render logic
16ed156 
Signed-off-by: Austin Abro <austinabro321@gmail.com>
@AustinAbro321
better error
f1d1886 
Signed-off-by: Austin Abro <austinabro321@gmail.com>
AustinAbro321 and others added 5 commits October 24, 2024 17:00
@AustinAbro321
newline
e825432 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
consistent creation
aa8c22b 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
verbiage
dc8bc71 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
Merge branch 'main' into try-out-ssa
dd5f752
@AustinAbro321
Merge branch 'try-out-ssa' of github.com:zarf-dev/zarf into try-out-ssa
f5e30a8
@AustinAbro321 AustinAbro321 marked this pull request as ready for review October 24, 2024 17:07
@AustinAbro321 AustinAbro321 requested review from a team as code owners October 24, 2024 17:07
@AustinAbro321 AustinAbro321 changed the title refactor: change create or update if not exists operations to use server side apply refactor: server side apply over create / update Oct 24, 2024
phillebaba
phillebaba reviewed Oct 25, 2024
View reviewed changes
Copy link
Member

@phillebaba phillebaba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comment. Additionally have we looked into if it is possible to set a global field manager?

src/pkg/cluster/state.go Show resolved Hide resolved
@AustinAbro321
Copy link
Contributor Author

@phillebaba As far as I can tell it's not possible to set a global field manager. I thought about making a function like cluster.DefaultApplyOpts(), lmk if you'd prefer that

AustinAbro321 and others added 2 commits October 29, 2024 14:13
@AustinAbro321
Merge branch 'main' into try-out-ssa
94376a5
@AustinAbro321
merge
ad93bcb 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@phillebaba
Copy link
Member

For now just make a constant called something like FieldManagerName so that we can track where it is being used.

AustinAbro321 and others added 5 commits November 1, 2024 16:40
@AustinAbro321
field manager name
a00784d 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
no nil needed
c15887e 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
remove maps equal logic
e1ff23f 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
newline
ac86e82 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
Merge branch 'main' into try-out-ssa
e6d25d3
@AustinAbro321
merge
245dfc4 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321
spinner
20f1069 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
@AustinAbro321 AustinAbro321 added this pull request to the merge queue Nov 6, 2024
Merged via the queue into main with commit e0040ab Nov 6, 2024
26 checks passed
@AustinAbro321 AustinAbro321 deleted the try-out-ssa branch November 6, 2024 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phillebaba phillebaba phillebaba approved these changes

Assignees
No one assigned
Labels
ready-for-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use Server Side Apply when creating Kubernetes resources
2 participants
@AustinAbro321 @phillebaba