Skip to content
This repository was archived by the owner on Jun 21, 2022. It is now read-only.

PMM-8459 Ability to disable updates. #808

Merged
JiriCtvrtka merged 22 commits into from
Aug 30, 2021
Merged

PMM-8459 Ability to disable updates. #808

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5da8245
PMM-8459 Update changes.
Jul 28, 2021
6402f10
PMM-8459 Fix test.
Jul 28, 2021
a528f68
PMM-8459 Fix.
Jul 28, 2021
7ff72c8
PMM-8459 Fix test.
Jul 28, 2021
a8eed3e
PMM-8459 Add another case.
Jul 28, 2021
5905f8b
PMM-8459 Required changes.
Jul 28, 2021
c1a7f06
PMM-8459 Required changes.
Jul 28, 2021
c8937e9
PMM-8459 Changes.
Jul 28, 2021
1fa4a8e
PMM-8459 Change.
Jul 28, 2021
67a6e24
PMM-8459 Changes.
Jul 28, 2021
9c03996
PMM-8459 Fix test.
Jul 28, 2021
4f4fe05
Merge branch 'PMM-2.0' into PMM-8459-disable-updates
Jul 29, 2021
c54872c
Merge branch 'PMM-2.0' into PMM-8459-disable-updates
Aug 3, 2021
5fbdea7
PMM-8459 Mod.
Aug 3, 2021
5125900
Merge branch 'PMM-2.0' into PMM-8459-disable-updates
Aug 26, 2021
a2c22d4
PMM-8459 Mod.
Aug 26, 2021
29da0d6
PMM-8459 Mod.
Aug 26, 2021
4890350
PMM-8459 Move api tests to managed.
Aug 27, 2021
bb04e5f
Merge branch 'PMM-2.0' into PMM-8459-disable-updates
Aug 27, 2021
1abb795
PMM-8459 Mod.
Aug 27, 2021
f5211db
PMM-8459 Fix test.
Aug 27, 2021
99699b7
PMM-8459 Mod.
Aug 30, 2021
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
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ require (
github.com/minio/minio-go/v7 v7.0.10
github.com/percona-platform/dbaas-api v0.0.0-20210726192627-670b3725d0ac
github.com/percona-platform/saas v0.0.0-20210628125953-5bf84e6eefa0
github.com/percona/pmm v0.0.0-20210727064651-c6dbbddd9850
github.com/percona/pmm v0.0.0-20210728065534-7a7bfe95ccd9
github.com/percona/promconfig v0.2.1
github.com/pkg/errors v0.9.1
github.com/pmezard/go-difflib v1.0.0
Expand Down
6 changes: 6 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,12 @@ github.com/percona/pmm v0.0.0-20210721092629-8d0bba52fe8a h1:YpPgxdSwBeBmz35DiOL
github.com/percona/pmm v0.0.0-20210721092629-8d0bba52fe8a/go.mod h1:Cm2JKvJMlMimtAhmF/1BUvz3qVJZ2O2zxQXRvtQh93Q=
github.com/percona/pmm v0.0.0-20210727064651-c6dbbddd9850 h1:sV3uHzOKjZ1OgAi/6Eiq5Il9lJGNCAO4QTacPDG2h68=
github.com/percona/pmm v0.0.0-20210727064651-c6dbbddd9850/go.mod h1:Cm2JKvJMlMimtAhmF/1BUvz3qVJZ2O2zxQXRvtQh93Q=
github.com/percona/pmm v0.0.0-20210728063029-294734c0726e h1:p/DeRRPr1xFYZikY5c1yIc8ul0UcJjx9L2BRtxQ/vag=
github.com/percona/pmm v0.0.0-20210728063029-294734c0726e/go.mod h1:Cm2JKvJMlMimtAhmF/1BUvz3qVJZ2O2zxQXRvtQh93Q=
github.com/percona/pmm v0.0.0-20210728064606-5e0a50d0b1a2 h1:k0XDHyfjYQ9rfUN7tSHil3tnFg3dssy7x2El+9XkO18=
github.com/percona/pmm v0.0.0-20210728064606-5e0a50d0b1a2/go.mod h1:Cm2JKvJMlMimtAhmF/1BUvz3qVJZ2O2zxQXRvtQh93Q=
github.com/percona/pmm v0.0.0-20210728065534-7a7bfe95ccd9 h1:4wErKnKvoyh63li/nLld6oQ4qTqXGDg/O9Zm/Byn1NU=
github.com/percona/pmm v0.0.0-20210728065534-7a7bfe95ccd9/go.mod h1:Cm2JKvJMlMimtAhmF/1BUvz3qVJZ2O2zxQXRvtQh93Q=
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
4 changes: 4 additions & 0 deletions models/settings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ type IntegratedAlerting struct {
type Settings struct {
PMMPublicAddress string `json:"pmm_public_address"`

Updates struct {
Disabled bool `json:"disabled"`
} `json:"updates"`

Telemetry struct {
Disabled bool `json:"disabled"`
UUID string `json:"uuid"`
Expand Down
17 changes: 14 additions & 3 deletions models/settings_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ func GetSettings(q reform.DBTX) (*Settings, error) {

// ChangeSettingsParams contains values to change data in settings table.
type ChangeSettingsParams struct {
// We don't save it to db
DisableUpdates bool
EnableUpdates bool

DisableTelemetry bool
EnableTelemetry bool
Expand Down Expand Up @@ -140,6 +140,12 @@ func UpdateSettings(q reform.DBTX, params *ChangeSettingsParams) (*Settings, err
return nil, err
}

if params.DisableUpdates {
settings.Updates.Disabled = true
}
if params.EnableUpdates {
settings.Updates.Disabled = false
}
if params.DisableTelemetry {
settings.Telemetry.Disabled = true
settings.Telemetry.UUID = ""
Expand Down Expand Up @@ -422,9 +428,14 @@ func validateSettingsConflicts(params *ChangeSettingsParams, settings *Settings)
if params.DisableTelemetry && !params.DisableSTT && settings.SaaS.STTEnabled {
return fmt.Errorf("Cannot disable telemetry while STT is enabled.") //nolint:golint,stylecheck
}

if params.LogOut && (params.Email != "" || params.SessionID != "") {
return fmt.Errorf("Cannot loguot while updating Percona Platform user data.") //nolint:golint,stylecheck
return fmt.Errorf("Cannot logout while updating Percona Platform user data.") //nolint:golint,stylecheck
}
if params.DisableUpdates && !params.DisableTelemetry && !settings.Telemetry.Disabled {
return fmt.Errorf("Cannot disable updates while telemetry is enabled.") //nolint:golint,stylecheck
}
Comment thread
JiriCtvrtka marked this conversation as resolved.
Outdated
if params.DisableUpdates && params.EnableTelemetry {
return fmt.Errorf("Cannot disable updates while enabling telemetry.") //nolint:golint,stylecheck
}

return nil
Expand Down
9 changes: 8 additions & 1 deletion services/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ func (s *Server) readUpdateAuthToken() (string, error) {
// convertSettings merges database settings and settings from environment variables into API response.
func (s *Server) convertSettings(settings *models.Settings) *serverpb.Settings {
res := &serverpb.Settings{
UpdatesDisabled: s.envSettings.DisableUpdates,
UpdatesDisabled: !settings.Updates.Disabled,
Comment thread
JiriCtvrtka marked this conversation as resolved.
Outdated
TelemetryEnabled: !settings.Telemetry.Disabled,
MetricsResolutions: &serverpb.MetricsResolutions{
Hr: durationpb.New(settings.MetricsResolutions.HR),
Expand Down Expand Up @@ -522,6 +522,13 @@ func (s *Server) validateChangeSettingsRequest(ctx context.Context, req *serverp

// check request parameters compatibility with environment variables

if req.DisableUpdates && s.envSettings.EnableUpdates {
return status.Error(codes.FailedPrecondition, "Updates are enabled via ENABLE_UPDATES environment variable.")
}
if req.DisableUpdates && s.envSettings.EnableTelemetry {
return status.Error(codes.FailedPrecondition, "Updates cannot be disabled because telemetry is enabled via ENABLE_TELEMETRY environment variable.")
}

@BupycHuk BupycHuk Jul 28, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updates and telemetry is independent, so we can remove this check.

@JiriCtvrtka JiriCtvrtka Jul 28, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here https://perconacorp.slack.com/archives/C03J8FZFU/p1626470324102700 is discussion that they are coupled. Ok I will change everything related to it.

@BupycHuk BupycHuk Jul 28, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please discuss it with Roma, Steve or Denys. But previously users were able to disable updates using env variable without disabling telemetry.

@denisok denisok Jul 28, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that was just assumption from Steve (that disable doesn't work because of telemetry). It is not a requirement.

@JiriCtvrtka JiriCtvrtka Jul 28, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@denisok Ok thanks for verifying.


Comment thread
BupycHuk marked this conversation as resolved.
// ignore req.DisableTelemetry and req.DisableStt even if they are present since that will not change anything
if req.EnableTelemetry && s.envSettings.DisableTelemetry {
return status.Error(codes.FailedPrecondition, "Telemetry is disabled via DISABLE_TELEMETRY environment variable.")
Expand Down
16 changes: 15 additions & 1 deletion services/server/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,22 @@ func TestServer(t *testing.T) {
RemoveAlertManagerRules: true,
}))

s.envSettings.DisableTelemetry = true
s.envSettings.EnableUpdates = true
expected = status.New(codes.FailedPrecondition, "Updates are enabled via ENABLE_UPDATES environment variable.")

@BupycHuk BupycHuk Jul 28, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have ENABLE_UPDATES env variable? As far as I know we have only DISABLE_UPDATES

@BupycHuk BupycHuk Jul 28, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's rewrite test taking into account that we can't enable updates if it's disabled.

tests.AssertGRPCError(t, expected, s.validateChangeSettingsRequest(ctx, &serverpb.ChangeSettingsRequest{
DisableUpdates: true,
}))
s.envSettings.EnableUpdates = false
s.envSettings.EnableTelemetry = true
expected = status.New(codes.FailedPrecondition, "Updates cannot be disabled because telemetry is enabled via ENABLE_TELEMETRY environment variable.")
tests.AssertGRPCError(t, expected, s.validateChangeSettingsRequest(ctx, &serverpb.ChangeSettingsRequest{
DisableUpdates: true,
}))
assert.NoError(t, s.validateChangeSettingsRequest(ctx, &serverpb.ChangeSettingsRequest{
EnableUpdates: true,
}))

s.envSettings.DisableTelemetry = true
expected = status.New(codes.FailedPrecondition, "Telemetry is disabled via DISABLE_TELEMETRY environment variable.")
tests.AssertGRPCError(t, expected, s.validateChangeSettingsRequest(ctx, &serverpb.ChangeSettingsRequest{
EnableTelemetry: true,
Expand Down