From eea66fb0e9477acb0e340da0f3c97b5ab650263d Mon Sep 17 00:00:00 2001 From: xhe Date: Wed, 2 Aug 2023 22:02:27 +0800 Subject: [PATCH] fix: add label overwrite for tiproxy component val compatibility (#5194) --- docs/api-references/docs.md | 16 +++++ hack/lib.sh | 2 +- hack/localtest/cluster.yaml | 3 +- hack/localtest/monitor.yaml | 24 +++++++ hack/localtest/test.sh | 4 +- manifests/crd.yaml | 64 +++++++++++++++++++ manifests/crd/v1/pingcap.com_dmclusters.yaml | 16 +++++ .../crd/v1/pingcap.com_tidbclusters.yaml | 36 +++++++++++ .../crd/v1/pingcap.com_tidbdashboards.yaml | 4 ++ .../crd/v1/pingcap.com_tidbngmonitorings.yaml | 8 +++ pkg/apis/pingcap/v1alpha1/component_spec.go | 9 ++- pkg/apis/pingcap/v1alpha1/types.go | 8 +++ .../pingcap/v1alpha1/zz_generated.deepcopy.go | 20 +++++- pkg/manager/member/tiproxy_upgrader.go | 19 +++++- 14 files changed, 224 insertions(+), 9 deletions(-) create mode 100644 hack/localtest/monitor.yaml diff --git a/docs/api-references/docs.md b/docs/api-references/docs.md index e2c6f6f2c4..55df96188c 100644 --- a/docs/api-references/docs.md +++ b/docs/api-references/docs.md @@ -25536,6 +25536,22 @@ LabelSelector is generated by component type See pkg/apis/pingcap/v1alpha1/tidbcluster_component.go#TopologySpreadConstraints()

+ + +matchLabels
+ +github.com/pingcap/tidb-operator/pkg/apis/label.Label + + + +(Optional) +

MatchLabels is used to overwrite generated corev1.TopologySpreadConstraints.LabelSelector +corev1.TopologySpreadConstraint generated in component_spec.go will set a +LabelSelector automatically with some KV. +Historically, it is l[“comp”] = “” for component tiproxy. And we will use +MatchLabels to keep l[“comp”] = “” for old clusters with tiproxy

+ +

TxnLocalLatches

diff --git a/hack/lib.sh b/hack/lib.sh index 241f6a158e..b59a47181a 100644 --- a/hack/lib.sh +++ b/hack/lib.sh @@ -266,7 +266,7 @@ function hack::ensure_aws_k8s_tester() { function hack::ensure_gen_crd_api_references_docs() { echo "Installing gen_crd_api_references_docs..." - GOBIN=$OUTPUT_BIN go install github.com/xhebox/gen-crd-api-reference-docs@e46d84594a6d158ec7123ff05acd57acf62e140f + GOBIN=$OUTPUT_BIN go install github.com/xhebox/gen-crd-api-reference-docs@a8a3b01e858f0de8bc8f9419d210da4334929e2d } function hack::ensure_misspell() { diff --git a/hack/localtest/cluster.yaml b/hack/localtest/cluster.yaml index dbaa1c122d..5c3d5e0ed7 100644 --- a/hack/localtest/cluster.yaml +++ b/hack/localtest/cluster.yaml @@ -2,6 +2,7 @@ apiVersion: pingcap.com/v1alpha1 kind: TidbCluster metadata: name: basic + annotations: spec: version: v6.3.0 timezone: UTC @@ -33,7 +34,7 @@ spec: maxFailoverCount: 0 config: {} tiproxy: - baseImage: xhebox/tiproxy + baseImage: pingcap/tiproxy version: latest imagePullPolicy: Always replicas: 1 diff --git a/hack/localtest/monitor.yaml b/hack/localtest/monitor.yaml new file mode 100644 index 0000000000..a0c5f4289b --- /dev/null +++ b/hack/localtest/monitor.yaml @@ -0,0 +1,24 @@ +apiVersion: pingcap.com/v1alpha1 +kind: TidbMonitor +metadata: + name: basic +spec: + replicas: 1 + clusters: + - name: basic + prometheus: + baseImage: prom/prometheus + version: v2.27.1 + grafana: + baseImage: grafana/grafana + version: 7.5.11 + initializer: + baseImage: pingcap/tidb-monitor-initializer + version: v6.5.0 + reloader: + baseImage: pingcap/tidb-monitor-reloader + version: v1.0.1 + prometheusReloader: + baseImage: quay.io/prometheus-operator/prometheus-config-reloader + version: v0.49.0 + imagePullPolicy: IfNotPresent diff --git a/hack/localtest/test.sh b/hack/localtest/test.sh index 107e64448b..fee252d5e8 100755 --- a/hack/localtest/test.sh +++ b/hack/localtest/test.sh @@ -12,8 +12,8 @@ if [ -n "$BUILD" ]; then make DOCKER_REPO=xx operator-docker fi -kubectl replace -f $BASE/../../manifests/crd.yaml -kubectl replace -f $BASE/../../manifests/advanced-statefulset-crd.v1.yaml +kubectl replace --force -f $BASE/../../manifests/crd.yaml +kubectl replace --force -f $BASE/../../manifests/advanced-statefulset-crd.v1.yaml kubectl delete --force namespace $namespace || true diff --git a/manifests/crd.yaml b/manifests/crd.yaml index 1ebb9a7608..41cb8839c7 100644 --- a/manifests/crd.yaml +++ b/manifests/crd.yaml @@ -7471,6 +7471,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -10010,6 +10014,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -10142,6 +10150,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -12610,6 +12622,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -18449,6 +18465,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -21024,6 +21044,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -23558,6 +23582,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -26069,6 +26097,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -28762,6 +28794,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -31311,6 +31347,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -33849,6 +33889,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -36331,6 +36375,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -36369,6 +36417,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -40319,6 +40371,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -47672,6 +47728,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -47810,6 +47870,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: diff --git a/manifests/crd/v1/pingcap.com_dmclusters.yaml b/manifests/crd/v1/pingcap.com_dmclusters.yaml index d152d8cd2d..640d48ead5 100644 --- a/manifests/crd/v1/pingcap.com_dmclusters.yaml +++ b/manifests/crd/v1/pingcap.com_dmclusters.yaml @@ -2866,6 +2866,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -5405,6 +5409,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -5537,6 +5545,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -8005,6 +8017,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: diff --git a/manifests/crd/v1/pingcap.com_tidbclusters.yaml b/manifests/crd/v1/pingcap.com_tidbclusters.yaml index 57da219678..7e768c9abd 100644 --- a/manifests/crd/v1/pingcap.com_tidbclusters.yaml +++ b/manifests/crd/v1/pingcap.com_tidbclusters.yaml @@ -2891,6 +2891,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -5466,6 +5470,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -8000,6 +8008,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -10511,6 +10523,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -13204,6 +13220,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -15753,6 +15773,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -18291,6 +18315,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -20773,6 +20801,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -20811,6 +20843,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: diff --git a/manifests/crd/v1/pingcap.com_tidbdashboards.yaml b/manifests/crd/v1/pingcap.com_tidbdashboards.yaml index 8651a3bbf8..1678964f5c 100644 --- a/manifests/crd/v1/pingcap.com_tidbdashboards.yaml +++ b/manifests/crd/v1/pingcap.com_tidbdashboards.yaml @@ -2534,6 +2534,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: diff --git a/manifests/crd/v1/pingcap.com_tidbngmonitorings.yaml b/manifests/crd/v1/pingcap.com_tidbngmonitorings.yaml index b3a046dda1..e3b76c4afd 100644 --- a/manifests/crd/v1/pingcap.com_tidbngmonitorings.yaml +++ b/manifests/crd/v1/pingcap.com_tidbngmonitorings.yaml @@ -4795,6 +4795,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: @@ -4933,6 +4937,10 @@ spec: topologySpreadConstraints: items: properties: + matchLabels: + additionalProperties: + type: string + type: object topologyKey: type: string required: diff --git a/pkg/apis/pingcap/v1alpha1/component_spec.go b/pkg/apis/pingcap/v1alpha1/component_spec.go index 0144def923..db693d422b 100644 --- a/pkg/apis/pingcap/v1alpha1/component_spec.go +++ b/pkg/apis/pingcap/v1alpha1/component_spec.go @@ -443,8 +443,15 @@ func (a *componentAccessorImpl) TopologySpreadConstraints() []corev1.TopologySpr case DMClusterKind: l = label.NewDM() } + if v, ok := tsc.MatchLabels[label.ComponentLabelKey]; ok { + componentLabelVal = v + } l[label.ComponentLabelKey] = componentLabelVal - l[label.InstanceLabelKey] = a.name + instanceLabelVal := a.name + if v, ok := tsc.MatchLabels[label.InstanceLabelKey]; ok { + instanceLabelVal = v + } + l[label.InstanceLabelKey] = instanceLabelVal ptsc.LabelSelector = &metav1.LabelSelector{ MatchLabels: map[string]string(l), } diff --git a/pkg/apis/pingcap/v1alpha1/types.go b/pkg/apis/pingcap/v1alpha1/types.go index a35d56bca8..3fea034866 100644 --- a/pkg/apis/pingcap/v1alpha1/types.go +++ b/pkg/apis/pingcap/v1alpha1/types.go @@ -21,6 +21,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "github.com/pingcap/tidb-operator/pkg/apis/label" "github.com/pingcap/tidb-operator/pkg/apis/util/config" ) @@ -3012,6 +3013,13 @@ type TopologySpreadConstraint struct { // LabelSelector is generated by component type // See pkg/apis/pingcap/v1alpha1/tidbcluster_component.go#TopologySpreadConstraints() TopologyKey string `json:"topologyKey"` + // MatchLabels is used to overwrite generated corev1.TopologySpreadConstraints.LabelSelector + // corev1.TopologySpreadConstraint generated in component_spec.go will set a + // LabelSelector automatically with some KV. + // Historically, it is l["comp"] = "" for component tiproxy. And we will use + // MatchLabels to keep l["comp"] = "" for old clusters with tiproxy + // +optional + MatchLabels label.Label `json:"matchLabels"` } // Failover contains the failover specification. diff --git a/pkg/apis/pingcap/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pingcap/v1alpha1/zz_generated.deepcopy.go index bba199a9ab..1a4aed7b28 100644 --- a/pkg/apis/pingcap/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pingcap/v1alpha1/zz_generated.deepcopy.go @@ -21,6 +21,7 @@ package v1alpha1 import ( time "time" + label "github.com/pingcap/tidb-operator/pkg/apis/label" model "github.com/prometheus/common/model" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" @@ -981,7 +982,9 @@ func (in *ComponentSpec) DeepCopyInto(out *ComponentSpec) { if in.TopologySpreadConstraints != nil { in, out := &in.TopologySpreadConstraints, &out.TopologySpreadConstraints *out = make([]TopologySpreadConstraint, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.SuspendAction != nil { in, out := &in.SuspendAction, &out.SuspendAction @@ -1284,7 +1287,9 @@ func (in *DMClusterSpec) DeepCopyInto(out *DMClusterSpec) { if in.TopologySpreadConstraints != nil { in, out := &in.TopologySpreadConstraints, &out.TopologySpreadConstraints *out = make([]TopologySpreadConstraint, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.SuspendAction != nil { in, out := &in.SuspendAction, &out.SuspendAction @@ -9044,7 +9049,9 @@ func (in *TidbClusterSpec) DeepCopyInto(out *TidbClusterSpec) { if in.TopologySpreadConstraints != nil { in, out := &in.TopologySpreadConstraints, &out.TopologySpreadConstraints *out = make([]TopologySpreadConstraint, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.SuspendAction != nil { in, out := &in.SuspendAction, &out.SuspendAction @@ -9773,6 +9780,13 @@ func (in *TikvAutoScalerStatus) DeepCopy() *TikvAutoScalerStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TopologySpreadConstraint) DeepCopyInto(out *TopologySpreadConstraint) { *out = *in + if in.MatchLabels != nil { + in, out := &in.MatchLabels, &out.MatchLabels + *out = make(label.Label, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } return } diff --git a/pkg/manager/member/tiproxy_upgrader.go b/pkg/manager/member/tiproxy_upgrader.go index eeb4fa3ab6..a316532cd4 100644 --- a/pkg/manager/member/tiproxy_upgrader.go +++ b/pkg/manager/member/tiproxy_upgrader.go @@ -15,6 +15,7 @@ package member import ( "fmt" + "strconv" "github.com/pingcap/advanced-statefulset/client/apis/apps/v1/helper" "github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1" @@ -22,10 +23,15 @@ import ( mngerutils "github.com/pingcap/tidb-operator/pkg/manager/utils" apps "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog/v2" podutil "k8s.io/kubernetes/pkg/api/v1/pod" ) +const ( + annoKeyTiProxyMinReadySeconds = "tiproxy.pingcap.com/tiproxy-min-ready-seconds" +) + type tiproxyUpgrader struct { deps *controller.Dependencies } @@ -63,6 +69,17 @@ func (u *tiproxyUpgrader) Upgrade(tc *v1alpha1.TidbCluster, oldSet *apps.Statefu return nil } + minReadySeconds := int(newSet.Spec.MinReadySeconds) + s, ok := tc.Annotations[annoKeyTiProxyMinReadySeconds] + if ok { + i, err := strconv.Atoi(s) + if err != nil { + klog.Warningf("tidbcluster: [%s/%s] annotation %s should be an integer: %v", ns, tcName, annoKeyTiProxyMinReadySeconds, err) + } else { + minReadySeconds = i + } + } + mngerutils.SetUpgradePartition(newSet, *oldSet.Spec.UpdateStrategy.RollingUpdate.Partition) podOrdinals := helper.GetPodOrdinals(*oldSet.Spec.Replicas, oldSet).List() for _i := len(podOrdinals) - 1; _i >= 0; _i-- { @@ -79,7 +96,7 @@ func (u *tiproxyUpgrader) Upgrade(tc *v1alpha1.TidbCluster, oldSet *apps.Statefu } if revision == tc.Status.TiProxy.StatefulSet.UpdateRevision { - if !podutil.IsPodReady(pod) { + if !podutil.IsPodAvailable(pod, int32(minReadySeconds), metav1.Now()) { return controller.RequeueErrorf("tidbcluster: [%s/%s]'s upgraded tiproxy pod: [%s] is not ready", ns, tcName, podName) } continue