-
Notifications
You must be signed in to change notification settings - Fork 564
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
JsonPatches doesnt work #1434
Comments
also, when running the helm x jsonPatch example I'm getting this:
|
so it appears it doesnt work at all xD |
@4c74356b41 Hey! Thanks for reporting. Could you provide me exact reproduction steps? I tried to reproduce it myself but had no luck. Here's the structure of the sample project I tried:
// Btw I did find a bug that helmfile was ignoring |
To fix the issue that adhoc json patches were not working on kustomize/raw manifests. Note that regular kustomize project was working. In other words, this only affetcts `chart: path/to/dir` combined with `jsonPatches: ...` when the `path/to/dir` points to a kustomize project or a local directory containing raw K8s manifests. Ref #1434 (comment)
what might help is that I'm using |
@4c74356b41 Maybe. Would you mind sharing me your Dockerfile/Makefile that you used for building the image? I tried to read https://hub.docker.com/layers/4c74356b41/helmfile/azure/images/sha256-c194fd5b089dd9a5013f73dcf848aa5f2a73ceb259cc30f359da54d6c59353f9?context=explore but it's too obfuscated to read... |
you can just pull the image, nothing harmful in there, but here's the dockerfile:
|
To fix the issue that adhoc json patches were not working on kustomize/raw manifests. Note that regular kustomize project was working. In other words, this only affetcts `chart: path/to/dir` combined with `jsonPatches: ...` when the `path/to/dir` points to a kustomize project or a local directory containing raw K8s manifests. Ref #1434 (comment)
I tried using your own docker image (quay.io/roboll/helmfile:helm3-v0.126.0) together with kustomize v3.8.1 (https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize%2Fv3.8.1) and it throws the exact same errors:
btw, why does your image not have kustomize in it? |
@mumoshu bump |
@4c74356b41 Could you share me all the files including helmfile.yaml and your local chart? I have no luck so far reproducing this. |
I was just too lazy to add and occasionally update it :) I'm open to review PRs to add/update it though |
okay, I figured it out. this happens when I try to override yamls from a chart, so doesnt throw when I have:
but throws when I have it like this;
unfortunately it doesnt work when it doesnt throw... I observe no changes to the yamls. using your exact snippet (from above), am I misunderstanding something? or can I rephrase, is there any other way to change the namespace\tolerations\etc of a static yaml with helmfile (say, I dont want to use a chart for prometheus-operator, or I'm using some service that doesn't yet have a chart) |
mate? can you confirm its something I'm doing, or its really not rendering any changes? @mumoshu |
@4c74356b41 I noticed I was using a very outdated version of kustomize (v3.2.1) so that might be the cause of your issue. Let me check this soon. |
So it does seem like changing the SA's namespace to |
There should be no way other than what I demonstrated above. |
Same with the helmfile image 🤔
|
okay, this just started working with v0.126.2, no changes on my end. so I'm not sure how you claim it was working for you prior to that helmfile version :) |
@4c74356b41 Glad to hear it worked to you! As I said before, it should work only after the fix mentioned in #1434 (comment), which is 94e01b7, that is included since v0.125.9. Also, If you tried any of v0.125.9, v0.126.0, v0.126.1 before finally trying v0.126.2 and it still didn't work at that time, there may be some potential bug that I had fixed unexpectedly. It's just that I tried it several times with v0.125.9 and versions greater than that and it had consistently worked for me. So I hope it was just 94e01b7. |
I'm now running into issues with prometheus-community/kube-prometheus-stack and
|
@yurrriq That's already fixed in the latest, unreleased version of helmfile. Could you try building helmfile from |
Well, closing as the original issue has been resolved. Please submit another issue for different issues. Thanks! |
linebreaks only for readability
this is the helmfile piece that produces the error:
the file that its rendering is pretty much this file. Any pointers?
The text was updated successfully, but these errors were encountered: