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

Support use local file or directory as config for monitor components #712

Merged
merged 41 commits into from
Aug 28, 2020

Conversation

lucklove
Copy link
Member

@lucklove lucklove commented Aug 25, 2020

TODOLIST:

  • Support config alertmanager
  • Support config prometheus
  • Support config grafana
  • Add unit test
  • Add integration test

What problem does this PR solve?

  • Support config alertmanager
  • Support config prometheus rules
  • Support config grafana dashboards

What is changed and how it works?

Make it possible to specify additional file (or dir) path in topology or meta.yml. In deploy and reload stage, tiup will copy these things to remote.

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported variable/fields change
  • Has persistent data change

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation

Release notes:

Support config monitor components

@lucklove lucklove marked this pull request as draft August 25, 2020 07:05
Signed-off-by: lucklove <[email protected]>
Signed-off-by: lucklove <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Aug 25, 2020

Codecov Report

Merging #712 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #712      +/-   ##
==========================================
+ Coverage   57.97%   58.00%   +0.02%     
==========================================
  Files         253      253              
  Lines       18530    18660     +130     
==========================================
+ Hits        10743    10823      +80     
- Misses       6365     6397      +32     
- Partials     1422     1440      +18     
Flag Coverage Δ
#coverage 58.00% <ø> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
...p/tiup/pkg/cluster/template/config/alertmanager.go 64.70% <0.00%> (-8.63%) ⬇️
...ub.com/pingcap/tiup/pkg/cluster/spec/prometheus.go 73.87% <0.00%> (-4.70%) ⬇️
....com/pingcap/tiup/components/dm/spec/prometheus.go 69.42% <0.00%> (-3.31%) ⬇️
...thub.com/pingcap/tiup/pkg/cluster/spec/instance.go 71.79% <0.00%> (-3.21%) ⬇️
...hub.com/pingcap/tiup/components/dm/spec/grafana.go 63.01% <0.00%> (-1.79%) ⬇️
...hub.com/pingcap/tiup/pkg/cluster/executor/local.go 69.56% <0.00%> (-1.27%) ⬇️
...om/pingcap/tiup/components/dm/spec/alertmanager.go 68.91% <0.00%> (-0.65%) ⬇️
...com/pingcap/tiup/components/dm/spec/topology_dm.go 73.36% <0.00%> (-0.41%) ⬇️
....com/pingcap/tiup/pkg/cluster/spec/alertmanager.go 64.40% <0.00%> (ø)
...thub.com/pingcap/tiup/pkg/cluster/spec/validate.go 96.78% <0.00%> (+0.16%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4978e39...478d3a3. Read the comment docs.

@lucklove lucklove added status/PTAL type/new-feature Categorizes pr as related to a new feature. labels Aug 25, 2020
@lucklove lucklove added this to the v1.1.0 milestone Aug 25, 2020
@lucklove lucklove marked this pull request as ready for review August 25, 2020 12:46
Signed-off-by: lucklove <[email protected]>
Signed-off-by: lucklove <[email protected]>
Signed-off-by: lucklove <[email protected]>
Signed-off-by: lucklove <[email protected]>
Copy link
Contributor

@lonng lonng left a comment

Choose a reason for hiding this comment

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

Can those *.json can be generated dynamically?

@lucklove
Copy link
Member Author

Can those *.json can be generated dynamically?

No, they have too many things.
However, in tests, we can generate the simplest ones, but it's of no significance since we can just replace these with local file (which are simplified)

Signed-off-by: lucklove <[email protected]>
Signed-off-by: lucklove <[email protected]>
Signed-off-by: lucklove <[email protected]>
Signed-off-by: lucklove <[email protected]>
Signed-off-by: lucklove <[email protected]>
Signed-off-by: lucklove <[email protected]>
Signed-off-by: lucklove <[email protected]>
Signed-off-by: lucklove <[email protected]>
pkg/cluster/spec/validate.go Outdated Show resolved Hide resolved
Signed-off-by: lucklove <[email protected]>
Signed-off-by: lucklove <[email protected]>
pkg/cluster/spec/grafana.go Outdated Show resolved Hide resolved
Copy link
Contributor

@AstroProfundis AstroProfundis left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 27, 2020
@lucklove lucklove merged commit d3003bf into pingcap:master Aug 28, 2020
@lucklove lucklove deleted the config-file branch August 28, 2020 02:56
lucklove added a commit to lucklove/docs-cn that referenced this pull request Aug 31, 2020
TomShawn added a commit to pingcap/docs-cn that referenced this pull request Sep 21, 2020
* Document for pingcap/tiup#712

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

* More detail

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

* Apply suggestions from code review

Co-authored-by: Lonng <[email protected]>

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: TomShawn <[email protected]>

* Update tiup/tiup-cluster.md

Co-authored-by: Lonng <[email protected]>
Co-authored-by: TomShawn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT1 Indicates that a PR has LGTM 1. status/need-doc Indicates that we should update document before merge a PR. type/new-feature Categorizes pr as related to a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants