search

pingcap / tidb-operator

TiDB operator creates and manages TiDB clusters running in Kubernetes.
https://docs.pingcap.com/tidb-in-kubernetes/
Apache License 2.0
1.23k stars 495 forks source link

TiDB operator cannot update nor contact the TiDB cluster after the TLS config is changed #5728

Open kos-team opened 4 weeks ago

kos-team commented 4 weeks ago

Bug Report

What version of Kubernetes are you using?

1.28.1

What version of TiDB Operator are you using?

1.6.0

What storage classes exist in the Kubernetes cluster and what are used for PD/TiKV pods?

standard

What's the status of the TiDB cluster pods?

What did you do?

We tried to change the TLS configuration of the TiDB cluster, but found that the operator fails to contact the TiDB cluster after we change the tlsCluster.enabled option. This bug happens both when we change it from true to false and false to true. The root cause of this bug is due to the mismatch of operator's pd client's TLS configuration and the server's config.

To reproduce this bug:

  1. First, create the certificate secrets according to the documentation: https://docs.pingcap.com/tidb-in-kubernetes/stable/enable-tls-between-components#using-cert-manager
  2. Install TiDB cluster by applying the CR with tlsCluster.enabled to true 
    apiVersion: pingcap.com/v1alpha1
kind: TidbCluster
metadata:
name: basic
spec:
version: v8.1.0
timezone: UTC
pvReclaimPolicy: Retain
enableDynamicConfiguration: true
configUpdateStrategy: RollingUpdate
discovery: {}
helper:
image: alpine:3.16.0
tlsCluster:
enabled: true
pd:
baseImage: pingcap/pd
maxFailoverCount: 0
replicas: 1
requests:
  storage: "1Gi"
config: {}
tikv:
baseImage: pingcap/tikv
maxFailoverCount: 0
# If only 1 TiKV is deployed, the TiKV region leader
# cannot be transferred during upgrade, so we have
# to configure a short timeout
evictLeaderTimeout: 1m
replicas: 1
requests:
  storage: "1Gi"
config:
  storage:
    # In basic examples, we set this to avoid using too much storage.
    reserve-space: "0MB"
    engine: "partitioned-raft-kv"
  rocksdb:
    # In basic examples, we set this to avoid the following error in some Kubernetes clusters:
    # "the maximum number of open file descriptors is too small, got 1024, expect greater or equal to 82920"
    max-open-files: 256
  raftdb:
    max-open-files: 256
tidb:
baseImage: pingcap/tidb
maxFailoverCount: 0
replicas: 1
service:
  type: ClusterIP
config: {}
  3. Change the tlsCluster.enabled to false 
    apiVersion: pingcap.com/v1alpha1
kind: TidbCluster
metadata:
name: basic
spec:
version: v8.1.0
timezone: UTC
pvReclaimPolicy: Retain
enableDynamicConfiguration: true
configUpdateStrategy: RollingUpdate
discovery: {}
helper:
image: alpine:3.16.0
tlsCluster:
enabled: true
pd:
baseImage: pingcap/pd
maxFailoverCount: 0
replicas: 1
requests:
  storage: "1Gi"
config: {}
tikv:
baseImage: pingcap/tikv
maxFailoverCount: 0
# If only 1 TiKV is deployed, the TiKV region leader
# cannot be transferred during upgrade, so we have
# to configure a short timeout
evictLeaderTimeout: 1m
replicas: 1
requests:
  storage: "1Gi"
config:
  storage:
    # In basic examples, we set this to avoid using too much storage.
    reserve-space: "0MB"
    engine: "partitioned-raft-kv"
  rocksdb:
    # In basic examples, we set this to avoid the following error in some Kubernetes clusters:
    # "the maximum number of open file descriptors is too small, got 1024, expect greater or equal to 82920"
    max-open-files: 256
  raftdb:
    max-open-files: 256
tidb:
baseImage: pingcap/tidb
maxFailoverCount: 0
replicas: 1
service:
  type: ClusterIP
config: {}

What did you expect to see? The operator should be able to reconfigure the TLS config for the TiDB cluster.

What did you see instead? The operator fails with the error logs:

| E0830 20:39:35.006167       1 pd_member_manager.go:201] failed to sync TidbCluster: [default/advanced-tidb]'s status, error: Get "https://advanced-tidb-pd.default:2379/pd/api/v1/health": EOF                   │
│ E0830 20:39:35.020171       1 tidb_cluster_controller.go:144] TidbCluster: default/advanced-tidb, sync failed tidbcluster: [default/advanced-tidb]'s pd status sync failed, can not to be upgraded, requeuing    │
│ E0830 20:39:41.747446       1 pd_member_manager.go:201] failed to sync TidbCluster: [default/advanced-tidb]'s status, error: Get "https://advanced-tidb-pd.default:2379/pd/api/v1/health": EOF                   │
│ E0830 20:39:41.760679       1 tidb_cluster_controller.go:144] TidbCluster: default/advanced-tidb, sync failed tidbcluster: [default/advanced-tidb]'s pd status sync failed, can not to be upgraded, requeuing

The root cause is the pd client's TLS configuration. After the tlsCluster configuration gets changed, the operator uses a different client with a different TLS configuration than what the TiDB cluster is currently using. This causes the healthcheck to fail due to the client error, although the cluster is already healthy.

https://github.com/pingcap/tidb-operator/blob/ff467a6e7a0563b31a3ace2fe5d060774012780d/pkg/pdapi/pd_control.go#L241C1-L252C3

csuzhangxc commented 3 weeks ago

@kos-team in the above example, the root cause is: after changed from TLS to non-TLS, the operator is still using a client with TLS enabled to connect to PD, right?

can you workaround it after restarting tidb-controller-manager?

kos-team commented 3 weeks ago

@csuzhangxc The root cause is because the operator switched to use a client with non-TLS, while the PD is still running with TLS. This causes the calls to the PD to fail. I believe restarting the tidb-operator won't help.

csuzhangxc commented 3 weeks ago

@csuzhangxc The root cause is because the operator switched to use a client with non-TLS, while the PD is still running with TLS. This causes the calls to the PD to fail. I believe restarting the tidb-operator won't help.

In your case, after updating the TidbCluster CR from TLS to non-TLS, is PD still using TLS? Is the ConfigMap of PD updated? if the ConfigMap is updated, this may be related to #5708

kos-team commented 3 weeks ago

The ConfigMap is not updated. The pd_member_manager early returns at this line: https://github.com/pingcap/tidb-operator/blob/ff467a6e7a0563b31a3ace2fe5d060774012780d/pkg/manager/member/pd_member_manager.go#L200 which is before where the ConfigMap is updated: https://github.com/pingcap/tidb-operator/blob/ff467a6e7a0563b31a3ace2fe5d060774012780d/pkg/manager/member/pd_member_manager.go#L209

And as mentioned in the issue report, the reason that syncTidbClusterStatus fails is because of the inconsistent client config. The client should use TLS config according to the server's current config, but not the desired TLS config specified in the CR.

csuzhangxc commented 3 weeks ago

Got it! Let's think about how to resolve it with minimal changes.