#2563: Expose Helm --kube-version argument for contextless diffs #2593
Conversation
<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
for Pulumi's contribution guidelines.
Help us merge your changes more quickly by adding more details such
as labels, milestones, and reviewers.-->
### Proposed changes
This PR exposes the helm template `--kube-version` argument so it can be manually specified in situations when there is no access to the Kubernetes API server (e.g running a diff in CSA mode with no connection to the cluster).
Prior to this merge:
* If there were zero k8s contexts --> this field was unset meaning helm would default to some old version (1.20.0 at the time of writing) if it couldn't connect to the k8s context
* 1+ k8s contexts --> would always try to contact the API server to get the version leading to an error if the API server was unreachable.
This variable will override the behavior in either case, but is mostly useful in the first case or when the context in the second case is malformed / unreachable.
### Related issues (optional)
<!--Refer to related PRs or issues: pulumi#1234, or 'Fixes pulumi#1234' or 'Closes pulumi#1234'.
Or link to full URLs to issues or pull requests in other GitHub repositories. -->
Closes pulumi#2563
|
PR is now waiting for a maintainer to run the acceptance tests. |
|
/run-acceptance-tests |
|
Please view the PR build: https://github.com/pulumi/pulumi-kubernetes/actions/runs/6396894182 |
|
Any update here? The link seems to show green tests but I don't see that reflected in the build status. |
There was a problem hiding this comment.
The changes in pkg/gen, were they done by hand?
I believe they're generated based on a schema file, but @rquitales will know more about it.
|
Yes I did those by hand because I couldn't find a reference to the helm chart opts in the schema. It is likely I just missed something and would love for this to be generated. |
|
Hello @hvpeteet, I want to express my appreciation for your work on this PR and your valuable contribution to our project! After an initial review, it appears that the code you've submitted is quite promising, and it's encouraging to see that the test suite is passing. Regarding the changes in While I haven't had the chance to build and run this locally to verify its functionality, I would like to suggest the inclusion of some program tests for this feature. If you're able to add tests, we'd be more than happy to provide guidance and assistance to help you create them. Once again, thank you for your contribution, and we look forward to seeing this enhancement progress and landed. If you have any questions, need assistance with tests, or if there's anything else you'd like to discuss, please feel free to reach out. Your efforts are instrumental in enhancing our project! |
I will work on some getting some tests added, thanks for confirming that I needed to edit these files by hand. |
|
PR is now waiting for a maintainer to run the acceptance tests. |
|
PR is now waiting for a maintainer to run the acceptance tests. |
|
PR is now waiting for a maintainer to run the acceptance tests. |
|
PR is now waiting for a maintainer to run the acceptance tests. |
|
@rquitales I added some tests but wasn't able to get the dotnet tests running on my laptop (ARM M2 macbook). Do you think you could take a look at the tests (especially dotnet) when you get a chance? I don't know how much more local dev I can get working easily |
|
/run-acceptance-tests |
|
Please view the PR build: https://github.com/pulumi/pulumi-kubernetes/actions/runs/6460586760 |
|
Thanks @rquitales, please let me know if I can do anything else here. IIUC the tests look green 🤞 |
|
Any updates here? |
|
@rquitales is anything blocking this from merging? |
|
PR is now waiting for a maintainer to run the acceptance tests. |
|
/run-acceptance-tests |
|
Please view the PR build: https://github.com/pulumi/pulumi-kubernetes/actions/runs/6567857895 |
|
@henry-fn Apologies for dropping the ball here, this fell into the void of my GitHub notifications. Re-running the tests after rebasing. Will merge and cut a release after this! |
Proposed changes
This PR exposes the helm template
--kube-versionargument so it can be manually specified in situations when there is no access to the Kubernetes API server (e.g running a diff in CSA mode with no connection to the cluster).Prior to this merge:
This variable will override the behavior in either case, but is mostly useful in the first case or when the context in the second case is malformed / unreachable.
Related issues (optional)
Closes #2563
Notes:
Unpack.cswas already at max length. I tried to consolidate the args in a similar style to the rest of the unpack logic but it is likely that this section in particular deserves some extra time whenever someone reviews this code.