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

ci: add helm generation checker pipeline #1150

Merged
merged 9 commits into from
Apr 11, 2023
Merged

ci: add helm generation checker pipeline #1150

merged 9 commits into from
Apr 11, 2023

Conversation

rakshitgondwal
Copy link
Member

@rakshitgondwal rakshitgondwal commented Mar 29, 2023 •
edited
Loading

Fixes #926

This pipeline checks whether helm was re-generated in a PR when new fields are added to the manifests.

@github-actions github-actions bot added the ops label Mar 29, 2023
@netlify
Copy link

netlify bot commented Mar 29, 2023 •
edited
Loading

✅ Deploy Preview for keptn-lifecycle-toolkit ready!

Name Link
🔨 Latest commit 740e907
🔍 Latest deploy log https://app.netlify.com/sites/keptn-lifecycle-toolkit/deploys/64353b9f2012450008e46425
😎 Deploy Preview https://deploy-preview-1150--keptn-lifecycle-toolkit.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@codecov
Copy link

codecov bot commented Mar 30, 2023 •
edited
Loading

Codecov Report

Merging #1150 (8978d0c) into main (fa53443) will decrease coverage by 0.03%.
The diff coverage is n/a.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1150      +/-   ##
==========================================
- Coverage   59.03%   59.00%   -0.03%     
==========================================
  Files         128      128              
  Lines       10040    10040              
==========================================
- Hits         5927     5924       -3     
- Misses       3896     3898       +2     
- Partials      217      218       +1

see 1 file with indirect coverage changes

Flag Coverage Δ
certificate-operator 64.15% <ø> (ø)
component-tests 60.35% <ø> (-0.22%) ⬇️
lifecycle-operator 78.24% <ø> (ø)
metrics-operator 77.57% <ø> (ø)
scheduler 21.75% <ø> (ø)

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

mowies
mowies reviewed Apr 4, 2023
View reviewed changes
Copy link
Member

@mowies mowies left a comment •
edited
Loading

Choose a reason for hiding this comment

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

can you please combine your new file with the helm docs validation file? they both handle the helm chart so i think it makes sense to put them into one pipeline with 2 jobs inside.
other than that, great work!

@rakshitgondwal
Copy link
Member Author

can you please combine your new file with the helm docs validation file? they both handle the helm chart so i think it makes sense to put them into one pipeline with 2 jobs inside. other than that, great work!

Do we require this pipeline on the type pull_request only? or on type push too?

@mowies
Copy link
Member

mowies commented Apr 5, 2023

I think both is good, the validate helm docs pipeline should also run on both.

@rakshitgondwal
Copy link
Member Author

So I should update the validate helm docs pipeline to type push too right?

@mowies
Copy link
Member

mowies commented Apr 5, 2023

yes pls :)

mowies
mowies reviewed Apr 5, 2023
View reviewed changes
Copy link
Member

@mowies mowies left a comment

Choose a reason for hiding this comment

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

IMHO validate-helm-chart.yaml is a better name for this pipeline since it doesn't only checks the helm docs anymore. I suggest changing it together with the pipeline name inside the file

mowies
mowies reviewed Apr 6, 2023
View reviewed changes
.github/workflows/validate-helm-chart.yml Outdated Show resolved Hide resolved
.github/workflows/validate-helm-chart.yml Outdated Show resolved Hide resolved
mowies
mowies previously approved these changes Apr 6, 2023
View reviewed changes
Copy link
Member

@mowies mowies left a comment

Choose a reason for hiding this comment

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

LGTM!

thisthat
thisthat previously approved these changes Apr 6, 2023
View reviewed changes
Copy link
Member

@thisthat thisthat left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @rakshitgondwal

odubajDT
odubajDT previously approved these changes Apr 6, 2023
View reviewed changes
Copy link
Contributor

@odubajDT odubajDT 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 for contribution! :)

@rakshitgondwal rakshitgondwal dismissed stale reviews from odubajDT, thisthat, and mowies via 07c4ea3 April 6, 2023 13:39
mowies
mowies previously approved these changes Apr 11, 2023
View reviewed changes
Copy link
Member

@mowies mowies left a comment

Choose a reason for hiding this comment

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

LGTM!

@rakshitgondwal
add helm generation checker pipeline
69862b5 
Signed-off-by: Rakshit Gondwal <[email protected]>
@rakshitgondwal
ci: remove install helm step
5641d84 
Signed-off-by: Rakshit Gondwal <[email protected]>
@rakshitgondwal
ci: fix after review
2ddc795 
Signed-off-by: Rakshit Gondwal <[email protected]>
mowies
mowies previously approved these changes Apr 11, 2023
View reviewed changes
Copy link
Member

@mowies mowies left a comment

Choose a reason for hiding this comment

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

LGTM!

@odubajDT odubajDT self-requested a review April 11, 2023 10:55
odubajDT
odubajDT requested changes Apr 11, 2023
View reviewed changes
Copy link
Contributor

@odubajDT odubajDT left a comment

Choose a reason for hiding this comment

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

Hello @rakshitgondwal , thank you very much for your contribution! Can you please check the failed linter check? Thank you!

@rakshitgondwal
fix: yamllint erros
8978d0c 
Signed-off-by: Rakshit Gondwal <[email protected]>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@mowies
Copy link
Member

mowies commented Apr 11, 2023

Thanks for the contribution @rakshitgondwal ! We will fix the missing auto-generated files for the helm chart in a separate PR after this one.

@mowies mowies merged commit d8d94f0 into keptn:main Apr 11, 2023
@rakshitgondwal
Copy link
Member Author

Thanks a lot for the merge!

@rakshitgondwal rakshitgondwal mentioned this pull request Apr 15, 2023
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RealAnna RealAnna RealAnna left review comments

@odubajDT odubajDT odubajDT approved these changes

@mowies mowies mowies approved these changes

@thisthat thisthat thisthat left review comments

@thschue thschue Awaiting requested review from thschue

@bacherfl bacherfl Awaiting requested review from bacherfl

@philipp-hinteregger philipp-hinteregger Awaiting requested review from philipp-hinteregger

Assignees
No one assigned
Labels
ops
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Helm generation checker pipeline
5 participants
@rakshitgondwal @mowies @thisthat @RealAnna @odubajDT