JSON deserialise exception when using PodV1Client.Delete()

[felixfbecker]

DELETE on a pod seems to return the deleted pod:

VERBOSE: KubeClient.KubeApiClient.Http: Completed DELETE request to 'https://localhost:6443/api/v1/namespaces/pskubectltest/pods/hello-world-bdf85c5f9-jhpxr' (OK).
VERBOSE: KubeClient.KubeApiClient.Http: Receive response body for DELETE request to 'https://localhost:6443/api/v1/namespaces/pskubectltest/pods/hello-world-bdf85c5f9-qhr89' (OK):
{"kind":"Pod","apiVersion":"v1","metadata":{"name":"hello-world-bdf85c5f9-qhr89","generateName":"hello-world-bdf85c5f9-","namespace":"pskubectltest","selfLink":"/api/v1/namespaces/pskubectltest/pods/hello-world-bdf85c5f9-qhr89","uid":"52ca2432-017b-11e9-9449-025000000001","resourceVersion":"3166","creationTimestamp":"2018-12-16T21:41:19Z","deletionTimestamp":"2018-12-16T21:44:42Z","deletionGracePeriodSeconds":30,"labels":{"app":"hello-world","pod-template-hash":"689417195"},"ownerReferences":[{"apiVersion":"extensions/v1beta1","kind":"ReplicaSet","name":"hello-world-bdf85c5f9","uid":"d94a88f4-017a-11e9-9449-025000000001","controller":true,"blockOwnerDeletion":true}]},"spec":{"volumes":[{"name":"default-token-6nk4d","secret":{"secretName":"default-token-6nk4d","defaultMode":420}}],"containers":[{"name":"hello-world","image":"strm/helloworld-http:latest","ports":[{"containerPort":80,"protocol":"TCP"}],"resources":{},"volumeMounts":[{"name":"default-token-6nk4d","readOnly":true,"mountPath":"/var/run/secrets/kubernetes.io/serviceaccount"}],"terminationMessagePath":"/dev/termination-log","terminationMessagePolicy":"File","imagePullPolicy":"IfNotPresent"}],"restartPolicy":"Always","terminationGracePeriodSeconds":30,"dnsPolicy":"ClusterFirst","serviceAccountName":"default","serviceAccount":"default","nodeName":"docker-for-desktop","securityContext":{},"schedulerName":"default-scheduler","tolerations":[{"key":"node.kubernetes.io/not-ready","operator":"Exists","effect":"NoExecute","tolerationSeconds":300},{"key":"node.kubernetes.io/unreachable","operator":"Exists","effect":"NoExecute","tolerationSeconds":300}]},"status":{"phase":"Running","conditions":[{"type":"Initialized","status":"True","lastProbeTime":null,"lastTransitionTime":"2018-12-16T21:41:19Z"},{"type":"Ready","status":"True","lastProbeTime":null,"lastTransitionTime":"2018-12-16T21:41:22Z"},{"type":"PodScheduled","status":"True","lastProbeTime":null,"lastTransitionTime":"2018-12-16T21:41:19Z"}],"hostIP":"192.168.65.3","podIP":"10.1.0.47","startTime":"2018-12-16T21:41:19Z","containerStatuses":[{"name":"hello-world","state":{"running":{"startedAt":"2018-12-16T21:41:22Z"}},"lastState":{},"ready":true,"restartCount":0,"image":"strm/helloworld-http:latest","imageID":"docker-pullable://strm/helloworld-http@sha256:bd44b0ca80c26b5eba984bf498a9c3bab0eb1c59d30d8df3cb2c073937ee4e45","containerID":"docker://7b55dace41825a4780fd66b34e4fd0cbccc39575273df8b4392f797bb0ee6a6b"}],"qosClass":"BestEffort"}}

but PodV1Client.delete() tries to deserialise it into a StatusV1 object and therefor always throws

Newtonsoft.Json.JsonReaderException: Unexpected character encountered while parsing value: {. Path 'status', line 1, position 1640.
   at Newtonsoft.Json.JsonTextReader.ReadStringValue(ReadType readType)
   at Newtonsoft.Json.JsonTextReader.ReadAsString()
   at Newtonsoft.Json.JsonReader.ReadForType(JsonContract contract, Boolean hasConverter)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
   at Newtonsoft.Json.JsonSerializer.Deserialize(TextReader reader, Type objectType)
   at HTTPlease.Formatters.Json.JsonFormatter.ReadAsync(InputFormatterContext context, Stream stream)
   at HTTPlease.Formatters.ContentExtensions.ReadAsAsync[TBody](HttpContent content, IInputFormatter formatter, InputFormatterContext formatterContext)
   at HTTPlease.FormatterResponseExtensions.ReadContentAsAsync[TBody](HttpResponseMessage responseMessage, IInputFormatter formatter, InputFormatterContext formatterContext)
   at HTTPlease.FormatterResponseExtensions.ReadContentAsAsync[TBody](HttpResponseMessage responseMessage, IFormatterCollection formatters)
   at HTTPlease.FormatterResponseExtensions.ReadContentAsAsync[TBody](HttpResponseMessage responseMessage)
   at HTTPlease.FormatterResponseExtensions.ReadContentAsAsync[TBody,TError](Task`1 response, HttpStatusCode[] successStatusCodes)
   at KubeClient.ResourceClients.HttpExtensions.ReadContentAsObjectV1Async[TObject](Task`1 response, String operationDescription, HttpStatusCode[] successStatusCodes)
   at KubeClient.ResourceClients.PodClientV1.Delete(String name, String kubeNamespace, CancellationToken cancellationToken)
   at Kubectl.Cmdlets.RemoveKubePodCmdlet.deletePod(String name, String kubeNamespace, CancellationToken cancellationToken) in /Users/felix/git/PSKubectl/src/Cmdlets/RemoveKubePodCmdlet.cs:line 62

Tested by trying to delete a pod in Docker Kubernetes.

[tintoy]

Actually it’s a good deal uglier than that - whether it returns a PodV1 or a StatusV1 depends on whether you’ve requested immediate deletion or not - I fixed this for some other resource type, will look it up when I get home :)

[felixfbecker]

It seems like most resource types return KubeObjectV1 which is a common super type of StatusV1 and PodV1/any other resource, but some others return StatusV1: https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/tintoy/dotnet-kube-client%24+%22+Delete%28%22#1

[felixfbecker]

DeploymentV1Client switches the deserialisation type based on the propagationPolicy passed: https://sourcegraph.com/github.com/tintoy/dotnet-kube-client/-/blob/src/KubeClient/ResourceClients/DeploymentClientV1.cs#L209-212
But PodV1Client doesn't have such a parameter

[felixfbecker]

Also the way that DeploymentClientV1 defaults propagationPolicy to Background actually seems to override the native Kubernetes default behaviour which depends on the resource:

Whether and how garbage collection will be performed. Either this field or OrphanDependents may be set, but not both. The default policy is decided by the existing finalizer set in the metadata.finalizers and the resource-specific default policy. Acceptable values are: 'Orphan' - orphan the dependents; 'Background' - allow the garbage collector to delete the dependents in the background; 'Foreground' - a cascading policy that deletes all dependents in the foreground.

Maybe instead of switching on parameters it should use a JSON deserialiser that looks at the returned Kind field?

[tintoy]

Yeah, probably - I was trying to avoid that but I think you’re right.

[felixfbecker]

Another thing I noticed: If the resource is not found, a Status object is returned with the status property being the string "Failure" instead of a PodStatusV1 object:

VERBOSE: KubeClient.KubeApiClient.Http: Performing DELETE request to 'https://localhost:6443/api/v1/namespaces/pskubectltest/pods/pod/hello-world-dbd74b87c-62v8n'.
VERBOSE: KubeClient.KubeApiClient.Http: Receive response body for DELETE request to 'https://localhost:6443/api/v1/namespaces/pskubectltest/pods/pod/hello-world-dbd74b87c-62v8n' (NotFound):
{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"the server could not find the requested resource","reason":"NotFound","details":{},"code":404}

