skip destroy monitor when ignore_exporter is true by Smityz · Pull Request #2197 · pingcap/tiup

Smityz · 2023-05-06T11:10:36Z

when ignore_exporter: true is setting, tiup should skip destroying node exporter

Signed-off-by: Smityz <smityz@qq.com>

ti-chi-bot · 2023-05-06T11:10:37Z

[REVIEW NOTIFICATION]

This pull request has not been approved.

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

codecov-commenter · 2023-05-06T11:14:06Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.97%. Comparing base (600390f) to head (7b297a0).
⚠️ Report is 248 commits behind head on master.

⚠️ Current head 7b297a0 differs from pull request most recent head e73a281

Please upload reports for the commit e73a281 to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2197      +/-   ##
==========================================
- Coverage   56.14%   55.97%   -0.17%     
==========================================
  Files         320      320              
  Lines       33866    33866              
==========================================
- Hits        19014    18956      -58     
- Misses      12583    12641      +58     
  Partials     2269     2269

Flag	Coverage Δ
cluster	`44.83% <100.00%> (-0.18%)`	⬇️
dm	`25.58% <100.00%> (-0.05%)`	⬇️
playground	`14.30% <ø> (-0.02%)`	⬇️
tiup	`13.00% <ø> (-0.03%)`	⬇️
unittest	`22.59% <ø> (-<0.01%)`	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

nexustar · 2023-05-16T07:36:02Z

I suggest to move this if to "instCount[inst.GetManageHost()]--".So it will have the same behave like deploy.(If there are two instant on one machine, only one has "nomonitor" config, tiup cluster will deyloy and destroy monitor on this machine)

Smityz · 2023-05-17T09:27:14Z

I suggest to move this if to "instCount[inst.GetManageHost()]--".So it will have the same behave like deploy.(If there are two instant on one machine, only one has "nomonitor" config, tiup cluster will deyloy and destroy monitor on this machine)

updated

ti-chi-bot · 2023-08-13T13:26:53Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: srstack
Once this PR has been reviewed and has the lgtm label, please assign kaaaaaaang for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2023-08-13T13:27:14Z

[LGTM Timeline notifier]

Timeline:

2023-08-13 13:26:53.89523038 +0000 UTC m=+464778.444246366: ☑️ agreed by srstack.
2023-08-13 13:27:13.742076099 +0000 UTC m=+464798.291092085: ✖️🔁 reset by srstack.

ti-chi-bot · 2023-08-13T13:27:15Z

New changes are detected. LGTM label has been removed.

ti-chi-bot · 2024-02-03T08:33:39Z

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Adapt the final fix from pingcap#2197 to current master. The original implementation and review update are in commits 4e06282 and 8025e3a. Signed-off-by: Smityz <smityz@qq.com> Signed-off-by: Ziqian Qin <eke@fastmail.com>

* cluster: honor ignore_exporter when destroying a cluster Adapt the final fix from #2197 to current master. The original implementation and review update are in commits 4e06282 and 8025e3a. Signed-off-by: Smityz <smityz@qq.com> Signed-off-by: Ziqian Qin <eke@fastmail.com> * cluster: test exporter preservation during destroy Cover shared and managed exporters, multiple instances per host, and force destroy. Verify exporter directories, systemd units, and port checks while keeping cleanup of cluster-owned components intact. Signed-off-by: Ziqian Qin <eke@fastmail.com> --------- Signed-off-by: Smityz <smityz@qq.com> Signed-off-by: Ziqian Qin <eke@fastmail.com> Co-authored-by: Smityz <smityz@qq.com>

* cluster: fix Grafana VM datasource replacement (#2732) (cherry picked from commit 17b30ee) * cluster: scrape node_exporter on dedicated tidb-dashboard hosts (#2734) (cherry picked from commit c1c95d7) * ci: migrate to Bookworm and fix concurrent TLS CA copying (#2738) * ci: migrate integration test nodes to Debian Bookworm Remove the unused JRE 11 dependency, declare the systemd and process-management tools needed by node tests, and use systemctl is-enabled instead of distro-specific status output. Signed-off-by: Ziqian Qin <eke@fastmail.com> * ci: export cluster failure logs from the control container Copy collected node logs to the runner before detection and upload, and retain separate artifacts for each matrix case. This makes the runtime failures exposed by the Bookworm migration diagnosable. Signed-off-by: Ziqian Qin <eke@fastmail.com> * ci: bump Bookworm scale tests off TiDB v4.0.12 TiFlash v4.0.12 cannot load on glibc 2.36 (GLIBC_PRIVATE in libpthread). The v4.0.12 TLS start path also hits empty-CA transfers more readily. Use versions already green on this PR: v6.2.0 for tools, v6.0.0 for core TLS. Signed-off-by: Ziqian Qin <eke@fastmail.com> * cluster: copy TLS CA from a per-instance cache file Parallel TLSCert tasks shared cache/ca.crt and could SCP a truncated file (#2727). Native-SSH rename on this Bookworm PR hits that race. Give each instance its own CA cache path, matching key/cert files. Remote dest remains ca.crt. Signed-off-by: Ziqian Qin <eke@fastmail.com> --------- Signed-off-by: Ziqian Qin <eke@fastmail.com> (cherry picked from commit 6f9f329) * cluster: honor ignore_exporter when destroying a cluster (#2737) * cluster: honor ignore_exporter when destroying a cluster Adapt the final fix from #2197 to current master. The original implementation and review update are in commits 4e06282 and 8025e3a. Signed-off-by: Smityz <smityz@qq.com> Signed-off-by: Ziqian Qin <eke@fastmail.com> * cluster: test exporter preservation during destroy Cover shared and managed exporters, multiple instances per host, and force destroy. Verify exporter directories, systemd units, and port checks while keeping cleanup of cluster-owned components intact. Signed-off-by: Ziqian Qin <eke@fastmail.com> --------- Signed-off-by: Smityz <smityz@qq.com> Signed-off-by: Ziqian Qin <eke@fastmail.com> Co-authored-by: Smityz <smityz@qq.com> (cherry picked from commit c00f987) * release: bump version to v1.17.1 (cherry picked from commit f965165) * ci: rerun release checks after mirror publication --------- Signed-off-by: Ziqian Qin <eke@fastmail.com> Signed-off-by: Smityz <smityz@qq.com> Co-authored-by: Hangjie Mo <mohangjie1995@gmail.com> Co-authored-by: mayjiang0203 <mayjiang0203@users.noreply.github.com> Co-authored-by: Smityz <smityz@qq.com>

skip destroy monitor when ignore_exporter is true

4e06282

Signed-off-by: Smityz <smityz@qq.com>

ti-chi-bot Bot requested review from lucklove and srstack May 6, 2023 11:10

ti-chi-bot Bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 6, 2023

update

8025e3a

srstack approved these changes Aug 13, 2023

View reviewed changes

ti-chi-bot Bot added the lgtm label Aug 13, 2023

Merge branch 'master' into fix-1

e73a281

ti-chi-bot Bot removed the lgtm label Aug 13, 2023

ti-chi-bot Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2024

pdkajonpai-thai mentioned this pull request Sep 1, 2026

tiup cluster destroy ignores ignore_exporter and deletes a shared node_exporter's files before checking the port #2733

Closed

ekexium mentioned this pull request Sep 7, 2026

cluster: honor ignore_exporter when destroying a cluster #2737

Merged

3 tasks

Navigation Menu

Uh oh!

skip destroy monitor when ignore_exporter is true - #2197

skip destroy monitor when ignore_exporter is true#2197
Smityz wants to merge 3 commits into
pingcap:masterfrom
Smityz:fix-1

Smityz commented May 6, 2023 •

edited

Loading

Uh oh!

ti-chi-bot Bot commented May 6, 2023

Uh oh!

codecov-commenter commented May 6, 2023 •

edited

Loading

Uh oh!

nexustar commented May 16, 2023

Uh oh!

Smityz commented May 17, 2023

Uh oh!

ti-chi-bot Bot commented Aug 13, 2023

Uh oh!

ti-chi-bot Bot commented Aug 13, 2023

Uh oh!

ti-chi-bot Bot commented Aug 13, 2023

Uh oh!

ti-chi-bot Bot commented Feb 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

Smityz commented May 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ti-chi-bot Bot commented May 6, 2023

Uh oh!

codecov-commenter commented May 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nexustar commented May 16, 2023

Uh oh!

Smityz commented May 17, 2023

Uh oh!

ti-chi-bot Bot commented Aug 13, 2023

Uh oh!

ti-chi-bot Bot commented Aug 13, 2023

[LGTM Timeline notifier]

Uh oh!

ti-chi-bot Bot commented Aug 13, 2023

Uh oh!

ti-chi-bot Bot commented Feb 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Smityz commented May 6, 2023 •

edited

Loading

codecov-commenter commented May 6, 2023 •

edited

Loading