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

Support to run karmadactl init within a pod #3338

Merged
merged 1 commit into from
Mar 29, 2023

Conversation

lonelyCZ
Copy link
Member

@lonelyCZ lonelyCZ commented Mar 28, 2023

What type of PR is this?
/kind feature

What this PR does / why we need it:
Support to run karmadactl init within a pod by InClusterConfig.

Which issue(s) this PR fixes:
Fixes #3335

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

`karmadactl`: Introduced support for running `init` within a pod.

@karmada-bot karmada-bot added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Mar 28, 2023
@karmada-bot karmada-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 28, 2023
@lonelyCZ
Copy link
Member Author

Hi @hzxuzhonghu , Could you please help test it in your env?

@@ -43,6 +44,13 @@ func RestConfig(context, kubeconfigPath string) (*rest.Config, error) {
kubeconfigPath = env.GetString("KUBECONFIG", defaultKubeConfig)
}
if !Exists(kubeconfigPath) {
klog.Warning("Neither --kubeconfig nor KUBECONFIG was specified and '~/.kube/config' doesn't exist. Using the inClusterConfig. This might not work.")
// if the kubeconfig path is not valid, try to load InClusterConfig
inClusterRestConfig, err := rest.InClusterConfig()
Copy link
Member

Choose a reason for hiding this comment

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

great, will test it

Copy link
Member

@hzxuzhonghu hzxuzhonghu left a comment

Choose a reason for hiding this comment

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

/lgtm

It can connect to kube-apiserver successfully, we just need to grant some permission to the service account.

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 28, 2023
@lonelyCZ
Copy link
Member Author

It can connect to kube-apiserver successfully, we just need to grant some permission to the service account.

Great, I will update the klog message to match our scenario.

@lonelyCZ lonelyCZ force-pushed the pr-incluster-config branch from 4dcfea8 to 9654407 Compare March 28, 2023 09:45
@karmada-bot karmada-bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 28, 2023
@lonelyCZ lonelyCZ changed the title [WIP]Support to run karmadactl init within a pod Support to run karmadactl init within a pod Mar 28, 2023
@karmada-bot karmada-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 28, 2023
@lonelyCZ
Copy link
Member Author

If there isn't kubeconfig, the message is that

[root@master67 karmada]# _output/bin/linux/amd64/karmadactl init
W0328 17:43:07.952514    1571 apiclient.go:47] Kubeconfig '/root/.kube/config' doesn't exist. Using the inClusterConfig. This might not work.
W0328 17:43:07.953343    1571 apiclient.go:53] error creating inClusterConfig: unable to load in-cluster configuration, KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT must be defined
error: Missing or incomplete configuration info.  Please point to an existing, complete config file:
  1. Via the command-line flag --kubeconfig
  2. Via the KUBECONFIG environment variable
  3. In your home directory as ~/.kube/config

What do you think?

/cc @RainbowMango

@RainbowMango
Copy link
Member

/assign

@@ -43,6 +44,13 @@ func RestConfig(context, kubeconfigPath string) (*rest.Config, error) {
kubeconfigPath = env.GetString("KUBECONFIG", defaultKubeConfig)
}
if !Exists(kubeconfigPath) {
klog.Warningf("Kubeconfig '%s' doesn't exist. Using the inClusterConfig. This might not work.", kubeconfigPath)
Copy link
Member

Choose a reason for hiding this comment

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

Since we are going to officially support the in-cluster mode, we can think of this(build rest config by InClusterConfig) as a normal way, so, this log should not be a warning.

Copy link
Member

Choose a reason for hiding this comment

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

How about this?

	if !Exists(kubeconfigPath) {
		// Given no kubeconfig is provided, give it a try to load the config by
		// in-cluster mode if the client running inside a pod running on kubernetes.
		host, port := os.Getenv("KUBERNETES_SERVICE_HOST"), os.Getenv("KUBERNETES_SERVICE_PORT")
		if len(host) > 0 || len(port) > 0 { // in-cluster mode
			inClusterRestConfig, err := rest.InClusterConfig()
			if err != nil {
				return nil, fmt.Errorf("failed to load rest config by in-cluster mode: %v", err)
			}
			return inClusterRestConfig, nil
		}

		return nil, ErrEmptyConfig
	}

If no kubeconfig is provided and it works inside a Pod, we assume that it is intended to be running inside, so we just return the in-cluster error.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is great.

Signed-off-by: lonelyCZ <chengzhe@zju.edu.cn>
@lonelyCZ lonelyCZ force-pushed the pr-incluster-config branch from 9654407 to 59ef627 Compare March 29, 2023 02:54
@karmada-bot karmada-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 29, 2023
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 29, 2023
@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu, RainbowMango

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:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@karmada-bot karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 29, 2023
@RainbowMango RainbowMango added this to the v1.6 milestone Mar 29, 2023
@karmada-bot karmada-bot merged commit 66ed564 into karmada-io:master Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed runing kubectl-karmada within a pod
4 participants