VERBOSE: KubeClient.KubeApiClient.Http: Completed DELETE request to 'https://localhost:6443/api/v1/namespaces/pskubectltest/pods/pod/hello-world-dbd74b87c-62v8n' (NotFound).
WARNING: Newtonsoft.Json.JsonSerializationException: Error converting value "Failure" to type 'KubeClient.Models.PodStatusV1'. Path 'status', line 1, position 67. ---> System.ArgumentException: Could not cast or convert from System.String to KubeClient.Models.PodStatusV1.
   at Newtonsoft.Json.Utilities.ConvertUtils.EnsureTypeAssignable(Object value, Type initialType, Type targetType)
   at Newtonsoft.Json.Utilities.ConvertUtils.ConvertOrCast(Object initialValue, CultureInfo culture, Type targetType)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.EnsureType(JsonReader reader, Object value, CultureInfo culture, JsonContract contract, Type targetType)
   --- End of inner exception stack trace ---
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.EnsureType(JsonReader reader, Object value, CultureInfo culture, JsonContract contract, Type targetType)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
   at Newtonsoft.Json.JsonSerializer.Deserialize(TextReader reader, Type objectType)
   at HTTPlease.Formatters.Json.JsonFormatter.ReadAsync(InputFormatterContext context, Stream stream)
   at HTTPlease.Formatters.ContentExtensions.ReadAsAsync[TBody](HttpContent content, IInputFormatter formatter, InputFormatterContext formatterContext)
   at HTTPlease.FormatterResponseExtensions.ReadContentAsAsync[TBody](HttpResponseMessage responseMessage, IInputFormatter formatter, InputFormatterContext formatterContext)
   at HTTPlease.FormatterResponseExtensions.ReadContentAsAsync[TBody](HttpResponseMessage responseMessage, IFormatterCollection formatters)
   at HTTPlease.FormatterResponseExtensions.ReadContentAsAsync[TBody](HttpResponseMessage responseMessage)
   at HTTPlease.FormatterResponseExtensions.ReadContentAsAsync[TBody,TError](Task`1 response, HttpStatusCode[] successStatusCodes)
   at KubeClient.ResourceClients.HttpExtensions.ReadContentAsObjectV1Async[TObject](Task`1 response, String operationDescription, HttpStatusCode[] successStatusCodes) in /Users/felix/git/dotnet-kube-client/src/KubeClient/ResourceClients/HttpExtensions.cs:line 73
   at KubeClient.ResourceClients.PodClientV1.Delete(String name, String kubeNamespace, CancellationToken cancellationToken) in /Users/felix/git/dotnet-kube-client/src/KubeClient/ResourceClients/PodClientV1.cs:line 247
   at Kubectl.Cmdlets.RemoveKubePodCmdlet.deletePod(String name, String kubeNamespace, CancellationToken cancellationToken) in /Users/felix/git/PSKubectl/src/Cmdlets/RemoveKubePodCmdlet.cs:line 66

[tintoy]

I’m starting to think maybe we should have thrown exceptions for those responses but it’s a bit late to change now :(

Let me have a think about it for a day or 2, unless you have an opinion on the best way to deal with it?

[tintoy]

BTW, I’m moving house this week - it may be a couple of days before I get to this :)

[tintoy]

(need to get internet connected at new place)

[felixfbecker]

I’m starting to think maybe we should have thrown exceptions for those responses but it’s a bit late to change now :(

Throwing on 404s is what I would expect, is that not the case?
It could be changed in a major version or with a config option

[tintoy]

I generally try to avoid making consumers catch exceptions for common scenarios, so we return null if we get a 404 and the returned StatusV1 indicates that the resource was not found (otherwise we throw).

[tintoy]

Hey, @felixfbecker - did the latest set of KubeResultV1 changes fix this for your use-case?

[felixfbecker]

Yup