Skip to content

[K8s Plugin] Implement K8S_BASELINE_ROLLOUT#5870

Merged
Warashi merged 14 commits into
masterfrom
k8s-plugin-baseline-rollout
May 29, 2025
Merged

[K8s Plugin] Implement K8S_BASELINE_ROLLOUT#5870
Warashi merged 14 commits into
masterfrom
k8s-plugin-baseline-rollout

Conversation

@Warashi
Copy link
Copy Markdown
Member

@Warashi Warashi commented May 26, 2025

What this PR does:

  • fix resource key annotation bug in the baseline/canary resources
  • implement K8S_BASELINE_ROLLOUT and its test

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

Warashi added 6 commits May 26, 2025 10:20
@Warashi
[K8s Plugin] Implement baseline rollout stage for Kubernetes deployment
884eea8 
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi
[K8s Plugin] Add tests and example configurations for baseline rollou…
ce2e596 
…t stage

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi
[K8s Plugin] Enhance variant labels and annotations handling
5af4d4e 
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi
[K8s Plugin] Add additional assertions for labels, annotations, and s…
cc8deb9 
…ervice properties in baseline tests

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi
Port fixes to k8s_multicluster plugin
460588f 
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi
Remove 'pipecd.dev/variant: baseline' label from deployment and servi…
09083ac 
…ce YAML files in baseline rollout tests

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

codecov Bot commented May 26, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 55.45455% with 49 lines in your changes missing coverage. Please review.

Project coverage is 27.89%. Comparing base (44a0c00) to head (daa15fa).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...p/pipedv1/plugin/kubernetes/deployment/baseline.go 52.88% 35 Missing and 14 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #5870      +/-   ##
==========================================
+ Coverage   27.85%   27.89%   +0.04%     
==========================================
  Files         519      519              
  Lines       55793    55900     +107     
==========================================
+ Hits        15539    15594      +55     
- Misses      39070    39108      +38     
- Partials     1184     1198      +14
Flag Coverage Δ
. 23.73% <ø> (-0.02%) ⬇️
.-pkg-app-pipedv1-plugin-example 0.00% <ø> (ø)
.-pkg-app-pipedv1-plugin-kubernetes 63.32% <55.45%> (-0.29%) ⬇️
.-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.58% <ø> (ø)
.-tool-actions-plan-preview 23.22% <ø> (ø)
.-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.

@Warashi
Refactor baseline test to use updated unstructured.NestedStringMap an…
1656704 
…d NestedSlice calls for service spec

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi Warashi marked this pull request as ready for review May 26, 2025 02:32
@Warashi Warashi requested a review from a team as a code owner May 26, 2025 02:32
t-kikuc
t-kikuc previously approved these changes May 26, 2025
View reviewed changes
ffjlabo
ffjlabo reviewed May 26, 2025
View reviewed changes
Comment thread pkg/app/pipedv1/plugin/kubernetes/deployment/misc.go Outdated

// addVariantLabelsAndAnnotations adds the variant label and annotation to the given manifests.
// This also adds the resource key to the manifest.
// This is because the resource key differs between variants.
Copy link
Copy Markdown
Member

@ffjlabo ffjlabo May 26, 2025

Choose a reason for hiding this comment

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

[ask] Can you show me an example of the expected case of this comment?

Copy link
Copy Markdown
Member Author

@Warashi Warashi May 26, 2025

Choose a reason for hiding this comment

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

Thank you.
I added it as a comment because it's better to have it in this comment.
https://github.com/pipe-cd/pipecd/pull/5870/files/1656704e0390b8235ee6d2ec7391bbd456252886..ff25ca493ff82d1b5840bfb686a950ca596494e6

Copy link
Copy Markdown
Member

@ffjlabo ffjlabo May 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you. I understood that.
IMO, it's better to fix the resource key annotation when generating variant manifests (insidegenerateBaselineManifests) to separate the responsibility for each function. WDYT?

Copy link
Copy Markdown
Member Author

@Warashi Warashi May 26, 2025

Choose a reason for hiding this comment

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

I agree with you; it's clear to do this when generating variant manifests.
I'll change the code.

Copy link
Copy Markdown
Member Author

@Warashi Warashi May 26, 2025

Choose a reason for hiding this comment

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

I changed the code.
a4b1510

Comment thread pkg/app/pipedv1/plugin/kubernetes_multicluster/deployment/misc.go Outdated
Comment on lines +42 to +43
variantLabel: variant,
provider.LabelResourceKey: m.Key().String(),
Copy link
Copy Markdown
Member

@ffjlabo ffjlabo May 26, 2025

Choose a reason for hiding this comment

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

[nit] add the same comment for k8s plugins' fixes

Copy link
Copy Markdown
Member Author

@Warashi Warashi May 26, 2025

Choose a reason for hiding this comment

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

Thank you.
added
https://github.com/pipe-cd/pipecd/pull/5870/files/1656704e0390b8235ee6d2ec7391bbd456252886..ff25ca493ff82d1b5840bfb686a950ca596494e6

@Warashi
Enhance documentation for addVariantLabelsAndAnnotations function
3ede487 
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
Warashi added 3 commits May 26, 2025 12:59
@Warashi
Update comment
ff25ca4 
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi
Move responsibility of adding resource key to the manifest generator
a4b1510 
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi
Fix test
41156d6 
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi Warashi requested review from ffjlabo and t-kikuc May 26, 2025 04:45
ffjlabo
ffjlabo previously approved these changes May 26, 2025
View reviewed changes
Copy link
Copy Markdown
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.

LGTM

t-kikuc
t-kikuc previously approved these changes May 27, 2025
View reviewed changes
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.

Let's merge after separating the SDK (and solving conflicts)

@Warashi
Merge branch 'master' into k8s-plugin-baseline-rollout
7533bc8 
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi Warashi dismissed stale reviews from t-kikuc and ffjlabo via 7533bc8 May 29, 2025 04:54
@Warashi
Run go fmt
00bc95f 
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi
Fix test imports
daa15fa 
Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>
@Warashi
Copy link
Copy Markdown
Member Author

Warashi commented May 29, 2025

Solved the conflicts and import error

@Warashi Warashi requested review from ffjlabo and t-kikuc May 29, 2025 05:51
@Warashi Warashi merged commit 23b62f8 into master May 29, 2025
40 checks passed
@Warashi Warashi deleted the k8s-plugin-baseline-rollout branch May 29, 2025 07:00
@github-actions github-actions Bot mentioned this pull request Jun 2, 2025
Merged
@github-actions github-actions Bot mentioned this pull request Jul 14, 2025
Merged
@github-actions github-actions Bot mentioned this pull request Aug 15, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ffjlabo ffjlabo ffjlabo approved these changes

@t-kikuc t-kikuc t-kikuc approved these changes

@khanhtc1202 khanhtc1202 Awaiting requested review from khanhtc1202 khanhtc1202 is a code owner automatically assigned from pipe-cd/pipecd-approvers

Assignees

No one assigned

Labels

area/go area/pipedv1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Warashi @ffjlabo @t-kikuc