diff --git a/api-tests/management/ia/channels_test.go b/api-tests/management/ia/channels_test.go index 858d7970bc..16bc2e9b83 100644 --- a/api-tests/management/ia/channels_test.go +++ b/api-tests/management/ia/channels_test.go @@ -30,10 +30,6 @@ import ( pmmapitests "github.com/percona/pmm-managed/api-tests" ) -// Note: Even though the IA services check for alerting enabled or disabled before returning results -// we don't enable or disable IA explicit in our tests since it is enabled by default through -// ENABLE_ALERTING env var. - func TestChannelsAPI(t *testing.T) { client := channelsClient.Default.Channels diff --git a/api-tests/management/ia/rules_test.go b/api-tests/management/ia/rules_test.go index 951bc33669..1a1fcdd9d5 100644 --- a/api-tests/management/ia/rules_test.go +++ b/api-tests/management/ia/rules_test.go @@ -35,9 +35,6 @@ import ( pmmapitests "github.com/percona/pmm-managed/api-tests" ) -// Note: Even though the IA services check for alerting enabled or disabled before returning results -// we don't enable or disable IA explicit in our tests since it is enabled by default through -// ENABLE_ALERTING env var. func TestRulesAPI(t *testing.T) { rulesClient := client.Default.Rules templatesClient := client.Default.Templates diff --git a/api-tests/management/ia/templates_test.go b/api-tests/management/ia/templates_test.go index f8c458b405..bcd73f2c52 100644 --- a/api-tests/management/ia/templates_test.go +++ b/api-tests/management/ia/templates_test.go @@ -37,9 +37,6 @@ import ( pmmapitests "github.com/percona/pmm-managed/api-tests" ) -// Note: Even though the IA services check for alerting enabled or disabled before returning results -// we don't enable or disable IA explicit in our tests since it is enabled by default through -// ENABLE_ALERTING env var. func assertTemplate(t *testing.T, expectedTemplate alert.Template, listTemplates []*templates.TemplatesItems0) { convertParamUnit := func(u string) alert.Unit { switch u { diff --git a/api-tests/server/settings_test.go b/api-tests/server/settings_test.go index 0e928f2807..9fb5b406b7 100644 --- a/api-tests/server/settings_test.go +++ b/api-tests/server/settings_test.go @@ -126,7 +126,6 @@ func TestSettings(t *testing.T) { slackURL := gofakeit.URL() res, err := serverClient.Default.Server.ChangeSettings(&server.ChangeSettingsParams{ Body: server.ChangeSettingsBody{ - EnableAlerting: true, EmailAlertingSettings: &server.ChangeSettingsParamsBodyEmailAlertingSettings{ From: email, Smarthost: smarthost, @@ -153,23 +152,6 @@ func TestSettings(t *testing.T) { assert.Equal(t, slackURL, res.Payload.Settings.SlackAlertingSettings.URL) }) - t.Run("InvalidBothEnableAndDisableAlerting", func(t *testing.T) { - defer restoreSettingsDefaults(t) - - res, err := serverClient.Default.Server.ChangeSettings(&server.ChangeSettingsParams{ - Body: server.ChangeSettingsBody{ - // since alerting is already enabled on managed by default using - // ENABLE_ALERTING env var, just passing DisableAlerting param satisfies - // the condition of both enable and disable alerting being true - DisableAlerting: true, - }, - Context: pmmapitests.Context, - }) - pmmapitests.AssertAPIErrorf(t, err, 400, codes.FailedPrecondition, - `Alerting is enabled via ENABLE_ALERTING environment variable.`) - assert.Empty(t, res) - }) - t.Run("InvalidBothSlackAlertingSettingsAndRemoveSlackAlertingSettings", func(t *testing.T) { defer restoreSettingsDefaults(t) diff --git a/docker-compose.yml b/docker-compose.yml index 5d81ce1102..dca14a0e62 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -12,7 +12,6 @@ services: - ENABLE_DBAAS=${ENABLE_DBAAS:-0} - AWS_ACCESS_KEY=${AWS_ACCESS_KEY} - AWS_SECRET_KEY=${AWS_SECRET_KEY} - - ENABLE_ALERTING=1 - ENABLE_BACKUP_MANAGEMENT=1 # for delve diff --git a/go.mod b/go.mod index 422c2a319d..3504ce0057 100644 --- a/go.mod +++ b/go.mod @@ -35,7 +35,7 @@ require ( github.com/minio/minio-go/v7 v7.0.10 github.com/percona-platform/dbaas-api v0.0.0-20211201151251-014259873599 github.com/percona-platform/saas v0.0.0-20211101203847-f65c32bc8770 - github.com/percona/pmm v0.0.0-20220105120335-0b304c3d1574 + github.com/percona/pmm v0.0.0-20220106113046-71030a073065 github.com/percona/promconfig v0.2.4-0.20211110115058-98687f586f54 github.com/pkg/errors v0.9.1 github.com/pmezard/go-difflib v1.0.0 diff --git a/go.sum b/go.sum index 1b07d738ed..a43a988e30 100644 --- a/go.sum +++ b/go.sum @@ -594,8 +594,8 @@ github.com/percona-platform/dbaas-api v0.0.0-20211201151251-014259873599 h1:dDlc github.com/percona-platform/dbaas-api v0.0.0-20211201151251-014259873599/go.mod h1:zgb9gTJusc8Jv2zRNAlWV0/XQ8IK+hwHMc9lIqfK0tM= github.com/percona-platform/saas v0.0.0-20211101203847-f65c32bc8770 h1:ZfS8TnnctQn0ZMB2YV7Q46LbN9odamb/nsyBJWFNaRY= github.com/percona-platform/saas v0.0.0-20211101203847-f65c32bc8770/go.mod h1:jJRyGMxxDJaSiU7AaHNS+8j1TFQQhX6lcYp8s0t8Knc= -github.com/percona/pmm v0.0.0-20220105120335-0b304c3d1574 h1:VFcUDmqA0/hIFLB12lOdS9cLUlX8egaOVhyj/ql1Uaw= -github.com/percona/pmm v0.0.0-20220105120335-0b304c3d1574/go.mod h1:OmWayvQAavtvlzLkvpea5tAqaWGGNNyG+xj4MJUsNm4= +github.com/percona/pmm v0.0.0-20220106113046-71030a073065 h1:8Xr7oiEONNePTlVB7+zN4WAVKPI6KNqL+wiSC/McSTI= +github.com/percona/pmm v0.0.0-20220106113046-71030a073065/go.mod h1:OmWayvQAavtvlzLkvpea5tAqaWGGNNyG+xj4MJUsNm4= github.com/percona/promconfig v0.2.1/go.mod h1:Y2uXi5QNk71+ceJHuI9poank+0S1kjxd3K105fXKVkg= github.com/percona/promconfig v0.2.4-0.20211110115058-98687f586f54 h1:aI1emmycDTGWKsBdxFPKZqohfBbK4y2ta9G4+RX7gVg= github.com/percona/promconfig v0.2.4-0.20211110115058-98687f586f54/go.mod h1:Y2uXi5QNk71+ceJHuI9poank+0S1kjxd3K105fXKVkg= diff --git a/models/settings.go b/models/settings.go index 94b518f755..48e3aa91ce 100644 --- a/models/settings.go +++ b/models/settings.go @@ -43,7 +43,6 @@ type SaaS struct { // IntegratedAlerting contains settings related to IntegratedAlerting. type IntegratedAlerting struct { - Enabled bool `json:"enabled"` EmailAlertingSettings *EmailAlertingSettings `json:"email_settings"` SlackAlertingSettings *SlackAlertingSettings `json:"slack_settings"` } diff --git a/models/settings_helpers.go b/models/settings_helpers.go index ab3897180e..9c1965d594 100644 --- a/models/settings_helpers.go +++ b/models/settings_helpers.go @@ -87,11 +87,6 @@ type ChangeSettingsParams struct { // Disable Azure Discover features. DisableAzurediscover bool - // Enable Integrated Alerting features. - EnableAlerting bool - // Disable Integrated Alerting features. - DisableAlerting bool - // Email config for Integrated Alerting. EmailAlertingSettings *EmailAlertingSettings // If true removes email alerting settings. @@ -248,14 +243,6 @@ func UpdateSettings(q reform.DBTX, params *ChangeSettingsParams) (*Settings, err settings.Azurediscover.Enabled = true } - if params.DisableAlerting { - settings.IntegratedAlerting.Enabled = false - } - - if params.EnableAlerting { - settings.IntegratedAlerting.Enabled = true - } - if params.RemoveEmailAlertingSettings { settings.IntegratedAlerting.EmailAlertingSettings = nil } @@ -328,9 +315,6 @@ func ValidateSettings(params *ChangeSettingsParams) error { if params.EnableVMCache && params.DisableVMCache { return errors.New("both enable_vm_cache and disable_vm_cache are present") } - if params.EnableAlerting && params.DisableAlerting { - return errors.New("both enable_alerting and disable_alerting are present") - } if err := validateEmailAlertingSettings(params); err != nil { return err } diff --git a/models/settings_helpers_test.go b/models/settings_helpers_test.go index 6f42c1e3bf..7743060589 100644 --- a/models/settings_helpers_test.go +++ b/models/settings_helpers_test.go @@ -342,19 +342,16 @@ func TestSettings(t *testing.T) { } slackSettings := &models.SlackAlertingSettings{URL: gofakeit.URL()} ns, err := models.UpdateSettings(sqlDB, &models.ChangeSettingsParams{ - EnableAlerting: true, EmailAlertingSettings: emailSettings, SlackAlertingSettings: slackSettings, }) require.NoError(t, err) - assert.True(t, ns.IntegratedAlerting.Enabled) assert.Equal(t, ns.IntegratedAlerting.EmailAlertingSettings, emailSettings) assert.Equal(t, ns.IntegratedAlerting.SlackAlertingSettings, slackSettings) // check that we don't lose settings on empty updates ns, err = models.UpdateSettings(sqlDB, &models.ChangeSettingsParams{}) require.NoError(t, err) - assert.True(t, ns.IntegratedAlerting.Enabled) assert.Equal(t, ns.IntegratedAlerting.EmailAlertingSettings, emailSettings) assert.Equal(t, ns.IntegratedAlerting.SlackAlertingSettings, slackSettings) @@ -411,20 +408,11 @@ func TestSettings(t *testing.T) { assert.EqualError(t, err, "invalid argument: invalid url value") ns, err = models.UpdateSettings(sqlDB, &models.ChangeSettingsParams{ - DisableAlerting: true, RemoveEmailAlertingSettings: true, RemoveSlackAlertingSettings: true, }) require.NoError(t, err) assert.Empty(t, ns.IntegratedAlerting.EmailAlertingSettings) - assert.False(t, ns.IntegratedAlerting.Enabled) - - _, err = models.UpdateSettings(sqlDB, &models.ChangeSettingsParams{ - DisableAlerting: true, - EnableAlerting: true, - }) - assert.True(t, errors.As(err, &errInvalidArgument)) - assert.EqualError(t, err, "invalid argument: both enable_alerting and disable_alerting are present") }) }) diff --git a/services/management/ia/alerts_service.go b/services/management/ia/alerts_service.go index eec3b42dd6..745ef6421e 100644 --- a/services/management/ia/alerts_service.go +++ b/services/management/ia/alerts_service.go @@ -57,12 +57,7 @@ func NewAlertsService(db *reform.DB, alertManager alertManager, templatesService // Enabled returns if service is enabled and can be used. func (s *AlertsService) Enabled() bool { - settings, err := models.GetSettings(s.db) - if err != nil { - s.l.WithError(err).Error("can't get settings") - return false - } - return settings.IntegratedAlerting.Enabled + return true } // ListAlerts returns list of existing alerts. diff --git a/services/management/ia/channels_service.go b/services/management/ia/channels_service.go index 2994b684fd..3eb269a417 100644 --- a/services/management/ia/channels_service.go +++ b/services/management/ia/channels_service.go @@ -45,12 +45,7 @@ func NewChannelsService(db *reform.DB, alertManager alertManager) *ChannelsServi // Enabled returns if service is enabled and can be used. func (s *ChannelsService) Enabled() bool { - settings, err := models.GetSettings(s.db) - if err != nil { - s.l.WithError(err).Error("can't get settings") - return false - } - return settings.IntegratedAlerting.Enabled + return true } // ListChannels returns list of available channels. diff --git a/services/management/ia/rules_service.go b/services/management/ia/rules_service.go index f608870699..c493cf4f49 100644 --- a/services/management/ia/rules_service.go +++ b/services/management/ia/rules_service.go @@ -81,12 +81,7 @@ func NewRulesService(db *reform.DB, templates *TemplatesService, vmalert vmAlert // Enabled returns if service is enabled and can be used. func (s *RulesService) Enabled() bool { - settings, err := models.GetSettings(s.db) - if err != nil { - s.l.WithError(err).Error("can't get settings") - return false - } - return settings.IntegratedAlerting.Enabled + return true } // TODO Move this and related types to https://github.com/percona/promconfig diff --git a/services/management/ia/rules_service_test.go b/services/management/ia/rules_service_test.go index cf5d222ede..0b01d834c9 100644 --- a/services/management/ia/rules_service_test.go +++ b/services/management/ia/rules_service_test.go @@ -46,13 +46,6 @@ func TestCreateAlertRule(t *testing.T) { sqlDB := testdb.Open(t, models.SkipFixtures, nil) db := reform.NewDB(sqlDB, postgresql.Dialect, reform.NewPrintfLogger(t.Logf)) - // Enable IA - settings, err := models.GetSettings(db) - require.NoError(t, err) - settings.IntegratedAlerting.Enabled = true - err = models.SaveSettings(db, settings) - require.NoError(t, err) - alertManager := new(mockAlertManager) alertManager.On("RequestConfigurationUpdate").Return() vmAlert := new(mockVmAlert) diff --git a/services/management/ia/templates_service.go b/services/management/ia/templates_service.go index ed72050c92..8729aafa22 100644 --- a/services/management/ia/templates_service.go +++ b/services/management/ia/templates_service.go @@ -112,12 +112,7 @@ func NewTemplatesService(db *reform.DB) (*TemplatesService, error) { // Enabled returns if service is enabled and can be used. func (s *TemplatesService) Enabled() bool { - settings, err := models.GetSettings(s.db) - if err != nil { - s.l.WithError(err).Error("can't get settings") - return false - } - return settings.IntegratedAlerting.Enabled + return true } func newParamTemplate() *template.Template { diff --git a/services/management/ia/templates_service_test.go b/services/management/ia/templates_service_test.go index 71230161b9..d2e510b512 100644 --- a/services/management/ia/templates_service_test.go +++ b/services/management/ia/templates_service_test.go @@ -108,13 +108,6 @@ func TestTemplateValidation(t *testing.T) { sqlDB := testdb.Open(t, models.SkipFixtures, nil) db := reform.NewDB(sqlDB, postgresql.Dialect, reform.NewPrintfLogger(t.Logf)) - // Enable IA - settings, err := models.GetSettings(db) - require.NoError(t, err) - settings.IntegratedAlerting.Enabled = true - err = models.SaveSettings(db, settings) - require.NoError(t, err) - t.Run("create a template with missing param", func(t *testing.T) { t.Parallel() diff --git a/services/server/server.go b/services/server/server.go index ba24e33f76..cfd4a1ee8c 100644 --- a/services/server/server.go +++ b/services/server/server.go @@ -448,7 +448,7 @@ func (s *Server) convertSettings(settings *models.Settings, connectedToPlatform AzurediscoverEnabled: settings.Azurediscover.Enabled, PmmPublicAddress: settings.PMMPublicAddress, - AlertingEnabled: settings.IntegratedAlerting.Enabled, + AlertingEnabled: true, BackupManagementEnabled: settings.BackupManagement.Enabled, ConnectedToPlatform: connectedToPlatform, @@ -531,11 +531,6 @@ func (s *Server) validateChangeSettingsRequest(ctx context.Context, req *serverp return status.Error(codes.FailedPrecondition, "Telemetry is disabled via DISABLE_TELEMETRY environment variable.") } - // ignore req.EnableAlerting even if they are present since that will not change anything - if req.DisableAlerting && s.envSettings.EnableAlerting { - return status.Error(codes.FailedPrecondition, "Alerting is enabled via ENABLE_ALERTING environment variable.") - } - // ignore req.DisableAzurediscover even if they are present since that will not change anything if req.DisableAzurediscover && s.envSettings.EnableAzurediscover { return status.Error(codes.FailedPrecondition, "Azure Discover is enabled via ENABLE_AZUREDISCOVER environment variable.") @@ -608,8 +603,6 @@ func (s *Server) ChangeSettings(ctx context.Context, req *serverpb.ChangeSetting PMMPublicAddress: req.PmmPublicAddress, RemovePMMPublicAddress: req.RemovePmmPublicAddress, - EnableAlerting: req.EnableAlerting, - DisableAlerting: req.DisableAlerting, RemoveEmailAlertingSettings: req.RemoveEmailAlertingSettings, RemoveSlackAlertingSettings: req.RemoveSlackAlertingSettings, EnableBackupManagement: req.EnableBackupManagement, @@ -678,18 +671,6 @@ func (s *Server) ChangeSettings(ctx context.Context, req *serverpb.ChangeSetting return nil, err } - // When IA moved from disabled state to enabled create rules files. - if !oldSettings.IntegratedAlerting.Enabled && req.EnableAlerting { - s.rulesService.WriteVMAlertRulesFiles() - } - - // When IA moved from enabled state to disables cleanup rules files. - if oldSettings.IntegratedAlerting.Enabled && req.DisableAlerting { - if err := s.rulesService.RemoveVMAlertRulesFiles(); err != nil { - s.l.Errorf("Failed to clean old alert rule files: %+v", err) - } - } - // If STT intervals are changed reset timers. if oldSettings.SaaS.STTCheckIntervals != newSettings.SaaS.STTCheckIntervals { s.checksService.UpdateIntervals( diff --git a/services/server/server_test.go b/services/server/server_test.go index 79fbc73d92..30e8fb4b12 100644 --- a/services/server/server_test.go +++ b/services/server/server_test.go @@ -222,7 +222,6 @@ func TestServer(t *testing.T) { server.UpdateSettingsFromEnv([]string{ "ENABLE_DBAAS=1", - "ENABLE_ALERTING=1", "ENABLE_AZUREDISCOVER=1", }) @@ -238,39 +237,9 @@ func TestServer(t *testing.T) { require.NoError(t, err) assert.True(t, settings.Settings.DbaasEnabled) - assert.True(t, settings.Settings.AlertingEnabled) + assert.True(t, settings.Settings.AlertingEnabled) //nolint:staticcheck assert.True(t, settings.Settings.AzurediscoverEnabled) }) - - t.Run("ChangeSettings IA", func(t *testing.T) { - server := newServer(t) - rs := new(mockRulesService) - server.rulesService = rs - server.UpdateSettingsFromEnv([]string{}) - - ctx := context.TODO() - rs.On("RemoveVMAlertRulesFiles").Return(nil) - defer rs.AssertExpectations(t) - s, err := server.ChangeSettings(ctx, &serverpb.ChangeSettingsRequest{ - DisableAlerting: true, - }) - require.NoError(t, err) - require.NotNil(t, s) - - rs.On("WriteVMAlertRulesFiles") - s, err = server.ChangeSettings(ctx, &serverpb.ChangeSettingsRequest{ - EnableAlerting: true, - }) - require.NoError(t, err) - require.NotNil(t, s) - - rs.On("RemoveVMAlertRulesFiles").Return(nil) - s, err = server.ChangeSettings(ctx, &serverpb.ChangeSettingsRequest{ - DisableAlerting: true, - }) - require.NoError(t, err) - require.NotNil(t, s) - }) } func TestServer_TestEmailAlertingSettings(t *testing.T) { diff --git a/services/telemetry/telemetry.go b/services/telemetry/telemetry.go index f96dab3d5d..da45bff3fb 100644 --- a/services/telemetry/telemetry.go +++ b/services/telemetry/telemetry.go @@ -275,7 +275,7 @@ func (s *Service) makeV2Payload(serverUUID string, settings *models.Settings) (* UpDuration: durationpb.New(time.Since(s.start)), DistributionMethod: s.tDistributionMethod, SttEnabled: wrapperspb.Bool(settings.SaaS.STTEnabled), - IaEnabled: wrapperspb.Bool(settings.IntegratedAlerting.Enabled), + IaEnabled: wrapperspb.Bool(true), } if err = event.Validate(); err != nil { diff --git a/services/telemetry/telemetry_test.go b/services/telemetry/telemetry_test.go index 397621ec80..ca33069792 100644 --- a/services/telemetry/telemetry_test.go +++ b/services/telemetry/telemetry_test.go @@ -80,9 +80,7 @@ func TestMakeV2Payload(t *testing.T) { SaaS: models.SaaS{ STTEnabled: true, }, - IntegratedAlerting: models.IntegratedAlerting{ - Enabled: false, - }, + IntegratedAlerting: models.IntegratedAlerting{}, }) require.NoError(t, err) assert.NoError(t, r.Validate()) @@ -101,7 +99,7 @@ func TestMakeV2Payload(t *testing.T) { assert.GreaterOrEqual(t, float64(uEv.UpDuration.Seconds), delay.Seconds()) assert.Equal(t, u, hex.EncodeToString(uEv.Id)) assert.Equal(t, wrapperspb.Bool(true), uEv.SttEnabled) - assert.Equal(t, wrapperspb.Bool(false), uEv.IaEnabled) + assert.Equal(t, wrapperspb.Bool(true), uEv.IaEnabled) } func TestSendV2Request(t *testing.T) { @@ -133,9 +131,7 @@ func TestSendV2Request(t *testing.T) { SaaS: models.SaaS{ STTEnabled: true, }, - IntegratedAlerting: models.IntegratedAlerting{ - Enabled: false, - }, + IntegratedAlerting: models.IntegratedAlerting{}, }) require.NoError(t, err) @@ -157,9 +153,7 @@ func TestSendV2Request(t *testing.T) { SaaS: models.SaaS{ STTEnabled: true, }, - IntegratedAlerting: models.IntegratedAlerting{ - Enabled: false, - }, + IntegratedAlerting: models.IntegratedAlerting{}, }) require.NoError(t, err) diff --git a/utils/envvars/parser.go b/utils/envvars/parser.go index b3549f5233..5757536295 100644 --- a/utils/envvars/parser.go +++ b/utils/envvars/parser.go @@ -59,7 +59,6 @@ func (e InvalidDurationError) Error() string { return string(e) } // - DISABLE_TELEMETRY is a boolean flag to enable or disable pmm telemetry (and disable STT if telemetry is disabled); // - METRICS_RESOLUTION, METRICS_RESOLUTION, METRICS_RESOLUTION_HR, METRICS_RESOLUTION_LR are durations of metrics resolution; // - DATA_RETENTION is the duration of how long keep time-series data in ClickHouse; -// - ENABLE_ALERTING enables Integrated Alerting; // - ENABLE_AZUREDISCOVER enables Azure Discover; // - ENABLE_DBAAS enables Database as a Service feature, it's a replacement for deprecated PERCONA_TEST_DBAAS which still works but will be removed eventually; // - the environment variables prefixed with GF_ passed as related to Grafana. @@ -123,11 +122,6 @@ func ParseEnvVars(envs []string) (envSettings *models.ChangeSettingsParams, errs // disable cache explicitly envSettings.DisableVMCache = true } - case "ENABLE_ALERTING": - envSettings.EnableAlerting, err = strconv.ParseBool(v) - if err != nil { - err = fmt.Errorf("invalid value %q for environment variable %q", v, k) - } case "ENABLE_AZUREDISCOVER": envSettings.EnableAzurediscover, err = strconv.ParseBool(v) if err != nil {