karmada
karmada copied to clipboard
karmada-io
Support multiple label selection ability in EtcdNodeSelectorLabels
What type of PR is this? /kind feature
What this PR does / why we need it: Supports the function of --etcd-node-selector-labels for multiple labels of nodes in the cluster
user@virtual-machine:karmada/_output/bin/linux/amd64# ./karmadactl init --etcd-node-selector-labels karmada.io/etcd=true,kubernetes.io/os=linux
I0830 09:58:51.083493 7186 deploy.go:111] No default release version found. build version: version.Info{GitVersion:"", GitCommit:"0c4ff310f2d3bba4c54a513c62bc4110e5353b32", GitTreeState:"clean", BuildDate:"2024-08-30T01:35:17Z", GoVersion:"go1.22.6", Compiler:"gc", Platform:"linux/amd64"}
I0830 09:58:51.083581 7186 enable_option.go:85] No default release version found. build version: version.Info{GitVersion:"", GitCommit:"0c4ff310f2d3bba4c54a513c62bc4110e5353b32", GitTreeState:"clean", BuildDate:"2024-08-30T01:35:17Z", GoVersion:"go1.22.6", Compiler:"gc", Platform:"linux/amd64"}
......
I0830 10:01:43.161625 7186 deploy.go:502] Create karmada webhook Deployment
I0830 10:01:43.167256 7186 idempotency.go:296] Service karmada-system/karmada-webhook has been created or updated.
------------------------------------------------------------------------------------------------------
█████ ████ █████████ ███████████ ██████ ██████ █████████ ██████████ █████████
░░███ ███░ ███░░░░░███ ░░███░░░░░███ ░░██████ ██████ ███░░░░░███ ░░███░░░░███ ███░░░░░███
░███ ███ ░███ ░███ ░███ ░███ ░███░█████░███ ░███ ░███ ░███ ░░███ ░███ ░███
░███████ ░███████████ ░██████████ ░███░░███ ░███ ░███████████ ░███ ░███ ░███████████
░███░░███ ░███░░░░░███ ░███░░░░░███ ░███ ░░░ ░███ ░███░░░░░███ ░███ ░███ ░███░░░░░███
░███ ░░███ ░███ ░███ ░███ ░███ ░███ ░███ ░███ ░███ ░███ ███ ░███ ░███
█████ ░░████ █████ █████ █████ █████ █████ █████ █████ █████ ██████████ █████ █████
░░░░░ ░░░░ ░░░░░ ░░░░░ ░░░░░ ░░░░░ ░░░░░ ░░░░░ ░░░░░ ░░░░░ ░░░░░░░░░░ ░░░░░ ░░░░░
------------------------------------------------------------------------------------------------------
Karmada is installed successfully.
Register Kubernetes cluster to Karmada control plane.
Register cluster with 'Push' mode
Step 1: Use "karmadactl join" command to register the cluster to Karmada control plane. --cluster-kubeconfig is kubeconfig of the member cluster.
(In karmada)~# MEMBER_CLUSTER_NAME=$(cat ~/.kube/config | grep current-context | sed 's/: /\n/g'| sed '1d')
(In karmada)~# karmadactl --kubeconfig /etc/karmada/karmada-apiserver.config join ${MEMBER_CLUSTER_NAME} --cluster-kubeconfig=$HOME/.kube/config
Step 2: Show members of karmada
(In karmada)~# kubectl --kubeconfig /etc/karmada/karmada-apiserver.config get clusters
Register cluster with 'Pull' mode
Step 1: Use "karmadactl register" command to register the cluster to Karmada control plane. "--cluster-name" is set to cluster of current-context by default.
(In member cluster)~# karmadactl register 172.18.0.2:32443 --token ofc912.rk5yj36jfc9es9by --discovery-token-ca-cert-hash sha256:82c3150ef0593fe5f581078cb6ccfc597a0043a45ff2af028b302d1cd53ad9f9
Step 2: Show members of karmada
(In karmada)~# kubectl --kubeconfig /etc/karmada/karmada-apiserver.config get clusters
user@virtual-machine:karmada/_output/bin/linux/amd64#
Which issue(s) this PR fixes: Fixes #5320 This PR is related to the documentation PR: https://github.com/karmada-io/karmada/pull/5277 Special notes for your reviewer: @liangyuanpeng Does this PR introduce a user-facing change?:
Hi, this is a bot comment, just put the label of ok-to-test here, you can comment /retest to rerun github workflow if github workflow is failing and it's not releated with this PR.
/ok-to-test
:warning: Please install the to ensure uploads and comments are reliably processed by Codecov.
Codecov Report
Attention: Patch coverage is 22.22222% with 14 lines in your changes missing coverage. Please review.
Project coverage is 31.68%. Comparing base (
e13518d) to head (2c57eba). Report is 81 commits behind head on master.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| pkg/karmadactl/cmdinit/kubernetes/deploy.go | 0.00% | 12 Missing :warning: |
| pkg/karmadactl/cmdinit/kubernetes/statefulset.go | 60.00% | 1 Missing and 1 partial :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## master #5321 +/- ##
==========================================
+ Coverage 30.97% 31.68% +0.71%
==========================================
Files 637 643 +6
Lines 44266 44456 +190
==========================================
+ Hits 13712 14088 +376
+ Misses 29563 29337 -226
- Partials 991 1031 +40
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 31.68% <22.22%> (+0.71%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
/lgtm
(need to squash commits before merge)
Probably because I made a mistake with the rabase operation, I might need to review the code again😭😭
Add a description of what this PR does and why it is needed so that other reviewers can understand the PR.
/cc @zhzhuang-zju for help to review,thanks.
Description for review:
Design Purpose
In certain scenarios, users may need to deploy Etcd on nodes with specific attributes, such as operating system type, hardware architecture, or custom labels. By allowing multiple labels to be specified, users can more accurately control the deployment of Etcd, thereby improving system robustness and performance.
Implementation Details
Command Flag: Users can specify multiple labels using the --EtcdNodeSelectorLabels flag. The format for labels is key1=value1,key2=value2, with each label separated by a comma.
- Label Parsing: A new function parseEtcdNodeSelectorLabelsMap was introduced to parse the label string in EtcdNodeSelectorLabels into a key-value pair map (map[string]string) for subsequent use in the node selector.
- Node Validation: In the Complete() function, the program iterates through each label to check if there are nodes in the cluster that match the criteria. If no matching nodes are found, an error message is returned.
- StatefulSet Configuration: When creating the Etcd StatefulSet, the makeETCDStatefulSet function uses the parsed label map to set the node selector.
Usage
Users can specify the EtcdNodeSelectorLabels as follows when using the karmadactl init command:
karmadactl init --EtcdNodeSelectorLabels="kubernetes.io/os=linux,hello=world"
This ensures that Etcd is only deployed on nodes that have both the kubernetes.io/os=linux and hello=world labels.
Testing and Validation
To ensure the correctness of this feature, corresponding unit tests have been added, including:
- Valid Label Parsing Tests: Tests whether multiple correctly formatted labels can be properly parsed into a map.
- Invalid Label Tests: Tests whether labels without an equal sign or with incorrect formatting trigger an appropriate error.
- Label Application Tests: Tests whether the parsed labels are correctly applied to the Etcd StatefulSet configuration.
for reviewers, this PR is coming from https://github.com/karmada-io/karmada/pull/5277#discussion_r1706325206
@tiansuo114 https://github.com/karmada-io/karmada/blob/d4bfbb5a707a6495d007d8e47a06c447bc936c9f/pkg/karmadactl/cmdinit/cmdinit.go#L143
We can refine the description of the flag etcd-node-selector-labels to clarify its purpose more clearly, and add a release note
@tiansuo114
https://github.com/karmada-io/karmada/blob/d4bfbb5a707a6495d007d8e47a06c447bc936c9f/pkg/karmadactl/cmdinit/cmdinit.go#L143
We can refine the description of the flag
etcd-node-selector-labelsto clarify its purpose more clearly, and add a release note
Ok, I made some preliminary additions. What do you think of this content flags.StringVarP(&opts.EtcdNodeSelectorLabels, "etcd-node-selector-labels", "", "", "Specify the node labels to target for scheduling etcd Pods. This flag is valid only when using hostPath storage mode. Example: --etcd-node-selector-labels karmada.io/etcd=true. Use this option to ensure that etcd Pods are scheduled on specific nodes with the desired characteristics, such as high-performance storage.")
What do you think of this content flags.StringVarP(&opts.EtcdNodeSelectorLabels, "etcd-node-selector-labels", "", "", "Specify the node labels to target for scheduling etcd Pods. This flag is valid only when using hostPath storage mode. Example: --etcd-node-selector-labels karmada.io/etcd=true. Use this option to ensure that etcd Pods are scheduled on specific nodes with the desired characteristics, such as high-performance storage.")
How about:
flags.StringVarP(&opts.EtcdNodeSelectorLabels, "etcd-node-selector-labels", "", "", "the labels used for etcd pod to select nodes, valid in hostPath mod, and with each label separated by a comma. ( e.g. --etcd-node-selector-labels karmada.io/etcd=true,kubernetes.io/os=linux)")
What do you think of this content flags.StringVarP(&opts.EtcdNodeSelectorLabels, "etcd-node-selector-labels", "", "", "Specify the node labels to target for scheduling etcd Pods. This flag is valid only when using hostPath storage mode. Example: --etcd-node-selector-labels karmada.io/etcd=true. Use this option to ensure that etcd Pods are scheduled on specific nodes with the desired characteristics, such as high-performance storage.")
How about:
flags.StringVarP(&opts.EtcdNodeSelectorLabels, "etcd-node-selector-labels", "", "", "the labels used for etcd pod to select nodes, valid in hostPath mod, and with each label separated by a comma. ( e.g. --etcd-node-selector-labels karmada.io/etcd=true,kubernetes.io/os=linux)")
I have corrected the two shortcomings mentioned in the communication, how does the code look now
Since there is no e2e test coverage, it would be more confident to merge this PR if you could echo the results of your manual tests here, it's mean use the karmada init --etcd-node-selector-labels karmada.io/etcd=true,kubernetes.io/os=linux with this PR
@tiansuo114
Since there is no e2e test coverage, it would be more confident to merge this PR if you could echo the results of your manual tests here, it's mean use the
karmada init --etcd-node-selector-labels karmada.io/etcd=true,kubernetes.io/os=linuxwith this PR @tiansuo114
Thank you for your feedback! To ensure I fully understand your request, would you like me to provide the results of running karmada init --etcd-node-selector-labels karmada.io/etcd=true,kubernetes.io/os=linux on my local machine? Additionally, should I include the relevant configuration details generated in the Kubernetes cluster after deployment to verify that the command was correctly executed and applied?
Please confirm, thank you! @liangyuanpeng
just show the console logs and it's enough, similar to https://github.com/karmada-io/karmada/pull/5392
like:
$ karmada init --etcd-node-selector-labels karmada.io/etcd=true,kubernetes.io/os=linux
...
just show the console logs and it's enough, similar to #5392只需显示控制台日志就足够了，类似于 #5392
like: 喜欢：
$ karmada init --etcd-node-selector-labels karmada.io/etcd=true,kubernetes.io/os=linux ...
Ok, I will add as soon as possible
just show the console logs and it's enough, similar to #5392
like:
$ karmada init --etcd-node-selector-labels karmada.io/etcd=true,kubernetes.io/os=linux ...
I added the required information in the What this PR does/why we need it: section in header comment, how does it look now
/lgtm /cc @zhzhuang-zju if you want to lgtm again. /assign @XiShanYongYe-Chang @chaunceyjiang for approval
Hi @tiansuo114, can you help add a release note?您好，您能帮忙添加发行说明吗？
What should I do? Is it related to modifications to the karmadactl documentation? If so, I think it can be included in the final supplementary documentation phase of my OSPP task
Add the description into the Does this PR introduce a user-facing change?: of your PR body.
Maybe: karmadactl: support multiple label selection ability with flag EtcdNodeSelectorLabels.
Add the description into the
Does this PR introduce a user-facing change?:of your PR body.将描述添加到 PR 正文的Does this PR introduce a user-facing change?:中。Maybe:
karmadactl: support multiple label selection ability with flag EtcdNodeSelectorLabels.也许：karmadactl: support multiple label selection ability with flag EtcdNodeSelectorLabels.
Okay, I put it in the pr body
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: XiShanYongYe-Chang
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~pkg/karmadactl/cmdinit/OWNERS~~ [XiShanYongYe-Chang]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
