Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use grafana provisioning to replace dashboard installer #388

Merged
merged 17 commits into from
Apr 15, 2019

Conversation

qiffang
Copy link
Contributor

@qiffang qiffang commented Apr 12, 2019

Function
In the higher version of grafana,. it improve the experience by adding a new active provisioning system that uses config files. So we can remove dashboard installer logic by the feature.
But there is a problem the file of dashboard is too large exceed the limitation of ConfigMap's annotation field, we have to compress it by gzip.
The logic is here:

  1. Make a ConfigMap which contains dashboard file named monitor-dashboard-configmap.yaml
    The content is binary data
  2. Make a ConfigMap which contains dasource file and dashboard provisioning file(This file declare a dashboard path), the name of file is monitor-configmap.yaml
  3. Update monitor-deployment file, the spec declare the ConfigMaps to the directory specified
  4. Add a poststart logic to decompress the files to the path which dashboard provisioning declare
    postStart: exec: command: - "/bin/sh" - "-c" - > gzip -dc /tmp/dashboard-gz/tidb.json.gz > /grafana-dashboard-definitions/tidb/tidb.json; gzip -dc /tmp/dashboard-gz/pd.json.gz > /grafana-dashboard-definitions/tidb/pd.json; gzip -dc /tmp/dashboard-gz/tikv.json.gz > /grafana-dashboard-definitions/tidb/tikv.json; gzip -dc /tmp/dashboard-gz/overview.json.gz > /grafana-dashboard-definitions/tidb/overview.json;
  5. Delete the dashboad installer content

Test

  1. There is not the Job pod to install dashboard and datasource by API
  2. The dashboard works well
    image

@tennix tennix requested review from tennix and aylei April 12, 2019 13:44
@tennix
Copy link
Member

tennix commented Apr 12, 2019

There should be some documents about how the zipped dashboards come from, so other contributors know how to update it when there are newer dashboards in the upstream.

Copy link
Contributor

@aylei aylei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! I just left a format suggestion and a question.

"type": "file"
}
]
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep a new line at the end of file, so do the rest files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accept

@qiffang qiffang force-pushed the use-grafana-provisioning branch from 9928a25 to 67e690f Compare April 13, 2019 05:23
@qiffang
Copy link
Contributor Author

qiffang commented Apr 13, 2019

There should be some documents about how the zipped dashboards come from, so other contributors know how to update it when there are newer dashboards in the upstream.

Accept

  1. Add description in monitor-dashboard-configmap.yaml file.
  2. Delete invalid graphs from TIDB-Cluster-Overview dashboard("System Info" and "Services Port Status") and expanded pd relation graphs in overview dashboard.
    3 . Will create another PR to push these dashboard files to tidb-docker-compose repository.

tennix
tennix previously approved these changes Apr 14, 2019
Copy link
Member

@tennix tennix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@xiaojingchen
Copy link
Contributor

why not build an image which contains the config file, avoid to save config files to the configmap?

@tennix
Copy link
Member

tennix commented Apr 15, 2019

why not build an image which contains the config file, avoid to save config files to the configmap?

@xiaojingchen It's more convient to use configmap instead of a static docker image, especially when the dashboards needs to be updated.

xiaojingchen
xiaojingchen previously approved these changes Apr 15, 2019
- "/bin/sh"
- "-c"
- >
gzip -dc /tmp/dashboard-gz/tidb.json.gz > /grafana-dashboard-definitions/tidb/tidb.json;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use && instead of ;?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qiffang qiffang dismissed stale reviews from xiaojingchen and tennix via 07db5fd April 15, 2019 07:35
@qiffang qiffang force-pushed the use-grafana-provisioning branch from 4115cf1 to 07db5fd Compare April 15, 2019 07:35
Copy link
Contributor

@weekface weekface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@tennix tennix merged commit b5deab3 into pingcap:master Apr 15, 2019
yahonda pushed a commit that referenced this pull request Dec 27, 2021
* Move tidb-operator architecture section to a new page

* Apply suggestions from code review

Co-authored-by: Ran <huangran@pingcap.com>

* Apply suggestions from code review

Co-authored-by: Ran <huangran@pingcap.com>

Co-authored-by: Ran <huangran@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants