Skip to content
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 2.8.0 with CRDs: unable to find api field in struct Unstructured for the json field "metadata" #3382

Closed
Stono opened this issue Jan 24, 2018 · 50 comments

Comments

Projects
None yet
@Stono
Copy link

commented Jan 24, 2018

Hi,
We're seeing this a lot since updating to 2.8.0. This only happens on subsequent upgrades to existing charts.

❯ helm upgrade --reset-values --debug --wait --install --namespace default --version 0.1.379 at-sauron-web-mobile . -f values.yaml -f dev.yaml
[debug] Created tunnel using local port: '56621'

[debug] SERVER: "127.0.0.1:56621"

Error: UPGRADE FAILED: failed to create patch: unable to find api field in struct Unstructured for the json field "metadata"

This particular chart is upgrading the following components:

  • configmap
  • service
  • deployment
  • ingress

If i take the --dry-run output and put it into a file and do kubectl apply -f myself, it works fine

@Stono

This comment has been minimized.

Copy link
Author

commented Jan 24, 2018

Hi,
This is proving to be a huge problem for us, we've got 10 or so applications having intermittent failures.

All clients and server are 2.8.0

Error: UPGRADE FAILED: failed to create patch: unable to find api field in struct Unstructured for the json field "metadata"

Would really, really appreciate some support

@thomastaylor312

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2018

@Stono I'll ping some people on this

@thomastaylor312 thomastaylor312 added the bug label Jan 24, 2018

@thomastaylor312 thomastaylor312 added this to the 2.8.1 - Bugfix milestone Jan 24, 2018

@Stono

This comment has been minimized.

Copy link
Author

commented Jan 24, 2018

@technosophos

This comment has been minimized.

Copy link
Member

commented Jan 24, 2018

From Slack:

  • Kube version is 1.8.6/GKE and Helm 2.7 was working fine
  • I suspect Deployment may be the culprit, since that had notable changes between Kube 1.8 and Kube 1.9, and here we have a library incompatibility

Probably the best testing procedure would be to dump the generated chart, and then dump the actually installed resources and look for structural differences that might tip us off to some sort of transform that happened on a resource.

@Stono

This comment has been minimized.

Copy link
Author

commented Jan 24, 2018

OK I think you might be right on the money.

  • Deployed: extensions/v1beta1
  • Helm Template: apiVersion: apps/v1beta2

my charts Deployment is apiVersion: apps/v1beta2, so it looks like after the initial deployment, for whatever reason what is on the server is v1beta1

@munnerz

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2018

@Stono the server will respond with a manifest in the version of whatever you ask it for, it isn't necessarily stored in this version in etcd.

You can ask for a specific version of a resource with something like:

kubectl get deployments.v1beta2.apps -o yaml

@mattfarina

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2018

The setup, as I understand it, is in both circumstances helm/tiller is talking to k8s 1.8.6. Helm 2.7.2 is built to k8s 1.8 libs (e.g., apimachinery) while helm 2.8 is built to k8s 1.9 versions of the libs.

I think the error is being generated inside of LookupPatchMetadataForStruct. This is notable because the error and function signature didn't exist until k8s 1.9. Here's the PR that changed it to the current state.

Looking at the type as Unstructured which looks like the following the few places it shows up in code:

type Unstructured struct {
	// Object is a JSON compatible map with string, float, int, bool, []interface{}, or
	// map[string]interface{}
	// children.
	Object map[string]interface{}
}

and that the field name is "metadata" may help us start to find the culprit.

@technosophos

This comment has been minimized.

Copy link
Member

commented Jan 24, 2018

Might have to ask @adamreese for help on this one. This is his area of expertise.

@mattfarina

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2018

@k8s-mirror-api-machinery-bugs any idea why we'd experience issues working with a k8s 1.8 cluster (client-go/api-machinery for k8s 1.9)?

@munnerz

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2018

I've bumped our e2e tests for a couple of our projects to use Helm 2.8.0 and am experiencing a similar thing in Kubernetes 1.9 and 1.7. I'd imagine also 1.8, but our e2e tests look like they flaked on that run:

Namely:

Error: UPGRADE FAILED: failed to create patch: unable to find api field in struct Unstructured for the json field "spec"

so it appears the issue isn't constrained to just metadata, nor isolated to an API version 😄. My guess would be that the way unstructured fields are handled in the new machinery (client side) is different.

