Support-Multi-version-TiDB-monitor #666

qiffang · 2019-07-18T05:54:59Z

What problem does this PR solve?

In this PR, it contains 3 functions

Support multi TiDB version
Support to dynamic reload rules
Update monitor deployment Strategy

1.TiDB multi version
In this PR, Every TiDB has own monitor initializer image.
And the docker receive 4 variables

GF_PROVISIONING_PATH // grafana provisioning path
TIDB_CLUSTER_NAME // TiDB cluster name
TIDB_ENABLE_BINLOG // whether enable binlog
PROM_CONFIG_PATH // proemtheus rules config path

TiDB Operator use the new monitor way to replace the old.

2.Dynamic update alert rules
In reload, it support a UI to update alert rules and it will call reload of Promtheus.
The UI likes this

Update strategy with ReCreate.

What is changed and how does it work?

Delete the old way and import a tidb-monitor-initializer image to init grafana dashboards and prometheus rules.

Check List

Tests

TiDB Cluster dashboards work well and ClusterName is replaced successful.
Promethes rules work well and ClusterName is replaced successful.
Alert rule can be updated dynamic.
Monitor deployment strategy is updated successful.

Related changes

Need to update the documentation

Does this PR introduce a user-facing change?:

Action required: Users should migrate the configs in values.yaml of previous chart releases to the new values.yaml of the new chart. Otherwise, the monitor pods will be failed when upgrading monitor with the new chart.
For example, configs in old values.yaml:
monitor:
 create: true
 # Also see rbac.create
 # If you set rbac.create to false, you need to provide a value here.
 # If you set rbac.create to true, you should leave this empty.
 # serviceAccount:
 persistent: false
 storageClassName: local-storage
 storage: 10Gi
 grafana:
   ...
 prometheus:
   image: prom/prometheus:v2.2.1
   ...

After migration, new values.yaml should be as below:
monitor:
 create: true
 # Also see rbac.create
 # If you set rbac.create to false, you need to provide a value here.
 # If you set rbac.create to true, you should leave this empty.
 # serviceAccount:
 persistent: false
 storageClassName: local-storage
 storage: 10Gi
 initializer:
   image: pingcap/tidb-monitor-initializer:v3.0.0
   imagePullPolicy: IfNotPresent
   reloader:
   create: true
   image: pingcap/tidb-monitor-reloader:v1.0.0
   imagePullPolicy: IfNotPresent
   service:
     type: NodePort
 grafana:
   ...
 prometheus:
   image: prom/prometheus:v2.11.1
   ...

charts/tidb-cluster/values.yaml

weekface · 2019-07-18T06:17:29Z

charts/tidb-cluster/values.yaml

@@ -330,6 +330,10 @@ monitor:
  persistent: false
  storageClassName: local-storage
  storage: 10Gi
+  initializer:
+    create: true
+    image: pingcap/tidb-monitor-initializer:v3.0.0-rc.1


$ docker pull pingcap/tidb-monitor-initializer:v3.0.0-rc.1 Error response from daemon: pull access denied for pingcap/tidb-monitor-initializer, repository does not exist or may require 'docker login'

https://github.com/pingcap/monitoring The images here are note uploaded to DockerHub yet.

The values in other places like test, terraform also have to be updated.

We could have a CI check to tell us that terraform values are out of date.

weekface · 2019-07-18T08:26:11Z

/run-e2e-tests

tennix · 2019-07-19T07:11:55Z

charts/tidb-cluster/templates/monitor-deployment.yaml

@@ -181,23 +185,10 @@ spec:
          - key: dashboard-config
            path: dashboards.yaml
        name: dashboards-provisioning
+      - emptyDir: {}


Is it better to use the same directory as the data directory?

数据目录是 PV，配置不需要PV

charts/tidb-cluster/templates/monitor-service.yaml

charts/tidb-cluster/templates/monitor-deployment.yaml

Signed-off-by: qiffang <947321353@qq.com>

weekface · 2019-07-24T08:46:10Z

charts/tidb-cluster/templates/monitor-deployment.yaml

@@ -11,6 +11,9 @@ metadata:
    helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+"  "_" }}
 spec:
  replicas: 1
+  strategy:
+    type: Recreate
+    rollingUpdate: null


remove this?

remove这个会有问题

Error: UPGRADE FAILED: Deployment.apps “tikv200-monitor” is invalid: spec.strategy.rollingUpdate: Forbidden: may not be specified when strategy `type` is ‘Recreate’

需要显示把这个置为null

weekface · 2019-07-24T09:16:34Z

/run-e2e-tests

Signed-off-by: qiffang <947321353@qq.com>

…ng/tidb-operator into Multi-version-TiDB-monitor

tennix · 2019-07-25T09:00:43Z

/run-e2e-tests

weekface · 2019-07-25T09:59:38Z

charts/tidb-cluster/templates/monitor-deployment.yaml

