Skip to content

feat: add better support and messaging around using helm with skaffold apply #7149

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

Conversation

aaron-prindle
Copy link
Contributor

@aaron-prindle aaron-prindle commented Mar 1, 2022

fixes #7101

This PR makes it so that if a single helm release is specified with a namespace option and that release is the only deployment, skaffold will set the kubectl namespace to that namespace. In the case that a helm release is specified with a namespace option and another deployment exists (eg: helm + kubectl deployments) or the case that multiple helm releases with namespaces are specified, skaffold apply will error stating:

skaffold apply called with 'deploy.helm.releases.*.namespace' field set which is not supported.  Remove the 'deploy.helm.releases.*.namespace' field(s) and run skaffold apply again

NOTE: for this to work properly, the apply call must also specify the profile used in the render call
skaffold apply -p dev --filename=skaffold.yaml manifest.yaml

^^ This is because render does not store anywhere (skaffold state, manifest files, etc) which profile was used to generate the manifests so it needs to be passed in both places

@aaron-prindle aaron-prindle requested a review from a team as a code owner March 1, 2022 22:27
@aaron-prindle aaron-prindle requested a review from gsquared94 March 1, 2022 22:27
@codecov
Copy link

codecov bot commented Mar 1, 2022

Codecov Report

Merging #7149 (dd38a3e) into main (290280e) will decrease coverage by 2.06%.
The diff coverage is 56.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7149      +/-   ##
==========================================
- Coverage   70.48%   68.41%   -2.07%     
==========================================
  Files         515      560      +45     
  Lines       23150    26271    +3121     
==========================================
+ Hits        16317    17974    +1657     
- Misses       5776     7059    +1283     
- Partials     1057     1238     +181     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/deploy.go 52.00% <ø> (-1.85%) ⬇️
cmd/skaffold/app/cmd/dev.go 84.61% <0.00%> (ø)
cmd/skaffold/app/cmd/render.go 36.66% <0.00%> (-4.72%) ⬇️
cmd/skaffold/skaffold.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/inspect_tests.go 62.50% <14.28%> (-1.14%) ⬇️
cmd/skaffold/app/cmd/lsp.go 28.12% <28.12%> (ø)
cmd/skaffold/app/cmd/fix.go 68.85% <40.00%> (-7.62%) ⬇️
cmd/skaffold/app/cmd/lint.go 42.85% <42.85%> (ø)
cmd/skaffold/app/cmd/find_configs.go 48.88% <50.00%> (+0.24%) ⬆️
cmd/skaffold/app/skaffold.go 76.19% <70.00%> (-8.43%) ⬇️
... and 216 more

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 1dbc7a2...dd38a3e. Read the comment docs.

@aaron-prindle aaron-prindle force-pushed the fix-7101-helm-render-and-apply branch 3 times, most recently from d5e402b to 01f186d Compare March 1, 2022 23:47
@aaron-prindle aaron-prindle force-pushed the fix-7101-helm-render-and-apply branch from 01f186d to 61d49ed Compare March 2, 2022 22:20
@aaron-prindle aaron-prindle force-pushed the fix-7101-helm-render-and-apply branch from 61d49ed to dd38a3e Compare March 2, 2022 22:20
@tejal29 tejal29 merged commit a35b47c into GoogleContainerTools:main Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Render and Apply - different behaviour with Helm namespaces
2 participants