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

feat: disable kbcli addons in batch #285

Closed
wants to merge 0 commits into from
Closed

feat: disable kbcli addons in batch #285

wants to merge 0 commits into from

Conversation

SuperAayush
Copy link
Contributor

@SuperAayush SuperAayush commented Mar 8, 2024

The PR contains the code to make the kbcli to disable more than one addon at once. I have not tested the code yet since I am not aware of how to setup kbcli locally for testing purposes and have tried reaching out to @shanshanying for the same.

I respect the busy schedule all the maintainers but would be helpful if I can get KT on how should once setup kbcli locally mainly for testing purposes so that I can make the PR for this issue as well: #278

I am aware that this PR still needs many changes hence I am opening this as a draft PR.

Fixes: #237

@github-actions github-actions bot added the size/S Denotes a PR that changes 10-29 lines. label Mar 8, 2024
@shanshanying
Copy link
Contributor

shanshanying commented Mar 20, 2024

  1. build with
make kbcli

and get follwing err msg, pls fix it first.
image

  1. To test, just run
make test

More unit tests for addons should be added to pkg/cmd/addon/addon_test.go .

  1. to play with your newly build kbcli binary
    a) you have to install KubeBlocks first: https://kubeblocks.io/docs/release-0.8/user_docs/installation/install-with-kbcli/install-kubeblocks-with-kbcli
    b) and you may get the list of addon using
 ./bin/kbcli addon list

pkg/cmd/addon/addon.go Outdated Show resolved Hide resolved
pkg/cmd/addon/addon.go Outdated Show resolved Hide resolved
@SuperAayush
Copy link
Contributor Author

SuperAayush commented Mar 27, 2024

I have been trying to add a test for the disable command part. Have tried to debug using multiple cases but all of them throw the same error - The connection to the server localhost:8080 was refused - did you specify the right host or port?. I understand the error is saying that my code addition is trying to connect to a K8s existing cluster but it throws the same error even if I make a fake K8s client.

Here are the 3 different code changes I executed but got the same error:

  1. https://gist.github.com/SuperAayush/c89a011d6e8c8acb6eeb1ff50818dc0b

  2. https://gist.github.com/SuperAayush/7ba7838f6dec8138396728070dc47e35

  3. https://gist.github.com/SuperAayush/7f6f7d63ba1b4f4f25b4d2e8e0c17c7c

I did also try to use clientfake instead of restfake but still no change in the error.

cc - @shanshanying

@shanshanying
Copy link
Contributor

Reference
The error is raised at line
https://github.com/superaayush/kbcli/blob/improvement/kbcli-disable-addons-in-batch-issue237/pkg/cmd/addon/addon.go#L303

When addon cmd calls ServerVersion().
To solve this issue, you can refer to how client-go mock the test for this function at:
https://github.com/kubernetes/client-go/blob/1518fca9f06c6a73fc091535b8966c71704e657b/discovery/discovery_client_test.go#L48

Hope it will help.

@github-actions github-actions bot added size/M Denotes a PR that changes 30-99 lines. and removed size/S Denotes a PR that changes 10-29 lines. labels Mar 30, 2024
@shanshanying shanshanying marked this pull request as ready for review April 8, 2024 12:18
@shanshanying
Copy link
Contributor

/approve

@shanshanying
Copy link
Contributor

resolve conflicts pls.


"github.com/apecloud/kbcli/pkg/action"
"github.com/apecloud/kbcli/pkg/testing"
"github.com/apecloud/kbcli/pkg/types"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import order does not need to be adjusted.

@github-actions github-actions bot added size/XS Denotes a PR that changes 0-9 lines. and removed size/M Denotes a PR that changes 30-99 lines. labels Apr 10, 2024
@github-actions github-actions bot added this to the Release 0.9.0 milestone Apr 10, 2024
@SuperAayush
Copy link
Contributor Author

Strange, I did not close the PR. No idea what happened here 🤔

@SuperAayush
Copy link
Contributor Author

Going for a new PR

@SuperAayush
Copy link
Contributor Author

Please head over to this PR: #313

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS Denotes a PR that changes 0-9 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improvement] kbcli disable addons in batch
4 participants