+          value: /grafana-dashboard-definitions/tidb
+        - name: TIDB_CLUSTER_NAME
+          value: {{ template "cluster.name" . }}
+        - name: TIDB_ENABLE_BINLOG


why add this env? TIDB_ENABLE_BINLOG

This env is used to decide whether Grafana show BinLog dashboard.

DanielZhangQD

LGTM

charts/tidb-cluster/values.yaml

Signed-off-by: qiffang <947321353@qq.com>

tennix

LGTM

tennix · 2019-07-29T00:29:36Z

/run-e2e-tests

(cherry picked from commit 5a227d8)

Signed-off-by: Ran <huangran@pingcap.com>

Multi-version-TiDB-monitor

4483b15

weekface reviewed Jul 18, 2019

View reviewed changes

qiffang added 2 commits July 18, 2019 14:22

Multi-version-TiDB-monitor

441a686

Multi-version-TiDB-monitor

4223832

weekface added this to the v1.0.0 milestone Jul 18, 2019

tennix reviewed Jul 19, 2019

View reviewed changes

qiffang added 2 commits July 23, 2019 19:48

Multi-version-TiDB-monitor

851647d

Multi-version-TiDB-monitor

f4f8436

tennix reviewed Jul 24, 2019

View reviewed changes

charts/tidb-cluster/templates/monitor-service.yaml Outdated Show resolved Hide resolved

tennix reviewed Jul 24, 2019

View reviewed changes

charts/tidb-cluster/templates/monitor-deployment.yaml Show resolved Hide resolved

Multi-version-TiDB-monitor

31d1092

Signed-off-by: qiffang <947321353@qq.com>

weekface reviewed Jul 24, 2019

View reviewed changes

qiffang added 3 commits July 24, 2019 17:47

Merge branch 'master' into Multi-version-TiDB-monitor

0874edb

Multi-version-TiDB-monitor

a33da70

Signed-off-by: qiffang <947321353@qq.com>

Merge branch 'Multi-version-TiDB-monitor' of https://github.com/qiffa…

18c72eb

…ng/tidb-operator into Multi-version-TiDB-monitor

weekface mentioned this pull request Jul 25, 2019

Rolling restart of the monitor services gets stuck with multiple monitor nodes #668

Closed

weekface reviewed Jul 25, 2019

View reviewed changes

weekface previously approved these changes Jul 25, 2019

View reviewed changes

DanielZhangQD reviewed Jul 26, 2019

View reviewed changes

tennix reviewed Jul 26, 2019

View reviewed changes

charts/tidb-cluster/values.yaml Outdated Show resolved Hide resolved

Multi-version-TiDB-monitor

c891d96

Signed-off-by: qiffang <947321353@qq.com>

qiffang dismissed weekface’s stale review via c891d96 July 28, 2019 11:59

tennix approved these changes Jul 29, 2019

View reviewed changes

weekface approved these changes Jul 29, 2019

View reviewed changes

Merge branch 'master' into Multi-version-TiDB-monitor

19f82d6

tennix merged commit 5a227d8 into pingcap:master Jul 29, 2019

weekface pushed a commit that referenced this pull request Jul 29, 2019

Support-Multi-version-TiDB-monitor (#666)

961a45e

(cherry picked from commit 5a227d8)

yahonda pushed a commit that referenced this pull request Dec 27, 2021

en: bump tidb version to 4.0.4 (#666)

52cb70b

Signed-off-by: Ran <huangran@pingcap.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support-Multi-version-TiDB-monitor #666

Support-Multi-version-TiDB-monitor #666

qiffang commented Jul 18, 2019 •

edited

Loading

weekface Jul 18, 2019

qiffang Jul 18, 2019 •

edited by tennix

Loading

tennix Jul 19, 2019

gregwebs Jul 22, 2019

weekface commented Jul 18, 2019

tennix Jul 19, 2019

qiffang Jul 24, 2019

weekface Jul 24, 2019

qiffang Jul 24, 2019

weekface Jul 24, 2019

weekface commented Jul 24, 2019

tennix commented Jul 25, 2019

weekface Jul 25, 2019

qiffang Jul 28, 2019

DanielZhangQD left a comment

tennix left a comment

tennix commented Jul 29, 2019

Support-Multi-version-TiDB-monitor #666

Support-Multi-version-TiDB-monitor #666

Conversation

qiffang commented Jul 18, 2019 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Does this PR introduce a user-facing change?:

Choose a reason for hiding this comment

qiffang Jul 18, 2019 • edited by tennix Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weekface commented Jul 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weekface commented Jul 24, 2019

tennix commented Jul 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielZhangQD left a comment

Choose a reason for hiding this comment

tennix left a comment

Choose a reason for hiding this comment

tennix commented Jul 29, 2019

qiffang commented Jul 18, 2019 •

edited

Loading

qiffang Jul 18, 2019 •

edited by tennix

Loading