Allow for user to specify memory limits for certain containers

With the work on memory management for this release, one of the
discoveries made involved how the OOM killer on Kubelets would interact
with PostgreSQL instances (as well as pgBackRest, pgBouncer) in ways
that could lead to some undesired behavior. As such, the ability to
implicitly set memory limits was removed for the time
being.

However, there may be cases where one may want to set a memory limit,
along side a memory request + CPU request/limit to get a Guaranteed
QoS on a Pod, perhaps for pgBouncer or pgBackRest. This patch allows
for this via the `--enable-memory-limit` flag, specifically:

  pgo create cluster --enable-memory-limit
  pgo create cluster --enable-pgbackrest-memory-limit
  pgo create cluster --enable-pgbouncer-memory-limit
  pgo update cluster --enable-memory-limit
  pgo update cluster --disable-memory-limit
  pgo update cluster --enable-pgbackrest-memory-limit
  pgo update cluster --disable-pgbackrest-memory-limit
  pgo create pgbouncer --enable-memory-limit
  pgo update pgbouncer --enable-memory-limit
  pgo update pgbouncer --disable-memory-limit

The net effect is that if a memory request is present (and all of
PostgreSQL, pgBackRest, and pgBouncer have default values for memory
requests), the limit will also be set if it is enabled. By default, the
PostgreSQL Operator does not setting the memory limit per the behavioral
issues mentioned above.

If the memory limit is enabled, it is set to be identical to the memory
request.

This setting is also reactive, i.e. if you modify it in the CR directly,
the PostgreSQL Operator will roll out the changes to the managed Pods.

Issue: [ch8043]

Author: Jonathan S. Katz

apis/crunchydata.com/v1/cluster.go

@@ -88,9 +88,15 @@ type PgclusterSpec struct {
 	// Now, for CPU, we set both the Request and the Limit, based on how
 	// Kubernetes interacts with these parameters
 	Resources v1.ResourceList `json:"resources"`
+	// EnableMemoryLimit specifies whether or not the "Limit" for memory
+	// should be set on the Pod. Defaults to false
+	EnableMemoryLimit bool `json:"enableMemoryLimit"`
 	// BackrestResources, if specified, contains the container request resources
 	// for the pgBackRest Deployment for this PostgreSQL cluster
 	BackrestResources v1.ResourceList `json:"backrestResources"`
+	// EnableBackrestMemoryLimit specifies whether or not the "Limit" for memory
+	// should be set on the Pod. Defaults to false
+	EnableBackrestMemoryLimit bool `json:"enableBackrestMemoryLimit"`
 	// PgBouncer contains all of the settings to properly maintain a pgBouncer
 	// implementation
 	PgBouncer          PgBouncerSpec            `json:"pgBouncer"`
@@ -172,6 +178,9 @@ type PodAntiAffinitySpec struct {
 // - what resources it should consume
 // - the total number of replicas
 type PgBouncerSpec struct {
+	// EnableMemoryLimit specifies whether or not the "Limit" for memory
+	// should be set on the Pod. Defaults to false
+	EnableMemoryLimit bool `json:"enableMemoryLimit"`
 	// Replicas represents the total number of Pods to deploy with pgBouncer,
 	// which effectively enables/disables the pgBouncer.
 	//

apiserver/clusterservice/clusterimpl.go

@@ -1045,6 +1045,11 @@ func getClusterParams(request *msgs.CreateClusterRequest, name string, userLabel
 		spec.Resources[v1.ResourceMemory] = apiserver.Pgo.Cluster.DefaultInstanceResourceMemory
 	}
 
+	// determine if we should apply a memory limit for the PostgreSQL clusters
+	if request.MemoryLimitStatus == msgs.MemoryLimitEnable {
+		spec.EnableMemoryLimit = true
+	}
+
 	// similarly, if there are any overriding pgBackRest repository container
 	// resource request values, set them here
 	if request.BackrestCPURequest != "" {
@@ -1061,6 +1066,11 @@ func getClusterParams(request *msgs.CreateClusterRequest, name string, userLabel
 		spec.BackrestResources[v1.ResourceMemory] = apiserver.Pgo.Cluster.DefaultBackrestResourceMemory
 	}
 
+	// determine if we should apply a memory limit for the pgBackRest repository
+	if request.BackrestMemoryLimitStatus == msgs.MemoryLimitEnable {
+		spec.EnableBackrestMemoryLimit = true
+	}
+
 	// if the pgBouncer flag is set to true, indicate that the pgBouncer
 	// deployment should be made available in this cluster
 	if request.PgbouncerFlag {
@@ -1071,6 +1081,11 @@ func getClusterParams(request *msgs.CreateClusterRequest, name string, userLabel
 		if request.PgBouncerReplicas > 0 {
 			spec.PgBouncer.Replicas = request.PgBouncerReplicas
 		}
+
+		// determine if we should apply a memory limit for the pgBouncer instances
+		if request.PgBouncerMemoryLimitStatus == msgs.MemoryLimitEnable {
+			spec.PgBouncer.EnableMemoryLimit = true
+		}
 	}
 
 	// similarly, if there are any overriding pgBouncer container resource request
@@ -1716,6 +1731,14 @@ func UpdateCluster(request *msgs.UpdateClusterRequest) msgs.UpdateClusterRespons
 			cluster.Spec.Resources[v1.ResourceMemory] = quantity
 		}
 
+		// determine if the memory limit should be applied to PostgreSQL instances
+		switch request.MemoryLimitStatus {
+		case msgs.MemoryLimitEnable:
+			cluster.Spec.EnableMemoryLimit = true
+		case msgs.MemoryLimitDisable:
+			cluster.Spec.EnableMemoryLimit = false
+		}
+
 		// ensure there is a value for BackrestResources
 		if cluster.Spec.BackrestResources == nil {
 			cluster.Spec.BackrestResources = v1.ResourceList{}
@@ -1733,6 +1756,14 @@ func UpdateCluster(request *msgs.UpdateClusterRequest) msgs.UpdateClusterRespons
 			cluster.Spec.BackrestResources[v1.ResourceMemory] = quantity
 		}
 
+		// determine if the memory limit should be applied to pgBackRest
+		switch request.BackrestMemoryLimitStatus {
+		case msgs.MemoryLimitEnable:
+			cluster.Spec.EnableBackrestMemoryLimit = true
+		case msgs.MemoryLimitDisable:
+			cluster.Spec.EnableBackrestMemoryLimit = false
+		}
+
 		// extract the parameters for the TablespaceMounts and put them in the
 		// format that is required by the pgcluster CRD
 		for _, tablespace := range request.Tablespaces {

apiserver/pgbouncerservice/pgbouncerimpl.go

@@ -117,6 +117,11 @@ func CreatePgbouncer(request *msgs.CreatePgbouncerRequest, ns, pgouser string) m
 			resources[v1.ResourceMemory] = apiserver.Pgo.Cluster.DefaultPgBouncerResourceMemory
 		}
 
+		// determine if we should apply a memory limit for the pgBouncer instances
+		if request.MemoryLimitStatus == msgs.MemoryLimitEnable {
+			cluster.Spec.PgBouncer.EnableMemoryLimit = true
+		}
+
 		cluster.Spec.PgBouncer.Resources = resources
 
 		// update the cluster CRD with these udpates. If there is an error
@@ -184,6 +189,9 @@ func DeletePgbouncer(request *msgs.DeletePgbouncerRequest, ns string) msgs.Delet
 
 		// Disable the pgBouncer Deploymnet, whcih means setting Replicas to 0
 		cluster.Spec.PgBouncer.Replicas = 0
+		// Set the MemoryLimit of pgBouncer to false as well, as this is the default
+		// setting
+		cluster.Spec.PgBouncer.EnableMemoryLimit = false
 
 		// update the cluster CRD with these udpates. If there is an error
 		if err := kubeapi.Updatepgcluster(apiserver.RESTClient, &cluster, cluster.Name, request.Namespace); err != nil {
@@ -373,6 +381,14 @@ func UpdatePgBouncer(request *msgs.UpdatePgBouncerRequest, namespace, pgouser st
 			}
 		}
 
+		// determine if we should apply a memory limit for the pgBouncer instances
+		switch request.MemoryLimitStatus {
+		case msgs.MemoryLimitEnable:
+			cluster.Spec.PgBouncer.EnableMemoryLimit = true
+		case msgs.MemoryLimitDisable:
+			cluster.Spec.PgBouncer.EnableMemoryLimit = false
+		}
+
 		// apply the replica count number if there is a change, i.e. replicas is not
 		// 0
 		if request.Replicas > 0 {

apiservermsgs/clustermsgs.go

@@ -121,20 +121,35 @@ type CreateClusterRequest struct {
 	// CPURequest is the value of how much CPU should be requested for deploying
 	// the PostgreSQL cluster
 	CPURequest string
+	// MemoryLimitStatus determines whether or not the Kubernetes Resource Limit
+	// for memory should be set on the request. Defaults to "do nothing", which
+	// effectively disable setting the memory limit, which is currently the
+	// preferred behavior
+	MemoryLimitStatus MemoryLimitStatus
 	// MemoryRequest is the value of how much RAM should be requested for
 	// deploying the PostgreSQL cluster
 	MemoryRequest string
 	// PgBouncerCPURequest, if specified, is the value of how much CPU should be
 	// requested for deploying pgBouncer instances. Defaults to not being
 	// requested
 	PgBouncerCPURequest string
+	// PgBouncerMemoryLimitStatus determines whether or not the Kubernetes
+	// Resource Limit for memory should be set on the request. Defaults to
+	// "do nothing", which effectively disable setting the memory limit, which is
+	// currently the preferred behavior
+	PgBouncerMemoryLimitStatus MemoryLimitStatus
 	// PgBouncerMemoryRequest, if specified, is the value of how much RAM should
 	// be requested for deploying pgBouncer instances. Defaults to the server
 	// specified default
 	PgBouncerMemoryRequest string
 	// BackrestCPURequest, if specified, is the value of how much CPU should be
 	// requested the pgBackRest repository. Defaults to not being requested
 	BackrestCPURequest string
+	// BackrestMemoryLimitStatus determines whether or not the Kubernetes
+	// Resource Limit for memory should be set on the request. Defaults to
+	// "do nothing", which effectively disable setting the memory limit, which is
+	// currently the preferred behavior
+	BackrestMemoryLimitStatus MemoryLimitStatus
 	// BackrestMemoryRequest, if specified, is the value of how much RAM should
 	// be requested for the pgBackRest repository. Defaults to the server
 	// specified default
@@ -312,12 +327,20 @@ type UpdateClusterRequest struct {
 	// BackrestCPURequest, if specified, is the value of how much CPU should be
 	// requested the pgBackRest repository.
 	BackrestCPURequest string
+	// BackrestMemoryLimitStatus determines whether or not the Kubernetes
+	// Resource Limit for memory should be set on the request. Defaults to
+	// "do nothing", which does nothing
+	BackrestMemoryLimitStatus MemoryLimitStatus
 	// BackrestMemoryRequest, if specified, is the value of how much RAM should
 	// be requested for the pgBackRest repository.
 	BackrestMemoryRequest string
 	// CPURequest is the value of how much CPU should be requested for deploying
 	// the PostgreSQL cluster
 	CPURequest string
+	// MemoryLimitStatus determines whether or not the Kubernetes Resource Limit
+	// for memory should be set on the request. Defaults to "do nothing", which
+	// does nothing
+	MemoryLimitStatus MemoryLimitStatus
 	// MemoryRequest is the value of how much RAM should be requested for
 	// deploying the PostgreSQL cluster
 	MemoryRequest string

apiservermsgs/common.go

@@ -47,3 +47,14 @@ type BasicAuthCredentials struct {
 func (b BasicAuthCredentials) HasUsernameAndPassword() bool {
 	return len(b.Username) > 0 && len(b.Password) > 0
 }
+
+// set the types for updating the memory limit
+type MemoryLimitStatus int
+
+// set the different values around updating the "memory limit" parameter for
+// setting whether or not the memory limit for a Pod should be set
+const (
+	MemoryLimitDoNothing MemoryLimitStatus = iota
+	MemoryLimitEnable
+	MemoryLimitDisable
+)

apiservermsgs/pgbouncermsgs.go

@@ -24,6 +24,11 @@ type CreatePgbouncerRequest struct {
 	// requested for deploying pgBouncer instances. Defaults to not being
 	// requested
 	CPURequest string
+	// MemoryLimitStatus determines whether or not the Kubernetes Resource Limit
+	// for memory should be set on the request. Defaults to "do nothing", which
+	// effectively disable setting the memory limit, which is currently the
+	// preferred behavior
+	MemoryLimitStatus MemoryLimitStatus
 	// MemoryRequest, if specified, is the value of how much RAM should
 	// be requested for deploying pgBouncer instances. Defaults to the server
 	// specified default
@@ -150,6 +155,12 @@ type UpdatePgBouncerRequest struct {
 	// requested
 	CPURequest string
 
+	// MemoryLimitStatus determines whether or not the Kubernetes Resource Limit
+	// for memory should be set on the request. Defaults to "do nothing", which
+	// effectively disable setting the memory limit, which is currently the
+	// preferred behavior
+	MemoryLimitStatus MemoryLimitStatus
+
 	// MemoryRequest, if specified, is the value of how much RAM should
 	// be requested for deploying pgBouncer instances. Defaults to the server
 	// specified default

conf/postgres-operator/container-resources.json

@@ -1,10 +1,13 @@
 {{ if or .RequestsMemory .RequestsCPU }}
 "resources": {
-  {{ if .RequestsCPU }}
   "limits": {
-    "cpu": "{{.RequestsCPU}}"
+    {{ if .RequestsCPU }}
+    "cpu": "{{.RequestsCPU}}"{{ if and .EnableMemoryLimit .RequestsMemory }},{{ end }}
+    {{ end }}
+    {{ if and .EnableMemoryLimit .RequestsMemory }}
+    "memory": "{{.RequestsMemory}}"
+    {{ end }}
   },
-  {{ end }}
   "requests": {
     {{ if .RequestsCPU }}
     "cpu": "{{.RequestsCPU}}"{{ if .RequestsMemory }},{{ end }}

controller/pgcluster/pgclustercontroller.go

@@ -212,7 +212,8 @@ func (c *Controller) onUpdate(oldObj, newObj interface{}) {
 
 	// see if any of the resource values have changed, and if so, update them
 	if oldcluster.Spec.Resources[v1.ResourceCPU] != newcluster.Spec.Resources[v1.ResourceCPU] ||
-		oldcluster.Spec.Resources[v1.ResourceMemory] != newcluster.Spec.Resources[v1.ResourceMemory] {
+		oldcluster.Spec.Resources[v1.ResourceMemory] != newcluster.Spec.Resources[v1.ResourceMemory] ||
+		oldcluster.Spec.EnableMemoryLimit != newcluster.Spec.EnableMemoryLimit {
 		if err := clusteroperator.UpdateResources(c.PgclusterClientset, c.PgclusterConfig, newcluster); err != nil {
 			log.Error(err)
 			return
@@ -222,7 +223,8 @@ func (c *Controller) onUpdate(oldObj, newObj interface{}) {
 	// see if any of the pgBackRest repository resource values have changed, and
 	// if so, update them
 	if oldcluster.Spec.BackrestResources[v1.ResourceCPU] != newcluster.Spec.BackrestResources[v1.ResourceCPU] ||
-		oldcluster.Spec.BackrestResources[v1.ResourceMemory] != newcluster.Spec.BackrestResources[v1.ResourceMemory] {
+		oldcluster.Spec.BackrestResources[v1.ResourceMemory] != newcluster.Spec.BackrestResources[v1.ResourceMemory] ||
+		oldcluster.Spec.EnableBackrestMemoryLimit != newcluster.Spec.EnableBackrestMemoryLimit {
 		if err := backrestoperator.UpdateResources(c.PgclusterClientset, c.PgclusterConfig, newcluster); err != nil {
 			log.Error(err)
 			return

docs/content/pgo-client/reference/pgo_create_cluster.md

@@ -25,6 +25,9 @@ pgo create cluster [flags]
       --custom-config string                  The name of a configMap that holds custom PostgreSQL configuration files used to override defaults.
   -d, --database string                       If specified, sets the name of the initial database that is created for the user. Defaults to the value set in the PostgreSQL Operator configuration, or if that is not present, the name of the cluster
       --disable-autofail                      Disables autofail capabitilies in the cluster following cluster initialization.
+      --enable-memory-limit                   Enables PostgreSQL instances to be set with a memory limit on top of the memory request.
+      --enable-pgbackrest-memory-limit        Enables the pgBackRest repository to be set with a memory limit on top of the memory request.
+      --enable-pgbouncer-memory-limit         Enables pgBouncer instances to be set with a memory limit on top of the memory request. This has no effect if there is no pgBouncer deployment.
   -h, --help                                  help for cluster
   -l, --labels string                         The labels to apply to this cluster.
       --memory string                         Set the amount of RAM to request, e.g. 1GiB. Overrides the default server value.
@@ -100,4 +103,4 @@ pgo create cluster [flags]
 
 * [pgo create](/pgo-client/reference/pgo_create/)	 - Create a Postgres Operator resource
 
-###### Auto generated by spf13/cobra on 27-Apr-2020
+###### Auto generated by spf13/cobra on 29-Apr-2020

docs/content/pgo-client/reference/pgo_create_pgbouncer.md

@@ -18,11 +18,12 @@ pgo create pgbouncer [flags]
 ### Options
 
 ```
-      --cpu string        Set the number of millicores to request for CPU for pgBouncer. Defaults to being unset.
-  -h, --help              help for pgbouncer
-      --memory string     Set the amount of Memory to request for pgBouncer. Defaults to server value (24Mi).
-      --replicas int32    Set the total number of pgBouncer instances to deploy. If not set, defaults to 1.
-  -s, --selector string   The selector to use for cluster filtering.
+      --cpu string            Set the number of millicores to request for CPU for pgBouncer. Defaults to being unset.
+      --enable-memory-limit   Enables pgBouncer instances to be set with a memory limit on top of the memory request.
+  -h, --help                  help for pgbouncer
+      --memory string         Set the amount of Memory to request for pgBouncer. Defaults to server value (24Mi).
+      --replicas int32        Set the total number of pgBouncer instances to deploy. If not set, defaults to 1.
+  -s, --selector string       The selector to use for cluster filtering.
 ```
 
 ### Options inherited from parent commands
@@ -42,4 +43,4 @@ pgo create pgbouncer [flags]
 
 * [pgo create](/pgo-client/reference/pgo_create/)	 - Create a Postgres Operator resource
 
-###### Auto generated by spf13/cobra on 21-Apr-2020
+###### Auto generated by spf13/cobra on 29-Apr-2020