I am also seeing test failures on another project of ours, this time when deploying a particular chart for the first time. This happens consistently against k8s 1.7, 1.8 and 1.9. I'll need to do a bit more digging to get us some better logs out of this one, as it seems to be hitting the --wait timeout. For now I'd theorise that these failures are unrelated to this issue (although it does seem to be a 2.8.0 regression): https://storage.googleapis.com/jetstack-logs/pr-logs/pull/jetstack_cert-manager/276/cert-manager-e2e-v1-8/89/build-log.txt

/cc @sttts are you aware of any changes to Unstructured since release-1.8 that could cause this sort of thing?

@sttts

This comment has been minimized.

Copy link

commented Jan 25, 2018

Looks like something related to strategic merge patches as the error messages come from LookupPatchMetadataForStruct called from https://github.com/kubernetes/kubernetes/blob/f1ad84a2c3d4c6680091250967e96108cc9dd652/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/meta.go#L74.

@Stono

This comment has been minimized.

Copy link
Author

commented Jan 31, 2018

Hey all, just wondering if you're managing to make any progress with this?

@thomastaylor312

This comment has been minimized.

Copy link
Collaborator

commented Feb 1, 2018

@Stono We just talked about this in the dev call. Pretty much all of the maintainers with free cycles will be trying to take a look at this today and tomorrow

@thomastaylor312

This comment has been minimized.

Copy link
Collaborator

commented Feb 1, 2018

@sttts Is there any chance you might have some time for debugging this with one of the maintainers today and tomorrow?

@jascott1

This comment has been minimized.

Copy link
Collaborator

commented Feb 1, 2018

I tried with a foo chart with Helm 2.8.0 and k8s 1.8.4 but could not reproduce. Changing cluster to 1.8.6 now but if anyone has a chart that demonstrates the problem please share.

@adamreese

This comment has been minimized.

Copy link
Member

commented Feb 1, 2018

I think this unrelated to 1.9. This test is with kubectl v1.8 on a v1.8 cluster.

Running kubectl apply on a apps/v1beta2 deployment creates a apps/v1beta1 but the version in kubectl.kubernetes.io/last-applied-configuration is v1beta2

$ cat <<EOF | kubectl apply -f -
apiVersion: apps/v1beta2
kind: Deployment
metadata:
  name: apps-bug
spec:
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - name: apps-bug
        image: nginx
EOF

deployment "apps-bug" created

$ kubectl get deployment apps-bug -o jsonpath="{.apiVersion}"
extensions/v1beta1

$ kubectl apply view-last-applied deployment apps-bug
apiVersion: apps/v1beta2
kind: Deployment
metadata:
  annotations: {}
  name: apps-bug
  namespace: default
spec:
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - image: nginx
        name: apps-bug
@bacongobbler

This comment has been minimized.

Copy link
Member

commented Feb 2, 2018

wat

@bacongobbler

This comment has been minimized.

Copy link
Member

commented Feb 2, 2018

I can confirm this on kubectl/kubernetes v1.9. I will downgrade to v1.8 and see if I can replicate. If that happens to be the case, then this is an issue somewhere at the API level and should be reported upstream.

@bacongobbler

This comment has been minimized.

Copy link
Member

commented Feb 2, 2018

Yep. I can also confirm that I was able to produce the same output from the API with kubectl v1.8.0 and kubernetes v1.8.7. This is an upstream thing and is not in any way related to #3234, so I'm going to remove it from the 2.8.1 milestone as it's out of our control at this point.

@bacongobbler bacongobbler removed this from the 2.8.1 - Bugfix milestone Feb 2, 2018

@mattfarina

This comment has been minimized.

Copy link
Collaborator

commented Feb 2, 2018

@adamreese @bacongobbler I'm not sure I understand the link to the original error in the issue which read:

Error: UPGRADE FAILED: failed to create patch: unable to find api field in struct Unstructured for the json field "metadata"

Also, when helm upgraded to client-go/apimachinery for k8s 1.8 we didn't have reported issues of this.

Can you fill me in on the link and how the "metadata" json error relates?

@bismarck

This comment has been minimized.

Copy link

commented Feb 2, 2018

We're seeing this issue with Helm when trying to update a CRD. I wasn't sure if this was related to kubernetes/kubernetes#58017?

@mattfarina

This comment has been minimized.

Copy link
Collaborator

commented Feb 2, 2018

I've done a little more digging and discovered that...

https://github.com/kubernetes/helm/blob/1f87bf031ab12d6444067880ec07df4f3889d3a4/pkg/kube/client.go#L390-L417

The versionedObject on line 405 is returning a struct type of Unstructured. If you dig down into strategicpatch.CreateTwoWayMergePatch and functions it calls this is turned into a schema and LookupPatchMetadataForStruct is called for fields like metadata on it which don't exist.

Still investigating.

@technosophos

This comment has been minimized.

Copy link
Member

commented Feb 2, 2018

Is everyone who has had this issue running on GKE? Can anyone reproduce on Minikube or any other distribution?

@mattfarina

This comment has been minimized.

Copy link
Collaborator

commented Feb 2, 2018

Folks, we're having a little trouble recreating the original problem in order to understand the root and test any fixes. If you are experiencing this problem could you please point us to a chart and setup we could use to recreate the problem. The more detail the better.

@Stono

This comment has been minimized.

Copy link
Author

commented Feb 2, 2018

@mattfarina see slack (helm-dev)

@bacongobbler

This comment has been minimized.

Copy link
Member

commented Feb 5, 2018

Updated the title to reflect the bug. @adamreese the bug you found is probably unrelated but might also be good to track separately just so we have a pulse on that one as well. I'll see if someone's already submitted that issue upstream.

@bacongobbler

This comment has been minimized.

Copy link
Member

commented Feb 5, 2018

filed kubernetes/kubernetes#59362 to track the other issue

@mattfarina

This comment has been minimized.

Copy link
Collaborator

commented Feb 5, 2018

FYI, I created a repo with a test folks can run to reproduce. I've used this against minikube. https://github.com/mattfarina/bad-chart-2.8-1.9

@mattfarina

This comment has been minimized.

Copy link
Collaborator

commented Feb 5, 2018

To close the loop, I've also posted to the SIG API Machinery list at https://groups.google.com/forum/#!topic/kubernetes-sig-api-machinery/Mv5ya3HcVE0. Their next meeting is on Feb 14th. I can't make it but if someone else wants to go and bring it up there (if we don't have direction already). You can find it on the calendar at https://kubernetes.io/community/

mattfarina added a commit to mattfarina/helm that referenced this issue Feb 5, 2018

fix(api-machinery): Fixes patching for unstructured objects
CRDs and other objects seen as unstructured cannot use strategic
merge patching. It has never been supported on CRDs. Previously,
cases like unstructured objects could have caused an unregistered
error. This is no longer the case.

This change explicitly looks for unstructured objects and handles
those using json merge patching.

Closes helm#3382

@mattfarina mattfarina added this to the 2.8.1 - Bugfix milestone Feb 5, 2018

@mattfarina

This comment has been minimized.

Copy link
Collaborator

commented Feb 5, 2018

Adding this back to the 2.8.1 bugfix release. I think #3459 fixes it.

Thanks to @liggitt for the help.

@Stono

This comment has been minimized.

Copy link
Author

commented Feb 5, 2018

Great work getting to the bottom of it and getting a fix into 2.8.1 :-)

mattfarina added a commit to mattfarina/helm that referenced this issue Feb 5, 2018

fix(api-machinery): Fixes patching for unstructured objects
CRDs and other objects seen as unstructured cannot use strategic
merge patching. It has never been supported on CRDs. Previously,
cases like unstructured objects could have caused an unregistered
error. This is no longer the case.

This change explicitly looks for unstructured objects and handles
those using json merge patching.

Closes helm#3382

bacongobbler added a commit that referenced this issue Feb 8, 2018

fix(api-machinery): Fixes patching for unstructured objects
CRDs and other objects seen as unstructured cannot use strategic
merge patching. It has never been supported on CRDs. Previously,
cases like unstructured objects could have caused an unregistered
error. This is no longer the case.

This change explicitly looks for unstructured objects and handles
those using json merge patching.

Closes #3382

(cherry picked from commit e6137ff)
@balboah

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2018

yay, love when things are fixed a couple of days before experiencing the issue :)

@flah00

This comment has been minimized.

Copy link

commented May 1, 2018

Sorry to resurrect a zombie, but I just ran into this, using helm-2.8.2, tiller-2.8.2, kubectl client 1.9.2 and kubectl server 1.8.4+coreos.0.

@macropin

This comment has been minimized.

Copy link

commented May 29, 2018

We're seeing this as well with helm v2.8.2, tiller v2.8.2, kubectl v1.9.7, kubernetes v1.9.8.

@liggitt

This comment has been minimized.

Copy link

commented May 29, 2018

We're seeing this as well with helm v2.8.2, tiller v2.8.2, kubectl v1.9.7, kubernetes v1.9.8.

On every patch request, or occasionally on contentious resources?

@macropin

This comment has been minimized.

Copy link

commented May 29, 2018

We're using helm to deploy a custom application. It appears to happen on every deployment. I'm going to try updating the deployments with

-apiVersion: extensions/v1beta1
+apiVersion: apps/v1beta1
@liggitt

This comment has been minimized.

Copy link

commented May 30, 2018

We're using helm to deploy a custom application. It appears to happen on every deployment.

This bug doesn't involve deployment objects, only custom resources. What are the custom resources involved? What component is submitting patches for those custom resources? Do you have details of the API requests (body, url, submitted content-type) that are returning this error?

@jimethn

This comment has been minimized.

Copy link

commented Aug 28, 2018

We're having this problem with normal Deployments, we don't have any CRDs. Helm 2.10.0 and Kubernetes 1.10.5.

@Stono

This comment has been minimized.

Copy link
Author

commented Aug 28, 2018

@jimethn - I've seen this error manifest when you're using integer values where they should be strings, for example in your env settings for your deployment, do you have value: 1, instead of value: "1"

@jimethn

This comment has been minimized.

Copy link

commented Aug 28, 2018

@Stono thank you for the swift reply. I'm not seeing that anywhere with integers, although we do use unquoted strings all over the place. Might that be related? My apologies for posting in this closed thread. As we refine the issue further we may open a fresh ticket and refer to this one.

@Stono

This comment has been minimized.

Copy link
Author

commented Aug 28, 2018

Ya another burn is using true instead of "true". I'm not saying this is your issue btw, you could have a genuine problem :d if it isn't this - i suggest a new issue where you paste your manifest.

@pkelleratwork

This comment has been minimized.

Copy link

commented Nov 19, 2018

I am experiencing this issue on

Client: &version.Version{SemVer:"v2.11.0", GitCommit:"2e55dbe1fdb5fdb96b75ff144a339489417b146b", GitTreeState:"clean"}
Server: &version.Version{SemVer:"v2.11.0", GitCommit:"2e55dbe1fdb5fdb96b75ff144a339489417b146b", GitTreeState:"clean"}

helm chart uses

    annotations:
      checksum/config: {{ include (print $.Template.BasePath "/checksum-configmap.yaml") . | sha256sum }}

getting error

Error: UPGRADE FAILED: failed to create patch: unable to find api field in struct PodTemplateSpec for the json field "annotations"
@cblecker

This comment has been minimized.

Copy link
Contributor

commented Nov 19, 2018

@pkelleratwork You're putting the annotations at the wrong level. They need to be under metadata, not directly as part of the PodTemplateSpec

splisson added a commit to splisson/helm that referenced this issue Dec 6, 2018

fix(api-machinery): Fixes patching for unstructured objects
CRDs and other objects seen as unstructured cannot use strategic
merge patching. It has never been supported on CRDs. Previously,
cases like unstructured objects could have caused an unregistered
error. This is no longer the case.

This change explicitly looks for unstructured objects and handles
those using json merge patching.

Closes helm#3382

splisson pushed a commit to splisson/helm that referenced this issue Dec 6, 2018

fix(api-machinery): Fixes patching for unstructured objects
CRDs and other objects seen as unstructured cannot use strategic
merge patching. It has never been supported on CRDs. Previously,
cases like unstructured objects could have caused an unregistered
error. This is no longer the case.

This change explicitly looks for unstructured objects and handles
those using json merge patching.

Closes helm#3382

jianghang8421 added a commit to jianghang8421/helm that referenced this issue Feb 17, 2019

fix(api-machinery): Fixes patching for unstructured objects
CRDs and other objects seen as unstructured cannot use strategic
merge patching. It has never been supported on CRDs. Previously,
cases like unstructured objects could have caused an unregistered
error. This is no longer the case.

This change explicitly looks for unstructured objects and handles
those using json merge patching.

Closes helm#3382
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.