Skip to content
This repository was archived by the owner on Jun 21, 2022. It is now read-only.
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions api-tests/management/ia/channels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,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 TestAddChannel(t *testing.T) {
client := channelsClient.Default.Channels

Expand Down
3 changes: 0 additions & 3 deletions api-tests/management/ia/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,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
channelsClient := client.Default.Channels
Expand Down
3 changes: 0 additions & 3 deletions api-tests/management/ia/templates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,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 {
Expand Down
17 changes: 0 additions & 17 deletions api-tests/server/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,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,
Expand All @@ -154,22 +153,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)

Expand Down
1 change: 0 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,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
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ require (
github.com/minio/minio-go/v7 v7.0.10
github.com/percona-platform/dbaas-api v0.0.0-20211006103141-a68316e565d2
github.com/percona-platform/saas v0.0.0-20210505183502-c18b6f47c932
github.com/percona/pmm v0.0.0-20211006103121-6dee93c2029b
github.com/percona/pmm v0.0.0-20211012222805-ca69392f29d6
github.com/percona/promconfig v0.2.1
github.com/pkg/errors v0.9.1
github.com/pmezard/go-difflib v1.0.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -440,8 +440,8 @@ github.com/percona-platform/dbaas-api v0.0.0-20211006103141-a68316e565d2 h1:TJX3
github.com/percona-platform/dbaas-api v0.0.0-20211006103141-a68316e565d2/go.mod h1:zgb9gTJusc8Jv2zRNAlWV0/XQ8IK+hwHMc9lIqfK0tM=
github.com/percona-platform/saas v0.0.0-20210505183502-c18b6f47c932 h1:EDe5KfOdEoEBKg33BsFsMrdazZu8vaRj4fTKdg4zjCc=
github.com/percona-platform/saas v0.0.0-20210505183502-c18b6f47c932/go.mod h1:jJRyGMxxDJaSiU7AaHNS+8j1TFQQhX6lcYp8s0t8Knc=
github.com/percona/pmm v0.0.0-20211006103121-6dee93c2029b h1:75eCD67rQr+JlyJAENQ68ZCUcQQXjBj/jRuaNizsrvg=
github.com/percona/pmm v0.0.0-20211006103121-6dee93c2029b/go.mod h1:OmWayvQAavtvlzLkvpea5tAqaWGGNNyG+xj4MJUsNm4=
github.com/percona/pmm v0.0.0-20211012222805-ca69392f29d6 h1:8UFKAU+qQwVaydV0g+j9yW62qjjxqb1D+Ml1Zb+3PgU=
github.com/percona/pmm v0.0.0-20211012222805-ca69392f29d6/go.mod h1:OmWayvQAavtvlzLkvpea5tAqaWGGNNyG+xj4MJUsNm4=
github.com/percona/promconfig v0.2.1 h1:LBbCDSQRfy0aTHFJMgrVQIE2WvmPkMTkIoznTfBAvj8=
github.com/percona/promconfig v0.2.1/go.mod h1:Y2uXi5QNk71+ceJHuI9poank+0S1kjxd3K105fXKVkg=
github.com/performancecopilot/speed v3.0.0+incompatible/go.mod h1:/CLtqpZ5gBg1M9iaPbIdPPGyKcA8hKdoy6hAWba7Yac=
Expand Down
1 change: 0 additions & 1 deletion models/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,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"`
}
Expand Down
16 changes: 0 additions & 16 deletions models/settings_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -259,14 +254,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
}
Expand Down Expand Up @@ -311,9 +298,6 @@ func ValidateSettings(params *ChangeSettingsParams) error {
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
}
if params.EnableBackupManagement && params.DisableBackupManagement {
return fmt.Errorf("Both enable_backup_management and disable_backup_management are present.") //nolint:golint,stylecheck
}
Expand Down
33 changes: 11 additions & 22 deletions models/settings_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,14 @@ func TestSettings(t *testing.T) {
AWSPartitions: []string{"aws", "aws-cn", "aws-cn"},
}
settings, err := models.UpdateSettings(sqlDB, s)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, []string{"aws", "aws-cn"}, settings.AWSPartitions)

s = &models.ChangeSettingsParams{
AWSPartitions: []string{},
}
settings, err = models.UpdateSettings(sqlDB, s)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, []string{"aws", "aws-cn"}, settings.AWSPartitions)

