Feature_1716:set registry and version variable in deploy-karmada.sh

[wuyingjun-lucky]

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:

Which issue(s) this PR fixes: Fixes # https://github.com/karmada-io/karmada/issues/1716 Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

[wuyingjun-lucky]

/assign @kevin-wangzefeng

[wuyingjun-lucky]

/assign @lonelyCZ

[wuyingjun-lucky]

/assign @RainbowMango

[lonelyCZ]

Thanks @wuyingjun-lucky , I will look it ASAP.

[lonelyCZ]

Should we put these parameters in function usage()

https://github.com/karmada-io/karmada/blob/dd75a65f1014460652018e60e0cbf4c74a694da4/hack/deploy-karmada.sh#L22-L32

[wuyingjun-lucky]

GOT IT And I have some other questions when I run local-up.sh and e2e.sh ; Could u help me ? @lonelyCZ @RainbowMango 1: I found other shell scripts or make file have similar env named VERSION and REGISTRY; should I use VERSION and REGISTRY and KUBE_REGISTRY in my pull request or change them to KARMADA_VERSION and KARMADA_REGISTRY and KUBE_REGISTRY for the code and env consistency like

2: local-up.sh use the deploy-karmada.sh to deploy clusters; But when I set env CHINA_MAINLAND the scripts will replace the registry value like this

For adapt my pull request , I want to modify like this and the env will work and replace in the new deploy-karmada.sh , do you think it is ok ?

[lonelyCZ]

1: I found other shell scripts or make file have similar env named VERSION and REGISTRY; should I use VERSION and REGISTRY and KUBE_REGISTRY in my pull request or change them to KARMADA_VERSION and KARMADA_REGISTRY and KUBE_REGISTRY for the code and env consistency

I think it can be unified to VERSION, REGISTRY and KUBE_REGISTRY. deploy-karmada.sh is more low-level and they don't conflict.

2: local-up.sh use the deploy-karmada.sh to deploy clusters; But when I set env CHINA_MAINLAND the scripts will replace the registry value like this

Right, perhaps we should fix the function CHINA_MAINLAND, we should not update the files in REPO_ROOT.

[wuyingjun-lucky]

1: I found other shell scripts or make file have similar env named VERSION and REGISTRY; should I use VERSION and REGISTRY and KUBE_REGISTRY in my pull request or change them to KARMADA_VERSION and KARMADA_REGISTRY and KUBE_REGISTRY for the code and env consistency

I think it can be unified to VERSION, REGISTRY and KUBE_REGISTRY. deploy-karmada.sh is more low-level and they don't conflict.

2: local-up.sh use the deploy-karmada.sh to deploy clusters; But when I set env CHINA_MAINLAND the scripts will replace the registry value like this

Right, perhaps we should fix the function CHINA_MAINLAND, we should not update the files in REPO_ROOT.

That means we can put it into deploy-karmada.sh ?

[lonelyCZ]

I find that if we modify REGISTRY here, can we not deploy it successfully? It appears that the yaml file below has not changed.

Changes are not currently supported and we need to consider the value of changing it.

https://github.com/karmada-io/karmada/blob/dd75a65f1014460652018e60e0cbf4c74a694da4/hack/local-up-karmada.sh#L122-L130

[wuyingjun-lucky]

I find that if we modify REGISTRY here, can we not deploy it successfully? It appears that the yaml file below has not changed

https://github.com/karmada-io/karmada/blob/dd75a65f1014460652018e60e0cbf4c74a694da4/hack/local-up-karmada.sh#L122-L130 That would not be failed because it use the default registry to make container and call deploy-karmada.sh use default env But it can not use the variable to call the deploy-karmada.sh

I found it when testing and modify like this (I have not changed the env name)

do u think it ok ?

[lonelyCZ]

I feel good about making the script more flexible. Perhaps we can look forward to @RainbowMango comments on such changes.

[wuyingjun-lucky]

I feel good about making the script more flexible. Perhaps we can look forward to @RainbowMango comments on such changes.

Ok, before that I will modify the code with your suggestion ; And please help me again

[karmada-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: To complete the pull request process, please ask for approval from kevin-wangzefeng after the PR has been reviewed.

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

[wuyingjun-lucky]

@RainbowMango Could you help me review the pull request ？

[RainbowMango]

Sure. /assign

[RainbowMango]

Generally looks good to me. But I guess I guess the deploy-karmada.sh is mainly used by developers, if you want to install Karmada it is recommended to refer to Installation Guide.

[RainbowMango]

@lfbear Could you please take a look?

[RainbowMango]

I see you did excellent work here. My concern is if this will make the scripts too complicated? Can you guys give me more hints about the benefits we got?

[wuyingjun-lucky]

I see you did excellent work here. My concern is if this will make the scripts too complicated? Can you guys give me more hints about the benefits we got?

I do not think the scripts is too complicated，you can use the scripts to deploy as before，we do not change the process, just set it variable. As a green hand, I think the process is clear, let alone other develpers. our team use it frequently; we had to pull the code and then modify the scripts to deploy clusters again and again. But we do not want to do this. we want our code is not dirty. I guess people who use the swr.ap-southeast-1.myhuaweicloud.com may think it is not necessary, But people who don not use swr.ap-southeast-1.myhuaweicloud.com and k8s.gcr.io will welcome it @RainbowMango

[RainbowMango]

our team use it frequently; we had to pull the code and then modify the scripts to deploy clusters again and again.

May I know why you are so dependent on deploy-karmada.sh? (not an objection, just curious). You can use karmadactl init to install Karmada, sure you can customize the images registry and version. Or use local-up-karmada.sh to install Karmada from the source, in this case you don't have to replace registry.

[wuyingjun-lucky]

Installation Guide 1: We use the deploy-karmada.sh for test and cd 2: kubectl karmada seems troublesome, The options looks to be optimized because we will set many images options. Maybe we can just set the registry and version like the pull request.

@RainbowMango I think we can bring advantages by improving the process of deploying experience

[wuyingjun-lucky]

@RainbowMango Should I close the change or modify the change ？

[chaunceyjiang]

Maybe this is a more elegant solution