kubernetes-client / c

Official C client library for Kubernetes
Apache License 2.0
146 stars 46 forks source link

Different return types of Delete Pod #31

Closed clearday4 closed 3 years ago

clearday4 commented 3 years ago

When calling CoreV1API_deleteNamespacedPod() if there is an error, returns a pointer of v1_status_t type. but if it succeeds, it returns a pointer of v1_pod_t type. The return type depends on the success / fail of the function call. How can we handle this?

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#delete-pod-v1-core According to the above link, v1_pod_t is returned when delete pod is success. and v1_status_t is returned when delete collection of pod is success. There is no mention of what structure type will be returned when delete command fails.

First, you need to change the return type of CoreV1API_deleteNamespacedPod() to v1_pod_t pointer type.

brendandburns commented 3 years ago

This code is generated, so unfortunately we really can't do this.

This is basically a fundamental disconnect between the implementation of the Kubernetes API (which as you note, can return multiple different types) and the Swagger/OpenAPI document for the API (which only describes one type)

There's a much larger discussion of the issue here: https://github.com/kubernetes-client/java/issues/86

clearday4 commented 3 years ago

In the current situation, if CoreV1API_deleteNamespacedPod() succeeds, app got a segfault. Shouldn't we modify this part?

ityuhui commented 3 years ago

Hi @clearday4

Which case will trigger a segfault by CoreV1API_deleteNamespacedPod ?

Case 1: Deleting succeeded ( e.g. The pod that will be deleted is valid) Or Case 2: Deleting failed ( e.g. The pod that will be deleted does not exist)

clearday4 commented 3 years ago

Hi Case 1 trigger a segfault. Because Jason returned a v1_pod_t structure type but library tries to parse v1_status_t structure type. v1_status_t *elementToReturn = v1_status_parseFromJSON(CoreV1APIlocalVarJSON);

ityuhui commented 3 years ago

When deleting pod succeeds, CoreV1API_deleteNamespacedPod will not raise a segment fault itself, but because CoreV1APIlocalVarJSON is not a valid v1_status_t json string, elementToReturn will be NULL. Using elementToReturn without checking will trigger a segment.

So, this segfault can be avoided by below gurad:

void delete_a_pod(apiClient_t * apiClient)
{
    v1_status_t *status = CoreV1API_deleteNamespacedPod(apiClient, 
                                                "test-pod-6",   // char *name
                                                "default",      // char *namespace
                                                NULL,           // char *pretty
                                                NULL,           // char *dryRun
                                                0,              // int gracePeriodSeconds
                                                0,              // int orphanDependents
                                                NULL,           // char *propagationPolicy
                                                NULL            // v1_delete_options_t *body
                                                );

    printf("The return code of HTTP request=%ld\n", apiClient->response_code);

    if ( 200 == apiClient->response_code ||
         202 == apiClient->response_code) {
        printf("delete pod succeed\n");
    } else {
        if (status && status->message) {
            printf("error message: %s\n", status->message);
        }
    }
}

As @brendandburns said, the parsing is generated according to Swagger doc. It's not easy to update by manual. Can the above example code resovle your problem ?

clearday4 commented 3 years ago

Thank you for the opinion we will change the code you suggested.

brendandburns commented 3 years ago

You can also use the generic delete: https://github.com/kubernetes-client/c/blob/master/kubernetes/src/generic.c#L118

Which will return the raw JSON.

clearday4 commented 3 years ago

closed