feat(store): support server labels for store #6638

liubog2008 · 2026-01-04T10:02:38Z

support store labels

Signed-off-by: liubo02 <[email protected]>

codecov-commenter · 2026-01-04T10:09:59Z

Codecov Report

❌ Patch coverage is 21.77419% with 97 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.33%. Comparing base (5cdabec) to head (7395197).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6638      +/-   ##
==========================================
+ Coverage   39.29%   39.33%   +0.04%     
==========================================
  Files         351      351              
  Lines       20692    20665      -27     
==========================================
- Hits         8130     8129       -1     
+ Misses      12562    12536      -26

Flag	Coverage Δ
unittest	`39.33% <21.77%> (+0.04%)`	⬆️

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

liubog2008 · 2026-01-05T00:36:41Z

/test pull-e2e

Signed-off-by: liubo02 <[email protected]>

liubog2008 · 2026-01-06T03:49:30Z

/test pull-e2e

Copilot

Pull request overview

This PR adds support for server labels on store components (TiKV and TiFlash), extending the existing server labels functionality that was previously available only for TiDB and TiProxy. The implementation refactors the label management to use a common pattern while preserving component-specific behaviors.

Key changes include:

Addition of Labels field to TiKV and TiFlash Server structs with CRD validation
Refactoring of PD client state management to support lazy initialization and caching
Introduction of a Contains utility function for map subset validation
Unification of server labels task implementation across all components

Reviewed changes

Copilot reviewed 65 out of 65 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`pkg/utils/map/map.go`	Adds Contains function to check if one map contains all key-value pairs from another
`pkg/state/pd_client.go`	Refactors PD client state to support lazy initialization with cluster context
`pkg/controllers/common/task.go`	Refactors TaskServerLabels to accept PD client manager parameter and removes component-specific logic
`pkg/controllers/tikv/tasks/store_labels.go`	Replaces custom implementation with common TaskServerLabels, uses Contains for comparison
`pkg/controllers/tiflash/tasks/store_labels.go`	Replaces custom implementation with common TaskServerLabels, uses Contains for comparison
`pkg/controllers/tidb/builder.go`	Updates to use refactored TaskServerLabels with compare-before-set logic
`pkg/controllers/tiproxy/builder.go`	Updates to use refactored TaskServerLabels signature
`pkg/pdapi/v1/client.go`	Changes SetStoreLabels signature from uint64 to string for storeID consistency
`pkg/tidbapi/v1/client.go`	Refactors SetServerLabels return handling and adds Labels field to ServerInfo
`pkg/tiproxyapi/v1/client.go`	Removes trailing slash from config API path
`pkg/runtime/zz_generated.runtime.*.go`	Adds ServerLabels() method to all instance types (generated code)
`api/core/v1alpha1/tikv_types.go`	Adds Labels field to TiKVServer struct with validation
`api/core/v1alpha1/tiflash_types.go`	Adds Labels field to TiFlashServer struct with validation
`api/core/v1alpha1/tidb_types.go`	Updates documentation for Labels field behavior
`api/core/v1alpha1/tiproxy_types.go`	Updates documentation for Labels field behavior
`manifests/crd/*.yaml`	Updates CRD manifests with Labels field definitions and documentation
`cmd/runtime-gen/generators/*.go`	Updates code generator to handle ServerLabels field mapping

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-06T04:12:18Z

pkg/utils/map/map.go

+func Contains[K comparable, V comparable](a, b map[K]V) bool {
+	for k, v := range b {
+		if a[k] != v {
+			return false
+		}
+	}


The Contains function checks if map 'a' contains all key-value pairs from map 'b', but it doesn't verify that the key exists in 'a' before comparing values. When a key from 'b' doesn't exist in 'a', accessing a[k] returns the zero value for the value type, which may incorrectly match if the value in 'b' is also the zero value. This could lead to false positives where the function returns true even when 'a' doesn't actually contain all elements from 'b'.

Copilot · 2026-01-06T04:12:19Z

pkg/utils/map/map.go

 	return ret
 }

+// Contains check whether map a contains all elem in map b


The comment has a grammatical error. It should say "checks whether" instead of "check whether".

Copilot · 2026-01-06T04:12:19Z

pkg/tidbapi/v1/client.go

 }

+// SetServerLabels will set labels of tidb server.
+// TODO(liubo02): now this function call is not safe because labels maybe changed by others after get info is called.


The comment contains a grammatical error. "maybe" should be "may be" (two words).

Copilot · 2026-01-06T04:12:19Z

manifests/crd/core.pingcap.com_tiproxygroups.yaml

+                              Operator will set these `labels` by API.
+                              If a label in this field is conflict with the config file, this field takes precedence.
+                              NOTE: If a label is removed, operator will not delete it automatically.
+                              NOTE: these label keys are managed by TiDB Operator, it will be set automatically and you can not modify them:


The documentation comment says "NOTE: If a label is removed, operator will not delete it automatically" but this behavior is inconsistent across components. For TiKV and TiFlash, the Contains check only verifies that expected labels are present but doesn't remove extra labels. However, for TiDB the comment says "Different from other components, TiDB will replace all labels but not only add/update", which suggests TiDB does delete removed labels. This inconsistency should be clarified in the documentation.

Suggested change

Operator will set these `labels` by API.

If a label in this field is conflict with the config file, this field takes precedence.

NOTE: If a label is removed, operator will not delete it automatically.

NOTE: these label keys are managed by TiDB Operator, it will be set automatically and you can not modify them:

TiDB Operator will set these labels via the TiProxy API.

If a label in this field conflicts with the config file, this field takes precedence.

TiDB Operator only adds or updates the labels specified here; it does not remove any existing

labels on the TiProxy server. Removing a label key from this field will NOT delete that label

from TiProxy automatically.

NOTE: The following label keys are managed automatically by TiDB Operator. They are set by the

operator and cannot be modified in this field:

Copilot · 2026-01-06T04:12:20Z

pkg/utils/map/map.go

+// Contains check whether map a contains all elem in map b
+func Contains[K comparable, V comparable](a, b map[K]V) bool {
+	for k, v := range b {
+		if a[k] != v {
+			return false
+		}
+	}
+
+	return true
+}


The newly introduced Contains function lacks test coverage. Given that this function is used in critical label synchronization logic for TiKV and TiFlash stores, it should have comprehensive test coverage to ensure it correctly validates that map 'a' contains all key-value pairs from map 'b'.

fgksgf · 2026-01-06T04:15:18Z

pkg/controllers/common/task.go

-		if zoneLabel := findZoneLabel(pdCfg); zoneLabel != "" {
-			serverLabels[dcLabel] = serverLabels[zoneLabel]
-		}


why remove this

fgksgf · 2026-01-06T04:15:32Z

pkg/controllers/common/task.go

-func findZoneLabel(cfg *pdapi.PDConfigFromAPI) string {
-	for _, zl := range topologyZoneLabels {
-		if slices.Contains(cfg.Replication.LocationLabels, zl) {
-			return zl
-		}
-	}
-	return ""
-}


why remove this

useless? It's for compatibility

fgksgf · 2026-01-06T04:16:40Z

pkg/controllers/tidb/builder.go

