kubernetes-client / c

Official C client library for Kubernetes
Apache License 2.0
148 stars 49 forks source link

add support to load kubernetes configuration for memory buffer #242

Closed DanyT closed 4 months ago

DanyT commented 4 months ago
ityuhui commented 4 months ago

Thanks for your PR. I will review it.

brendandburns commented 4 months ago

Minor comments.

brendandburns commented 4 months ago

This looks good to me. Thanks for the quick turn around. I'll wait for @ityuhui to take a look though in case there are additional comments.

ityuhui commented 4 months ago

Thanks @brendandburns

@DanyT Can you please fix the issues reported by the code-static-check and code-sytle-check https://github.com/kubernetes-client/c/actions/runs/9781006686/job/27005098287?pr=242 in your code changes ? (Just update your code changes)

You can run the checks locally with

sh ./code-check/code-static-check.sh ./kubernetes/config/

and

find ./kubernetes/config/ -type f -regextype posix-extended -regex ".*\.(c|h)" -exec sh ./code-check/code-style-check.sh {} \;
ityuhui commented 4 months ago

@DanyT Could you please write an example that uses your new function so we can add it to the automated tests in case your functions are broken by future code changes. We also have memory leak testing in automated testing.

FYI https://github.com/kubernetes-client/c/blob/dc7d67098361c3579e3dd8153ccc6c4c30ddfedb/examples/Makefile#L33

DanyT commented 4 months ago

@ityuhui I have added:

Let me know if there is something that i missed.

ityuhui commented 4 months ago

@DanyT

It seems that the build failed

cd list_pod_buffer; make test
make[1]: Entering directory '/home/runner/work/c/c/examples/list_pod_buffer'
./list_pod_buffer_bin
Cannot get kubernetes configuration from file.
make[1]: *** [Makefile:11: test] Error 255
make[1]: Leaving directory '/home/runner/work/c/c/examples/list_pod_buffer'
make: *** [Makefile:40: test] Error 2
Error: Process completed with exit code 2.
DanyT commented 4 months ago

Interesting... it is not the build itself that fails but at runtime it cannot find/load the kube config file. I will look into it. Is there a special location where the kube config file is delivered in this test environment? @ityuhui

ityuhui commented 4 months ago

Nothing special, the configuration is located in $HOME/.kube/config

FYI https://github.com/kubernetes-client/c/blob/dc7d67098361c3579e3dd8153ccc6c4c30ddfedb/kubernetes/config/kube_config.c#L130

DanyT commented 4 months ago

@ityuhui Sorry for the delay. This should fix the issue.

ityuhui commented 4 months ago

Thank you @DanyT

Can you add your example to the memory leak test ?Sorry for not reminding you earlier.

https://github.com/kubernetes-client/c/blob/dc7d67098361c3579e3dd8153ccc6c4c30ddfedb/examples/Makefile#L53

DanyT commented 4 months ago

Done

ityuhui commented 4 months ago

I have no question about this PR. Thanks to @DanyT for the contribution !

@brendandburns I'm OK with the changes.

brendandburns commented 4 months ago

/lgtm /approve

k8s-ci-robot commented 4 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns, DanyT, ityuhui

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-client/c/blob/master/OWNERS)~~ [brendandburns,ityuhui] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment