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

"Kubernetes Resource Mistype" error with CoreV1API.deleteNamespace #4

Closed
janslow opened this issue Jul 18, 2022 · 7 comments
Closed

Comments

@janslow
Copy link

janslow commented Jul 18, 2022

Hey, thanks for the work you've done on this project!

I've encountered an issue with a couple of the delete methods on the API classes:

    error: Error: Kubernetes Resource Mistype: Expected "v1/Status", but was given "v1/Namespace". This is likely a library bug. Feel free to file an issue with a stack trace.
      throw new Error(`Kubernetes Resource Mistype: `+
            ^
        at Module.assertOrAddApiVersionAndKind (https://deno.land/x/kubernetes_apis@v0.3.2/common.ts:56:9)
        at Module.toStatus (https://deno.land/x/kubernetes_apis@v0.3.2/builtin/meta@v1/structs.ts:419:10)
        at CoreV1Api.deleteNamespace (https://deno.land/x/kubernetes_apis@v0.3.2/builtin/core@v1/mod.ts:193:19)

Namespace deletion is asynchronous, so the first API call returns a v1/Namespace object (with phase: Terminating), then only returns a v1/Status object when the deletion is complete.

Unfortunately, the API Reference doesn't explain/document this behaviour.

I've also encountered the same issue on the ApiextensionsV1Api.deleteCustomResourceDefinition method.

    error: Error: Kubernetes Resource Mistype: Expected "v1/Status", but was given "apiextensions.k8s.io/v1/CustomResourceDefinition". This is likely a library bug. Feel free to file an issue with a stack trace.
      throw new Error(`Kubernetes Resource Mistype: `+
            ^
        at Module.assertOrAddApiVersionAndKind (https://deno.land/x/kubernetes_apis@v0.3.2/common.ts:56:9)
        at Module.toStatus (https://deno.land/x/kubernetes_apis@v0.3.2/builtin/meta@v1/structs.ts:419:10)
        at ApiextensionsV1Api.deleteCustomResourceDefinition (https://deno.land/x/kubernetes_apis@v0.3.2/builtin/apiextensions.k8s.io@v1/mod.ts:80:19)
@danopia
Copy link
Member

danopia commented Jul 18, 2022

Hey, thanks for writing in.

The inconsistent response bodies on deletion of certain Kinds is something I've run into previously. I used to use a silly workaround for it:

policyApi
        .namespace(unusedBudget.namespace)
        .deletePodDisruptionBudget(unusedBudget.name, {})
        .catch(err => 'TODO: deletion has a response parsing error')

I added a fix at some point and was able to remove my workarounds when updating to /x/kubernetes_apis@v0.2.0.

I'll have to revisit and see what is going on with those specific kinds..

@danopia
Copy link
Member

danopia commented Jul 18, 2022

Ok, my previous workaround was less forward-thinking than I hoped:

// Known remaining instances of https://github.com/kubernetes/kubernetes/issues/59501
const returnOnDeleteOps = new Set([
'batch/deleteJob',
]);

There's a bunch more known Kinds that return the deleted resource. But the kubernetes java thread said that even the kinds that return the deleted resource will switch to returning just a Status if called again (e.g. if there wasn't a resource to delete). So the actual correct fix must be returning e.g. metav1.Status | corev1.Namespace from deleteNamespace(), and letting the consumer narrow the response type if desired.

This would be a breaking change of course!

Too bad the openapi doesn't seem to have any of this info, it would be nice to know which delete methods never return the deleted resources.

References:

grep -r "ReturnDeletedObject: true" pkg
pkg/registry/core/pod/storage/storage.go:               ReturnDeletedObject: true,
pkg/registry/core/podtemplate/storage/storage.go:               ReturnDeletedObject: true,
pkg/registry/core/persistentvolumeclaim/storage/storage.go:             ReturnDeletedObject: true,
pkg/registry/core/namespace/storage/storage.go:         ReturnDeletedObject: true,
pkg/registry/core/persistentvolume/storage/storage.go:          ReturnDeletedObject: true,
pkg/registry/core/serviceaccount/storage/storage.go:            ReturnDeletedObject: true,
pkg/registry/core/resourcequota/storage/storage.go:             ReturnDeletedObject: true,
pkg/registry/storage/volumeattachment/storage/storage.go:               ReturnDeletedObject: true,
pkg/registry/storage/csidriver/storage/storage.go:              ReturnDeletedObject: true,
pkg/registry/storage/storageclass/storage/storage.go:           ReturnDeletedObject: true,
pkg/registry/storage/csinode/storage/storage.go:                ReturnDeletedObject: true,
pkg/registry/policy/podsecuritypolicy/storage/storage.go:               ReturnDeletedObject: true,

@janslow
Copy link
Author

janslow commented Jul 18, 2022

Ok, my previous workaround was less forward-thinking than I hoped:

Yeah, I've ended up just creating a variant which returns metav1.Status | corev1.Namespace (then I can call it repeatedly until it returns a metav1.Status)

This would be a breaking change of course!

Yeah, although it's worth it IMO, seeing as the only time you'll get a metav1.Status is if you try to delete something that has already been fully deleted (which you can't do as the first request fails), which makes the methods pretty hard to use as is.

Alternatively, it could be done as an additional option, e.g., expectStatus: false, (similar to how setting expectJson: true on the RestClient.performRequest changes the type), that changes the signature from Promise<metav1.Status> to Promise<metav1.Status | corev1.Namespace>.

Too bad the openapi doesn't seem to have any of this info, it would be nice to know which delete methods never return the deleted resources.

Yeah, it seems that every issue raised against the kubernetes/kubernetes project to fix the docs has just gone stale 😢

I guess there's a few options:

  1. Add more entries to returnOnDeleteOps (and fix it to handle metav1.Status)
  2. Assumed that any DELETE operation may return metav1.Status | $deletedObject (which is what kube-rs has done)
  3. Coerce the response into a metav1.Status, even if it's a corev1.Namespace (e.g., like the Java client)
  4. Make this an opt-in behaviour by allowing an extra option (e.g., expectStatus: false) to be used to change the signature.

I'm not convinced that (1) is a good option, as there's inevitably going to be missing entries. E.g., CRDs also exhibit this behaviour, but aren't included in the grep you posted.

(2) will provide the most consistent API, although it will require a breaking change (and consumers may need to add their own type assertions to narrow types)

(3) feels really hacky (I'm guessing they chose it for Java because of the lack of union types) and it'd make it harder to work out whether a resource is deleted or just in the process of being deleted, although it avoids breaking changes.

(4) also avoids breaking changes, but is the most complex to implement and use (e.g., I think it'd need to be clearly documented)

I'd be happy to raise a PR to implement one of these options, if you'd like.

@adb-sh
Copy link

adb-sh commented Jan 23, 2023

hey :) got a similar issue when i tried to delete a service with coreApi.deleteService(name). the service will get deleted, but i also receive the following error message:

Kubernetes Resource Mistype: Expected "v1/Status", but was given "v1/Service"

@mjacobsson
Copy link

I'm also hit by this issue:

Error: Kubernetes Resource Mistype: Expected "v1/Status", but was given "argoproj.io/v1alpha1/Application". This is likely a library bug. Feel free to file an issue with a stack trace.
at Module.assertOrAddApiVersionAndKind (https://deno.land/x/kubernetes_apis@v0.3.2/common.ts:56:9)
at Module.toStatus (https://deno.land/x/kubernetes_apis@v0.3.2/builtin/meta@v1/structs.ts:419:10)
at ArgoprojIoV1alpha1NamespacedApi.deleteApplication (https://deno.land/x/kubernetes_apis@v0.3.2/argo-cd/argoproj.io@v1alpha1/mod.ts:144:19)

@danopia
Copy link
Member

danopia commented Feb 9, 2023

Thanks for bumping this issue. Clearly it comes up pretty often to users 😢

A workaround would be swallowing the library bug errors when you call delete APIs:

await api.deleteWhatever(...).catch(err => err.message.includes("library bug") ? null : Promise.reject(err))

To be frank I've simply done .catch(() => {}) in my own scripts, since it's just for deletes.

The proper fix will be returning metav1.Status | T from delete functions, which is a breaking change (types-wise) but still needs to be done.

@danopia
Copy link
Member

danopia commented Feb 10, 2023

I have a patch to address this released in v0.4.0. I couldn't actually reproduce some of the results yall reported (e.g. ArgoprojIoV1alpha1NamespacedApi.deleteApplication returning the deleted Application) but well that's the point of returning a union.

I also dug into the Kubernetes apiserver code to check if deleteXyzList() functions have this issue, and it seems they do not; the delete collection functions always return the original resources they found to-be-deleted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants