Object loses TypeMeta after Patch

[vincepri]

I have this code in a defer function running in a Reconciler:

if err := r.Client.Patch(ctx, cluster, patchCluster); err != nil {
	klog.Errorf("Error Patching Cluster %q in namespace %q: %v", cluster.Name, cluster.Namespace, err)
}
if err := r.Client.Status().Patch(ctx, cluster, patchCluster); err != nil {
	klog.Errorf("Error Patching Cluster status %q in namespace %q: %v", cluster.Name, cluster.Namespace, err)
}

which fails with the following error

E0719 19:31:54.850045 1 cluster_controller.go:120] Error Patching Cluster status "cluster-kxg94" in namespace "clusterapi-test-956xc": unmarshalerDecoder: Object 'Kind' is missing in '{"metadata":{"creationTimestamp":"2019-07-19T19:31:54Z","finalizers":["cluster.cluster.sigs.k8s.io"],"generateName":"cluster-","generation":1,"name":"cluster-kxg94","namespace":"clusterapi-test-956xc","resourceVersion":"529","uid":"fe7ee04d-0ef2-4d7c-bdac-85a011d7b54e"},"spec":{"clusterNetwork":{"pods":{"cidrBlocks":["192.168.0.0/16"]},"serviceDomain":"mydomain.com","services":{"cidrBlocks":["10.96.0.0/12"]}}},"status":{"phase":"pending"}}', error found in #10 byte of ...|pending"}}|..., bigger context ...|:["10.96.0.0/12"]}}},"status":{"phase":"pending"}}|...

I added some spew.Dump lines between the updates and it seems that the first Patch that goes through (no matter the order) makes the original object to lose TypeMeta.

Before

(*v1alpha2.Cluster)(0xc00024f680)({
 TypeMeta: (v1.TypeMeta) &TypeMeta{Kind:Cluster,APIVersion:cluster.sigs.k8s.io/v1alpha2,},
 ObjectMeta: (v1.ObjectMeta) &ObjectMeta{Name:cluster-kxg94,GenerateName:cluster-,Namespace:clusterapi-test-956xc,SelfLink:/apis/cluster.sigs.k8s.io/v1alpha2/namespaces/clusterapi-test-956xc/clusters/cluster-kxg94,UID:fe7ee04d-0ef2-4d7c-bdac-85a011d7b54e,ResourceVersion:619,Generation:2,CreationTimestamp:2019-07-19 19:31:54 +0000 UTC,DeletionTimestamp:2019-07-19 19:32:59 +0000 UTC,DeletionGracePeriodSeconds:*0,Labels:map[string]string{},Annotations:map[string]string{},OwnerReferences:[],Finalizers:[],ClusterName:,Initializers:nil,ManagedFields:[],},
 Spec: (v1alpha2.ClusterSpec) {
  ClusterNetwork: (*v1alpha2.ClusterNetworkingConfig)(0xc0004a66c0)({
   Services: (v1alpha2.NetworkRanges) {
    CIDRBlocks: ([]string) (len=1 cap=1) {
     (string) (len=12) "10.96.0.0/12"
    }
   },
   Pods: (v1alpha2.NetworkRanges) {
    CIDRBlocks: ([]string) (len=1 cap=1) {
     (string) (len=14) "192.168.0.0/16"
    }
   },
   ServiceDomain: (string) (len=12) "mydomain.com"
  }),
  InfrastructureRef: (*v1.ObjectReference)(<nil>)
 },
 Status: (v1alpha2.ClusterStatus) {
  APIEndpoint: (v1alpha2.APIEndpoint) {
   Host: (string) "",
   Port: (int) 0
  },
  ErrorReason: (*common.ClusterStatusError)(<nil>),
  ErrorMessage: (*string)(<nil>),
  Phase: (*string)(0xc000391b90)((len=8) "deleting"),
  InfrastructureReady: (bool) false
 }
})

After:

(*v1alpha2.Cluster)(0xc00024f680)({
 TypeMeta: (v1.TypeMeta) &TypeMeta{Kind:,APIVersion:,},
 ObjectMeta: (v1.ObjectMeta) &ObjectMeta{Name:cluster-kxg94,GenerateName:cluster-,Namespace:clusterapi-test-956xc,SelfLink:/apis/cluster.sigs.k8s.io/v1alpha2/namespaces/clusterapi-test-956xc/clusters/cluster-kxg94,UID:fe7ee04d-0ef2-4d7c-bdac-85a011d7b54e,ResourceVersion:619,Generation:2,CreationTimestamp:2019-07-19 19:31:54 +0000 UTC,DeletionTimestamp:2019-07-19 19:32:59 +0000 UTC,DeletionGracePeriodSeconds:*0,Labels:map[string]string{},Annotations:map[string]string{},OwnerReferences:[],Finalizers:[],ClusterName:,Initializers:nil,ManagedFields:[],},
 Spec: (v1alpha2.ClusterSpec) {
  ClusterNetwork: (*v1alpha2.ClusterNetworkingConfig)(0xc0004a66c0)({
   Services: (v1alpha2.NetworkRanges) {
    CIDRBlocks: ([]string) (len=1 cap=1) {
     (string) (len=12) "10.96.0.0/12"
    }
   },
   Pods: (v1alpha2.NetworkRanges) {
    CIDRBlocks: ([]string) (len=1 cap=1) {
     (string) (len=14) "192.168.0.0/16"
    }
   },
   ServiceDomain: (string) (len=12) "mydomain.com"
  }),
  InfrastructureRef: (*v1.ObjectReference)(<nil>)
 },
 Status: (v1alpha2.ClusterStatus) {
  APIEndpoint: (v1alpha2.APIEndpoint) {
   Host: (string) "",
   Port: (int) 0
  },
  ErrorReason: (*common.ClusterStatusError)(<nil>),
  ErrorMessage: (*string)(<nil>),
  Phase: (*string)(0xc000391b90)((len=8) "deleting"),
  InfrastructureReady: (bool) false
 }
})

Probably related to #406

[vincepri]

/cc @DirectXMan12

[ncdc]

@liggitt do you have any advice for a reasonable fix here? We could have controller-runtime record the GVK pre-patch, then restore it using defer, but I'm wondering if there is something more fundamental we're either doing wrong or should be doing differently?

[liggitt]

I'm not familiar with these client methods... where is it clearing the GVK?

[liggitt]

@smarterclayton I'm not familiar with the full client stack, but this may be the direct encoder/decoder dropping GVK on decode. There are a couple related issues:

• Decoding should not clear apiVersion/kind kubernetes/kubernetes#80609 (just opened)
• apiVersion/kind exist on top-level REST responses (GET/LIST, WATCH events), but not on individual items within a LIST response kubernetes/kubernetes#3030 - individual list items do not contain kind/apiVersion, only the outer list does (wouldn't directly impact this update issue, but would mean an item in a cache populated from an initial LIST API call would not have kind/apiVersion set)

[ncdc]

@liggitt I'm guessing you came to the same/similar conclusion as kubernetes/apimachinery#19 with your new issue #80609?

[liggitt]

yes