From 7529cb847a5dd2d870b3fc46829828f39e33e891 Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Tue, 29 Dec 2020 16:04:10 +0300 Subject: [PATCH 1/4] PMM-7246 Remove --search.disableCache flag --- models/settings.go | 3 ++- models/settings_helpers.go | 16 ---------------- services/supervisord/supervisord.go | 2 -- services/supervisord/supervisord_test.go | 1 - testdata/supervisord.d/victoriametrics.ini | 1 - utils/envvars/parser.go | 19 +++++++------------ 6 files changed, 9 insertions(+), 33 deletions(-) diff --git a/models/settings.go b/models/settings.go index daa0678df4..eae817cf6c 100644 --- a/models/settings.go +++ b/models/settings.go @@ -52,7 +52,8 @@ type Settings struct { AlertManagerURL string `json:"alert_manager_url"` VictoriaMetrics struct { - CacheEnabled bool `json:"cache_enabled"` + // Deprecated: does nothing now but kept to ensure that we don't reuse that JSON name. + DeprecatedCacheEnabled bool `json:"cache_enabled"` } `json:"victoria_metrics"` // Saas config options diff --git a/models/settings_helpers.go b/models/settings_helpers.go index 81769d73ba..4d26de305b 100644 --- a/models/settings_helpers.go +++ b/models/settings_helpers.go @@ -102,11 +102,6 @@ type ChangeSettingsParams struct { // LogOut user from Percona Platform, i.e. remove user email and session id LogOut bool - // EnableVMCache enables caching for vmdb search queries - EnableVMCache bool - // DisableVMCache disables caching for vmdb search queries - DisableVMCache bool - // PMM Server public address. PMMPublicAddress string RemovePMMPublicAddress bool @@ -207,14 +202,6 @@ func UpdateSettings(q reform.DBTX, params *ChangeSettingsParams) (*Settings, err settings.SaaS.Email = params.Email } - if params.DisableVMCache { - settings.VictoriaMetrics.CacheEnabled = false - } - - if params.EnableVMCache { - settings.VictoriaMetrics.CacheEnabled = true - } - if params.PMMPublicAddress != "" { settings.PMMPublicAddress = params.PMMPublicAddress } @@ -260,9 +247,6 @@ func ValidateSettings(params *ChangeSettingsParams) error { if params.EnableSTT && params.DisableSTT { return fmt.Errorf("Both enable_stt and disable_stt are present.") //nolint:golint,stylecheck } - if params.EnableVMCache && params.DisableVMCache { - return fmt.Errorf("Both enable_vm_cache and disable_vm_cache are present.") //nolint:golint,stylecheck - } if params.EnableAlerting && params.DisableAlerting { return fmt.Errorf("Both enable_alerting and disable_alerting are present.") //nolint:golint,stylecheck } diff --git a/services/supervisord/supervisord.go b/services/supervisord/supervisord.go index 6f291bb53b..2117dcc79d 100644 --- a/services/supervisord/supervisord.go +++ b/services/supervisord/supervisord.go @@ -405,7 +405,6 @@ func (s *Service) marshalConfig(tmpl *template.Template, settings *models.Settin "DataRetentionHours": int(settings.DataRetention.Hours()), "DataRetentionDays": int(settings.DataRetention.Hours() / 24), "VMAlertFlags": s.vmParams.VMAlertFlags, - "VMDBCacheDisable": !settings.VictoriaMetrics.CacheEnabled, "PerconaTestDbaas": settings.DBaaS.Enabled, } if err := addAlertManagerParams(settings.AlertManagerURL, templateParams); err != nil { @@ -576,7 +575,6 @@ command = --retentionPeriod={{ .DataRetentionDays }}d --storageDataPath=/srv/victoriametrics/data --httpListenAddr=127.0.0.1:9090 - --search.disableCache={{ .VMDBCacheDisable }} --search.maxQueryLen=72KB --search.maxUniqueTimeseries=1500000 --prometheusDataPath=/srv/prometheus/data diff --git a/services/supervisord/supervisord_test.go b/services/supervisord/supervisord_test.go index 5e709f3fdc..f2d21657b9 100644 --- a/services/supervisord/supervisord_test.go +++ b/services/supervisord/supervisord_test.go @@ -42,7 +42,6 @@ func TestConfig(t *testing.T) { DataRetention: 30 * 24 * time.Hour, AlertManagerURL: "https://external-user:passw!,ord@external-alertmanager:6443/alerts", } - settings.VictoriaMetrics.CacheEnabled = false for _, tmpl := range templates.Templates() { n := tmpl.Name() diff --git a/testdata/supervisord.d/victoriametrics.ini b/testdata/supervisord.d/victoriametrics.ini index 12db64942d..0c8b4e9ae4 100644 --- a/testdata/supervisord.d/victoriametrics.ini +++ b/testdata/supervisord.d/victoriametrics.ini @@ -8,7 +8,6 @@ command = --retentionPeriod=30d --storageDataPath=/srv/victoriametrics/data --httpListenAddr=127.0.0.1:9090 - --search.disableCache=true --search.maxQueryLen=72KB --search.maxUniqueTimeseries=1500000 --prometheusDataPath=/srv/prometheus/data diff --git a/utils/envvars/parser.go b/utils/envvars/parser.go index d93082f5f1..3bd6484821 100644 --- a/utils/envvars/parser.go +++ b/utils/envvars/parser.go @@ -106,27 +106,22 @@ func ParseEnvVars(envs []string) (envSettings *models.ChangeSettingsParams, errs if envSettings.DataRetention, err = parseStringDuration(v); err != nil { err = formatEnvVariableError(err, env, v) } - case "ENABLE_VM_CACHE": - envSettings.EnableVMCache, err = strconv.ParseBool(v) - if err != nil { - err = fmt.Errorf("invalid value %q for environment variable %q", v, k) - } - if !envSettings.EnableVMCache { - // disable cache explicitly - envSettings.DisableVMCache = true - } - case "PERCONA_TEST_IA": // FIXME remove - warns = append(warns, fmt.Sprintf("Environment variable %q WILL BE REMOVED SOON, please use %q instead.", k, "ENABLE_ALERTING")) - fallthrough case "ENABLE_ALERTING": envSettings.EnableAlerting, err = strconv.ParseBool(v) if err != nil { err = fmt.Errorf("invalid value %q for environment variable %q", v, k) } + // deprecated variables that produce warnings + case "ENABLE_VM_CACHE": // keep it forever since it does not have PERCONA_TEST_ prefix + warns = append(warns, fmt.Sprintf("environment variable %q does nothing now", k)) + + // removed variables that produce errors case "PERCONA_TEST_AUTH_HOST", "PERCONA_TEST_CHECKS_HOST", "PERCONA_TEST_TELEMETRY_HOST": err = fmt.Errorf("environment variable %q is removed and replaced by %q", k, envSaaSHost) + case "PERCONA_TEST_IA": + err = fmt.Errorf("environment variable %q is removed and replaced by %q", k, "ENABLE_ALERTING") default: // handle prefixes From 1205b5e7dddc3b3d097acfee8f6798dedda262ab Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Tue, 29 Dec 2020 16:07:31 +0300 Subject: [PATCH 2/4] PMM-7141 Add search.latencyOffset flag --- services/supervisord/supervisord.go | 17 +++++++++-------- testdata/supervisord.d/victoriametrics.ini | 17 +++++++++-------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/services/supervisord/supervisord.go b/services/supervisord/supervisord.go index 2117dcc79d..a105e1a2fc 100644 --- a/services/supervisord/supervisord.go +++ b/services/supervisord/supervisord.go @@ -571,14 +571,15 @@ redirect_stderr = true priority = 7 command = /usr/sbin/victoriametrics - --promscrape.config=/etc/victoriametrics-promscrape.yml - --retentionPeriod={{ .DataRetentionDays }}d - --storageDataPath=/srv/victoriametrics/data - --httpListenAddr=127.0.0.1:9090 - --search.maxQueryLen=72KB - --search.maxUniqueTimeseries=1500000 - --prometheusDataPath=/srv/prometheus/data - --http.pathPrefix=/prometheus + -http.pathPrefix=/prometheus + -httpListenAddr=127.0.0.1:9090 + -prometheusDataPath=/srv/prometheus/data + -promscrape.config=/etc/victoriametrics-promscrape.yml + -retentionPeriod={{ .DataRetentionDays }}d + -search.latencyOffset=5s + -search.maxQueryLen=72KB + -search.maxUniqueTimeseries=1500000 + -storageDataPath=/srv/victoriametrics/data user = pmm autorestart = true autostart = true diff --git a/testdata/supervisord.d/victoriametrics.ini b/testdata/supervisord.d/victoriametrics.ini index 0c8b4e9ae4..1ec4ba50db 100644 --- a/testdata/supervisord.d/victoriametrics.ini +++ b/testdata/supervisord.d/victoriametrics.ini @@ -4,14 +4,15 @@ priority = 7 command = /usr/sbin/victoriametrics - --promscrape.config=/etc/victoriametrics-promscrape.yml - --retentionPeriod=30d - --storageDataPath=/srv/victoriametrics/data - --httpListenAddr=127.0.0.1:9090 - --search.maxQueryLen=72KB - --search.maxUniqueTimeseries=1500000 - --prometheusDataPath=/srv/prometheus/data - --http.pathPrefix=/prometheus + -http.pathPrefix=/prometheus + -httpListenAddr=127.0.0.1:9090 + -prometheusDataPath=/srv/prometheus/data + -promscrape.config=/etc/victoriametrics-promscrape.yml + -retentionPeriod=30d + -search.latencyOffset=5s + -search.maxQueryLen=72KB + -search.maxUniqueTimeseries=1500000 + -storageDataPath=/srv/victoriametrics/data user = pmm autorestart = true autostart = true From 1d021ebe8244dbcc3cb3c5487c46e5493c0c99f2 Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Tue, 29 Dec 2020 16:34:25 +0300 Subject: [PATCH 3/4] PMM-7248 Tweak VMAlert flags --- models/victoriametrics.go | 4 ++-- models/victoriametrics_test.go | 2 +- services/supervisord/supervisord.go | 20 ++++++++++---------- testdata/supervisord.d/vmalert.ini | 20 ++++++++++---------- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/models/victoriametrics.go b/models/victoriametrics.go index ca9e59e028..ae5612fb1a 100644 --- a/models/victoriametrics.go +++ b/models/victoriametrics.go @@ -72,10 +72,10 @@ func (vmp *VictoriaMetricsParams) loadVMAlertParams() error { } vmalertFlags := make([]string, 0, len(vmp.VMAlertFlags)) for _, r := range cfg.RuleFiles { - vmalertFlags = append(vmalertFlags, "--rule="+r) + vmalertFlags = append(vmalertFlags, "-rule="+r) } if cfg.GlobalConfig.EvaluationInterval != 0 { - vmalertFlags = append(vmalertFlags, "--evaluationInterval="+cfg.GlobalConfig.EvaluationInterval.String()) + vmalertFlags = append(vmalertFlags, "-evaluationInterval="+cfg.GlobalConfig.EvaluationInterval.String()) } vmp.VMAlertFlags = vmalertFlags diff --git a/models/victoriametrics_test.go b/models/victoriametrics_test.go index 965d1dd2d4..41f926cb52 100644 --- a/models/victoriametrics_test.go +++ b/models/victoriametrics_test.go @@ -30,6 +30,6 @@ func TestVictoriaMetricsParams(t *testing.T) { t.Run("check params for VMAlert", func(t *testing.T) { vmp, err := NewVictoriaMetricsParams("../testdata/victoriametrics/prometheus.external.alerts.yml") require.NoError(t, err) - require.Equal(t, []string{"--rule=/srv/external_rules/rul1.yml", "--rule=/srv/external_rules/rule2.yml", "--evaluationInterval=10s"}, vmp.VMAlertFlags) + require.Equal(t, []string{"-rule=/srv/external_rules/rul1.yml", "-rule=/srv/external_rules/rule2.yml", "-evaluationInterval=10s"}, vmp.VMAlertFlags) }) } diff --git a/services/supervisord/supervisord.go b/services/supervisord/supervisord.go index a105e1a2fc..c0559895e3 100644 --- a/services/supervisord/supervisord.go +++ b/services/supervisord/supervisord.go @@ -598,16 +598,16 @@ redirect_stderr = true priority = 7 command = /usr/sbin/vmalert - --notifier.url="{{ .AlertmanagerURL }}" - --notifier.basicAuth.password='{{ .AlertManagerPassword }}' - --notifier.basicAuth.username="{{ .AlertManagerUser }}" - --external.url=http://localhost:9090/prometheus - --datasource.url=http://127.0.0.1:9090/prometheus - --remoteRead.url=http://127.0.0.1:9090/prometheus - --remoteWrite.url=http://127.0.0.1:9090/prometheus - --rule=/srv/prometheus/rules/*.yml - --rule=/etc/ia/rules/*.yml - --httpListenAddr=127.0.0.1:8880 + -datasource.url=http://127.0.0.1:9090/prometheus + -external.url=http://localhost/ + -httpListenAddr=127.0.0.1:8880 + -notifier.basicAuth.password='{{ .AlertManagerPassword }}' + -notifier.basicAuth.username="{{ .AlertManagerUser }}" + -notifier.url="{{ .AlertmanagerURL }}" + -remoteRead.url=http://127.0.0.1:9090/prometheus + -remoteWrite.url=http://127.0.0.1:9090/prometheus + -rule=/etc/ia/rules/*.yml + -rule=/srv/prometheus/rules/*.yml {{- range $index, $param := .VMAlertFlags }} {{ $param }} {{- end }} diff --git a/testdata/supervisord.d/vmalert.ini b/testdata/supervisord.d/vmalert.ini index 7fb914a565..b1444753b7 100644 --- a/testdata/supervisord.d/vmalert.ini +++ b/testdata/supervisord.d/vmalert.ini @@ -4,16 +4,16 @@ priority = 7 command = /usr/sbin/vmalert - --notifier.url="http://127.0.0.1:9093/alertmanager,https://external-alertmanager:6443/alerts" - --notifier.basicAuth.password=',"passw!,ord"' - --notifier.basicAuth.username=",external-user" - --external.url=http://localhost:9090/prometheus - --datasource.url=http://127.0.0.1:9090/prometheus - --remoteRead.url=http://127.0.0.1:9090/prometheus - --remoteWrite.url=http://127.0.0.1:9090/prometheus - --rule=/srv/prometheus/rules/*.yml - --rule=/etc/ia/rules/*.yml - --httpListenAddr=127.0.0.1:8880 + -datasource.url=http://127.0.0.1:9090/prometheus + -external.url=http://localhost/ + -httpListenAddr=127.0.0.1:8880 + -notifier.basicAuth.password=',"passw!,ord"' + -notifier.basicAuth.username=",external-user" + -notifier.url="http://127.0.0.1:9093/alertmanager,https://external-alertmanager:6443/alerts" + -remoteRead.url=http://127.0.0.1:9090/prometheus + -remoteWrite.url=http://127.0.0.1:9090/prometheus + -rule=/etc/ia/rules/*.yml + -rule=/srv/prometheus/rules/*.yml user = pmm autorestart = true autostart = true From 439a6a6910a4f34ade9eb1457a3fa542dc72ea55 Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Tue, 29 Dec 2020 16:34:37 +0300 Subject: [PATCH 4/4] PMM-7248 Add TODO --- models/victoriametrics.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/models/victoriametrics.go b/models/victoriametrics.go index ae5612fb1a..6414809c59 100644 --- a/models/victoriametrics.go +++ b/models/victoriametrics.go @@ -47,6 +47,9 @@ func NewVictoriaMetricsParams(basePath string) (*VictoriaMetricsParams, error) { // UpdateParams - reads configuration file and updates corresponding flags. func (vmp *VictoriaMetricsParams) UpdateParams() error { + // TODO read settings, set -external.url and -external.alert.source + // https://jira.percona.com/browse/PMM-7248 + if err := vmp.loadVMAlertParams(); err != nil { return errors.Wrap(err, "cannot update VMAlertFlags config param") }