settings = &models.Settings{AWSPartitions: []string{}}
Expand Down Expand Up @@ -173,7 +173,7 @@ func TestSettings(t *testing.T) {
ns, err := models.UpdateSettings(sqlDB, &models.ChangeSettingsParams{
DisableUpdates: false,
})
assert.NoError(t, err)
require.NoError(t, err)
assert.False(t, ns.Updates.Disabled)

_, err = models.UpdateSettings(sqlDB, &models.ChangeSettingsParams{
Expand All @@ -185,7 +185,7 @@ func TestSettings(t *testing.T) {
ns, err = models.UpdateSettings(sqlDB, &models.ChangeSettingsParams{
DisableUpdates: true,
})
assert.NoError(t, err)
require.NoError(t, err)
assert.True(t, ns.Updates.Disabled)
})

Expand All @@ -195,7 +195,7 @@ func TestSettings(t *testing.T) {
EnableTelemetry: true,
DisableSTT: true,
})
assert.NoError(t, err)
require.NoError(t, err)
assert.False(t, ns.Telemetry.Disabled)
assert.False(t, ns.SaaS.STTEnabled)

Expand All @@ -222,7 +222,7 @@ func TestSettings(t *testing.T) {
EnableSTT: true,
EnableTelemetry: true,
})
assert.NoError(t, err)
require.NoError(t, err)
assert.False(t, ns.Telemetry.Disabled)
assert.True(t, ns.SaaS.STTEnabled)

Expand All @@ -236,7 +236,7 @@ func TestSettings(t *testing.T) {
DisableSTT: true,
DisableTelemetry: true,
})
assert.NoError(t, err)
require.NoError(t, err)
assert.True(t, ns.Telemetry.Disabled)
assert.False(t, ns.SaaS.STTEnabled)

Expand All @@ -250,7 +250,7 @@ func TestSettings(t *testing.T) {
EnableTelemetry: true,
DisableSTT: true,
})
assert.NoError(t, err)
require.NoError(t, err)
assert.False(t, ns.Telemetry.Disabled)
assert.False(t, ns.SaaS.STTEnabled)
})
Expand Down Expand Up @@ -362,19 +362,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,
})
assert.NoError(t, err)
assert.True(t, ns.IntegratedAlerting.Enabled)
require.NoError(t, err)
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{})
assert.NoError(t, err)
assert.True(t, ns.IntegratedAlerting.Enabled)
require.NoError(t, err)
assert.Equal(t, ns.IntegratedAlerting.EmailAlertingSettings, emailSettings)
assert.Equal(t, ns.IntegratedAlerting.SlackAlertingSettings, slackSettings)

Expand All @@ -391,19 +388,11 @@ func TestSettings(t *testing.T) {
assert.EqualError(t, err, "Both slack_alerting_settings and remove_slack_alerting_settings are present.")

ns, err = models.UpdateSettings(sqlDB, &models.ChangeSettingsParams{
DisableAlerting: true,
RemoveEmailAlertingSettings: true,
RemoveSlackAlertingSettings: true,
})
assert.NoError(t, err)
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.EqualError(t, err, "Both enable_alerting and disable_alerting are present.")
})
})
}
3 changes: 0 additions & 3 deletions services/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,3 @@ import (

// ErrSTTDisabled means that STT checks are disabled and can't be executed.
var ErrSTTDisabled = errors.New("STT is disabled")

// ErrAlertingDisabled means Integrated Alerting is disabled and IA APIs can't be executed.
var ErrAlertingDisabled = errors.New("Alerting is disabled")
7 changes: 1 addition & 6 deletions services/management/ia/alerts_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 1 addition & 6 deletions services/management/ia/channels_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Comment thread
oter marked this conversation as resolved.
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.
Expand Down
7 changes: 1 addition & 6 deletions services/management/ia/rules_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,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 {
Comment thread
oter marked this conversation as resolved.
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
Expand Down
7 changes: 0 additions & 7 deletions services/management/ia/rules_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 1 addition & 6 deletions services/management/ia/templates_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,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 {
Expand Down
7 changes: 0 additions & 7 deletions services/management/ia/templates_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,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()

Expand Down
21 changes: 1 addition & 20 deletions services/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ func (s *Server) convertSettings(settings *models.Settings) *serverpb.Settings {
AzurediscoverEnabled: settings.Azurediscover.Enabled,
PmmPublicAddress: settings.PMMPublicAddress,

AlertingEnabled: settings.IntegratedAlerting.Enabled,
AlertingEnabled: true,
BackupManagementEnabled: settings.BackupManagement.Enabled,
}

Expand Down Expand Up @@ -532,11 +532,6 @@ func (s *Server) validateChangeSettingsRequest(ctx context.Context, req *serverp
return status.Error(codes.FailedPrecondition, "STT cannot be enabled because 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.")
Expand Down Expand Up @@ -610,8 +605,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,
Expand Down Expand Up @@ -673,18 +666,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(
Expand Down
Loading