-
Notifications
You must be signed in to change notification settings - Fork 117
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
Helm Chart Issue - wrong version of .Capabilities.KubeVersion.GitVersion passed? #2345
Comments
Thanks for filing. I believe this is a Kubernetes/Helm question so I'll transfer your issue to that repository and they should be better able to help you. |
Hello, from a brief look at this it looks related to the fact that charts are executed locally. Using Release instead of Chart might address this issue as the Release will execute the template in the context of the cluster. The downside of the Release is that Pulumi won't be aware of any resources created by the template. |
Hi @danielrbradley, pulumi runs into a azure pipeline and all my CLI are up to date in there. I specifically dump my versions into the pipeline to make sure I am using the right ones: Helm: kubectl is at: and of course I use the latest pulumi kubernetes nuget. So why doesn't it resolve the version properly while templating the chart? |
Also, maybe this might be related but I get something similar when trying to render a keda helm chart > 2.8.1.. There is a condition that tests the version and I get this error during the preview: stderr: error: Running program '/home/vsts/work/1/s/src/---/bin/Debug/net6.0/---.dll' failed with an unhandled exception: |
Just to make sure we understand each other (I am not always clear ;)) the way to make it work would be that pulumi passes a '--kube-version' to the helm command line. For example, in the case of the latest keda chart (2.10.2) the default behavior of So maybe the Chart class would need an argument in its ChartArgs class so that we could specify which kube-version to use? BTW I found this issue but it seems that would not cover the case here. (#2155) thanks, |
@yann-soubeyrand is it possible that there is something wrong with your fix (#2155) when it runs into the context of an Azure Pipeline? thanks, |
Hello, pulumi-kubernetes/provider/pkg/provider/invoke_helm_template.go Lines 246 to 278 in 58b8c44
Can you confirm that you don’t set APIVersions in your chart args?Could it be that Pulumi doesn’t have access to your cluster at the time the chart resource is evaluated? |
Thanks for the quick response.
|
Can you run |
See this post #2345 (comment) I did not do it for |
OK. And apart from these Helm related errors, does your Kubernetes provider work as expected? I mean, can you create Kubernetes ressources using Pulumi? |
Yes, everything else works. There is only those issues with the Chart object and there is so little verbose logs that it is hard to troubleshoot. |
Sorry to ask this question, but is your Chart resource using your configured Kubernetes provider and not using the default (maybe not working) provider? |
Here is what I have for what you asked:
|
Using the proper provider with the kubeconfig returned from the cluster. As I said, everything else works except the Chart wich seems to be missing the proper version passed to it. |
Sorry, I meant running this command using the same kubeconfig used by the Pulumi Kubernetes provider. Also, can you provide a copy of this kubeconfig? |
Would you have a code sample for what you need me do to (C#)? I am not that familiar with that functionality sorry |
Sorry, I’m not familiar at all with C#. Here’s the documentation to output your kubeconfig: https://www.pulumi.com/learn/building-with-pulumi/stack-outputs/. Also, I’m not a Pulumi developer (I just contributed the PR you linked), so maybe you’ll have more luck finding someone who can help you troubleshooting your issue on Slack. |
Ok, will look into that. But as mentioned everything else works properly (even the use of ConfigFile to install Itsio/prometheus). Is it possible to add more verbose logs into that part of the code to indicate what is passed in the KubeVersion as well as any info retrieved from the DiscoveryClient ? |
OK, I think I am getting closer to the issue. I think the problem is related to a Parent/Child provider issue.
I am not jumping straight to conclusions, but I tried passing directly the Provider to the Chart object and it seems now to behave properly. Maybe there is a bug in there which does not pass down the proper provider for child resources? |
Seems you’re getting closer indeed 😉 I’ve just check in our code though, and we do not pass the provider directly, our charts are children of component resources which receive a provider map as follow:
But I guess you have the same on your side, right? |
There are two ways to instantiate a ComponentResource. One way passing a list of providers (key, value) and one way using a single provider. We use the later so we only use one provider down the chain which is the Kubernetes one. It seems it is not passed down properly and does not match your "kubernetes" one? |
Hi, I am currently debugging this exact issue for installing the k8s cert-manager. Same steps
Here's my code if anything obvious could be problematic // create cluster etc...
k8sProvider, err := kubernetes.NewProvider(ctx, "k8s-provider", &kubernetes.ProviderArgs{
Kubeconfig: clusterKubeconfig,
})
if err != nil {
return err
}
_, err = helm.NewChart(ctx, "k8s-cert-manager", helm.ChartArgs{
Chart: pulumi.String("cert-manager"),
Version: pulumi.String("1.11.1"),
Namespace: pulumi.String("cert-manager"),
FetchArgs: helm.FetchArgs{
Repo: pulumi.String("https://charts.jetstack.io"),
},
Values: pulumi.Map{
"installCRDs": pulumi.Bool(true),
},
},
pulumi.Provider(k8sProvider),
)
if err != nil {
return err
} |
@hampusohlsson are you sure this is the same issue? IIUC, @Phil1972 said that passing the provider directly to the chart resource fixed the issue for him. @Phil1972 you’re right that these two way of passing the providers should end with the same result, but could you try passing the providers using a map? Who knows, maybe we’ll uncover an interesting bug! |
Perhaps not, but it definitely seems related to a wrong version getting passed to Helm, as using |
Yes, that’s what makes me think it’s not the same issue 😉 Which version of the Kubernetes provider are you using? |
Using the 'Providers' (with an S) allows to pass a list of providers. (Not sure how I can pass a map). I tried using that one passing a list of my sole provider but it did not work either. As for hampusohlsson, I am not sure it is the same issue indeed since he is not using composite objects. So it might be a different issue but gives him the same result as me ;) |
Here is what I sort of have in pseudo code:
Note that I am properly setting the parent and passing the provider in the argument and in the base constructor. |
In Go, there’s a Again, I’m not familiar with C#, but your code seems really similar to what we do in Go, which is working for us. I’m wondering if this line isn’t missing the pulumi-kubernetes/sdk/dotnet/Helm/V3/Chart.cs Line 363 in 58b8c44
In the Go SDK, all the resource options which are also invoke options (which is the case of the Parent option) are passed: pulumi-kubernetes/sdk/go/kubernetes/helm/v3/chart.go Lines 247 to 268 in 58b8c44
Maybe @lblackstone or another maintainer can confirm or inform what I’m supposing? |
TypeScript SDK seems to set pulumi-kubernetes/sdk/nodejs/helm/v3/helm.ts Lines 243 to 258 in 58b8c44
|
If anyone can verify/confirm this, it would be a big one for us using c# ;) |
I've solved my previous issue, and like you all suspected it's a different issue - turns out it was bad spacing in my generated cluster kubeconfig.yaml 🤦♂️ I guess it was deemed invalid and Helm provider set the version to some default of 1.20.0 |
@lblackstone when is this supposed to be merge into the latest release? I do not see any info on the resolution PR/release version Thanks, |
@Phil1972 Sorry, I reread the entire thread and it looks like I closed this in error while I was triaging issues the other day. I saw #2345 (comment) and assumed it was fixed. |
Perfect because I confirm that the issue is still present as of the latest release. Thanks, |
Hi @Phil1972, |
To recap the discussion, a bug (in some SDKs) with provider option propagation causes a confusing error message. In some cases, a possible workaround is to set |
<!--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 Epic: #2254 Fixes: #1214 Fixes: #2345 This PR standardizes the option propagation logic for the component resources in the pulumi-kubernetes .NET SDK. The general approach is: 1. In the component resource constructor, compute the child options to be propagated to any children. The child options consist of the component as parent, and with `version` and `pluginDownloadURL` if specified. 2. Compute the invoke options by copying the child options. ### Specification The component resource is responsible for computing sub-options for invokes and for child resource declarations. This table outlines the expected behavior for each [resource option](https://www.pulumi.com/docs/concepts/options/) when presented to a component resource. | | Propagated | Remarks | |---|---|---| | `additionalSecretOutputs` | no | "does not apply to component resources" | | `aliases` | no | Inherited via parent-child relationship. | | `customTimeouts` | no | "does not apply to component resources" | | `deleteBeforeReplace` | no | | | `deletedWith` | no | | | `dependsOn` | no | The children implicitly wait for the dependency. | | `ignoreChanges` | no | Nonsensical to apply directly to children (see [discussion](pulumi/pulumi#8969)). | | `import` | no | | | `parent` | **yes** | The component becomes the parent. | | `protect` | no | Inherited. | | `provider` | no | Combined into providers map, then inherited via parent-child relationship. | | `providers` | no | Inherited. | | `replaceOnChanges` | no | "does not apply to component resources" | | `retainOnDelete` | no | "does not apply to component resources" | | `transformations` | no | Inherited. | | `version` | **yes** | Influences default provider selection logic during invokes.<br/>Should propagate when child resource is from the same provider type. | | `pluginDownloadURL` | **yes** | Influences default provider selection logic during invokes.<br/>Should propagate when child resource is from the same provider type. | ### Testing A new test case is provided ([test case](https://github.com/pulumi/pulumi-kubernetes/blob/db3cd71f3e5780bfe0bf4584d667b10e446761f7/tests/sdk/dotnet/dotnet_test.go#L316), [test program](https://github.com/pulumi/pulumi-kubernetes/blob/db3cd71f3e5780bfe0bf4584d667b10e446761f7/tests/sdk/dotnet/options/Program.cs)) that exercises option propagation across the component resources: - [kubernetes.helm.sh.v3.Chart](https://www.pulumi.com/registry/packages/kubernetes/api-docs/helm/v3/chart/) - [kubernetes.kustomize.Directory](https://www.pulumi.com/registry/packages/kubernetes/api-docs/kustomize/directory/) - [kubernetes.yaml.ConfigGroup](https://www.pulumi.com/registry/packages/kubernetes/api-docs/yaml/configgroup/) - [kubernetes.yaml.ConfigFile](https://www.pulumi.com/registry/packages/kubernetes/api-docs/yaml/configfile/) Upgrade testing must be done manually, with an emphasis on avoiding replacement due to reparenting. ### Upgrade Considerations In previous versions, the `ConfigFile` and `ConfigGroup` component resources inadvertently assigned the wrong parent to the child resource(s). This would happen when the component resource itself had a parent; the child would be assigned that same parent. This also had the effect of disregarding the component resource's provider in favor of the parent's provider. For example, here's a before/after look at the component hierarchy: Before: ``` ├─ pkg:index:MyComponent parent │ ├─ kubernetes:core/v1:ConfigMap cg-options-cg-options-cm-1 │ ├─ kubernetes:yaml:ConfigFile cg-options-testdata/options/configgroup/manifest.yaml │ ├─ kubernetes:core/v1:ConfigMap cg-options-configgroup-cm-1 │ ├─ kubernetes:yaml:ConfigFile cg-options-testdata/options/configgroup/empty.yaml │ └─ kubernetes:yaml:ConfigGroup cg-options ``` After: ``` └─ pkg:index:MyComponent parent └─ kubernetes:yaml:ConfigGroup cg-options ├─ kubernetes:yaml:ConfigFile cg-options-testdata/options/configgroup/manifest.yaml │ └─ kubernetes:core/v1:ConfigMap cg-options-configgroup-cm-1 └─ kubernetes:core/v1:ConfigMap cg-options-cg-options-cm-1 ``` This PR addresses this issue and attempts to heal existing stacks using aliases. This is effective at avoiding a replacement except in the case where the child was created with the wrong provider. In this case, **Pulumi will suggest a replacement of the child resource(s)**, such that they use the correct provider. ### Related issues (optional)
What happened?
It seems an issue related to the fact that the version of k8s passed to helm might be wrong or invalid. I have the latest nuget and when we run our Azure pipeline, it seems that the capability passed is not right since it resolves to the 'else'.
This is in istiod 1.17.1 chart.
{{- if .Values.global.defaultPodDisruptionBudget.enabled }}
{{- if (semverCompare ">=1.21-0" .Capabilities.KubeVersion.GitVersion) }}
apiVersion: policy/v1
{{- else }}
apiVersion: policy/v1beta1
{{- end }}
kind: PodDisruptionBudget
is there a way to know what version is passed in .Capabilities.KubeVersion.GitVersion ?
Thanks,
Expected Behavior
"apiVersion: policy/v1" should been resolved in the chart
Steps to reproduce
Output of
pulumi about
Additional context
No response
Contributing
Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).
The text was updated successfully, but these errors were encountered: