From 90db0e545b7f0850866024a3569fd2438818b476 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Thu, 8 Nov 2018 13:49:30 +0100 Subject: [PATCH] Add set_memory_request_to_limit option --- README.md | 2 ++ docs/administrator.md | 6 ++--- docs/reference/operator_parameters.md | 3 +++ manifests/complete-postgres-manifest.yaml | 4 ++-- manifests/configmap.yaml | 1 + pkg/apis/acid.zalan.do/v1/postgresql_type.go | 4 ++-- pkg/apis/acid.zalan.do/v1/util_test.go | 4 ++-- .../acid.zalan.do/v1/zz_generated.deepcopy.go | 2 +- pkg/cluster/k8sres.go | 24 ++++++++++++++----- pkg/controller/controller.go | 15 ++++++++++++ pkg/util/config/config.go | 1 + 11 files changed, 50 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 18ea97538..13adae24d 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,8 @@ cd postgres-operator ./run_operator_locally.sh ``` +Note we provide the `/manifests` directory as an example only; you should consider adjusting the manifests to your particular setting. + ## Running and testing the operator The best way to test the operator is to run it locally in [minikube](https://kubernetes.io/docs/getting-started-guides/minikube/). See developer docs(`docs/developer.yaml`) for details. diff --git a/docs/administrator.md b/docs/administrator.md index 83594ef1f..f2c747cce 100644 --- a/docs/administrator.md +++ b/docs/administrator.md @@ -41,12 +41,12 @@ manifests: ```bash $ kubectl create namespace test - $ kubectl config set-context --namespace=test + $ kubectl config set-context $(kubectl config current-context) --namespace=test ``` All subsequent `kubectl` commands will work with the `test` namespace. The -operator will run in this namespace and look up needed resources - such as its -config map - there. +operator will run in this namespace and look up needed resources - such as its +config map - there. Please note that the namespace for service accounts and cluster role bindings in [operator RBAC rules](manifests/operator-service-account-rbac.yaml) needs to be adjusted to the non-default value. ## Specify the namespace to watch diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index 76109c890..168c1e9b1 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -221,6 +221,9 @@ CRD-based configuration. memory limits for the postgres containers, unless overridden by cluster-specific settings. The default is `1Gi`. +* **set_memory_request_to_limit** + Set `memory_request` to `memory_limit` for all Postgres clusters (the default value is also increased). This prevents certain cases of memory overcommitment at the cost of overprovisioning memory and potential scheduling problems for Postgres pods with high memory limits due to the lack of memory on Kubernetes cluster nodes. This affects sidecar containers as well. The default is `false`. + ## Operator timeouts This set of parameters define various timeouts related to some operator diff --git a/manifests/complete-postgres-manifest.yaml b/manifests/complete-postgres-manifest.yaml index 60d39c94b..e0f76e4d4 100644 --- a/manifests/complete-postgres-manifest.yaml +++ b/manifests/complete-postgres-manifest.yaml @@ -6,7 +6,7 @@ metadata: spec: teamId: "ACID" volume: - size: 5Gi + size: 1Gi numberOfInstances: 2 users: #Application/Robot users zalando: @@ -31,7 +31,7 @@ spec: memory: 100Mi limits: cpu: 300m - memory: 3000Mi + memory: 300Mi patroni: initdb: encoding: "UTF8" diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index ed7652907..44202ca37 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -15,6 +15,7 @@ data: secret_name_template: '{username}.{cluster}.credentials' super_username: postgres enable_teams_api: "false" + set_memory_request_to_limit: "true" # postgres_superuser_teams: "postgres_superusers" # enable_team_superuser: "false" # team_admin_role: "admin" diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go index 380ba68d7..aedb0512f 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go +++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go @@ -90,8 +90,8 @@ type ResourceDescription struct { // Resources describes requests and limits for the cluster resouces. type Resources struct { - ResourceRequest ResourceDescription `json:"requests,omitempty"` - ResourceLimits ResourceDescription `json:"limits,omitempty"` + ResourceRequests ResourceDescription `json:"requests,omitempty"` + ResourceLimits ResourceDescription `json:"limits,omitempty"` } // Patroni contains Patroni-specific configuration diff --git a/pkg/apis/acid.zalan.do/v1/util_test.go b/pkg/apis/acid.zalan.do/v1/util_test.go index 99e3f2b7c..b6a27542c 100644 --- a/pkg/apis/acid.zalan.do/v1/util_test.go +++ b/pkg/apis/acid.zalan.do/v1/util_test.go @@ -240,8 +240,8 @@ var unmarshalCluster = []struct { Slots: map[string]map[string]string{"permanent_logical_1": {"type": "logical", "database": "foo", "plugin": "pgoutput"}}, }, Resources: Resources{ - ResourceRequest: ResourceDescription{CPU: "10m", Memory: "50Mi"}, - ResourceLimits: ResourceDescription{CPU: "300m", Memory: "3000Mi"}, + ResourceRequests: ResourceDescription{CPU: "10m", Memory: "50Mi"}, + ResourceLimits: ResourceDescription{CPU: "300m", Memory: "3000Mi"}, }, TeamID: "ACID", 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 4496f8c0a..55a5ac075 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -573,7 +573,7 @@ func (in *ResourceDescription) DeepCopy() *ResourceDescription { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Resources) DeepCopyInto(out *Resources) { *out = *in - out.ResourceRequest = in.ResourceRequest + out.ResourceRequests = in.ResourceRequests out.ResourceLimits = in.ResourceLimits return } diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 66ac55388..3f1795915 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -92,18 +92,18 @@ func (c *Cluster) makeDefaultResources() acidv1.Resources { defaultRequests := acidv1.ResourceDescription{CPU: config.DefaultCPURequest, Memory: config.DefaultMemoryRequest} defaultLimits := acidv1.ResourceDescription{CPU: config.DefaultCPULimit, Memory: config.DefaultMemoryLimit} - return acidv1.Resources{ResourceRequest: defaultRequests, ResourceLimits: defaultLimits} + return acidv1.Resources{ResourceRequests: defaultRequests, ResourceLimits: defaultLimits} } func generateResourceRequirements(resources acidv1.Resources, defaultResources acidv1.Resources) (*v1.ResourceRequirements, error) { var err error - specRequests := resources.ResourceRequest + specRequests := resources.ResourceRequests specLimits := resources.ResourceLimits result := v1.ResourceRequirements{} - result.Requests, err = fillResourceList(specRequests, defaultResources.ResourceRequest) + result.Requests, err = fillResourceList(specRequests, defaultResources.ResourceRequests) if err != nil { return nil, fmt.Errorf("could not fill resource requests: %v", err) } @@ -377,8 +377,8 @@ func generateSidecarContainers(sidecars []acidv1.Sidecar, resources, err := generateResourceRequirements( makeResources( - sidecar.Resources.ResourceRequest.CPU, - sidecar.Resources.ResourceRequest.Memory, + sidecar.Resources.ResourceRequests.CPU, + sidecar.Resources.ResourceRequests.Memory, sidecar.Resources.ResourceLimits.CPU, sidecar.Resources.ResourceLimits.Memory, ), @@ -625,7 +625,7 @@ func getBucketScopeSuffix(uid string) string { func makeResources(cpuRequest, memoryRequest, cpuLimit, memoryLimit string) acidv1.Resources { return acidv1.Resources{ - ResourceRequest: acidv1.ResourceDescription{ + ResourceRequests: acidv1.ResourceDescription{ CPU: cpuRequest, Memory: memoryRequest, }, @@ -644,6 +644,18 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*v1beta1.State podTemplate *v1.PodTemplateSpec volumeClaimTemplate *v1.PersistentVolumeClaim ) + + if c.OpConfig.SetMemoryRequestToLimit { + + if spec.Resources.ResourceLimits.Memory > spec.Resources.ResourceRequests.Memory { + c.logger.Warningf("The memory request of %v for the Postgres container is increased to match the memory limit of %v.", spec.Resources.ResourceRequests.Memory, spec.Resources.ResourceLimits.Memory) + spec.Resources.ResourceRequests.Memory = spec.Resources.ResourceLimits.Memory + } + + // controller adjusts default and Spilo sidecar container requests (those do not need Sync) + + } + defaultResources := c.makeDefaultResources() resourceRequirements, err := generateResourceRequirements(spec.Resources, defaultResources) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 1bc4e08e0..8bff4cba1 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -110,6 +110,21 @@ func (c *Controller) initOperatorConfig() { c.opConfig = config.NewFromMap(configMapData) c.warnOnDeprecatedOperatorParameters() + if c.opConfig.SetMemoryRequestToLimit { + + if c.opConfig.DefaultMemoryLimit > c.opConfig.DefaultMemoryRequest { + c.logger.Warningf("The default memory request of %v for Postgres containers is increased to match the default memory limit of %v.", c.opConfig.DefaultMemoryRequest, c.opConfig.DefaultMemoryLimit) + c.opConfig.DefaultMemoryRequest = c.opConfig.DefaultMemoryLimit + } + + if c.opConfig.ScalyrMemoryLimit > c.opConfig.ScalyrMemoryRequest { + c.logger.Warningf("The memory request of %v for the Scalyr sidecar container is increased to match the memory limit of %v.", c.opConfig.ScalyrMemoryRequest, c.opConfig.ScalyrMemoryLimit) + c.opConfig.ScalyrMemoryRequest = c.opConfig.ScalyrMemoryLimit + } + + // generateStatefulSet adjusts values for individual Postgres clusters + } + } func (c *Controller) modifyConfigFromEnvironment() { diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 92fd3fd73..2bd7924ad 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -104,6 +104,7 @@ type Config struct { PodTerminateGracePeriod time.Duration `name:"pod_terminate_grace_period" default:"5m"` ProtectedRoles []string `name:"protected_role_names" default:"admin"` PostgresSuperuserTeams []string `name:"postgres_superuser_teams" default:""` + SetMemoryRequestToLimit bool `name:"set_memory_request_to_limit" defaults:"false"` } // MustMarshal marshals the config or panics