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

Add support for helm --force flag #1499

Closed
wants to merge 7 commits into from

Conversation

tjerkw
Copy link

@tjerkw tjerkw commented Jan 20, 2019

We're using skaffold and helm to deploy. In our ci-cluster deploys sometimes fail,
but then skaffold cannot install it anymore if it finds a failed helm installation.
The --force flag solves this.

See the --force functionailtiy added in helm 2.5.0:
helm/helm#2280

Potential solution for #1328
Makes it more easy to use skaffold in a ci-pipeline. (deploy from a clean sheet)

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@tjerkw
Copy link
Author

tjerkw commented Jan 20, 2019

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@codecov-io
Copy link

codecov-io commented Jan 20, 2019

Codecov Report

Merging #1499 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1499      +/-   ##
==========================================
- Coverage   44.87%   44.85%   -0.02%     
==========================================
  Files         115      115              
  Lines        4771     4773       +2     
==========================================
  Hits         2141     2141              
- Misses       2409     2410       +1     
- Partials      221      222       +1
Impacted Files Coverage Δ
pkg/skaffold/deploy/helm.go 64.09% <0%> (-0.59%) ⬇️

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 51bcca6...dbf77d2. Read the comment docs.

@dgageot dgageot added the kokoro:run runs the kokoro jobs on a PR label Jan 28, 2019
@priyawadhwa priyawadhwa added kokoro:run runs the kokoro jobs on a PR and removed kokoro:run runs the kokoro jobs on a PR labels Jan 29, 2019
Copy link
Contributor

@priyawadhwa priyawadhwa left a comment

Choose a reason for hiding this comment

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

Just one comment, but once that is addressed this should be ready!

@@ -183,6 +183,7 @@ deploy:

# helm:
# helm releases to deploy.
# Uses "helm install" if not installed yet, otherwise "helm upgrade" is executed
Copy link
Contributor

Choose a reason for hiding this comment

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

So, unfortunately because of the way we have to release config changes, this change also needs to be made in integration/examples/annotated-skaffold.yaml. Would you mind adding it there as well? Thanks!

@priyawadhwa priyawadhwa added kokoro:run runs the kokoro jobs on a PR and removed kokoro:run runs the kokoro jobs on a PR labels Jan 29, 2019
@tjerkw
Copy link
Author

tjerkw commented Feb 11, 2019

As agreed here, lets use the flag based approach:
#1506

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kokoro:run runs the kokoro jobs on a PR needs-rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants