introduce kube-image-tag flag to karmadactl init #2840

helen-frank · 2022-11-20T15:49:50Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
Choose a specific Kubernetes version for the control plane.
Which issue(s) this PR fixes:
Fixes #
see #2541 #2655
Special notes for your reviewer:
TEST

./karmadactl init --private-image-registry=daocloud.io/atsctoo --kube-image-tag=v1.18.20 --crds=/root/karmada/crds.tar.gz

Does this PR introduce a user-facing change?:

`karmadactl`: Introduced `--kube-image-tag` flag to the `init` command to specify the Kubernetes image version.

helen-frank · 2022-11-20T15:56:09Z

According to our current application scenario, we are more concerned about the version of kube (some problems of interface compatibility), so I limited the scope of tag parameters discussed earlier to kube.
What do you think ? @RainbowMango @carlory @lonelyCZ

codecov-commenter · 2022-11-20T16:04:41Z

Codecov Report

Merging #2840 (5eaf574) into master (8aa58ec) will increase coverage by 0.71%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #2840      +/-   ##
==========================================
+ Coverage   37.89%   38.60%   +0.71%     
==========================================
  Files         190      206      +16     
  Lines       17681    18846    +1165     
==========================================
+ Hits         6700     7276     +576     
- Misses      10574    11140     +566     
- Partials      407      430      +23

Flag	Coverage Δ
unittests	`38.60% <0.00%> (+0.71%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/karmadactl/cmdinit/kubernetes/deploy.go	`3.26% <0.00%> (ø)`
pkg/util/helper/binding.go	`64.56% <0.00%> (-17.54%)`	⬇️
pkg/registry/cluster/storage/proxy.go	`59.09% <0.00%> (-9.34%)`	⬇️
pkg/scheduler/core/division_algorithm.go	`84.39% <0.00%> (-6.52%)`	⬇️
pkg/util/worker.go	`66.66% <0.00%> (-4.77%)`	⬇️
pkg/util/helper/policy.go	`86.58% <0.00%> (-2.84%)`	⬇️
pkg/util/serviceaccount.go	`87.71% <0.00%> (-2.56%)`	⬇️
...ceinterpreter/configurableinterpreter/luavm/lua.go	`57.14% <0.00%> (-0.91%)`	⬇️
pkg/scheduler/event_handler.go	`27.27% <0.00%> (-0.70%)`	⬇️
pkg/karmadactl/cmdinit/karmada/rbac.go	`88.00% <0.00%> (-0.58%)`	⬇️
... and 53 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

jwcesign · 2022-11-21T01:32:15Z

/lgtm

carlory · 2022-11-21T02:02:28Z

LGTM

lonelyCZ · 2022-11-21T02:21:38Z

pkg/karmadactl/cmdinit/cmdinit.go

@@ -97,6 +97,7 @@ func NewCmdInit(parentCommand string) *cobra.Command {
 	// kube image registry
 	flags.StringVarP(&opts.KubeImageMirrorCountry, "kube-image-mirror-country", "", "", "Country code of the kube image registry to be used. For Chinese mainland users, set it to cn")
 	flags.StringVarP(&opts.KubeImageRegistry, "kube-image-registry", "", "", "Kube image registry. For Chinese mainland users, you may use local gcr.io mirrors such as registry.cn-hangzhou.aliyuncs.com/google_containers to override default kube image registry")
+	flags.StringVar(&opts.KubeVersion, "kube-version", "v1.25.2", "Choose a specific Kubernetes version for the control plane.")


How about kube-image-tag? It seems to account for the relation with kube-image-registry.

I refer to https://github.com/kubernetes/kubernetes/blob/57eb5d631ccd615cd161b6da36afc759af004b93/cmd/kubeadm/app/cmd/options/generic.go#L79

I think these two can be a definition @lonelyCZ

I refer to
https://github.com/kubernetes/kubernetes/blob/57eb5d631ccd615cd161b6da36afc759af004b93/cmd/kubeadm/app/cmd/options/generic.go#L79

kubeadm is specially to install Kubernetes that is a little different from my scenario. After all, we only two kube images, and I think kube-image-tag, kube-image-registry and kube-image-mirror-country could be better togather. What do you think? @helen-frank @RainbowMango

agree
What do you think? @RainbowMango

kubeadm is specially to install Kubernetes that is a little different from my scenario.

Totally agree!

@helen-frank Can you help to describe why we need a new flag? I'm kind of lost from it.

We already have the following flags:

--karmada-kube-controller-manager-image=''

--etcd-image=''

--etcd-init-image='docker.io/alpine:3.15.1'

--karmada-apiserver-image=''

currently, the platform cannot upgrade kube versions with karmada to adapt

What do you mean?

Some of our existing business platforms have different requirements for kube versions, so there is often a requirement for setting up kube versions when using karmadactl to install karmada, while we are not particularly concerned about others. For the Karmada itself, we use the latest stable version.

I think this is a common situation. What do you think

Yes, I agree it's a reasonable case.
Do we have any other alternatives in addition to kube-image-tag and --kube-version?

At present, there is no better idea. Individuals are more inclined to kube-version. kube-image-tag is also acceptable.

helen-frank · 2022-12-14T03:42:50Z

I want to use kube-version function now. Can I continue this topic? @lonelyCZ @RainbowMango

lonelyCZ · 2022-12-14T06:32:31Z

I think it can be move forward. Now the main question is the name of the option. Perhaps, we can vote for deciding it.

I perfer kube-image-tag that is more match to kube-image-registry and can express more semantics .

lonelyCZ · 2022-12-14T06:34:21Z

/cc @RainbowMango @carlory @XiShanYongYe-Chang @prodanlabs @Poor12

carlory · 2022-12-14T07:56:21Z

kube-image-tag +1

RainbowMango · 2022-12-14T08:14:37Z

I'm ok with it.

XiShanYongYe-Chang · 2022-12-14T08:22:48Z

In terms of contextual consistency, it seems that kube-image-tag looks a little more readable.

helen-frank · 2022-12-14T08:55:15Z

OK, everyone seems to approve kube-image-tag more

helen-frank · 2022-12-15T01:53:55Z

/cc @lonelyCZ

lonelyCZ · 2022-12-15T02:45:05Z

pkg/karmadactl/cmdinit/kubernetes/deploy.go

@@ -92,6 +90,7 @@ type CommandInitOption struct {
 	ImageRegistry                      string
 	KubeImageRegistry                  string
 	KubeImageMirrorCountry             string
+	KubeVersion                        string


Suggested change

KubeVersion string

KubeImageTag string

Signed-off-by: helen <haitao.zhang@daocloud.io>

lonelyCZ · 2022-12-15T02:55:39Z

/lgtm

Thanks @helen-frank , and please update the PR description.

helen-frank · 2022-12-16T06:12:20Z

/cc @RainbowMango

RainbowMango

/approve

I updated the release notes, by the way.

karmada-bot · 2022-12-16T07:21:17Z

[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:

~~pkg/karmadactl/cmdinit/OWNERS~~ [RainbowMango]

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

helen-frank · 2022-12-16T07:27:49Z

Thank you😂

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

karmada-bot requested review from lonelyCZ and prodanlabs November 20, 2022 15:49

karmada-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 20, 2022

helen-frank force-pushed the feature/AddKubeVersion branch from c103729 to 91ced92 Compare November 20, 2022 15:50

karmada-bot assigned jwcesign Nov 21, 2022

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

lonelyCZ reviewed Nov 21, 2022

View reviewed changes

helen-frank requested review from lonelyCZ and removed request for prodanlabs November 21, 2022 02:54

helen-frank requested review from RainbowMango and removed request for lonelyCZ November 28, 2022 12:09

helen-frank requested review from lonelyCZ and removed request for RainbowMango December 13, 2022 16:17

karmada-bot requested review from carlory, Poor12, prodanlabs, RainbowMango and XiShanYongYe-Chang December 14, 2022 06:34

helen-frank force-pushed the feature/AddKubeVersion branch from 91ced92 to b5af5da Compare December 14, 2022 08:56

karmada-bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 14, 2022

lonelyCZ reviewed Dec 15, 2022

View reviewed changes

introduce --kube-version flag to karmadactl init

5eaf574

Signed-off-by: helen <haitao.zhang@daocloud.io>

helen-frank force-pushed the feature/AddKubeVersion branch from b5af5da to 5eaf574 Compare December 15, 2022 02:51

karmada-bot assigned lonelyCZ Dec 15, 2022

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

helen-frank changed the title ~~introduce --kube-version flag to karmadactl init~~ introduce kube-image-tag flag to karmadactl init Dec 15, 2022

helen-frank requested review from lonelyCZ and removed request for carlory, Poor12, prodanlabs, RainbowMango and XiShanYongYe-Chang December 15, 2022 04:58

karmada-bot requested a review from RainbowMango December 16, 2022 06:12

RainbowMango approved these changes Dec 16, 2022

View reviewed changes

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

karmada-bot merged commit 9b626c0 into karmada-io:master Dec 16, 2022

helen-frank deleted the feature/AddKubeVersion branch December 16, 2022 07:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

introduce kube-image-tag flag to karmadactl init #2840

introduce kube-image-tag flag to karmadactl init #2840

helen-frank commented Nov 20, 2022 •

edited by RainbowMango

Loading

helen-frank commented Nov 20, 2022

codecov-commenter commented Nov 20, 2022 •

edited

Loading

jwcesign commented Nov 21, 2022

carlory commented Nov 21, 2022

lonelyCZ Nov 21, 2022

helen-frank Nov 21, 2022

lonelyCZ Nov 21, 2022

helen-frank Nov 22, 2022

RainbowMango Nov 22, 2022

RainbowMango Nov 23, 2022

helen-frank Nov 23, 2022

helen-frank Nov 23, 2022

RainbowMango Nov 23, 2022

helen-frank Nov 24, 2022

helen-frank commented Dec 14, 2022

lonelyCZ commented Dec 14, 2022

lonelyCZ commented Dec 14, 2022

carlory commented Dec 14, 2022

RainbowMango commented Dec 14, 2022

XiShanYongYe-Chang commented Dec 14, 2022

helen-frank commented Dec 14, 2022

helen-frank commented Dec 15, 2022

lonelyCZ Dec 15, 2022

helen-frank Dec 15, 2022

lonelyCZ commented Dec 15, 2022

helen-frank commented Dec 16, 2022

RainbowMango left a comment

karmada-bot commented Dec 16, 2022

helen-frank commented Dec 16, 2022

introduce kube-image-tag flag to karmadactl init #2840

introduce kube-image-tag flag to karmadactl init #2840

Conversation

helen-frank commented Nov 20, 2022 • edited by RainbowMango Loading

helen-frank commented Nov 20, 2022

codecov-commenter commented Nov 20, 2022 • edited Loading

Codecov Report

jwcesign commented Nov 21, 2022

carlory commented Nov 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

helen-frank commented Dec 14, 2022

lonelyCZ commented Dec 14, 2022

lonelyCZ commented Dec 14, 2022

carlory commented Dec 14, 2022

RainbowMango commented Dec 14, 2022

XiShanYongYe-Chang commented Dec 14, 2022

helen-frank commented Dec 14, 2022

helen-frank commented Dec 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lonelyCZ commented Dec 15, 2022

helen-frank commented Dec 16, 2022

RainbowMango left a comment

Choose a reason for hiding this comment

karmada-bot commented Dec 16, 2022

helen-frank commented Dec 16, 2022

helen-frank commented Nov 20, 2022 •

edited by RainbowMango

Loading

codecov-commenter commented Nov 20, 2022 •

edited

Loading