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/README.md b/README.md index 7ca8e9963..4b2799053 100644 --- a/README.md +++ b/README.md @@ -242,6 +242,14 @@ 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. + +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/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 diff --git a/manifests/complete-postgres-manifest.yaml b/manifests/complete-postgres-manifest.yaml index f9923f350..68bda81cf 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 + allowedSourceRanges: # load balancers' source ranges for both master and replica services - 127.0.0.1/32 databases: foo: zalando diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index d630fb42d..7d5c742c8 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -37,7 +37,10 @@ data: team_admin_role: "admin" teams_api_url: http://fake-teams-api.default.svc.cluster.local workers: "4" - enable_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: "false" api_port: "8080" ring_log_lines: "100" cluster_history_entries: "1000" diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index e2bb86d57..ea82265a7 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) } @@ -480,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) @@ -589,9 +586,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) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 2cd956950..d16706842 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -669,6 +669,48 @@ func (c *Cluster) generateSingleUserSecret(namespace string, pgUser spec.PgUser) return &secret } +func (c *Cluster) shouldCreateLoadBalancerForService(role PostgresRole, spec *spec.PostgresSpec) bool { + + switch role { + + 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 + } + + // `enable_load_balancer`` governs LB for a master service + // 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 + } + + return c.OpConfig.EnableMasterLoadBalancer + + default: + panic(fmt.Sprintf("Unknown role %v", role)) + } + +} + func (c *Cluster) generateService(role PostgresRole, spec *spec.PostgresSpec) *v1.Service { var dnsName string @@ -689,12 +731,11 @@ 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) { // safe default value: lock load balancer to only local address unless overridden explicitly. sourceRanges := []string{localHost} + allowedSourceRanges := spec.AllowedSourceRanges if len(allowedSourceRanges) >= 0 { sourceRanges = allowedSourceRanges @@ -707,6 +748,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{ 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) + } + } +} diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 955253f1a..5a77e658b 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) @@ -133,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 { @@ -165,11 +155,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 @@ -178,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 { diff --git a/pkg/spec/postgresql.go b/pkg/spec/postgresql.go index e124c10c1..7dac7992b 100644 --- a/pkg/spec/postgresql.go +++ b/pkg/spec/postgresql.go @@ -93,19 +93,29 @@ type PostgresSpec struct { Patroni `json:"patroni,omitempty"` Resources `json:"resources,omitempty"` - TeamID string `json:"teamId"` + 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 + EnableMasterLoadBalancer *bool `json:"enableMasterLoadBalancer,omitempty"` + EnableReplicaLoadBalancer *bool `json:"enableReplicaLoadBalancer,omitempty"` + + // deprecated load balancer settings mantained for backward compatibility + // 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"` - DockerImage string `json:"dockerImage,omitempty"` - // EnableLoadBalancer is a pointer, since it is important to know if that parameters is omitted from the manifest - 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"` + + 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. diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 2c83b36b3..0e653ce0b 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -67,20 +67,23 @@ 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"` + 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"` + // 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"`