From 2ef069ee9397533f6f836825fe92f143d985962b Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Tue, 27 Feb 2018 17:10:49 +0100 Subject: [PATCH 01/21] Create/delete replica service regardless of load balancer setup --- pkg/cluster/cluster.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index e2bb86d57..084c8926b 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -217,9 +217,7 @@ func (c *Cluster) Create() error { c.setStatus(spec.ClusterStatusCreating) for _, role := range []PostgresRole{Master, Replica} { - if role == Replica && !c.Spec.ReplicaLoadBalancer { - continue - } + if c.Endpoints[role] != nil { return fmt.Errorf("%s endpoint already exists in the cluster", role) } @@ -589,9 +587,6 @@ func (c *Cluster) Delete() error { } for _, role := range []PostgresRole{Master, Replica} { - if role == Replica && !c.Spec.ReplicaLoadBalancer { - continue - } if err := c.deleteEndpoint(role); err != nil { return fmt.Errorf("could not delete %s endpoint: %v", role, err) From b107d781e8a4210283a92bc56b33a060a39a5bdf Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Tue, 27 Feb 2018 17:16:00 +0100 Subject: [PATCH 02/21] Do not delete replica service w/o load balancer during sync --- pkg/cluster/sync.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 955253f1a..79e099fe9 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -108,11 +108,6 @@ func (c *Cluster) syncService(role PostgresRole) error { svc, err := c.KubeClient.Services(c.Namespace).Get(c.serviceName(role), metav1.GetOptions{}) if err == nil { - if role == Replica && !c.Spec.ReplicaLoadBalancer { - if err := c.deleteService(role); err != nil { - return fmt.Errorf("could not delete %s service", role) - } - } desiredSvc := c.generateService(role, &c.Spec) match, reason := k8sutil.SameService(svc, desiredSvc) From 28fed268459164813e77da384f50b35dea1ba1c4 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Tue, 27 Feb 2018 17:18:30 +0100 Subject: [PATCH 03/21] Do not delete an endpoint for the replica service w/o load balancer during sync --- pkg/cluster/sync.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 79e099fe9..2b7cda8dd 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -160,11 +160,6 @@ func (c *Cluster) syncEndpoint(role PostgresRole) error { ep, err := c.KubeClient.Endpoints(c.Namespace).Get(c.endpointName(role), metav1.GetOptions{}) if err == nil { - if role == Replica && !c.Spec.ReplicaLoadBalancer { - if err := c.deleteEndpoint(role); err != nil { - return fmt.Errorf("could not delete %s endpoint", role) - } - } c.Endpoints[role] = ep return nil From fb21246fcd812fc0c7589157a8b560664691cb45 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Tue, 27 Feb 2018 17:21:51 +0100 Subject: [PATCH 04/21] Remove early stopping conditions that rely on the relica service being absent --- pkg/cluster/sync.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 2b7cda8dd..5a77e658b 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -128,11 +128,6 @@ func (c *Cluster) syncService(role PostgresRole) error { } c.Services[role] = nil - // Service does not exist - if role == Replica && !c.Spec.ReplicaLoadBalancer { - return nil - } - c.logger.Infof("could not find the cluster's %s service", role) if svc, err := c.createService(role); err != nil { @@ -168,10 +163,6 @@ func (c *Cluster) syncEndpoint(role PostgresRole) error { } c.Endpoints[role] = nil - if role == Replica && !c.Spec.ReplicaLoadBalancer { - return nil - } - c.logger.Infof("could not find the cluster's %s endpoint", role) if ep, err := c.createEndpoint(role); err != nil { From e74c05fec9deb49323e12f71f89a5fa677aaec30 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Fri, 2 Mar 2018 11:47:51 +0100 Subject: [PATCH 05/21] Document intended usage of useLoadBalancer --- pkg/spec/postgresql.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/spec/postgresql.go b/pkg/spec/postgresql.go index e124c10c1..51dc3e9ce 100644 --- a/pkg/spec/postgresql.go +++ b/pkg/spec/postgresql.go @@ -96,7 +96,8 @@ type PostgresSpec struct { TeamID string `json:"teamId"` AllowedSourceRanges []string `json:"allowedSourceRanges"` DockerImage string `json:"dockerImage,omitempty"` - // EnableLoadBalancer is a pointer, since it is important to know if that parameters is omitted from the manifest + // EnableLoadBalancer is a pointer, since it is important to know if that parameters is omitted from the Postgres manifest + // in that case UseLoadBalancer == nil and the value is taken from the operator config UseLoadBalancer *bool `json:"useLoadBalancer,omitempty"` ReplicaLoadBalancer bool `json:"replicaLoadBalancer,omitempty"` NumberOfInstances int32 `json:"numberOfInstances"` From 18741750f51320c04cbbe85767f5a9797318bab2 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Fri, 2 Mar 2018 12:00:02 +0100 Subject: [PATCH 06/21] Make ReplicaLoadBalancer a pointer to handle the case when it is unset --- pkg/spec/postgresql.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/spec/postgresql.go b/pkg/spec/postgresql.go index 51dc3e9ce..501a77724 100644 --- a/pkg/spec/postgresql.go +++ b/pkg/spec/postgresql.go @@ -98,8 +98,9 @@ type PostgresSpec struct { DockerImage string `json:"dockerImage,omitempty"` // EnableLoadBalancer is a pointer, since it is important to know if that parameters is omitted from the Postgres manifest // in that case UseLoadBalancer == nil and the value is taken from the operator config - UseLoadBalancer *bool `json:"useLoadBalancer,omitempty"` - ReplicaLoadBalancer bool `json:"replicaLoadBalancer,omitempty"` + UseLoadBalancer *bool `json:"useLoadBalancer,omitempty"` + // if ReplicaLoadBalancer == nil (is unset), value of UseLoadBalancer takes priority + ReplicaLoadBalancer *bool `json:"replicaLoadBalancer,omitempty"` NumberOfInstances int32 `json:"numberOfInstances"` Users map[string]UserFlags `json:"users"` MaintenanceWindows []MaintenanceWindow `json:"maintenanceWindows,omitempty"` From 2aeff096f7afb67128b35154780292e1e35c981d Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Fri, 2 Mar 2018 13:35:25 +0100 Subject: [PATCH 07/21] Make ReplicaLoadBalancer a separate toggler --- pkg/cluster/cluster.go | 3 +-- pkg/cluster/k8sres.go | 20 +++++++++++++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 084c8926b..ea82265a7 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -478,8 +478,7 @@ func (c *Cluster) Update(oldSpec, newSpec *spec.Postgresql) error { // Service if !reflect.DeepEqual(c.generateService(Master, &oldSpec.Spec), c.generateService(Master, &newSpec.Spec)) || - !reflect.DeepEqual(c.generateService(Replica, &oldSpec.Spec), c.generateService(Replica, &newSpec.Spec)) || - oldSpec.Spec.ReplicaLoadBalancer != newSpec.Spec.ReplicaLoadBalancer { + !reflect.DeepEqual(c.generateService(Replica, &oldSpec.Spec), c.generateService(Replica, &newSpec.Spec)) { c.logger.Debugf("syncing services") if err := c.syncServices(); err != nil { c.logger.Errorf("could not sync services: %v", err) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 369f30b59..785e242c4 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -669,6 +669,20 @@ func (c *Cluster) generateSingleUserSecret(namespace string, pgUser spec.PgUser) return &secret } +func (c *Cluster) shouldCreateLoadBalancerForService(role PostgresRole, spec *spec.PostgresSpec) bool { + + // handle LB for the replica service separately if ReplicaLoadBalancer is set in the Postgres manifest + if role == Replica && spec.ReplicaLoadBalancer != nil { + return *spec.ReplicaLoadBalancer + } + + // create a load balancer for the master service if either Postgres or operator manifests tell to do so + // if ReplicaLoadBalancer is unset and LB for master service is created, also create a balancer for replicas + return (spec.UseLoadBalancer != nil && *spec.UseLoadBalancer) || + (spec.UseLoadBalancer == nil && c.OpConfig.EnableLoadBalancer) + +} + func (c *Cluster) generateService(role PostgresRole, spec *spec.PostgresSpec) *v1.Service { var dnsName string @@ -689,12 +703,12 @@ func (c *Cluster) generateService(role PostgresRole, spec *spec.PostgresSpec) *v var annotations map[string]string - // Examine the per-cluster load balancer setting, if it is not defined - check the operator configuration. - if (spec.UseLoadBalancer != nil && *spec.UseLoadBalancer) || - (spec.UseLoadBalancer == nil && c.OpConfig.EnableLoadBalancer) { + if c.shouldCreateLoadBalancerForService(role, spec) == true { // safe default value: lock load balancer to only local address unless overridden explicitly. sourceRanges := []string{localHost} + + // source ranges are the same for balancers for master and replica services allowedSourceRanges := spec.AllowedSourceRanges if len(allowedSourceRanges) >= 0 { sourceRanges = allowedSourceRanges From 5ff562a607bea6fc5c62db60978ac1b2aa89cd27 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Fri, 2 Mar 2018 14:03:41 +0100 Subject: [PATCH 08/21] Minor improvements --- pkg/cluster/k8sres.go | 2 +- pkg/spec/postgresql.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 785e242c4..8c136cce6 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -703,7 +703,7 @@ func (c *Cluster) generateService(role PostgresRole, spec *spec.PostgresSpec) *v var annotations map[string]string - if c.shouldCreateLoadBalancerForService(role, spec) == true { + if c.shouldCreateLoadBalancerForService(role, spec) { // safe default value: lock load balancer to only local address unless overridden explicitly. sourceRanges := []string{localHost} diff --git a/pkg/spec/postgresql.go b/pkg/spec/postgresql.go index 501a77724..460e62237 100644 --- a/pkg/spec/postgresql.go +++ b/pkg/spec/postgresql.go @@ -99,7 +99,7 @@ type PostgresSpec struct { // EnableLoadBalancer is a pointer, since it is important to know if that parameters is omitted from the Postgres manifest // in that case UseLoadBalancer == nil and the value is taken from the operator config UseLoadBalancer *bool `json:"useLoadBalancer,omitempty"` - // if ReplicaLoadBalancer == nil (is unset), value of UseLoadBalancer takes priority + // if ReplicaLoadBalancer == nil (is unset), value of UseLoadBalancer determines if a balancer for replicas is created ReplicaLoadBalancer *bool `json:"replicaLoadBalancer,omitempty"` NumberOfInstances int32 `json:"numberOfInstances"` Users map[string]UserFlags `json:"users"` From 5bc5e70c81d23c5916940ba45b9b6af86f368eb4 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Mon, 12 Mar 2018 16:48:44 +0100 Subject: [PATCH 09/21] Log if replica service has no load balancer --- pkg/cluster/k8sres.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 8c136cce6..4de577318 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -721,6 +721,10 @@ func (c *Cluster) generateService(role PostgresRole, spec *spec.PostgresSpec) *v constants.ZalandoDNSNameAnnotation: dnsName, constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, } + } else if role == Replica { + // before PR #258, the replica service was only created if allocated a LB + // now we always create the service but warn if the LB is absent + c.logger.Debugf("No load balancer created for the replica service") } service := &v1.Service{ From ac6c5bcf09210b67c19b98654d1c0561cbae2603 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Wed, 14 Mar 2018 11:53:03 +0100 Subject: [PATCH 10/21] Explicitly name replica and master load balancer params in PostgresSpec --- manifests/complete-postgres-manifest.yaml | 5 +++-- pkg/cluster/k8sres.go | 8 ++++---- pkg/spec/postgresql.go | 24 +++++++++++------------ 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/manifests/complete-postgres-manifest.yaml b/manifests/complete-postgres-manifest.yaml index f9923f350..e5908df03 100644 --- a/manifests/complete-postgres-manifest.yaml +++ b/manifests/complete-postgres-manifest.yaml @@ -12,8 +12,9 @@ spec: zalando: - superuser - createdb - useLoadBalancer: true - allowedSourceRanges: #Load balancer source ranges + enableMasterLoadBalancer: true + # enableReplicaLoadBalancer: true # uncomment to enable a LB for a replica k8s service + allowedSourceRanges: # load balancers' source ranges for both master and replica services - 127.0.0.1/32 databases: foo: zalando diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 4de577318..58a5529bf 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -672,14 +672,14 @@ func (c *Cluster) generateSingleUserSecret(namespace string, pgUser spec.PgUser) func (c *Cluster) shouldCreateLoadBalancerForService(role PostgresRole, spec *spec.PostgresSpec) bool { // handle LB for the replica service separately if ReplicaLoadBalancer is set in the Postgres manifest - if role == Replica && spec.ReplicaLoadBalancer != nil { - return *spec.ReplicaLoadBalancer + if role == Replica && spec.EnableReplicaLoadBalancer != nil { + return *spec.EnableReplicaLoadBalancer } // create a load balancer for the master service if either Postgres or operator manifests tell to do so // if ReplicaLoadBalancer is unset and LB for master service is created, also create a balancer for replicas - return (spec.UseLoadBalancer != nil && *spec.UseLoadBalancer) || - (spec.UseLoadBalancer == nil && c.OpConfig.EnableLoadBalancer) + return (spec.EnableMasterLoadBalancer != nil && *spec.EnableMasterLoadBalancer) || + (spec.EnableMasterLoadBalancer == nil && c.OpConfig.EnableLoadBalancer) } diff --git a/pkg/spec/postgresql.go b/pkg/spec/postgresql.go index 460e62237..a197912d4 100644 --- a/pkg/spec/postgresql.go +++ b/pkg/spec/postgresql.go @@ -96,18 +96,18 @@ type PostgresSpec struct { TeamID string `json:"teamId"` AllowedSourceRanges []string `json:"allowedSourceRanges"` DockerImage string `json:"dockerImage,omitempty"` - // EnableLoadBalancer is a pointer, since it is important to know if that parameters is omitted from the Postgres manifest - // in that case UseLoadBalancer == nil and the value is taken from the operator config - UseLoadBalancer *bool `json:"useLoadBalancer,omitempty"` - // if ReplicaLoadBalancer == nil (is unset), value of UseLoadBalancer determines if a balancer for replicas is created - ReplicaLoadBalancer *bool `json:"replicaLoadBalancer,omitempty"` - NumberOfInstances int32 `json:"numberOfInstances"` - Users map[string]UserFlags `json:"users"` - MaintenanceWindows []MaintenanceWindow `json:"maintenanceWindows,omitempty"` - Clone CloneDescription `json:"clone"` - ClusterName string `json:"-"` - Databases map[string]string `json:"databases,omitempty"` - Tolerations []v1.Toleration `json:"tolerations,omitempty"` + // EnableMasterLoadBalancer is a pointer, since it is important to know if that parameters is omitted from the Postgres manifest + // in that case EnableMasterLoadBalancer == nil and the value is taken from the operator config + EnableMasterLoadBalancer *bool `json:"enableMasterLoadBalancer,omitempty"` + // if EnableReplicaLoadBalancer == nil (is unset), value of UseLoadBalancer determines if a balancer for replicas is created + EnableReplicaLoadBalancer *bool `json:"enableReplicaLoadBalancer,omitempty"` + NumberOfInstances int32 `json:"numberOfInstances"` + Users map[string]UserFlags `json:"users"` + MaintenanceWindows []MaintenanceWindow `json:"maintenanceWindows,omitempty"` + Clone CloneDescription `json:"clone"` + ClusterName string `json:"-"` + Databases map[string]string `json:"databases,omitempty"` + Tolerations []v1.Toleration `json:"tolerations,omitempty"` } // PostgresqlList defines a list of PostgreSQL clusters. From 0986e5622604467c1efa096e0b5208fd5a703cf0 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Wed, 14 Mar 2018 12:10:10 +0100 Subject: [PATCH 11/21] Add separate params for master and replica load balancers to operator configuration --- manifests/complete-postgres-manifest.yaml | 2 +- manifests/configmap.yaml | 3 +- pkg/cluster/k8sres.go | 2 +- pkg/util/config/config.go | 49 ++++++++++++----------- 4 files changed, 29 insertions(+), 27 deletions(-) diff --git a/manifests/complete-postgres-manifest.yaml b/manifests/complete-postgres-manifest.yaml index e5908df03..5663dcbc2 100644 --- a/manifests/complete-postgres-manifest.yaml +++ b/manifests/complete-postgres-manifest.yaml @@ -13,7 +13,7 @@ spec: - superuser - createdb enableMasterLoadBalancer: true - # enableReplicaLoadBalancer: true # uncomment to enable a LB for a replica k8s service + # enableReplicaLoadBalancer: true allowedSourceRanges: # load balancers' source ranges for both master and replica services - 127.0.0.1/32 databases: diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index cc0af9fe0..edb308a37 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -36,7 +36,8 @@ data: team_admin_role: "admin" teams_api_url: http://fake-teams-api.default.svc.cluster.local workers: "4" - enable_load_balancer: "true" + enable_master_load_balancer: "true" + # enable_replica_load_balancer: "true" api_port: "8080" ring_log_lines: "100" cluster_history_entries: "1000" diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 58a5529bf..edd1d4cf9 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -679,7 +679,7 @@ func (c *Cluster) shouldCreateLoadBalancerForService(role PostgresRole, spec *sp // create a load balancer for the master service if either Postgres or operator manifests tell to do so // if ReplicaLoadBalancer is unset and LB for master service is created, also create a balancer for replicas return (spec.EnableMasterLoadBalancer != nil && *spec.EnableMasterLoadBalancer) || - (spec.EnableMasterLoadBalancer == nil && c.OpConfig.EnableLoadBalancer) + (spec.EnableMasterLoadBalancer == nil && c.OpConfig.EnableMasterLoadBalancer) } diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 2c83b36b3..76a9d6b70 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -67,30 +67,31 @@ type Config struct { Resources Auth Scalyr - WatchedNamespace string `name:"watched_namespace"` // special values: "*" means 'watch all namespaces', the empty string "" means 'watch a namespace where operator is deployed to' - EtcdHost string `name:"etcd_host" default:"etcd-client.default.svc.cluster.local:2379"` - DockerImage string `name:"docker_image" default:"registry.opensource.zalan.do/acid/spiloprivate-9.6:1.2-p4"` - ServiceAccountName string `name:"service_account_name" default:"operator"` - DbHostedZone string `name:"db_hosted_zone" default:"db.example.com"` - EtcdScope string `name:"etcd_scope" default:"service"` - WALES3Bucket string `name:"wal_s3_bucket"` - KubeIAMRole string `name:"kube_iam_role"` - DebugLogging bool `name:"debug_logging" default:"true"` - EnableDBAccess bool `name:"enable_database_access" default:"true"` - EnableTeamsAPI bool `name:"enable_teams_api" default:"true"` - EnableTeamSuperuser bool `name:"enable_team_superuser" default:"false"` - TeamAdminRole string `name:"team_admin_role" default:"admin"` - EnableLoadBalancer bool `name:"enable_load_balancer" default:"true"` - MasterDNSNameFormat stringTemplate `name:"master_dns_name_format" default:"{cluster}.{team}.{hostedzone}"` - ReplicaDNSNameFormat stringTemplate `name:"replica_dns_name_format" default:"{cluster}-repl.{team}.{hostedzone}"` - PDBNameFormat stringTemplate `name:"pdb_name_format" default:"postgres-{cluster}-pdb"` - Workers uint32 `name:"workers" default:"4"` - APIPort int `name:"api_port" default:"8080"` - RingLogLines int `name:"ring_log_lines" default:"100"` - ClusterHistoryEntries int `name:"cluster_history_entries" default:"1000"` - TeamAPIRoleConfiguration map[string]string `name:"team_api_role_configuration" default:"log_statement:all"` - PodTerminateGracePeriod time.Duration `name:"pod_terminate_grace_period" default:"5m"` - ProtectedRoles []string `name:"protected_role_names" default:"admin"` + WatchedNamespace string `name:"watched_namespace"` // special values: "*" means 'watch all namespaces', the empty string "" means 'watch a namespace where operator is deployed to' + EtcdHost string `name:"etcd_host" default:"etcd-client.default.svc.cluster.local:2379"` + DockerImage string `name:"docker_image" default:"registry.opensource.zalan.do/acid/spiloprivate-9.6:1.2-p4"` + ServiceAccountName string `name:"service_account_name" default:"operator"` + DbHostedZone string `name:"db_hosted_zone" default:"db.example.com"` + EtcdScope string `name:"etcd_scope" default:"service"` + WALES3Bucket string `name:"wal_s3_bucket"` + KubeIAMRole string `name:"kube_iam_role"` + DebugLogging bool `name:"debug_logging" default:"true"` + EnableDBAccess bool `name:"enable_database_access" default:"true"` + EnableTeamsAPI bool `name:"enable_teams_api" default:"true"` + EnableTeamSuperuser bool `name:"enable_team_superuser" default:"false"` + TeamAdminRole string `name:"team_admin_role" default:"admin"` + EnableMasterLoadBalancer bool `name:"enable_master_load_balancer" default:"true"` + EnableReplicaLoadBalancer bool `name:"enable_replica_load_balancer" default:"false"` + MasterDNSNameFormat stringTemplate `name:"master_dns_name_format" default:"{cluster}.{team}.{hostedzone}"` + ReplicaDNSNameFormat stringTemplate `name:"replica_dns_name_format" default:"{cluster}-repl.{team}.{hostedzone}"` + PDBNameFormat stringTemplate `name:"pdb_name_format" default:"postgres-{cluster}-pdb"` + Workers uint32 `name:"workers" default:"4"` + APIPort int `name:"api_port" default:"8080"` + RingLogLines int `name:"ring_log_lines" default:"100"` + ClusterHistoryEntries int `name:"cluster_history_entries" default:"1000"` + TeamAPIRoleConfiguration map[string]string `name:"team_api_role_configuration" default:"log_statement:all"` + PodTerminateGracePeriod time.Duration `name:"pod_terminate_grace_period" default:"5m"` + ProtectedRoles []string `name:"protected_role_names" default:"admin"` } // MustMarshal marshals the config or panics From 20f30d3739601c2d2ebaad0e49908975097aab54 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Wed, 14 Mar 2018 12:46:58 +0100 Subject: [PATCH 12/21] Update the method for deciding about load balancers --- pkg/cluster/k8sres.go | 28 ++++++++++++++++++++-------- pkg/spec/postgresql.go | 7 +++---- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index edd1d4cf9..0fe7dfa58 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -671,15 +671,27 @@ func (c *Cluster) generateSingleUserSecret(namespace string, pgUser spec.PgUser) func (c *Cluster) shouldCreateLoadBalancerForService(role PostgresRole, spec *spec.PostgresSpec) bool { - // handle LB for the replica service separately if ReplicaLoadBalancer is set in the Postgres manifest - if role == Replica && spec.EnableReplicaLoadBalancer != nil { - return *spec.EnableReplicaLoadBalancer - } + switch role { - // create a load balancer for the master service if either Postgres or operator manifests tell to do so - // if ReplicaLoadBalancer is unset and LB for master service is created, also create a balancer for replicas - return (spec.EnableMasterLoadBalancer != nil && *spec.EnableMasterLoadBalancer) || - (spec.EnableMasterLoadBalancer == nil && c.OpConfig.EnableMasterLoadBalancer) + case Replica: + + // if the value is explicitly set in a Postgresql manifest, follow this setting + if spec.EnableReplicaLoadBalancer != nil { + return *spec.EnableReplicaLoadBalancer + } + // otherwise, follow the operator configuration + return c.OpConfig.EnableReplicaLoadBalancer + + case Master: + + if spec.EnableMasterLoadBalancer != nil { + return *spec.EnableMasterLoadBalancer + } + return c.OpConfig.EnableMasterLoadBalancer + + default: + panic(fmt.Sprintf("Unknown role %v", role)) + } } diff --git a/pkg/spec/postgresql.go b/pkg/spec/postgresql.go index a197912d4..a948f1698 100644 --- a/pkg/spec/postgresql.go +++ b/pkg/spec/postgresql.go @@ -96,10 +96,9 @@ type PostgresSpec struct { TeamID string `json:"teamId"` AllowedSourceRanges []string `json:"allowedSourceRanges"` DockerImage string `json:"dockerImage,omitempty"` - // EnableMasterLoadBalancer is a pointer, since it is important to know if that parameters is omitted from the Postgres manifest - // in that case EnableMasterLoadBalancer == nil and the value is taken from the operator config - EnableMasterLoadBalancer *bool `json:"enableMasterLoadBalancer,omitempty"` - // if EnableReplicaLoadBalancer == nil (is unset), value of UseLoadBalancer determines if a balancer for replicas is created + // vars that enable load balancers are pointers because it is important to know if any of them is omitted from the Postgres manifest + // in that case the var evaluates to nil and the value is taken from the operator config + EnableMasterLoadBalancer *bool `json:"enableMasterLoadBalancer,omitempty"` EnableReplicaLoadBalancer *bool `json:"enableReplicaLoadBalancer,omitempty"` NumberOfInstances int32 `json:"numberOfInstances"` Users map[string]UserFlags `json:"users"` From 27837e567252bd2fecc43a0c2904394d686560f2 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Wed, 14 Mar 2018 13:09:16 +0100 Subject: [PATCH 13/21] Document usage of load balancers --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 6814d332c..44bbdf4f2 100644 --- a/README.md +++ b/README.md @@ -233,6 +233,10 @@ As a preventive measure, one can restrict the minimum and the maximum number of If either `min_instances` or `max_instances` is set to a non-zero value, the operator may adjust the number of instances specified in the cluster manifest to match either the min or the max boundary. For instance, of a cluster manifest has 1 instance and the min_instances is set to 3, the cluster will be created with 3 instances. By default, both parameters are set to -1. +### Load balancers + +For any Postgresql/Spilo cluster an operator creates two separate k8s services: one for the master pod and one for replica pods. To expose these services to an outer network, one can attach load balancers to them by setting `enableMasterLoadBalancer` and/or `enableReplicaLoadBalancer` to `true` in the cluster manifest. In the case any of these variables is omitted from the manifest, the operator configmap's settings `enable_master_load_balancer` and `enable_replica_load_balancer` apply. Note that the operator settings affect all Postgresql services running in a namespace watched by the operator. + # Setup development environment The following steps guide you through the setup to work on the operator itself. From 145689c9506d57798e330fc6ac5f2e63dbbf4b85 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Fri, 16 Mar 2018 13:18:13 +0100 Subject: [PATCH 14/21] Disable load balancer for master service by default (it may cost money) --- pkg/util/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 76a9d6b70..a9788221c 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -80,7 +80,7 @@ type Config struct { EnableTeamsAPI bool `name:"enable_teams_api" default:"true"` EnableTeamSuperuser bool `name:"enable_team_superuser" default:"false"` TeamAdminRole string `name:"team_admin_role" default:"admin"` - EnableMasterLoadBalancer bool `name:"enable_master_load_balancer" default:"true"` + EnableMasterLoadBalancer bool `name:"enable_master_load_balancer" default:"false"` EnableReplicaLoadBalancer bool `name:"enable_replica_load_balancer" default:"false"` MasterDNSNameFormat stringTemplate `name:"master_dns_name_format" default:"{cluster}.{team}.{hostedzone}"` ReplicaDNSNameFormat stringTemplate `name:"replica_dns_name_format" default:"{cluster}-repl.{team}.{hostedzone}"` From 386d7b6bdb0b98179040ef30f785d4a3f8657f0c Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Fri, 16 Mar 2018 13:19:26 +0100 Subject: [PATCH 15/21] Implement backward compatibility with older load balancer settings --- README.md | 2 ++ pkg/cluster/k8sres.go | 13 +++++++++++++ pkg/spec/postgresql.go | 25 ++++++++++++++++--------- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 44bbdf4f2..b12949b5b 100644 --- a/README.md +++ b/README.md @@ -237,6 +237,8 @@ For instance, of a cluster manifest has 1 instance and the min_instances is set For any Postgresql/Spilo cluster an operator creates two separate k8s services: one for the master pod and one for replica pods. To expose these services to an outer network, one can attach load balancers to them by setting `enableMasterLoadBalancer` and/or `enableReplicaLoadBalancer` to `true` in the cluster manifest. In the case any of these variables is omitted from the manifest, the operator configmap's settings `enable_master_load_balancer` and `enable_replica_load_balancer` apply. Note that the operator settings affect all Postgresql services running in a namespace watched by the operator. +For backward compatibility with already configured clusters we maintain in a cluster manifest older parameter names, namely `useLoadBalancer` for enabling the master service's load balancer and `replicaLoadBalancer` for the replica service. If set, these params take precedence over the newer `enableMasterLoadBalancer` and `enableReplicaLoadBalancer`. Note that in older versions of the operator (before PR #258) `replicaLoadBalancer` was responsible for both creating the replica service and attaching an LB to it; now the service is always created (since k8s service typically is free in the cloud setting), and this param only attaches an LB (that typically costs money). + # Setup development environment The following steps guide you through the setup to work on the operator itself. diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 0fe7dfa58..4bc399481 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -675,18 +675,31 @@ func (c *Cluster) shouldCreateLoadBalancerForService(role PostgresRole, spec *sp case Replica: + // deprecated option takes priority for backward compatibility + if spec.ReplicaLoadBalancer != nil { + c.logger.Debugf("The Postgres manifest for the cluster %v sets the deprecated `replicaLoadBalancer` param. Consider using the `enableReplicaLoadBalancer` instead.", c.Name) + return *spec.ReplicaLoadBalancer + } + // if the value is explicitly set in a Postgresql manifest, follow this setting if spec.EnableReplicaLoadBalancer != nil { return *spec.EnableReplicaLoadBalancer } + // otherwise, follow the operator configuration return c.OpConfig.EnableReplicaLoadBalancer case Master: + if spec.UseLoadBalancer != nil { + c.logger.Debugf("The Postgres manifest for the cluster %v sets the deprecated `useLoadBalancer` param. Consider using the `enableMasterLoadBalancer` instead.", c.Name) + return *spec.UseLoadBalancer + } + if spec.EnableMasterLoadBalancer != nil { return *spec.EnableMasterLoadBalancer } + return c.OpConfig.EnableMasterLoadBalancer default: diff --git a/pkg/spec/postgresql.go b/pkg/spec/postgresql.go index a948f1698..1b47ca162 100644 --- a/pkg/spec/postgresql.go +++ b/pkg/spec/postgresql.go @@ -96,17 +96,24 @@ type PostgresSpec struct { TeamID string `json:"teamId"` AllowedSourceRanges []string `json:"allowedSourceRanges"` DockerImage string `json:"dockerImage,omitempty"` + // vars that enable load balancers are pointers because it is important to know if any of them is omitted from the Postgres manifest // in that case the var evaluates to nil and the value is taken from the operator config - EnableMasterLoadBalancer *bool `json:"enableMasterLoadBalancer,omitempty"` - EnableReplicaLoadBalancer *bool `json:"enableReplicaLoadBalancer,omitempty"` - NumberOfInstances int32 `json:"numberOfInstances"` - Users map[string]UserFlags `json:"users"` - MaintenanceWindows []MaintenanceWindow `json:"maintenanceWindows,omitempty"` - Clone CloneDescription `json:"clone"` - ClusterName string `json:"-"` - Databases map[string]string `json:"databases,omitempty"` - Tolerations []v1.Toleration `json:"tolerations,omitempty"` + EnableMasterLoadBalancer *bool `json:"enableMasterLoadBalancer,omitempty"` + EnableReplicaLoadBalancer *bool `json:"enableReplicaLoadBalancer,omitempty"` + + // deprecated load balancer settings mantained for backward compatibility + // see "Load balancers" operator docs (PR #258) + UseLoadBalancer *bool `json:"useLoadBalancer,omitempty"` + ReplicaLoadBalancer *bool `json:"replicaLoadBalancer,omitempty"` + + NumberOfInstances int32 `json:"numberOfInstances"` + Users map[string]UserFlags `json:"users"` + MaintenanceWindows []MaintenanceWindow `json:"maintenanceWindows,omitempty"` + Clone CloneDescription `json:"clone"` + ClusterName string `json:"-"` + Databases map[string]string `json:"databases,omitempty"` + Tolerations []v1.Toleration `json:"tolerations,omitempty"` } // PostgresqlList defines a list of PostgreSQL clusters. From 931b48fcbb5035fc7a5ea9230d6cc3137dab747a Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Fri, 16 Mar 2018 15:36:42 +0100 Subject: [PATCH 16/21] Respond to code reviews --- manifests/complete-postgres-manifest.yaml | 2 +- pkg/spec/postgresql.go | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/manifests/complete-postgres-manifest.yaml b/manifests/complete-postgres-manifest.yaml index 5663dcbc2..b8f8d4906 100644 --- a/manifests/complete-postgres-manifest.yaml +++ b/manifests/complete-postgres-manifest.yaml @@ -12,7 +12,7 @@ spec: zalando: - superuser - createdb - enableMasterLoadBalancer: true + # enableMasterLoadBalancer: true # enableReplicaLoadBalancer: true allowedSourceRanges: # load balancers' source ranges for both master and replica services - 127.0.0.1/32 diff --git a/pkg/spec/postgresql.go b/pkg/spec/postgresql.go index 1b47ca162..7dac7992b 100644 --- a/pkg/spec/postgresql.go +++ b/pkg/spec/postgresql.go @@ -93,9 +93,8 @@ type PostgresSpec struct { Patroni `json:"patroni,omitempty"` Resources `json:"resources,omitempty"` - TeamID string `json:"teamId"` - AllowedSourceRanges []string `json:"allowedSourceRanges"` - DockerImage string `json:"dockerImage,omitempty"` + TeamID string `json:"teamId"` + DockerImage string `json:"dockerImage,omitempty"` // vars that enable load balancers are pointers because it is important to know if any of them is omitted from the Postgres manifest // in that case the var evaluates to nil and the value is taken from the operator config @@ -103,10 +102,13 @@ type PostgresSpec struct { EnableReplicaLoadBalancer *bool `json:"enableReplicaLoadBalancer,omitempty"` // deprecated load balancer settings mantained for backward compatibility - // see "Load balancers" operator docs (PR #258) + // see "Load balancers" operator docs UseLoadBalancer *bool `json:"useLoadBalancer,omitempty"` ReplicaLoadBalancer *bool `json:"replicaLoadBalancer,omitempty"` + // load balancers' source ranges are the same for master and replica services + AllowedSourceRanges []string `json:"allowedSourceRanges"` + NumberOfInstances int32 `json:"numberOfInstances"` Users map[string]UserFlags `json:"users"` MaintenanceWindows []MaintenanceWindow `json:"maintenanceWindows,omitempty"` From a8862aeee14705e0660c1279276da41d0ea3f6ef Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Mon, 19 Mar 2018 17:09:26 +0100 Subject: [PATCH 17/21] Enable backward compatibility for enable_load_balancer setting from operator configmap --- README.md | 2 ++ manifests/configmap.yaml | 4 ++- pkg/cluster/k8sres.go | 7 ++++-- pkg/util/config/config.go | 52 ++++++++++++++++++++------------------- 4 files changed, 37 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index b12949b5b..3039a0dbe 100644 --- a/README.md +++ b/README.md @@ -239,6 +239,8 @@ For any Postgresql/Spilo cluster an operator creates two separate k8s services: For backward compatibility with already configured clusters we maintain in a cluster manifest older parameter names, namely `useLoadBalancer` for enabling the master service's load balancer and `replicaLoadBalancer` for the replica service. If set, these params take precedence over the newer `enableMasterLoadBalancer` and `enableReplicaLoadBalancer`. Note that in older versions of the operator (before PR #258) `replicaLoadBalancer` was responsible for both creating the replica service and attaching an LB to it; now the service is always created (since k8s service typically is free in the cloud setting), and this param only attaches an LB (that typically costs money). +For the same reason of compatibility, we maintain the `enable_load_balancer` setting in the operator config map that was previously used to attach a LB to the master service. Its value is examined after the deprecated `useLoadBalancer` setting from the Postgresql manifest but before the recommended `enableMasterLoadBalancer`. There is no equivalent option for the replica service since the service used to be always created with a load balancer. + # Setup development environment The following steps guide you through the setup to work on the operator itself. diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index edb308a37..be3fddf3b 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -36,7 +36,9 @@ data: team_admin_role: "admin" teams_api_url: http://fake-teams-api.default.svc.cluster.local workers: "4" - enable_master_load_balancer: "true" + # turn on/off load balancers for all Postgres clusters managed by the operator + # LB settings in cluster manifests take priority over these settings + # enable_master_load_balancer: "true" # enable_replica_load_balancer: "true" api_port: "8080" ring_log_lines: "100" diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 4bc399481..d0b81a040 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -696,8 +696,11 @@ func (c *Cluster) shouldCreateLoadBalancerForService(role PostgresRole, spec *sp return *spec.UseLoadBalancer } - if spec.EnableMasterLoadBalancer != nil { - return *spec.EnableMasterLoadBalancer + // `enable_load_balancer`` governs LB for a master service + // there is no equivalent operator configmap option for the replica LB + if c.OpConfig.EnableLoadBalancer != nil { + c.logger.Debugf("The operator configmap sets the deprecated `enable_load_balancer` param. Consider using the `enable_master_load_balancer` or `enable_replica_load_balancer` instead.", c.Name) + return *c.OpConfig.EnableLoadBalancer } return c.OpConfig.EnableMasterLoadBalancer diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index a9788221c..2127e6038 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -67,31 +67,33 @@ type Config struct { Resources Auth Scalyr - WatchedNamespace string `name:"watched_namespace"` // special values: "*" means 'watch all namespaces', the empty string "" means 'watch a namespace where operator is deployed to' - EtcdHost string `name:"etcd_host" default:"etcd-client.default.svc.cluster.local:2379"` - DockerImage string `name:"docker_image" default:"registry.opensource.zalan.do/acid/spiloprivate-9.6:1.2-p4"` - ServiceAccountName string `name:"service_account_name" default:"operator"` - DbHostedZone string `name:"db_hosted_zone" default:"db.example.com"` - EtcdScope string `name:"etcd_scope" default:"service"` - WALES3Bucket string `name:"wal_s3_bucket"` - KubeIAMRole string `name:"kube_iam_role"` - DebugLogging bool `name:"debug_logging" default:"true"` - EnableDBAccess bool `name:"enable_database_access" default:"true"` - EnableTeamsAPI bool `name:"enable_teams_api" default:"true"` - EnableTeamSuperuser bool `name:"enable_team_superuser" default:"false"` - TeamAdminRole string `name:"team_admin_role" default:"admin"` - EnableMasterLoadBalancer bool `name:"enable_master_load_balancer" default:"false"` - EnableReplicaLoadBalancer bool `name:"enable_replica_load_balancer" default:"false"` - MasterDNSNameFormat stringTemplate `name:"master_dns_name_format" default:"{cluster}.{team}.{hostedzone}"` - ReplicaDNSNameFormat stringTemplate `name:"replica_dns_name_format" default:"{cluster}-repl.{team}.{hostedzone}"` - PDBNameFormat stringTemplate `name:"pdb_name_format" default:"postgres-{cluster}-pdb"` - Workers uint32 `name:"workers" default:"4"` - APIPort int `name:"api_port" default:"8080"` - RingLogLines int `name:"ring_log_lines" default:"100"` - ClusterHistoryEntries int `name:"cluster_history_entries" default:"1000"` - TeamAPIRoleConfiguration map[string]string `name:"team_api_role_configuration" default:"log_statement:all"` - PodTerminateGracePeriod time.Duration `name:"pod_terminate_grace_period" default:"5m"` - ProtectedRoles []string `name:"protected_role_names" default:"admin"` + WatchedNamespace string `name:"watched_namespace"` // special values: "*" means 'watch all namespaces', the empty string "" means 'watch a namespace where operator is deployed to' + EtcdHost string `name:"etcd_host" default:"etcd-client.default.svc.cluster.local:2379"` + DockerImage string `name:"docker_image" default:"registry.opensource.zalan.do/acid/spiloprivate-9.6:1.2-p4"` + ServiceAccountName string `name:"service_account_name" default:"operator"` + DbHostedZone string `name:"db_hosted_zone" default:"db.example.com"` + EtcdScope string `name:"etcd_scope" default:"service"` + WALES3Bucket string `name:"wal_s3_bucket"` + KubeIAMRole string `name:"kube_iam_role"` + DebugLogging bool `name:"debug_logging" default:"true"` + EnableDBAccess bool `name:"enable_database_access" default:"true"` + EnableTeamsAPI bool `name:"enable_teams_api" default:"true"` + EnableTeamSuperuser bool `name:"enable_team_superuser" default:"false"` + TeamAdminRole string `name:"team_admin_role" default:"admin"` + EnableMasterLoadBalancer bool `name:"enable_master_load_balancer" default:"false"` + EnableReplicaLoadBalancer bool `name:"enable_replica_load_balancer" default:"false"` + // deprecated and kept for backward compatibility + EnableLoadBalancer *bool `name:"enable_load_balancer" default:"true"` + MasterDNSNameFormat stringTemplate `name:"master_dns_name_format" default:"{cluster}.{team}.{hostedzone}"` + ReplicaDNSNameFormat stringTemplate `name:"replica_dns_name_format" default:"{cluster}-repl.{team}.{hostedzone}"` + PDBNameFormat stringTemplate `name:"pdb_name_format" default:"postgres-{cluster}-pdb"` + Workers uint32 `name:"workers" default:"4"` + APIPort int `name:"api_port" default:"8080"` + RingLogLines int `name:"ring_log_lines" default:"100"` + ClusterHistoryEntries int `name:"cluster_history_entries" default:"1000"` + TeamAPIRoleConfiguration map[string]string `name:"team_api_role_configuration" default:"log_statement:all"` + PodTerminateGracePeriod time.Duration `name:"pod_terminate_grace_period" default:"5m"` + ProtectedRoles []string `name:"protected_role_names" default:"admin"` } // MustMarshal marshals the config or panics From ced770a827b41f2eb9fe7bb04b018aff5bcf76d2 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Mon, 26 Mar 2018 11:07:32 +0200 Subject: [PATCH 18/21] Respond to code review --- pkg/cluster/k8sres.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index d0b81a040..18a2253ae 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -697,7 +697,7 @@ func (c *Cluster) shouldCreateLoadBalancerForService(role PostgresRole, spec *sp } // `enable_load_balancer`` governs LB for a master service - // there is no equivalent operator configmap option for the replica LB + // there is no equivalent deprecated operator option for the replica LB if c.OpConfig.EnableLoadBalancer != nil { c.logger.Debugf("The operator configmap sets the deprecated `enable_load_balancer` param. Consider using the `enable_master_load_balancer` or `enable_replica_load_balancer` instead.", c.Name) return *c.OpConfig.EnableLoadBalancer @@ -736,7 +736,6 @@ func (c *Cluster) generateService(role PostgresRole, spec *spec.PostgresSpec) *v // safe default value: lock load balancer to only local address unless overridden explicitly. sourceRanges := []string{localHost} - // source ranges are the same for balancers for master and replica services allowedSourceRanges := spec.AllowedSourceRanges if len(allowedSourceRanges) >= 0 { sourceRanges = allowedSourceRanges From 96d46252f551ec9b930af94f2f7dfd01997c3dfd Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Mon, 26 Mar 2018 11:43:46 +0200 Subject: [PATCH 19/21] Change the default values to closer match previous behaviour --- manifests/complete-postgres-manifest.yaml | 4 ++-- manifests/configmap.yaml | 4 ++-- pkg/util/config/config.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/manifests/complete-postgres-manifest.yaml b/manifests/complete-postgres-manifest.yaml index b8f8d4906..68bda81cf 100644 --- a/manifests/complete-postgres-manifest.yaml +++ b/manifests/complete-postgres-manifest.yaml @@ -12,8 +12,8 @@ spec: zalando: - superuser - createdb - # enableMasterLoadBalancer: true - # enableReplicaLoadBalancer: true + enableMasterLoadBalancer: true + enableReplicaLoadBalancer: true allowedSourceRanges: # load balancers' source ranges for both master and replica services - 127.0.0.1/32 databases: diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index be3fddf3b..49ec872e9 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -38,8 +38,8 @@ data: workers: "4" # turn on/off load balancers for all Postgres clusters managed by the operator # LB settings in cluster manifests take priority over these settings - # enable_master_load_balancer: "true" - # enable_replica_load_balancer: "true" + enable_master_load_balancer: "true" + enable_replica_load_balancer: "false" api_port: "8080" ring_log_lines: "100" cluster_history_entries: "1000" diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 2127e6038..0e653ce0b 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -80,7 +80,7 @@ type Config struct { EnableTeamsAPI bool `name:"enable_teams_api" default:"true"` EnableTeamSuperuser bool `name:"enable_team_superuser" default:"false"` TeamAdminRole string `name:"team_admin_role" default:"admin"` - EnableMasterLoadBalancer bool `name:"enable_master_load_balancer" default:"false"` + EnableMasterLoadBalancer bool `name:"enable_master_load_balancer" default:"true"` EnableReplicaLoadBalancer bool `name:"enable_replica_load_balancer" default:"false"` // deprecated and kept for backward compatibility EnableLoadBalancer *bool `name:"enable_load_balancer" default:"true"` From 8967a3be2c4838b5a0ac505e5b91ee8f402bfae4 Mon Sep 17 00:00:00 2001 From: erthalion <9erthalion6@gmail.com> Date: Tue, 27 Mar 2018 12:11:46 +0200 Subject: [PATCH 20/21] Add tests for load balancer function logic --- pkg/cluster/k8sres_test.go | 94 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 pkg/cluster/k8sres_test.go diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go new file mode 100644 index 000000000..43405a2ec --- /dev/null +++ b/pkg/cluster/k8sres_test.go @@ -0,0 +1,94 @@ +package cluster + +import ( + "github.com/zalando-incubator/postgres-operator/pkg/spec" + "github.com/zalando-incubator/postgres-operator/pkg/util/config" + "github.com/zalando-incubator/postgres-operator/pkg/util/k8sutil" + "testing" +) + +func True() *bool { + b := true + return &b +} + +func False() *bool { + b := false + return &b +} + +func TestCreateLoadBalancerLogic(t *testing.T) { + var cluster = New( + Config{ + OpConfig: config.Config{ + ProtectedRoles: []string{"admin"}, + Auth: config.Auth{ + SuperUsername: superUserName, + ReplicationUsername: replicationUserName, + }, + }, + }, k8sutil.KubernetesClient{}, spec.Postgresql{}, logger) + + testName := "TestCreateLoadBalancerLogic" + tests := []struct { + subtest string + role PostgresRole + spec *spec.PostgresSpec + opConfig config.Config + result bool + }{ + { + subtest: "new format, load balancer is enabled for replica", + role: Replica, + spec: &spec.PostgresSpec{EnableReplicaLoadBalancer: True()}, + opConfig: config.Config{}, + result: true, + }, + { + subtest: "new format, load balancer is disabled for replica", + role: Replica, + spec: &spec.PostgresSpec{EnableReplicaLoadBalancer: False()}, + opConfig: config.Config{}, + result: false, + }, + { + subtest: "new format, load balancer isn't specified for replica", + role: Replica, + spec: &spec.PostgresSpec{EnableReplicaLoadBalancer: nil}, + opConfig: config.Config{EnableReplicaLoadBalancer: true}, + result: true, + }, + { + subtest: "new format, load balancer isn't specified for replica", + role: Replica, + spec: &spec.PostgresSpec{EnableReplicaLoadBalancer: nil}, + opConfig: config.Config{EnableReplicaLoadBalancer: false}, + result: false, + }, + { + subtest: "old format, load balancer is enabled for replica", + role: Replica, + spec: &spec.PostgresSpec{ReplicaLoadBalancer: True()}, + opConfig: config.Config{}, + result: true, + }, + { + subtest: "old format has priority", + role: Replica, + spec: &spec.PostgresSpec{ + ReplicaLoadBalancer: True(), + EnableReplicaLoadBalancer: False(), + }, + opConfig: config.Config{}, + result: true, + }, + } + for _, tt := range tests { + cluster.OpConfig = tt.opConfig + result := cluster.shouldCreateLoadBalancerForService(tt.role, tt.spec) + if tt.result != result { + t.Errorf("%s %s: Load balancer is %t, expect %t for role %#v and spec %#v", + testName, tt.subtest, result, tt.result, tt.role, tt.spec) + } + } +} From 34518a4eb03a30878d4fd56cc43851ff58db7c3e Mon Sep 17 00:00:00 2001 From: erthalion <9erthalion6@gmail.com> Date: Tue, 27 Mar 2018 16:05:13 +0200 Subject: [PATCH 21/21] Some fixes for travis ci and cdp remove cmd package from travis (it's complaining because there are no tests), and add apt-get update for cdp. --- .travis.yml | 2 +- delivery.yaml | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 199408aea..f22275d9c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,4 +17,4 @@ install: - make deps script: - - travis_wait 20 goveralls -service=travis-ci -package ./pkg/... -package ./cmd/... -v + - travis_wait 20 goveralls -service=travis-ci -package ./pkg/... -v diff --git a/delivery.yaml b/delivery.yaml index d2b176234..502aa75b2 100644 --- a/delivery.yaml +++ b/delivery.yaml @@ -6,6 +6,9 @@ pipeline: GOPATH: /root/go OPERATOR_TOP_DIR: /root/go/src/github.com/zalando-incubator commands: + - desc: 'Update' + cmd: | + apt-get update - desc: 'Install required build software' cmd: | apt-get install -y make git apt-transport-https ca-certificates curl