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

[k8s plugin] Prepare for implementing livestate apis #5510

Merged
merged 6 commits into from
Jan 31, 2025

Conversation

Warashi
Copy link
Contributor

@Warashi Warashi commented Jan 24, 2025

What this PR does:

  • [refactor] Implement GetLiveResources in the provider package and use it from the deployment package
  • [feature] initial implementation of BuildApplicationLiveState to implement the livestate feature

Why we need it:

To implement the livestate API for plugin-architectured piped.

Which issue(s) this PR fixes:

Part of #4980

Does this PR introduce a user-facing change?:

  • How are users affected by this change:
  • Is this breaking change:
  • How to migrate (if breaking change):

…cation

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

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
…ve state management

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

codecov bot commented Jan 24, 2025

Codecov Report

Attention: Patch coverage is 58.18182% with 23 lines in your changes missing coverage. Please review.

Project coverage is 26.55%. Comparing base (90170ba) to head (1ab511c).
Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
...ipedv1/plugin/kubernetes/provider/liveresources.go 58.49% 21 Missing and 1 partial ⚠️
...g/app/pipedv1/plugin/kubernetes/deployment/sync.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5510      +/-   ##
==========================================
+ Coverage   26.19%   26.55%   +0.35%     
==========================================
  Files         464      465       +1     
  Lines       49815    49958     +143     
==========================================
+ Hits        13049    13266     +217     
+ Misses      35733    35633     -100     
- Partials     1033     1059      +26     

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

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi Warashi marked this pull request as ready for review January 27, 2025 00:51
Copy link
Member

@t-kikuc t-kikuc 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.

Thank you!

Comment on lines +47 to +48
// BuildApplicationLiveState builds the live state of the application from the given manifests.
func BuildApplicationLiveState(deploytarget string, manifests []Manifest, now time.Time) *model.ApplicationLiveState {
Copy link
Member

Choose a reason for hiding this comment

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

memo: If possible, it would be nice to implement it under the live state package.
I got the thought to hide the Manifest.body from the other package.
Not consider for now.

@Warashi Warashi merged commit c803e2c into master Jan 31, 2025
18 checks passed
@Warashi Warashi deleted the k8s-plugin-build-livestates branch January 31, 2025 07:56
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.

3 participants