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

chore: test the helm chart in the PR #829

Merged
merged 2 commits into from
Jan 5, 2022

Conversation

krol3
Copy link
Contributor

@krol3 krol3 commented Nov 24, 2021

  • Run the testing helm chart in the PR (only 'deploy/helm/**')
  • Separate the release chart in another job.

@codecov
Copy link

codecov bot commented Nov 24, 2021

Codecov Report

Merging #829 (53b7173) into main (df182b7) will increase coverage by 0.10%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #829      +/-   ##
==========================================
+ Coverage   65.61%   65.72%   +0.10%     
==========================================
  Files          58       58              
  Lines        6460     6460              
==========================================
+ Hits         4239     4246       +7     
+ Misses       1791     1789       -2     
+ Partials      430      425       -5     
Impacted Files Coverage Δ
pkg/operator/controller/configauditreport.go 64.41% <0.00%> (-0.30%) ⬇️
pkg/operator/controller/ciskubebenchreport.go 59.38% <0.00%> (+1.31%) ⬆️
pkg/operator/controller/vulnerabilityreport.go 64.68% <0.00%> (+1.74%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df182b7...53b7173. Read the comment docs.

@krol3
Copy link
Contributor Author

krol3 commented Dec 7, 2021

@danielpacak please validate the idea of test the manifest of the helm in the PR, if make sense.

@danielpacak danielpacak self-requested a review December 10, 2021 15:14
Copy link
Contributor

@danielpacak danielpacak 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 working on this @krol3 ! I think it's great idea to test the Helm chart as part of our build pipeline.

We have already grouped all checks related to pull requests in a single file called .github/worflow/build.yaml so I'd add such job in there and run it after all integration tests passed for few reasons:

  • If tests fail we gonna restart build anyway so doing all these Helm checks would be wasteful.
    starboard-ci-workflow
  • Maybe not in scope of this PR but ... after creating a Helm release it would be very nice to run actual integration tests to see the the operator deployed with Helm really works (not just that Helm release was created).
  • We'd like to get rid of the workflow_dispatch event and publish to Helm repository (and MkDocs) when the release workflow is complete.

@krol3
Copy link
Contributor Author

krol3 commented Jan 3, 2022

@danielpacak the "Test Helm Chart" was moved to the build job.

@krol3 krol3 requested a review from danielpacak January 3, 2022 20:39
Signed-off-by: Daniel Pacak <pacak.daniel@gmail.com>
Copy link
Contributor

@danielpacak danielpacak left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @krol3 I just slightly updated the workflow to run Helm chart tests only after integration tests passed (as suggested in my previous review) 🍀 🌏 .

@danielpacak danielpacak merged commit 8f637cf into aquasecurity:main Jan 5, 2022
@danielpacak danielpacak added this to the Release v0.14.0 milestone Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants