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

fix(k8s): ensure rendered helm chart contain runtime values [WIP] #1812

Closed
wants to merge 2 commits into from

Conversation

eysi09
Copy link
Collaborator

@eysi09 eysi09 commented Apr 30, 2020

What this PR does / why we need it:

The helm template command doesn't render values that need to be
retrieved from the cluster. We therefore use helm install|upgrade --dry-run instead.

Which issue(s) this PR fixes:

Fixes #1795

Special notes for your reviewer:

N/A

@eysi09 eysi09 force-pushed the fix-helm-render branch 2 times, most recently from 2737f30 to e30c9a9 Compare May 1, 2020 15:15
@eysi09
Copy link
Collaborator Author

eysi09 commented May 1, 2020

Not quite sure why this is failing, and especially why it's not failing consistently.

@eysi09 eysi09 force-pushed the fix-helm-render branch from e30c9a9 to 507456e Compare May 1, 2020 19:19
@eysi09 eysi09 changed the title fix(k8s): ensure rendered helm chart contain runtime values fix(k8s): ensure rendered helm chart contain runtime values [WIP] May 4, 2020
The `helm template` command doesn't render values that need to be
retrieved from the cluster. We therefore use `helm install|upgrade
--dry-run` instead.
@eysi09 eysi09 force-pushed the fix-helm-render branch from b80b93d to aa48d9c Compare May 11, 2020 08:00
@edvald
Copy link
Collaborator

edvald commented May 12, 2020

@eysi09 it does appear to be failing consistently, on the same test. Perhaps something to do with how the output from the helm install command differs from the helm template command?

@eysi09
Copy link
Collaborator Author

eysi09 commented May 13, 2020

This is after making some changes that broke all the things. Before, it only failed for K8s version 1.15 and up (IIRC). And yes, it did fail consistently for those. (In case you were looking at the CI results after making the change).

@edvald
Copy link
Collaborator

edvald commented Jun 17, 2020

Closing in favor of a slightly different approach (I'll build off this branch but make some further changes).

@edvald edvald closed this Jun 17, 2020
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.

Garden fails to find helm deployed resources where apiVersions don't match
2 participants