Failing to update chart containing child HelmRelease and nested values

[stefansedich]

Describe the bug

Given the following chart https://github.com/stefansedich/helm-test if one were to apply the outer release.yaml it would install the first time without issue however after an update say bumping the nested image tag we would see the following error: failed to create patch: merging an object in json but data type is not struct, instead is: map

Installing directly with helm using helm upgrade --install podinfo ./chart --values values.yaml works without issue. And strangely enough if you were to reduce the nesting of podinfo.some.nested.image to podinfo.some.image helm-operator is able to install and update without issue.

To Reproduce

Steps to reproduce the behaviour:

1. kubectl apply -f release.yaml
2. See that it installs fine
3. Change tag in outer release.yaml
4. See the error

Expected behavior

Subsequent chart upgrades to work without issue.

Logs

6 upgrade.go:280] warning: Upgrade "podinfo" failed: failed to create patch: merging an object in json but data type is not struct, instead is: map
ts=2019-12-28T01:35:42.221288399Z caller=release.go:217 component=release release=podinfo targetNamespace=default resource=default:helmrelease/podinfo helmVersion=v3 error="Helm release failed" revision=18651c21dc303955f861fb3e50234afbf9a6620e err="failed to upgrade chart for release [podinfo]: failed to create patch: merging an object in json but data type is not struct, instead is: map"

Additional context

Add any other context about the problem here, e.g

• Helm operator version: rc5
• Targeted Helm version: v3
• Kubernetes version: 1.14
• Git provider: GitHub
• Container registry provider: N/A

[hiddeco]

This is due to Helm using scheme.Scheme to get versioned objects, if it can get a versioned object, it will attempt a 3-way-merge patch.

Due to our versioned client injecting the HelmRelease resource into runtime.Scheme, Helm decides that it should execute a 3-way-merge patch, which is not supported for custom resources.

The best way to patch this would be to patch Helm, I was able to get rid of the issue by using runtime.NewScheme() in above linked converter.go, but I need to consult with the Helm team if this would break other parts of Helm, and above all, if they would be open to this change.

[stefansedich]

Thanks @hiddeco all makes sense, do you have any ideas for workaround on our end for the time being? I currently am unable to upgrade a few services due to this and would be fine with running a custom build/hack just to keep pushing forward.

[hiddeco]

@stefansedich the following image should work, it has been built based on the work in the helm-patched branch: hiddeco/helm-operator:v3-patched-scheme

[stefanprodan]

Fixing this bug requires upstream changes, ref: helm/helm#7401

[stefansedich]

@hiddeco @stefanprodan have you guys had any luck getting this one pushed through?

[hiddeco]

Nop, but on a second thought we may be able to work around it by getting rid of the usage of scheme.Scheme here and here (and replace it with a likewise thing as proposed on the Helm side).

If you are feeling adventurous; a contribution is always welcome. I did not find time (yet) to confirm or debunk this theory.

[stefansedich]

@hiddeco ok let me have a look, I need to deploy something that is blocking atm due to this bug so I have a vested interest in solving it 😆

Update: Attempted the suggested fix and as discussed with @hiddeco looks like this might not actually work.