diff --git a/charts/postgres-operator/crds/operatorconfigurations.yaml b/charts/postgres-operator/crds/operatorconfigurations.yaml index b74ebbb0a..043129516 100644 --- a/charts/postgres-operator/crds/operatorconfigurations.yaml +++ b/charts/postgres-operator/crds/operatorconfigurations.yaml @@ -131,6 +131,10 @@ spec: major_version_upgrade_mode: type: string default: "off" + major_version_upgrade_team_allow_list: + type: array + items: + type: string minimal_major_version: type: string default: "9.6" diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index d660de0ab..65619845a 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -64,6 +64,10 @@ configUsers: configMajorVersionUpgrade: # "off": no upgrade, "manual": manifest triggers action, "full": minimal version violation triggers too major_version_upgrade_mode: "off" + # upgrades will only be carried out for clusters of listed teams when mode is "off" + # major_version_upgrade_team_allow_list: + # - acid + # minimal Postgres major version that will not automatically be upgraded minimal_major_version: "9.6" # target Postgres major version when upgrading clusters automatically diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index bba917d88..00febcf89 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -184,6 +184,10 @@ CRD-configuration, they are grouped under the `major_version_upgrade` key. Note, that with all three modes increasing the version in the manifest will trigger a rolling update of the pods. The default is `"off"`. +* **major_version_upgrade_team_allow_list** + Upgrades will only be carried out for clusters of listed teams when mode is + set to "off". The default is empty. + * **minimal_major_version** The minimal Postgres major version that will not automatically be upgraded when `major_version_upgrade_mode` is set to `"full"`. The default is `"9.6"`. diff --git a/docs/user.md b/docs/user.md index e0873f274..a2a65e63f 100644 --- a/docs/user.md +++ b/docs/user.md @@ -603,10 +603,9 @@ spec: ``` Some extensions require SUPERUSER rights on creation unless they are not -whitelisted by the [pgextwlist](https://github.com/dimitri/pgextwlist) -extension, that is shipped with the Spilo image. To see which extensions are -on the list check the `extwlist.extension` parameter in the postgresql.conf -file. +allowed by the [pgextwlist](https://github.com/dimitri/pgextwlist) extension, +that is shipped with the Spilo image. To see which extensions are on the list +check the `extwlist.extension` parameter in the postgresql.conf file. ```bash SHOW extwlist.extensions; diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index e91e2f19e..7d3e14ce3 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -77,6 +77,7 @@ data: logical_backup_s3_sse: "AES256" logical_backup_schedule: "30 00 * * *" major_version_upgrade_mode: "manual" + # major_version_upgrade_team_allow_list: "" master_dns_name_format: "{cluster}.{team}.{hostedzone}" # master_pod_move_timeout: 20m # max_instances: "-1" diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml index d202125f3..bb64995ab 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -129,6 +129,10 @@ spec: major_version_upgrade_mode: type: string default: "off" + major_version_upgrade_team_allow_list: + type: array + items: + type: string minimal_major_version: type: string default: "9.6" diff --git a/manifests/postgresql-operator-default-configuration.yaml b/manifests/postgresql-operator-default-configuration.yaml index 24d496b36..02d558543 100644 --- a/manifests/postgresql-operator-default-configuration.yaml +++ b/manifests/postgresql-operator-default-configuration.yaml @@ -28,6 +28,8 @@ configuration: super_username: postgres major_version_upgrade: major_version_upgrade_mode: "off" + # major_version_upgrade_team_allow_list: + # - acid minimal_major_version: "9.6" target_major_version: "14" kubernetes: diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index f1805af5a..b0c88e264 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -1052,6 +1052,14 @@ var OperatorConfigCRDResourceValidation = apiextv1.CustomResourceValidation{ "major_version_upgrade_mode": { Type: "string", }, + "major_version_upgrade_team_allow_list": { + Type: "array", + Items: &apiextv1.JSONSchemaPropsOrArray{ + Schema: &apiextv1.JSONSchemaProps{ + Type: "string", + }, + }, + }, "minimal_major_version": { Type: "string", }, diff --git a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go index 6d0dd136a..f8eb5b5d1 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -43,9 +43,10 @@ type PostgresUsersConfiguration struct { // MajorVersionUpgradeConfiguration defines how to execute major version upgrades of Postgres. type MajorVersionUpgradeConfiguration struct { - MajorVersionUpgradeMode string `json:"major_version_upgrade_mode" default:"off"` // off - no actions, manual - manifest triggers action, full - manifest and minimal version violation trigger upgrade - MinimalMajorVersion string `json:"minimal_major_version" default:"9.6"` - TargetMajorVersion string `json:"target_major_version" default:"14"` + MajorVersionUpgradeMode string `json:"major_version_upgrade_mode" default:"off"` // off - no actions, manual - manifest triggers action, full - manifest and minimal version violation trigger upgrade + MajorVersionUpgradeTeamAllowList []string `json:"major_version_upgrade_team_allow_list,omitempty"` + MinimalMajorVersion string `json:"minimal_major_version" default:"9.6"` + TargetMajorVersion string `json:"target_major_version" default:"14"` } // KubernetesMetaConfiguration defines k8s conf required for all Postgres clusters and the operator itself diff --git a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go index 52595b362..08cc7a3a9 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -318,6 +318,11 @@ func (in *MaintenanceWindow) DeepCopy() *MaintenanceWindow { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MajorVersionUpgradeConfiguration) DeepCopyInto(out *MajorVersionUpgradeConfiguration) { *out = *in + if in.MajorVersionUpgradeTeamAllowList != nil { + in, out := &in.MajorVersionUpgradeTeamAllowList, &out.MajorVersionUpgradeTeamAllowList + *out = make([]string, len(*in)) + copy(*out, *in) + } return } @@ -386,7 +391,7 @@ func (in *OperatorConfigurationData) DeepCopyInto(out *OperatorConfigurationData } } out.PostgresUsersConfiguration = in.PostgresUsersConfiguration - out.MajorVersionUpgrade = in.MajorVersionUpgrade + in.MajorVersionUpgrade.DeepCopyInto(&out.MajorVersionUpgrade) in.Kubernetes.DeepCopyInto(&out.Kubernetes) out.PostgresPodResources = in.PostgresPodResources out.Timeouts = in.Timeouts diff --git a/pkg/cluster/connection_pooler.go b/pkg/cluster/connection_pooler.go index 5bde71458..c5c55350f 100644 --- a/pkg/cluster/connection_pooler.go +++ b/pkg/cluster/connection_pooler.go @@ -712,7 +712,8 @@ func (c *Cluster) syncConnectionPooler(oldSpec, newSpec *acidv1.Postgresql, Look if (!needSync && len(masterChanges) <= 0 && len(replicaChanges) <= 0) && ((!needConnectionPooler(&newSpec.Spec) && (c.ConnectionPooler == nil || !needConnectionPooler(&oldSpec.Spec))) || (c.ConnectionPooler != nil && needConnectionPooler(&newSpec.Spec) && - (c.ConnectionPooler[Master].LookupFunction || c.ConnectionPooler[Replica].LookupFunction))) { + ((c.ConnectionPooler[Master] != nil && c.ConnectionPooler[Master].LookupFunction) || + (c.ConnectionPooler[Replica] != nil && c.ConnectionPooler[Replica].LookupFunction)))) { c.logger.Debugln("syncing pooler is not required") return nil, nil } diff --git a/pkg/cluster/majorversionupgrade.go b/pkg/cluster/majorversionupgrade.go index edb55c882..60048e20d 100644 --- a/pkg/cluster/majorversionupgrade.go +++ b/pkg/cluster/majorversionupgrade.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/zalando/postgres-operator/pkg/spec" + "github.com/zalando/postgres-operator/pkg/util" v1 "k8s.io/api/core/v1" ) @@ -44,9 +45,25 @@ func (c *Cluster) GetDesiredMajorVersion() string { return c.Spec.PgVersion } +func (c *Cluster) isUpgradeAllowedForTeam(owningTeam string) bool { + allowedTeams := c.OpConfig.MajorVersionUpgradeTeamAllowList + + if len(allowedTeams) == 0 { + return false + } + + return util.SliceContains(allowedTeams, owningTeam) +} + +/* + Execute upgrade when mode is set to manual or full or when the owning team is allowed for upgrade (and mode is "off"). + + Manual upgrade means, it is triggered by the user via manifest version change + Full upgrade means, operator also determines the minimal version used accross all clusters and upgrades violators. +*/ func (c *Cluster) majorVersionUpgrade() error { - if c.OpConfig.MajorVersionUpgradeMode == "off" { + if c.OpConfig.MajorVersionUpgradeMode == "off" && !c.isUpgradeAllowedForTeam(c.Spec.TeamID) { return nil } diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index f078c6434..48a82ee04 100644 --- a/pkg/cluster/resources.go +++ b/pkg/cluster/resources.go @@ -496,18 +496,17 @@ func (c *Cluster) deleteEndpoint(role PostgresRole) error { func (c *Cluster) deleteSecrets() error { c.setProcessName("deleting secrets") - var errors []string - errorCount := 0 + errors := make([]string, 0) + for uid, secret := range c.Secrets { err := c.deleteSecret(uid, *secret) if err != nil { errors = append(errors, fmt.Sprintf("%v", err)) - errorCount++ } } - if errorCount > 0 { - return fmt.Errorf("could not delete all secrets: %v", errors) + if len(errors) > 0 { + return fmt.Errorf("could not delete all secrets: %v", strings.Join(errors, `', '`)) } return nil diff --git a/pkg/cluster/sync_test.go b/pkg/cluster/sync_test.go index d57d5cc85..872887baa 100644 --- a/pkg/cluster/sync_test.go +++ b/pkg/cluster/sync_test.go @@ -196,17 +196,15 @@ func TestCheckAndSetGlobalPostgreSQLConfiguration(t *testing.T) { cluster.patroni = p mockPod := newMockPod("192.168.100.1") - // simulate existing config that differs with cluster.Spec + // simulate existing config that differs from cluster.Spec tests := []struct { subtest string - pod *v1.Pod patroni acidv1.Patroni pgParams map[string]string restartMaster bool }{ { subtest: "Patroni and Postgresql.Parameters differ - restart replica first", - pod: mockPod, patroni: acidv1.Patroni{ TTL: 30, // desired 20 }, @@ -218,7 +216,6 @@ func TestCheckAndSetGlobalPostgreSQLConfiguration(t *testing.T) { }, { subtest: "multiple Postgresql.Parameters differ - restart replica first", - pod: mockPod, patroni: acidv1.Patroni{ TTL: 20, }, @@ -230,7 +227,6 @@ func TestCheckAndSetGlobalPostgreSQLConfiguration(t *testing.T) { }, { subtest: "desired max_connections bigger - restart replica first", - pod: mockPod, patroni: acidv1.Patroni{ TTL: 20, }, @@ -242,7 +238,6 @@ func TestCheckAndSetGlobalPostgreSQLConfiguration(t *testing.T) { }, { subtest: "desired max_connections smaller - restart master first", - pod: mockPod, patroni: acidv1.Patroni{ TTL: 20, }, @@ -255,7 +250,7 @@ func TestCheckAndSetGlobalPostgreSQLConfiguration(t *testing.T) { } for _, tt := range tests { - requireMasterRestart, err := cluster.checkAndSetGlobalPostgreSQLConfiguration(tt.pod, tt.patroni, cluster.Spec.Patroni, tt.pgParams, cluster.Spec.Parameters) + requireMasterRestart, err := cluster.checkAndSetGlobalPostgreSQLConfiguration(mockPod, tt.patroni, cluster.Spec.Patroni, tt.pgParams, cluster.Spec.Parameters) assert.NoError(t, err) if requireMasterRestart != tt.restartMaster { t.Errorf("%s - %s: unexpect master restart strategy, got %v, expected %v", testName, tt.subtest, requireMasterRestart, tt.restartMaster) diff --git a/pkg/cluster/volumes.go b/pkg/cluster/volumes.go index 4962319ed..78533ef54 100644 --- a/pkg/cluster/volumes.go +++ b/pkg/cluster/volumes.go @@ -88,7 +88,7 @@ func (c *Cluster) syncUnderlyingEBSVolume() error { awsGp3 := aws.String("gp3") awsIo2 := aws.String("io2") - errors := []string{} + errors := make([]string, 0) for _, volume := range c.EBSVolumes { var modifyIops *int64 diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go index fc56dbf96..275898d8e 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -56,6 +56,7 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur // major version upgrade config result.MajorVersionUpgradeMode = util.Coalesce(fromCRD.MajorVersionUpgrade.MajorVersionUpgradeMode, "off") + result.MajorVersionUpgradeTeamAllowList = fromCRD.MajorVersionUpgrade.MajorVersionUpgradeTeamAllowList result.MinimalMajorVersion = util.Coalesce(fromCRD.MajorVersionUpgrade.MinimalMajorVersion, "9.6") result.TargetMajorVersion = util.Coalesce(fromCRD.MajorVersionUpgrade.TargetMajorVersion, "14") diff --git a/pkg/controller/util.go b/pkg/controller/util.go index 8aa891c09..563734ac9 100644 --- a/pkg/controller/util.go +++ b/pkg/controller/util.go @@ -195,13 +195,12 @@ func (c *Controller) getInfrastructureRoleDefinitions() []*config.Infrastructure func (c *Controller) getInfrastructureRoles( rolesSecrets []*config.InfrastructureRole) ( - map[string]spec.PgUser, []error) { - - var errors []error - var noRolesProvided = true + map[string]spec.PgUser, error) { + errors := make([]string, 0) + noRolesProvided := true roles := []spec.PgUser{} - uniqRoles := map[string]spec.PgUser{} + uniqRoles := make(map[string]spec.PgUser) // To be compatible with the legacy implementation we need to return nil if // the provided secret name is empty. The equivalent situation in the @@ -214,37 +213,39 @@ func (c *Controller) getInfrastructureRoles( } if noRolesProvided { - return nil, nil + return uniqRoles, nil } for _, secret := range rolesSecrets { infraRoles, err := c.getInfrastructureRole(secret) if err != nil || infraRoles == nil { - c.logger.Debugf("Cannot get infrastructure role: %+v", *secret) + c.logger.Debugf("cannot get infrastructure role: %+v", *secret) if err != nil { - errors = append(errors, err) + errors = append(errors, fmt.Sprintf("%v", err)) } continue } - for _, r := range infraRoles { - roles = append(roles, r) - } + roles = append(roles, infraRoles...) } for _, r := range roles { if _, exists := uniqRoles[r.Name]; exists { - msg := "Conflicting infrastructure roles: roles[%s] = (%q, %q)" + msg := "conflicting infrastructure roles: roles[%s] = (%q, %q)" c.logger.Debugf(msg, r.Name, uniqRoles[r.Name], r) } uniqRoles[r.Name] = r } - return uniqRoles, errors + if len(errors) > 0 { + return uniqRoles, fmt.Errorf(strings.Join(errors, `', '`)) + } + + return uniqRoles, nil } // Generate list of users representing one infrastructure role based on its diff --git a/pkg/controller/util_test.go b/pkg/controller/util_test.go index d8e4c3782..a4ca17728 100644 --- a/pkg/controller/util_test.go +++ b/pkg/controller/util_test.go @@ -7,6 +7,7 @@ import ( b64 "encoding/base64" + "github.com/stretchr/testify/assert" "github.com/zalando/postgres-operator/pkg/spec" "github.com/zalando/postgres-operator/pkg/util/config" "github.com/zalando/postgres-operator/pkg/util/k8sutil" @@ -90,21 +91,21 @@ func TestClusterWorkerID(t *testing.T) { // not exist, or empty) and the old format. func TestOldInfrastructureRoleFormat(t *testing.T) { var testTable = []struct { - secretName spec.NamespacedName - expectedRoles map[string]spec.PgUser - expectedErrors []error + secretName spec.NamespacedName + expectedRoles map[string]spec.PgUser + expectedError error }{ { // empty secret name spec.NamespacedName{}, - nil, + map[string]spec.PgUser{}, nil, }, { // secret does not exist spec.NamespacedName{Namespace: v1.NamespaceDefault, Name: "null"}, map[string]spec.PgUser{}, - []error{fmt.Errorf(`could not get infrastructure roles secret default/null: NotFound`)}, + fmt.Errorf(`could not get infrastructure roles secret default/null: NotFound`), }, { spec.NamespacedName{ @@ -129,7 +130,7 @@ func TestOldInfrastructureRoleFormat(t *testing.T) { }, } for _, test := range testTable { - roles, errors := utilTestController.getInfrastructureRoles( + roles, err := utilTestController.getInfrastructureRoles( []*config.InfrastructureRole{ &config.InfrastructureRole{ SecretName: test.secretName, @@ -140,22 +141,9 @@ func TestOldInfrastructureRoleFormat(t *testing.T) { }, }) - if len(errors) != len(test.expectedErrors) { + if err != nil && err.Error() != test.expectedError.Error() { t.Errorf("expected error '%v' does not match the actual error '%v'", - test.expectedErrors, errors) - } - - for idx := range errors { - err := errors[idx] - expectedErr := test.expectedErrors[idx] - - if err != expectedErr { - if err != nil && expectedErr != nil && err.Error() == expectedErr.Error() { - continue - } - t.Errorf("expected error '%v' does not match the actual error '%v'", - expectedErr, err) - } + test.expectedError, err) } if !reflect.DeepEqual(roles, test.expectedRoles) { @@ -169,9 +157,8 @@ func TestOldInfrastructureRoleFormat(t *testing.T) { // corresponding secrets. Here we test the new format. func TestNewInfrastructureRoleFormat(t *testing.T) { var testTable = []struct { - secrets []spec.NamespacedName - expectedRoles map[string]spec.PgUser - expectedErrors []error + secrets []spec.NamespacedName + expectedRoles map[string]spec.PgUser }{ // one secret with one configmap { @@ -196,7 +183,6 @@ func TestNewInfrastructureRoleFormat(t *testing.T) { Flags: []string{"createdb"}, }, }, - nil, }, // multiple standalone secrets { @@ -224,7 +210,6 @@ func TestNewInfrastructureRoleFormat(t *testing.T) { MemberOf: []string{"new-test-inrole2"}, }, }, - nil, }, } for _, test := range testTable { @@ -239,27 +224,8 @@ func TestNewInfrastructureRoleFormat(t *testing.T) { }) } - roles, errors := utilTestController.getInfrastructureRoles(definitions) - if len(errors) != len(test.expectedErrors) { - t.Errorf("expected error does not match the actual error:\n%+v\n%+v", - test.expectedErrors, errors) - - // Stop and do not do any further checks - return - } - - for idx := range errors { - err := errors[idx] - expectedErr := test.expectedErrors[idx] - - if err != expectedErr { - if err != nil && expectedErr != nil && err.Error() == expectedErr.Error() { - continue - } - t.Errorf("expected error '%v' does not match the actual error '%v'", - expectedErr, err) - } - } + roles, err := utilTestController.getInfrastructureRoles(definitions) + assert.NoError(t, err) if !reflect.DeepEqual(roles, test.expectedRoles) { t.Errorf("expected roles output/the actual:\n%#v\n%#v", diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 78e0a6c49..71bf406e4 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -212,6 +212,7 @@ type Config struct { EnablePgVersionEnvVar bool `name:"enable_pgversion_env_var" default:"true"` EnableSpiloWalPathCompat bool `name:"enable_spilo_wal_path_compat" default:"false"` MajorVersionUpgradeMode string `name:"major_version_upgrade_mode" default:"off"` + MajorVersionUpgradeTeamAllowList []string `name:"major_version_upgrade_team_allow_list" default:""` MinimalMajorVersion string `name:"minimal_major_version" default:"9.6"` TargetMajorVersion string `name:"target_major_version" default:"14"` } diff --git a/pkg/util/users/users.go b/pkg/util/users/users.go index 3da933644..821350bb7 100644 --- a/pkg/util/users/users.go +++ b/pkg/util/users/users.go @@ -101,7 +101,7 @@ func (strategy DefaultUserSyncStrategy) ProduceSyncRequests(dbUsers spec.PgUserM // ExecuteSyncRequests makes actual database changes from the requests passed in its arguments. func (strategy DefaultUserSyncStrategy) ExecuteSyncRequests(requests []spec.PgSyncUserRequest, db *sql.DB) error { var reqretries []spec.PgSyncUserRequest - var errors []string + errors := make([]string, 0) for _, request := range requests { switch request.Kind { case spec.PGSyncUserAdd: @@ -138,7 +138,7 @@ func (strategy DefaultUserSyncStrategy) ExecuteSyncRequests(requests []spec.PgSy return err } } else { - return fmt.Errorf("could not execute sync requests for users: %v", errors) + return fmt.Errorf("could not execute sync requests for users: %v", strings.Join(errors, `', '`)) } }