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

Add force-sync-by-replace annotation option #5175

Merged
merged 4 commits into from
Sep 3, 2024
Merged

Conversation

Warashi
Copy link
Contributor

@Warashi Warashi commented Sep 3, 2024

What this PR does / why we need it:

Add an option to replace the Kubernetes resources forcefully.

Which issue(s) this PR fixes:

Fixes #5174

Does this PR introduce a user-facing change?:
Yes

  • How are users affected by this change:
    Users can replace resources forcefully.
    This allows users to run the k8s job each time deployment occurs.

  • Is this breaking change: No

  • How to migrate (if breaking change): N/A

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Copy link

codecov bot commented Sep 3, 2024

Codecov Report

Attention: Patch coverage is 38.54167% with 59 lines in your changes missing coverage. Please review.

Project coverage is 22.90%. Comparing base (a295485) to head (05f4d15).
Report is 32 commits behind head on master.

Files with missing lines Patch % Lines
...g/app/piped/platformprovider/kubernetes/kubectl.go 0.00% 33 Missing ⚠️
...g/app/piped/platformprovider/kubernetes/applier.go 0.00% 26 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5175      +/-   ##
==========================================
+ Coverage   22.88%   22.90%   +0.02%     
==========================================
  Files         416      416              
  Lines       44607    44690      +83     
==========================================
+ Hits        10208    10238      +30     
- Misses      33607    33660      +53     
  Partials      792      792              

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

ffjlabo
ffjlabo previously approved these changes Sep 3, 2024
Copy link
Member

@ffjlabo ffjlabo left a comment

Choose a reason for hiding this comment

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

Only a comment fix is needed, and the other seems good.

@@ -137,6 +139,32 @@ func (a *applier) ReplaceManifest(ctx context.Context, manifest Manifest) error
return err
}

// ReplaceManifest uses kubectl to replace the given manifests.
Copy link
Member

Choose a reason for hiding this comment

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

[nit] ReplaceManifest -> ForceReplaceManifest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 05f4d15

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@ffjlabo ffjlabo left a comment

Choose a reason for hiding this comment

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

🚀

@Warashi Warashi merged commit e9dd4c5 into master Sep 3, 2024
17 checks passed
@Warashi Warashi deleted the force-sync-by-replace branch September 3, 2024 06:28
github-actions bot pushed a commit that referenced this pull request Sep 4, 2024
* Add test for force-sync-by-replace

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

* Add ForceReplaceManifest methods

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

* Add force-sync-by-replace handling

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

* Fix the godoc comment

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

---------

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: pipecd-bot <pipecd.dev@gmail.com>
@github-actions github-actions bot mentioned this pull request Sep 4, 2024
Warashi added a commit that referenced this pull request Sep 4, 2024
* Add test for force-sync-by-replace



* Add ForceReplaceManifest methods



* Add force-sync-by-replace handling



* Fix the godoc comment



---------

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: pipecd-bot <pipecd.dev@gmail.com>
Co-authored-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@github-actions github-actions bot mentioned this pull request Sep 10, 2024
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.

Support kubectl replace with force option
3 participants