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

Bug Report

1. What did you do?

Two TiDB clusters share the same PD hosts. Cluster A was deployed first and owns node_exporter (9100) and blackbox_exporter (9115) on those hosts. Cluster B was deployed later with ignore_exporter: true on every PD instance so that it reuses cluster A's agents, which is the use case introduced by #1492.

Relevant part of cluster B's meta.yaml:

monitored:
  node_exporter_port: 9100
  blackbox_exporter_port: 9115
  deploy_dir: /data/tidb-deploy/monitor-9100
  data_dir: /data/tidb-data/monitor-9100
  log_dir: /data/tidb-deploy/monitor-9100/log
pd_servers:
  - host: tidbpd-hn1-dev
    ignore_exporter: true

Then ran:

tiup cluster destroy <cluster-B>

2. What did you expect to see?

destroy should skip the monitored agents on hosts whose instances are marked ignore_exporter: true, the same way scale-in, start and stop already do, and leave cluster A's exporters untouched.

3. What did you see instead?

Destroying monitored tidbpd-hn1-dev
tidbpd-hn1-dev failed to destroy node exporter: timed out waiting for port 9100 to be stopped after 1m0s

The failure is only the visible part. Before the port wait, DestroyMonitored() had already executed rm -rf on /data/tidb-deploy/monitor-9100, /data/tidb-data/monitor-9100, the log dir, /etc/systemd/system/node_exporter-9100.service and blackbox_exporter-9115.service. Cluster A's exporters keep running only as orphaned processes and disappear on the next reboot or service restart. Running destroy --force suppresses the error while still deleting the files.

Root cause (verified on master 17b30ee and tag v1.17.0):

pkg/cluster/operation/destroy.go, Destroy() lines 55 to 66, calls DestroyMonitored() for every host once its last instance is removed, with no IgnoreMonitorAgent() check:

if instCount[inst.GetManageHost()] == 0 {
    if cluster.GetMonitoredOptions() != nil {
        if err := DestroyMonitored(ctx, inst, cluster.GetMonitoredOptions(), ...

The scale-in path in the same file, StopAndDestroyInstance() line 139, does check it:

if monitoredOptions != nil && !instance.IgnoreMonitorAgent() {

as do Start() and Stop() in pkg/cluster/operation/action.go via noAgentHosts.

A one-line fix already exists in #2197 (&& !inst.IgnoreMonitorAgent() on that if), approved on 2023-08-13 but never rebased or merged. Reviewer feedback there suggested placing the check next to instCount[inst.GetManageHost()]-- so that a host with mixed ignore_exporter values across instances is handled consistently; that suggestion still applies.

Secondary issue: DestroyMonitored() deletes files before PortStopped(). Even with the guard added, the ordering means any future misconfiguration destroys a live agent before the check fails. Consider checking or stopping the service first, then deleting.

Related: ignore_exporter is not mentioned anywhere in the topology reference (https://docs.pingcap.com/tidb/stable/tiup-cluster-topology-reference/); a docs issue will be filed separately.

4. What version of TiUP are you using (tiup --version)?

Reproduced by code inspection on v1.17.0 and master 17b30ee; the customer's exact tiup --version output will be added once received.

Reporter context: PingCAP presales, on behalf of Kakao Corp DB Engineering team.