Support Dashboard Metrics Storage Key #2483
Conversation
…b-operator into support_dashboard_key
|
/run-e2e-in-kind |
| if err != nil { | ||
| return err | ||
| } | ||
| return tcsm.syncDashboardMetricStorage(tc, tm) |
There was a problem hiding this comment.
Status manager should focus on the status update, can we move the etcd key operation to a separate function out of the status manager?
There was a problem hiding this comment.
Currently, i think it is not necessary as we only sync etcd key for MonitorRef. If there are more new actions for ectd operation, it will be ok for me to split it as an independent manager.
pkg/pdapi/pdetcd.go
Outdated
| ) | ||
|
|
||
| type PDEtcdApi interface { | ||
| type PDEtcdClient interface { | ||
| // PutKey would put key to the target pd etcd cluster |
There was a problem hiding this comment.
why use would here? At the end of the function, the put/delete operation may not finish?
There was a problem hiding this comment.
I think the comment is misleading, I should use will here.
pkg/pdapi/pdapi.go
Outdated
| @@ -72,6 +75,32 @@ func GetTLSConfig(kubeCli kubernetes.Interface, namespace Namespace, tcName stri | |||
| return crypto.LoadTlsConfigFromSecret(secret, caCert) | |||
| } | |||
|
|
|||
| func (pdc *defaultPDControl) GetPDEtcdClient(namespace Namespace, tcName string, tlsEnabled bool) (PDEtcdClient, error) { | |||
| pdc.mutex.Lock() | |||
There was a problem hiding this comment.
Suggest define separate mutex here, it's an independent client with existing PDClient.
|
/run-e2e-in-kind |
|
/merge |
|
Your auto merge job has been accepted, waiting for:
|
|
/run-all-tests |
|
@Yisaer merge failed. |
|
/run-e2e-tests |
1 similar comment
|
/run-e2e-tests |
|
/merge |
|
/run-all-tests |
|
/run-all-tests |
|
/run-all-tests |
|
@Yisaer merge failed. |
|
/run-cherry-picker |
Signed-off-by: sre-bot <sre-bot@pingcap.com>
|
cherry pick to release-1.1 in PR #2644 |
What problem does this PR solve?
close #2331
This request sync prometheus and grafana information into pd etcd with tidb nightly verison. I leave alertmanager into ucp. Here is the demo:
Does this PR introduce a user-facing change?: