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 12 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 @@ -37,7 +37,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 @@ -442,8 +442,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 @@ -80,12 +80,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