Skip to content

[K8s Plugin] Add helper functions for canary stage#5892

Merged
ffjlabo merged 7 commits into
masterfrom
add-helper-functions-for-canary-stage
Jun 3, 2025
Merged

[K8s Plugin] Add helper functions for canary stage#5892
ffjlabo merged 7 commits into
masterfrom
add-helper-functions-for-canary-stage

Conversation

@ffjlabo
Copy link
Copy Markdown
Member

@ffjlabo ffjlabo commented May 28, 2025

What this PR does:

Added 4 functions to use in the canary stage logic from pipedv0.

Why we need it:

To support k8s pipeline sync in the plugin

Which issue(s) this PR fixes:

Part of #5764

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):

@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2025

Codecov Report

Attention: Patch coverage is 66.44295% with 50 lines in your changes missing coverage. Please review.

Project coverage is 27.99%. Comparing base (6f5d4d8) to head (46bd3ff).
Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
...app/pipedv1/plugin/kubernetes/deployment/canary.go 56.00% 31 Missing and 13 partials ⚠️
...bernetes/deployment/yamlprocessor/yamlprocessor.go 87.75% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5892      +/-   ##
==========================================
+ Coverage   27.89%   27.99%   +0.10%     
==========================================
  Files         519      520       +1     
  Lines       55923    56070     +147     
==========================================
+ Hits        15600    15699      +99     
- Misses      39125    39158      +33     
- Partials     1198     1213      +15     
Flag Coverage Δ
. 23.75% <ø> (ø)
.-pkg-app-pipedv1-plugin-example 0.00% <ø> (ø)
.-pkg-app-pipedv1-plugin-kubernetes 63.54% <66.44%> (+0.22%) ⬆️
.-pkg-app-pipedv1-plugin-kubernetes_multicluster 64.93% <ø> (ø)
.-pkg-app-pipedv1-plugin-wait 35.51% <ø> (ø)
.-pkg-plugin-sdk 49.25% <ø> (ø)
.-tool-actions-gh-release 19.23% <ø> (ø)
.-tool-actions-plan-preview 22.83% <ø> (ø)
.-tool-codegen-protoc-gen-auth 0.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ffjlabo ffjlabo changed the title Add helper functions for canary stage [K8s Plugin] Add helper functions for canary stage May 28, 2025
@ffjlabo ffjlabo force-pushed the add-helper-functions-for-canary-stage branch from fccf6a5 to 77f23ed Compare May 29, 2025 03:01
@ffjlabo ffjlabo marked this pull request as ready for review May 29, 2025 03:41
@ffjlabo ffjlabo requested a review from a team as a code owner May 29, 2025 03:41
@Warashi
Copy link
Copy Markdown
Member

Warashi commented May 29, 2025

Sorry, we updated the CI and branch protection rules.
I updated the branch of this PR by pushing the Update branch button, which is the GitHub feature.
Please let me know if you see any unexpected behavior.

@ffjlabo ffjlabo enabled auto-merge (squash) May 29, 2025 06:06
@ffjlabo ffjlabo marked this pull request as draft May 29, 2025 08:31
auto-merge was automatically disabled May 29, 2025 08:31

Pull request was converted to draft

ffjlabo added 3 commits May 29, 2025 17:36
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
@ffjlabo ffjlabo force-pushed the add-helper-functions-for-canary-stage branch from 8fb033d to 396d17e Compare May 29, 2025 08:37
@ffjlabo ffjlabo marked this pull request as ready for review May 29, 2025 08:45
@ffjlabo ffjlabo enabled auto-merge (squash) May 29, 2025 08:45
Copy link
Copy Markdown
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.

Almost LGTM

I got it, testcases are also copied from pipedv0.

)

func Test_findConfigMapManifests(t *testing.T) {
tests := []struct {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

t.Parallel() is possible? (if not, please ignore)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, added on f1c0444

Comment thread pkg/app/pipedv1/plugin/kubernetes/deployment/canary.go Outdated

"github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/config"
"github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/provider"
"github.com/pipe-cd/pipecd/pkg/yamlprocessor"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this yamlprocessor package be part of the SDK as well? 🤔

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, now that you mention it, it might be true.

IMO, it should stay as is for now.
It seems this package is mainly for piped logic and is used only for the canary stage as executor logic for now.
WDYT?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO, official plugins should not depend on pipe-cd/pipecd/pkg/... as much as possible because it causes dependency chaos.

However, I have no idea of the new place for now 😅 (internal/ dir??)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I see.

@khanhtc1202 @t-kikuc
So I think there are two patterns for now. Which is better for you?

  1. Add same package to SDK
  2. Add same package to k8s plugin

IMO, 2 is better because it is not expected to be used for other executors for now.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We decided to choose 2.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copied on 94ca4e8

ffjlabo added 4 commits May 29, 2025 19:05
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
@ffjlabo ffjlabo requested a review from khanhtc1202 June 2, 2025 02:03
@ffjlabo ffjlabo requested a review from t-kikuc June 2, 2025 02:03
Copy link
Copy Markdown
Member

@Warashi Warashi left a comment

Choose a reason for hiding this comment

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

LGTM

@ffjlabo ffjlabo merged commit d989499 into master Jun 3, 2025
40 checks passed
@ffjlabo ffjlabo deleted the add-helper-functions-for-canary-stage branch June 3, 2025 05:24
@github-actions github-actions Bot mentioned this pull request Jul 14, 2025
@github-actions github-actions Bot mentioned this pull request Aug 15, 2025
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.

4 participants