-			return state.TiDBClient.SetServerLabels(ctx, labels)
+			info, err := state.TiDBClient.GetInfo(ctx)
+			if err != nil {
+				return err


format the error with fmt.Errorf?

fgksgf · 2026-01-06T04:17:40Z

pkg/controllers/tiflash/tasks/ctx.go

-		state.StoreLabels = make([]*metapb.StoreLabel, len(s.Labels))
-		for k, v := range s.Labels {
-			state.StoreLabels = append(state.StoreLabels, &metapb.StoreLabel{Key: k, Value: v})
-		}
-


why remove this

use state.Store.Labels directly

fgksgf · 2026-01-06T04:21:37Z

pkg/controllers/tiproxy/builder.go

 		tasks.TaskPod(state, r.Client),
-		common.TaskServerLabels[scope.TiProxy](state, r.Client, func(ctx context.Context, labels map[string]string) error {
+		common.TaskServerLabels[scope.TiProxy](state, r.Client, r.PDClientManager, func(ctx context.Context, labels map[string]string) error {
+			// TODO(liubo02): compare before setting


why not do it in this PR

Signed-off-by: liubo02 <[email protected]>

ti-chi-bot · 2026-01-06T06:11:02Z

[LGTM Timeline notifier]

Timeline:

2026-01-06 06:11:02.30270764 +0000 UTC m=+682618.121016082: ☑️ agreed by fgksgf.

ti-chi-bot · 2026-01-06T06:11:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fgksgf

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [fgksgf]

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

feat(store): support server labels for store

8b847c4

Signed-off-by: liubo02 <[email protected]>

liubog2008 force-pushed the liubo02/support-label-store branch from 4b91662 to 8b847c4 Compare January 4, 2026 10:02

ti-chi-bot bot requested a review from howardlau1999 January 4, 2026 10:02

github-actions bot added the v2 for operator v2 label Jan 4, 2026

ti-chi-bot bot added the size/XXL label Jan 4, 2026

liubog2008 changed the title ~~Liubo02/support label store~~ feat(store): support server labels for store Jan 4, 2026

fix(tidb): fix set labels of tidb

2605f58

Signed-off-by: liubo02 <[email protected]>

liubog2008 force-pushed the liubo02/support-label-store branch from 0d23a9a to 2605f58 Compare January 5, 2026 03:13

liubog2008 added 2 commits January 5, 2026 14:43

fix e2e

a866929

Signed-off-by: liubo02 <[email protected]>

fix ut

441caab

Signed-off-by: liubo02 <[email protected]>

fgksgf requested a review from Copilot January 6, 2026 04:02

Copilot started reviewing on behalf of fgksgf January 6, 2026 04:03 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

fgksgf reviewed Jan 6, 2026

View reviewed changes

liubog2008 added 2 commits January 6, 2026 13:44

address comments

b532168

Signed-off-by: liubo02 <[email protected]>

fix lint

7395197

Signed-off-by: liubo02 <[email protected]>

fgksgf approved these changes Jan 6, 2026

View reviewed changes

ti-chi-bot bot added the lgtm label Jan 6, 2026

ti-chi-bot bot added the approved label Jan 6, 2026

ti-chi-bot bot merged commit 7b359cb into pingcap:main Jan 6, 2026
10 checks passed

-                              Operator will set these `labels` by API.
-                              If a label in this field is conflict with the config file, this field takes precedence.
-                              NOTE: If a label is removed, operator will not delete it automatically.
-                              NOTE: these label keys are managed by TiDB Operator, it will be set automatically and you can not modify them:
+                              TiDB Operator will set these labels via the TiProxy API.
+                              If a label in this field conflicts with the config file, this field takes precedence.
+                              TiDB Operator only adds or updates the labels specified here; it does not remove any existing
+                              labels on the TiProxy server. Removing a label key from this field will NOT delete that label
+                              from TiProxy automatically.
+                              NOTE: The following label keys are managed automatically by TiDB Operator. They are set by the
+                              operator and cannot be modified in this field:

feat(store): support server labels for store #6638

feat(store): support server labels for store #6638

Uh oh!

Conversation

liubog2008 commented Jan 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jan 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

liubog2008 commented Jan 5, 2026

Uh oh!

liubog2008 commented Jan 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

fgksgf Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

fgksgf Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

liubog2008 Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

fgksgf Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

fgksgf Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

liubog2008 Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

fgksgf Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Jan 6, 2026

[LGTM Timeline notifier]

Uh oh!

ti-chi-bot bot commented Jan 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

liubog2008 commented Jan 4, 2026 •

edited

Loading

codecov-commenter commented Jan 4, 2026 •

edited

Loading