Fix: Chart v4 fails on update #3046
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
EronWright
commented
Jun 4, 2024
Does the PR have any schema changes?Looking good! No breaking changes found. |
rquitales
reviewed
Jun 4, 2024
EronWright
commented
Jun 4, 2024
Comment on lines
+182
to
+184
| cmd.DisableOpenAPIValidation = true | ||
| cmd.Validate = false | ||
| cmd.ClientOnly = true |
There was a problem hiding this comment.
Emulates:
helm template --dry-run=server --validate=false --disable-openapi-validation \
--kube-version $VERSION --api-versions $APIVERSIONS
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3046 +/- ##
==========================================
+ Coverage 36.67% 36.77% +0.09%
==========================================
Files 71 71
Lines 9249 9262 +13
==========================================
+ Hits 3392 3406 +14
+ Misses 5520 5518 -2
- Partials 337 338 +1 ☔ View full report in Codecov by Sentry. |
rquitales
approved these changes
Jun 4, 2024
rquitales
added a commit
that referenced
this pull request
Jun 4, 2024
### Proposed changes This PR explicitly sets the `opttest.SkipInstall()` option for our yaml tests. It is observed that the test framework/automation API is unable successfully run `pulumi install` prior to the other pulumi operations. This error was hit in the presubmit for #3046, but went unnoticed in the past due to the `TestTerraformConvert` installing the right plugins before the other tests hit the installation step. Manual validation: - Deleted the local `~/.pulumi/plugin` folder - Run the tests without the changes and notice the test errors - Run the tests with the changes here and they pass ### Related issues (optional) Fixes: #3048
rshade
reviewed
Jun 4, 2024
This was referenced Jun 4, 2024
Merged
EronWright
added a commit
that referenced
this pull request
Jun 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
This PR fixes a problem with how Chart v4 uses the Helm library. The design goal is to allow for connectivity during template rendering, to support the lookup function (see helm/helm#9426) and to provide an accurate Capabilities object. Unfortunately we were slightly too aggressive and caused some of Helm's "non-template" code to execute.
This fix works by turning off the
helm template --validationflag, so that the internalClientOnlyflag is true thus avoiding this block of code that causes the unexpected error. A side-effect ofClientOnlybeing true is that the capabilities aren't automatically set, and so we set them using the provider's kube client (akin to using--kube-version).Detailed changes:
.CapabilitiesRelated issues (optional)
Closes #3045