karmada-io / karmada

Open, Multi-Cloud, Multi-Cluster Kubernetes Orchestration
https://karmada.io
Apache License 2.0
4.12k stars 806 forks source link

set karmadactl default config value from “” to karmada-apiserver.config #1770

Closed wuyingjun-lucky closed 1 year ago

wuyingjun-lucky commented 1 year ago

Signed-off-by: wuyingjun wuyingjun_yewu@cmss.chinamobile.com

What type of PR is this? /kind bug

What this PR does / why we need it: fix unjoin cluster example when dry-run option is set Which issue(s) this PR fixes: Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE
karmada-bot commented 1 year ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: To complete the pull request process, please assign rainbowmango after the PR has been reviewed. You can assign the PR to them by writing /assign @rainbowmango in a comment when ready.

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

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/karmada-io/karmada/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
lonelyCZ commented 1 year ago

Why using dry-run? We will remove resource of member cluster in control plane, but not to delete resource in the member cluster.

https://github.com/karmada-io/karmada/blob/da78076e261f796a03c6bb2306cb72664af956e1/pkg/karmadactl/unjoin.go#L171-L175

wuyingjun-lucky commented 1 year ago

Why using dry-run? We will remove resource of member cluster in control plane, but not to delete resource in the member cluster.

https://github.com/karmada-io/karmada/blob/da78076e261f796a03c6bb2306cb72664af956e1/pkg/karmadactl/unjoin.go#L171-L175

I may misunderstanding the usage of dry-run; But the origin usage do not work

image image
lonelyCZ commented 1 year ago

It seem not cluster created by Karmada. You can use kubectl --kubeconfig /etc/karmada/karmada-apiserver.config get clusters to get cluster in Karmada control plane.

wuyingjun-lucky commented 1 year ago

It seem not cluster created by Karmada. You can use kubectl --kubeconfig /etc/karmada/karmada-apiserver.config get clusters to get cluster in Karmada control plane.

I can get the clusters , that are joined an hour before

image

But it can be not be removed without setting the kubeconfig

lonelyCZ commented 1 year ago

Could you give me the logs when you unjoin it.

kubectl karmada unjoin cluster37 -v=6

wuyingjun-lucky commented 1 year ago

Could you give me the logs when you unjoin it.

kubectl karmada unjoin cluster37 -v=6

image
lonelyCZ commented 1 year ago

you need to set --kubeconfig=/etc/karmada/karmada-apiserver.config

wuyingjun-lucky commented 1 year ago

--kubeconfig=/etc/karmada/karmada-apiserver.config

Yes When I set the config, It works But I use the first one, it failed as it described in the help example

image
wuyingjun-lucky commented 1 year ago

you need to set --kubeconfig=/etc/karmada/karmada-apiserver.config so we set should karmada-controller config always and the cluster config is optional ?

lonelyCZ commented 1 year ago

But I use the first one, it failed as it described in the help example.

Oh, the usage example omit --kubeconfig by default.

wuyingjun-lucky commented 1 year ago

But I use the first one, it failed as it described in the help example.

Oh, the usage example omit --kubeconfig by default.

yes It misguide me, can I modify the usage in the pull request ?

wuyingjun-lucky commented 1 year ago

@lonelyCZ please check and review again

lonelyCZ commented 1 year ago

Yes, but we could add a global prompt in karmadactl

karmadactl as same as kubectl, checking kubeconfig in the following order.

1. Via the command-line flag --kubeconfig
2. Via the KUBECONFIG environment variable
3. In your home directory as ~/.kube/config
lonelyCZ commented 1 year ago

We should add --kubeconfig for all example, or set a global prompt. What do you think?

cc @RainbowMango @XiShanYongYe-Chang

wuyingjun-lucky commented 1 year ago

Yes, but we could add a global prompt in karmadactl

karmadactl as same as kubectl, checking kubeconfig in the following order.

1. Via the command-line flag --kubeconfig
2. Via the KUBECONFIG environment variable
3. In your home directory as ~/.kube/config

Do you think is it better by setting the default value from "" to /etc/karmada/karmada-apiserver.config

image
lonelyCZ commented 1 year ago

Do you think is it better by setting the default value from "" to /etc/karmada/karmada-apiserver.config

Great, but there are some problem, the path to karmada-kubeconfig is different for each deployment such as script, cli, and if it is null, it can be found in $HOME/.kube/config by default.

wuyingjun-lucky commented 1 year ago

Do you think is it better by setting the default value from "" to /etc/karmada/karmada-apiserver.config

Great, but there are some problem, the path to karmada-kubeconfig is different for each deployment such as script, cli, and if it is null, it can be found in $HOME/.kube/config by default.

I am sorry I can not understand for each deployment such asscript,cli , what does it means

XiShanYongYe-Chang commented 1 year ago

We should add --kubeconfig for all example, or set a global prompt. What do you think?

Personally, I prefer the latter.

wuyingjun-lucky commented 1 year ago

Do you think is it better by setting the default value from "" to /etc/karmada/karmada-apiserver.config

Great, but there are some problem, the path to karmada-kubeconfig is different for each deployment such as script, cli, and if it is null, it can be found in $HOME/.kube/config by default. is different for each deployment such as script, cli do you mean local-up.sh use the karmadactl to join member ? I am sorry I do not understand. @lonelyCZ

lonelyCZ commented 1 year ago

When we deploy karmada by hack/remote-up-karmada.sh, we put karmada-kubeconfig into host-kubeconfig file, usually locate at $HOME/.kube/config, which is different of karmadactl init.

wuyingjun-lucky commented 1 year ago

When we deploy karmada by hack/remote-up-karmada.sh, we put karmada-kubeconfig into host-kubeconfig file, usually locate at $HOME/.kube/config, which is different of karmadactl init.

All right , I got it. But can we to find a way to resolve it ? Just like use kubectl. I think hack/remote-up-karmada.sh can be different from the kubectl-karmada because hack/remote-up-karmada.sh override the kubeconfig but kubectl-karmada init make a new file.

lonelyCZ commented 1 year ago

Perhaps, we can read karmada-kubeconfig from $HOME/.kube/karmada.config by default, prompt users copy /etc/karmada/karmada-apiserver.config to here.

Like when we deploy kubernetes by kubeadm.

Your Kubernetes control-plane has initialized successfully!

To start using your cluster, you need to run the following as a regular user:

  mkdir -p $HOME/.kube
  sudo cp -i /etc/kubernetes/admin.conf $HOME/.kube/config
  sudo chown $(id -u):$(id -g) $HOME/.kube/config
wuyingjun-lucky commented 1 year ago

But the problem is we should use the host cluster config some times . So it just a message and how to use depend on themself?

wuyingjun-lucky commented 1 year ago

Perhaps, we can read karmada-kubeconfig from $HOME/.kube/karmada.config by default, prompt users copy /etc/karmada/karmada-apiserver.config to here.

Like when we deploy kubernetes by kubeadm.

Your Kubernetes control-plane has initialized successfully!

To start using your cluster, you need to run the following as a regular user:

  mkdir -p $HOME/.kube
  sudo cp -i /etc/kubernetes/admin.conf $HOME/.kube/config
  sudo chown $(id -u):$(id -g) $HOME/.kube/config

But the problem is we should use the host cluster config some times . So it just a message and how to use depend on themself?

lonelyCZ commented 1 year ago

But the problem is we should use the host cluster config some times . So it just a message and how to use depend on themself?

Yes, now the karmadactl use host cluster kubeconfig. We can set the default location of kubeconfig.

https://github.com/karmada-io/karmada/blob/3e057819f5605a1437a9b2b95f2cda78e182a8be/pkg/karmadactl/cmdinit/kubernetes/deploy.go#L38

wuyingjun-lucky commented 1 year ago

But the problem is we should use the host cluster config some times . So it just a message and how to use depend on themself?

Yes, now the karmadactl use host cluster kubeconfig. We can set the default location of kubeconfig.

https://github.com/karmada-io/karmada/blob/3e057819f5605a1437a9b2b95f2cda78e182a8be/pkg/karmadactl/cmdinit/kubernetes/deploy.go#L38

You mean we can modify the default-kube-config location https://github.com/karmada-io/karmada/blob/3e057819f5605a1437a9b2b95f2cda78e182a8be/pkg/karmadactl/cmdinit/kubernetes/deploy.go#:~:text=host%20kubernetes%20cluster-,if%20i.KubeConfig%20%3D%3D%20%22%22%20%7B,%7D,-restConfig%2C%20err with lowest priority but do not set the default value in the flags ?

lonelyCZ commented 1 year ago

Default values of flag may be required, but /etc/karmada/karmada-apiserver.config is not suitable.

wuyingjun-lucky commented 1 year ago

Default values of flag may be required, but /etc/karmada/karmada-apiserver.config is not suitable.

all right So should I close the pr ?