Introduce `--kube-image-mirror-country` and `kube-image-registry` to karmadactl `init` #1764

wuyingjun-lucky · 2022-05-11T02:26:12Z

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

What type of PR is this?
/kind feature

What this PR does / why we need it:
support kube-image-mirror-country and kube-image-registry
Which issue(s) this PR fixes:
Fixes #
#1753
Special notes for your reviewer:

Does this PR introduce a user-facing change?:

`karmadactl`: introduced `--kube-image-mirror-country` and `--kube-image-registry` flags to `init` subcommand for Chinese mainland users.

RainbowMango · 2022-05-11T02:28:02Z

/assign @lonelyCZ

RainbowMango · 2022-05-11T02:28:12Z

/assign @prodanlabs

wuyingjun-lucky · 2022-05-12T08:26:42Z

@lonelyCZ help to review

karmada-bot · 2022-05-12T08:26:46Z

@wuyingjun-lucky: GitHub didn't allow me to request PR reviews from the following users: review, to.

Note that only karmada-io members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @lonelyCZ help to review

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lonelyCZ · 2022-05-12T08:29:23Z

@lonelyCZ help to review

Ok, I will review it today.

lonelyCZ · 2022-05-12T11:18:46Z

pkg/karmadactl/cmdinit/cmdinit.go

@@ -18,7 +18,7 @@ const (

 // NewCmdInit install karmada on kubernetes
 func NewCmdInit(cmdOut io.Writer, parentCommand string) *cobra.Command {
-	opts := kubernetes.CommandInitOption{}
+	opts := &kubernetes.CommandInitOption{}


Why using &?

lonelyCZ · 2022-05-12T11:28:56Z

pkg/karmadactl/cmdinit/kubernetes/deploy.go

+	return imageRepositories["global"]
+}
+
+// get kube api server image


Suggested change

// get kube api server image

// get kube-apiserver image

lonelyCZ · 2022-05-12T11:37:43Z

pkg/karmadactl/cmdinit/kubernetes/deploy.go

@@ -71,6 +90,7 @@ type CommandInitOption struct {
 	CertAndKeyFileData                 map[string][]byte
 	RestConfig                         *rest.Config
 	KarmadaAPIServerIP                 []net.IP
+	FlagSet                            *pflag.FlagSet


FlagSet is not recommended as an option.

@lonelyCZ help to review again
I want to know if the user had set the kubeapiserver image manally.
three ways I have thought :
1: paas flagset to option and tell deploy.go if user had set the kubeapiserver image manally ( not recommended)
2: https://github.com/karmada-io/karmada/pull/1764/files#:~:text=flags.StringVarP(%26opts.KarmadaAPIServerImage%2C%20%22karmada%2Dapiserver%2Dimage%22%2C%20%22%22%2C%20%22k8s.gcr.io/kube%2Dapiserver%3Av1.21.7%22%2C%20%22Kubernetes%20apiserver%20image%22)) set apiserver image default value == "" and we use default value to judge user had set the value (code push now )
3: add some options in opts like apiserverset . we set the apiserverset value by visit when flags init

lonelyCZ · 2022-05-12T13:09:30Z

I want to know if the user had set the kubeapiserver image manally.

Yes, some users have their own image registry.

2: https://github.com/karmada-io/karmada/pull/1764/files#:~:text=flags.StringVarP(%26opts.KarmadaAPIServerImage%2C%20%22karmada%2Dapiserver%2Dimage%22%2C%20%22%22%2C%20%22k8s.gcr.io/kube%2Dapiserver%3Av1.21.7%22%2C%20%22Kubernetes%20apiserver%20image%22)) set apiserver image default value == "" and we use default value to judge user had set the value (code push now )

This is good and easy. We no longer need this default value.

wuyingjun-lucky · 2022-05-12T13:11:25Z

Yes, some users have their own image registry.

Ok， please review again @lonelyCZ

pkg/karmadactl/cmdinit/cmdinit.go

pkg/karmadactl/cmdinit/kubernetes/deploy.go

wuyingjun-lucky · 2022-05-12T13:29:08Z

@lonelyCZ
How can I squash the commits in gitlab
or I must squash in my computer ？

lonelyCZ · 2022-05-12T13:35:42Z

@lonelyCZ How can I squash the commits in gitlab or I must squash in my computer ？

I think so latter.

wuyingjun-lucky · 2022-05-12T13:38:30Z

@lonelyCZ How can I squash the commits in gitlab or I must squash in my computer ？

I think so latter.

The git commit message format is ok ？

lonelyCZ · 2022-05-12T13:44:59Z

The git commit message format is ok ？

Thanks, it is ok to keep track of your contributions.

lonelyCZ · 2022-05-12T14:21:08Z

I just tested it that worked fine. Just a little advice, rank the two new use cases to the front that are second and third.

Examples:

# Install Karmada in Kubernetes cluster
# The karmada-apiserver binds the master node's IP by default
karmadactl init

# Specify the URL to download CRD tarball
karmadactl init --crds https://github.com/karmada-io/karmada/releases/download/v1.1.0/crds.tar.gz

# Specify the local CRD tarball
karmadactl init --crds /root/crds.tar.gz

# Use PVC to persistent storage etcd data
karmadactl init --etcd-storage-mode PVC --storage-classes-name {StorageClassesName}

# Use hostPath to persistent storage etcd data. For data security, only 1 etcd pod can run in hostPath mode
karmadactl init --etcd-storage-mode hostPath  --etcd-replicas 1

# Use hostPath to persistent storage etcd data but select nodes by labels
karmadactl init --etcd-storage-mode hostPath --etcd-node-selector-labels karmada.io/etcd=true

# Private registry can be specified for all images
karmadactl init --etcd-image local.registry.com/library/etcd:3.5.1-0

# Kube registry can be specified by using kube-image-registry
karmadactl init --kube-image-registry=registry.cn-hangzhou.aliyuncs.com/google_containers

# China mainland registry mirror can be specified by using kube-image-mirror-country
karmadactl init --kube-image-mirror-country=cn

Update pkg/karmadactl/cmdinit/kubernetes/deploy.go Co-authored-by: Zhe Cheng <47743202+lonelyCZ@users.noreply.github.com> Update pkg/karmadactl/cmdinit/cmdinit.go Co-authored-by: Zhe Cheng <47743202+lonelyCZ@users.noreply.github.com> Apply suggestions from code review Co-authored-by: Zhe Cheng <47743202+lonelyCZ@users.noreply.github.com> Signed-off-by: wuyingjun <wuyingjun_yewu@cmss.chinamobile.com>

wuyingjun-lucky · 2022-05-12T14:32:25Z

I just tested it that worked fine. Just a little advice, rank the two new use cases to the front that are second and third.

Examples:

# Install Karmada in Kubernetes cluster
# The karmada-apiserver binds the master node's IP by default
karmadactl init

# Specify the URL to download CRD tarball
karmadactl init --crds https://github.com/karmada-io/karmada/releases/download/v1.1.0/crds.tar.gz

# Specify the local CRD tarball
karmadactl init --crds /root/crds.tar.gz

# Use PVC to persistent storage etcd data
karmadactl init --etcd-storage-mode PVC --storage-classes-name {StorageClassesName}

# Use hostPath to persistent storage etcd data. For data security, only 1 etcd pod can run in hostPath mode
karmadactl init --etcd-storage-mode hostPath  --etcd-replicas 1

# Use hostPath to persistent storage etcd data but select nodes by labels
karmadactl init --etcd-storage-mode hostPath --etcd-node-selector-labels karmada.io/etcd=true

# Private registry can be specified for all images
karmadactl init --etcd-image local.registry.com/library/etcd:3.5.1-0

# Kube registry can be specified by using kube-image-registry
karmadactl init --kube-image-registry=registry.cn-hangzhou.aliyuncs.com/google_containers

# China mainland registry mirror can be specified by using kube-image-mirror-country
karmadactl init --kube-image-mirror-country=cn

fixed it

lonelyCZ · 2022-05-13T01:49:24Z

Thanks, @wuyingjun-lucky

/lgtm

wuyingjun-lucky · 2022-05-13T07:20:29Z

/assign @RainbowMango

RainbowMango

/approve

karmada-bot · 2022-05-13T07:33:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

~~OWNERS~~ [RainbowMango]

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

karmada-bot added the kind/feature Categorizes issue or PR as related to a new feature. label May 11, 2022

karmada-bot requested review from lonelyCZ and prodanlabs May 11, 2022 02:26

karmada-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 11, 2022

karmada-bot assigned lonelyCZ May 11, 2022

karmada-bot assigned prodanlabs May 11, 2022

wuyingjun-lucky force-pushed the debug_fot branch 2 times, most recently from 9691281 to 93267b7 Compare May 11, 2022 03:11

karmada-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 11, 2022

lonelyCZ reviewed May 12, 2022

View reviewed changes

wuyingjun-lucky force-pushed the debug_fot branch from 93267b7 to 59ea0d1 Compare May 12, 2022 12:47

karmada-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 12, 2022

wuyingjun-lucky force-pushed the debug_fot branch from 59ea0d1 to 4089506 Compare May 12, 2022 12:59

lonelyCZ reviewed May 12, 2022

View reviewed changes

pkg/karmadactl/cmdinit/cmdinit.go Outdated Show resolved Hide resolved

pkg/karmadactl/cmdinit/cmdinit.go Outdated Show resolved Hide resolved

pkg/karmadactl/cmdinit/kubernetes/deploy.go Outdated Show resolved Hide resolved

wuyingjun-lucky force-pushed the debug_fot branch from d4e4cea to cd36747 Compare May 12, 2022 13:37

wuyingjun-lucky force-pushed the debug_fot branch from cd36747 to 78cb729 Compare May 12, 2022 14:30

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label May 13, 2022

karmada-bot assigned RainbowMango May 13, 2022

RainbowMango approved these changes May 13, 2022

View reviewed changes

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 13, 2022

karmada-bot merged commit a356722 into karmada-io:master May 13, 2022

RainbowMango changed the title ~~feature_1753:support kube-image-mirror-country and kube-image-registry~~ Introduce --kube-image-mirror-country and kube-image-registry to karmadactl init May 13, 2022

wuyingjun-lucky mentioned this pull request May 17, 2022

Support --image-mirror-country or --image-repositor like minikube #1753

Closed

wuyingjun-lucky deleted the debug_fot branch November 8, 2022 14:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce `--kube-image-mirror-country` and `kube-image-registry` to karmadactl `init` #1764

Introduce `--kube-image-mirror-country` and `kube-image-registry` to karmadactl `init` #1764

wuyingjun-lucky commented May 11, 2022 •

edited by RainbowMango

RainbowMango commented May 11, 2022

RainbowMango commented May 11, 2022

wuyingjun-lucky commented May 12, 2022 •

edited

karmada-bot commented May 12, 2022

lonelyCZ commented May 12, 2022

lonelyCZ May 12, 2022

lonelyCZ May 12, 2022

lonelyCZ May 12, 2022

wuyingjun-lucky May 12, 2022 •

edited

lonelyCZ commented May 12, 2022

wuyingjun-lucky commented May 12, 2022

wuyingjun-lucky commented May 12, 2022

lonelyCZ commented May 12, 2022

wuyingjun-lucky commented May 12, 2022

lonelyCZ commented May 12, 2022

lonelyCZ commented May 12, 2022

wuyingjun-lucky commented May 12, 2022

lonelyCZ commented May 13, 2022

wuyingjun-lucky commented May 13, 2022

RainbowMango left a comment

karmada-bot commented May 13, 2022

Introduce --kube-image-mirror-country and kube-image-registry to karmadactl init #1764

Introduce --kube-image-mirror-country and kube-image-registry to karmadactl init #1764

Conversation

wuyingjun-lucky commented May 11, 2022 • edited by RainbowMango

RainbowMango commented May 11, 2022

RainbowMango commented May 11, 2022

wuyingjun-lucky commented May 12, 2022 • edited

karmada-bot commented May 12, 2022

lonelyCZ commented May 12, 2022

lonelyCZ May 12, 2022

Choose a reason for hiding this comment

lonelyCZ May 12, 2022

Choose a reason for hiding this comment

lonelyCZ May 12, 2022

Choose a reason for hiding this comment

wuyingjun-lucky May 12, 2022 • edited

Choose a reason for hiding this comment

lonelyCZ commented May 12, 2022

wuyingjun-lucky commented May 12, 2022

wuyingjun-lucky commented May 12, 2022

lonelyCZ commented May 12, 2022

wuyingjun-lucky commented May 12, 2022

lonelyCZ commented May 12, 2022

lonelyCZ commented May 12, 2022

wuyingjun-lucky commented May 12, 2022

lonelyCZ commented May 13, 2022

wuyingjun-lucky commented May 13, 2022

RainbowMango left a comment

Choose a reason for hiding this comment

karmada-bot commented May 13, 2022

Introduce `--kube-image-mirror-country` and `kube-image-registry` to karmadactl `init` #1764

Introduce `--kube-image-mirror-country` and `kube-image-registry` to karmadactl `init` #1764

wuyingjun-lucky commented May 11, 2022 •

edited by RainbowMango

wuyingjun-lucky commented May 12, 2022 •

edited

wuyingjun-lucky May 12, 2022 •

edited