Repository: karmada-io/karmada

Issue:
karmadactl: Optimize the helper for the register command
<!-- Please use this template while publishing a good first issue. Thanks!
-->

**Task description**:
Referring to https://github.com/karmada-io/karmada/pull/6901#discussion_r2489223155, the `karmada-apiserver-endpoint` argument in `karmadactl register [karmada-apiserver-endpoint]` has specific format requirements, but this information is not currently exposed to users.  

This issue aims to enhance the helper text for `karmadactl register` to make these requirements explicit and improve user experience.


**Solution**:
We can add relevant descriptions in the examples of the `register` command, like:
```bash
+++ b/pkg/karmadactl/register/register.go
@@ -69,6 +69,10 @@ var (
                # If '--cluster-name' isn't specified, the cluster of current-context will be used by default.
                %[1]s register [karmada-apiserver-endpoint] --cluster-name=<CLUSTER_NAME> --token=<TOKEN>  --discovery-token-ca-cert-hash=<CA-CERT-HASH>

+               # The 'karmada-apiserver-endpoint' argument is an address of the Karmada API server from which info will be fetched. 
+               # It should be provided in the format '<host>:<port>'.
+               %[1]s register 172.18.0.2:5443 --cluster-name=<CLUSTER_NAME> --token=<TOKEN>  --discovery-token-ca-cert-hash=<CA-CERT-HASH>
+
                # UnsafeSkipCAVerification allows token-based discovery without CA verification via CACertHashes. This can weaken
                # the security of register command since other clusters can impersonate the control-plane.
                %[1]s register [karmada-apiserver-endpoint] --token=<TOKEN>  --discovery-token-unsafe-skip-ca-verification=true


```


**Who can join or take the task**:

The good first issue is intended for `first-time contributors` to get started on his/her contributor journey.

After a contributor has successfully completed 1-2 good first issue's,
they should be ready to move on to `help wanted` items, saving the remaining `good first issue` for other new contributors.

**How to join or take the task**:

Just reply on the issue with the message `/assign` in a separate line.

Then, the issue will be assigned to you.

**How to ask for help**:

If you need help or have questions, please feel free to ask on this issue.
The issue author or other members of the community will guide you through the contribution process.

Discussion:
@7h3-3mp7y-m4n: Can I pick this up?  @zhzhuang-zju @zhzhuang-zju: Sure! 
/assign @7h3-3mp7y-m4n 
Feel free to ask me if you have any questions@7h3-3mp7y-m4n: Thanks You :)

Solution patch:
diff --git a/.github/workflows/installation-chart.yaml b/.github/workflows/installation-chart.yaml
index 0dc164405..acfada816 100644
--- a/.github/workflows/installation-chart.yaml
+++ b/.github/workflows/installation-chart.yaml
@@ -86,7 +86,7 @@ jobs:
           check-latest: true
 
       - name: Set up chart-testing
-        uses: helm/chart-testing-action@v2.7.0
+        uses: helm/chart-testing-action@v2.8.0
 
       - name: Add dependency chart repos
         run: |
diff --git a/pkg/descheduler/core/helper.go b/pkg/descheduler/core/helper.go
index c5e257534..e6aa68ddd 100644
--- a/pkg/descheduler/core/helper.go
+++ b/pkg/descheduler/core/helper.go
@@ -34,7 +34,7 @@ import (
 // SchedulingResultHelper is a helper to wrap the ResourceBinding and its target cluster result.
 type SchedulingResultHelper struct {
 	*workv1alpha2.ResourceBinding
-	TargetClusters []*TargetClusterWrapper
+	TargetClusterReplicaStatus []*ClusterReplicaStatus
 }
 
 // NewSchedulingResultHelper returns a new SchedulingResultHelper based on ResourceBinding.
@@ -43,22 +43,22 @@ func NewSchedulingResultHelper(binding *workv1alpha2.ResourceBinding) *Schedulin
 	readyReplicas := getReadyReplicas(binding)
 	for i := range binding.Spec.Clusters {
 		targetCluster := &binding.Spec.Clusters[i]
-		targetClusterHelper := &TargetClusterWrapper{
+		targetClusterReplicaStatus := &ClusterReplicaStatus{
 			ClusterName: targetCluster.Name,
 			Spec:        targetCluster.Replicas,
 		}
 		if ready, exist := readyReplicas[targetCluster.Name]; exist {
-			targetClusterHelper.Ready = ready
+			targetClusterReplicaStatus.Ready = ready
 		} else {
-			targetClusterHelper.Ready = estimatorclient.UnauthenticReplica
+			targetClusterReplicaStatus.Ready = estimatorclient.UnauthenticReplica
 		}
-		h.TargetClusters = append(h.TargetClusters, targetClusterHelper)
+		h.TargetClusterReplicaStatus = append(h.TargetClusterReplicaStatus, targetClusterReplicaStatus)
 	}
 	return h
 }
 
 // FillUnschedulableReplicas will detect the unschedulable replicas of member cluster by calling
-// unschedulable replica estimators and fill the unschedulable field of TargetClusterWrapper.
+// unschedulable replica estimators and fill the unschedulable field of ClusterReplicaStatus.
 func (h *SchedulingResultHelper) FillUnschedulableReplicas(unschedulableThreshold time.Duration) {
 	reference := &h.Spec.Resource
 	undesiredClusters, undesiredClusterNames := h.GetUndesiredClusters()
@@ -96,10 +96,10 @@ func (h *SchedulingResultHelper) FillUnschedulableReplicas(unschedulableThreshol
 }
 
 // GetUndesiredClusters returns the cluster which of ready replicas are not reach the ready ones.
-func (h *SchedulingResultHelper) GetUndesiredClusters() ([]*TargetClusterWrapper, []string) {
-	var clusters []*TargetClusterWrapper
+func (h *SchedulingResultHelper) GetUndesiredClusters() ([]*ClusterReplicaStatus, []string) {
+	var clusters []*ClusterReplicaStatus
 	var names []string
-	for _, cluster := range h.TargetClusters {
+	for _, cluster := range h.TargetClusterReplicaStatus {
 		if cluster.Ready < cluster.Spec {
 			clusters = append(clusters, cluster)
 			names = append(names, cluster.ClusterName)
@@ -108,12 +108,18 @@ func (h *SchedulingResultHelper) GetUndesiredClusters() ([]*TargetClusterWrapper
 	return clusters, names
 }
 
-// TargetClusterWrapper is a wrapper to wrap the target cluster name, spec replicas,
-// ready replicas and unschedulable replicas.
-type TargetClusterWrapper struct {
-	ClusterName   string
-	Spec          int32
-	Ready         int32
+// ClusterReplicaStatus represents the replicas status of a workload on a particular cluster.
+type ClusterReplicaStatus struct {
+	// ClusterName is the name of the member cluster targeted by ResourceBinding.
+	ClusterName string
+
+	// Spec is the desired replicas assigned to this cluster (from ResourceBinding.spec.clusters).
+	Spec int32
+
+	// Ready is the number of replicas currently reported as ready in this cluster.
+	Ready int32
+
+	// Unschedulable is the estimated count of replicas that cannot be scheduled in this cluster.
 	Unschedulable int32
 }
 
diff --git a/pkg/descheduler/descheduler.go b/pkg/descheduler/descheduler.go
index 827bfe3c6..1dcd4e2f2 100644
--- a/pkg/descheduler/descheduler.go
+++ b/pkg/descheduler/descheduler.go
@@ -199,19 +199,25 @@ func (d *Descheduler) worker(key util.QueueKey) error {
 	}
 
 	h.FillUnschedulableReplicas(d.unschedulableThreshold)
-	klog.V(3).Infof("Unschedulable result of resource(%s): %v", namespacedName, h.TargetClusters)
+	klog.V(3).Infof("Unschedulable result of resource(%s): %v", namespacedName, h.TargetClusterReplicaStatus)
 
 	return d.updateScheduleResult(h)
 }
 
+// updateScheduleResult adjusts per-cluster replica targets based on unschedulable estimates
+// and updates the corresponding ResourceBinding. It also records an event summarizing
+// how many replicas were descheduled per cluster and in total.
 func (d *Descheduler) updateScheduleResult(h *core.SchedulingResultHelper) error {
 	unschedulableSum := int32(0)
 	message := descheduleSuccessMessage
 	binding := h.ResourceBinding.DeepCopy()
-	for i, cluster := range h.TargetClusters {
+	for i, cluster := range h.TargetClusterReplicaStatus {
+		// Only act when estimator reports some unschedulable replicas and the
+		// desired replicas (Spec) can cover that reduction.
 		if cluster.Unschedulable > 0 && cluster.Spec >= cluster.Unschedulable {
 			target := cluster.Spec - cluster.Unschedulable
-			// The target cluster replicas must not be less than ready replicas.
+			// Ensure we never reduce below the currently ready replicas (safety cap),
+			// but only if ready <= desired Spec to avoid increasing above Spec.
 			if target < cluster.Ready && cluster.Ready <= cluster.Spec {
 				target = cluster.Ready
 			}
@@ -222,12 +228,14 @@ func (d *Descheduler) updateScheduleResult(h *core.SchedulingResultHelper) error
 		}
 	}
 	if unschedulableSum == 0 {
+		// Nothing changed; skip API update and event recording logic.
 		return nil
 	}
 	message += fmt.Sprintf(", %d total descheduled replica(s)", unschedulableSum)
 
 	var err error
 	defer func() {
+		// Always record an event for observability, including failure reasons if any.
 		d.recordDescheduleResultEventForResourceBinding(binding, message, err)
 	}()
 
diff --git a/pkg/karmadactl/register/register.go b/pkg/karmadactl/register/register.go
index 8ebb557f5..52db0c332 100644
--- a/pkg/karmadactl/register/register.go
+++ b/pkg/karmadactl/register/register.go
@@ -68,11 +68,15 @@ var (
 		# Register cluster into karmada control plane with Pull mode.
 		# If '--cluster-name' isn't specified, the cluster of current-context will be used by default.
 		%[1]s register [karmada-apiserver-endpoint] --cluster-name=<CLUSTER_NAME> --token=<TOKEN>  --discovery-token-ca-cert-hash=<CA-CERT-HASH>
-		
+	
+		# The 'karmada-apiserver-endpoint' argument is an address of the Karmada API server from which info will be fetched.
+		# It should be provided in the format '<host>:<port>'.
+		%[1]s register 172.18.0.2:5443 --cluster-name=<CLUSTER_NAME> --token=<TOKEN> --discovery-token-ca-cert-hash=<CA-CERT-HASH>
+	
 		# UnsafeSkipCAVerification allows token-based discovery without CA verification via CACertHashes. This can weaken
 		# the security of register command since other clusters can impersonate the control-plane.
-		%[1]s register [karmada-apiserver-endpoint] --token=<TOKEN>  --discovery-token-unsafe-skip-ca-verification=true
-		`)
+		%[1]s register [karmada-apiserver-endpoint] --token=<TOKEN> --discovery-token-unsafe-skip-ca-verification=true
+	`)
 )
 
 const (



Test patch:
diff --git a/pkg/descheduler/core/helper_test.go b/pkg/descheduler/core/helper_test.go
index 154006bda..6702ff246 100644
--- a/pkg/descheduler/core/helper_test.go
+++ b/pkg/descheduler/core/helper_test.go
@@ -59,7 +59,7 @@ func TestNewSchedulingResultHelper(t *testing.T) {
 				},
 			},
 			expected: &SchedulingResultHelper{
-				TargetClusters: []*TargetClusterWrapper{
+				TargetClusterReplicaStatus: []*ClusterReplicaStatus{
 					{ClusterName: "cluster1", Spec: 3, Ready: 2},
 					{ClusterName: "cluster2", Spec: 2, Ready: 1},
 				},
@@ -88,7 +88,7 @@ func TestNewSchedulingResultHelper(t *testing.T) {
 				},
 			},
 			expected: &SchedulingResultHelper{
-				TargetClusters: []*TargetClusterWrapper{
+				TargetClusterReplicaStatus: []*ClusterReplicaStatus{
 					{ClusterName: "cluster1", Spec: 3, Ready: 2},
 					{ClusterName: "cluster2", Spec: 2, Ready: client.UnauthenticReplica},
 				},
@@ -112,7 +112,7 @@ func TestSchedulingResultHelper_FillUnschedulableReplicas(t *testing.T) {
 		name           string
 		helper         *SchedulingResultHelper
 		mockEstimator  *mockUnschedulableReplicaEstimator
-		expected       []*TargetClusterWrapper
+		expected       []*ClusterReplicaStatus
 		expectedErrLog string
 	}{
 		{
@@ -128,13 +128,13 @@ func TestSchedulingResultHelper_FillUnschedulableReplicas(t *testing.T) {
 						},
 					},
 				},
-				TargetClusters: []*TargetClusterWrapper{
+				TargetClusterReplicaStatus: []*ClusterReplicaStatus{
 					{ClusterName: "cluster1", Spec: 3, Ready: 2},
 					{ClusterName: "cluster2", Spec: 2, Ready: 1},
 				},
 			},
 			mockEstimator: &mockUnschedulableReplicaEstimator{},
-			expected: []*TargetClusterWrapper{
+			expected: []*ClusterReplicaStatus{
 				{ClusterName: "cluster1", Spec: 3, Ready: 2, Unschedulable: 1},
 				{ClusterName: "cluster2", Spec: 2, Ready: 1, Unschedulable: 1},
 			},
@@ -152,12 +152,12 @@ func TestSchedulingResultHelper_FillUnschedulableReplicas(t *testing.T) {
 						},
 					},
 				},
-				TargetClusters: []*TargetClusterWrapper{
+				TargetClusterReplicaStatus: []*ClusterReplicaStatus{
 					{ClusterName: "cluster1", Spec: 3, Ready: 2},
 				},
 			},
 			mockEstimator: &mockUnschedulableReplicaEstimator{shouldError: true},
-			expected: []*TargetClusterWrapper{
+			expected: []*ClusterReplicaStatus{
 				{ClusterName: "cluster1", Spec: 3, Ready: 2, Unschedulable: 0},
 			},
 			expectedErrLog: "Max cluster unschedulable replicas error: mock error",
@@ -175,13 +175,13 @@ func TestSchedulingResultHelper_FillUnschedulableReplicas(t *testing.T) {
 						},
 					},
 				},
-				TargetClusters: []*TargetClusterWrapper{
+				TargetClusterReplicaStatus: []*ClusterReplicaStatus{
 					{ClusterName: "cluster1", Spec: 3, Ready: 2},
 					{ClusterName: "cluster2", Spec: 2, Ready: 1},
 				},
 			},
 			mockEstimator: &mockUnschedulableReplicaEstimator{unauthenticCluster: "cluster2"},
-			expected: []*TargetClusterWrapper{
+			expected: []*ClusterReplicaStatus{
 				{ClusterName: "cluster1", Spec: 3, Ready: 2, Unschedulable: 1},
 				{ClusterName: "cluster2", Spec: 2, Ready: 1, Unschedulable: 0},
 			},
@@ -195,8 +195,8 @@ func TestSchedulingResultHelper_FillUnschedulableReplicas(t *testing.T) {
 
 			tt.helper.FillUnschedulableReplicas(time.Minute)
 
-			if !reflect.DeepEqual(tt.helper.TargetClusters, tt.expected) {
-				t.Errorf("FillUnschedulableReplicas() = %v, want %v", tt.helper.TargetClusters, tt.expected)
+			if !reflect.DeepEqual(tt.helper.TargetClusterReplicaStatus, tt.expected) {
+				t.Errorf("FillUnschedulableReplicas() = %v, want %v", tt.helper.TargetClusterReplicaStatus, tt.expected)
 			}
 		})
 	}
@@ -206,19 +206,19 @@ func TestSchedulingResultHelper_GetUndesiredClusters(t *testing.T) {
 	tests := []struct {
 		name             string
 		helper           *SchedulingResultHelper
-		expectedClusters []*TargetClusterWrapper
+		expectedClusters []*ClusterReplicaStatus
 		expectedNames    []string
 	}{
 		{
 			name: "Mixed desired and undesired clusters",
 			helper: &SchedulingResultHelper{
-				TargetClusters: []*TargetClusterWrapper{
+				TargetClusterReplicaStatus: []*ClusterReplicaStatus{
 					{ClusterName: "cluster1", Spec: 3, Ready: 2},
 					{ClusterName: "cluster2", Spec: 2, Ready: 2},
 					{ClusterName: "cluster3", Spec: 4, Ready: 1},
 				},
 			},
-			expectedClusters: []*TargetClusterWrapper{
+			expectedClusters: []*ClusterReplicaStatus{
 				{ClusterName: "cluster1", Spec: 3, Ready: 2},
 				{ClusterName: "cluster3", Spec: 4, Ready: 1},
 			},
@@ -227,7 +227,7 @@ func TestSchedulingResultHelper_GetUndesiredClusters(t *testing.T) {
 		{
 			name: "All clusters desired",
 			helper: &SchedulingResultHelper{
-				TargetClusters: []*TargetClusterWrapper{
+				TargetClusterReplicaStatus: []*ClusterReplicaStatus{
 					{ClusterName: "cluster1", Spec: 2, Ready: 2},
 					{ClusterName: "cluster2", Spec: 3, Ready: 3},
 				},