make minimum limits boundaries configurable (#808)

* make minimum limits boundaries configurable
* add e2e test

charts/postgres-operator/crds/operatorconfigurations.yaml

@@ -179,6 +179,12 @@ spec:
                 default_memory_request:
                   type: string
                   pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$'
+                min_cpu_limit:
+                  type: string
+                  pattern: '^(\d+m|\d+(\.\d{1,3})?)$'
+                min_memory_limit:
+                  type: string
+                  pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$'
             timeouts:
               type: object
               properties:

charts/postgres-operator/values-crd.yaml

@@ -115,13 +115,17 @@ configKubernetes:
 # configure resource requests for the Postgres pods
 configPostgresPodResources:
   # CPU limits for the postgres containers
-  default_cpu_limit: "3"
+  default_cpu_limit: "1"
-  # cpu request value for the postgres containers
+  # CPU request value for the postgres containers
   default_cpu_request: 100m
   # memory limits for the postgres containers
-  default_memory_limit: 1Gi
+  default_memory_limit: 500Mi
   # memory request value for the postgres containers
   default_memory_request: 100Mi
+  # hard CPU minimum required to properly run a Postgres cluster
+  min_cpu_limit: 250m
+  # hard memory minimum required to properly run a Postgres cluster
+  min_memory_limit: 250Mi
 
 # timeouts related to some operator actions
 configTimeouts:
@@ -251,7 +255,7 @@ configScalyr:
   # CPU rquest value for the Scalyr sidecar
   scalyr_cpu_request: 100m
   # Memory limit value for the Scalyr sidecar
-  scalyr_memory_limit: 1Gi
+  scalyr_memory_limit: 500Mi
   # Memory request value for the Scalyr sidecar
   scalyr_memory_request: 50Mi
 
@@ -272,13 +276,13 @@ serviceAccount:
 
 priorityClassName: ""
 
-resources: {}
+resources:
-  # limits:
+  limits:
-  #   cpu: 100m
+    cpu: 500m
-  #   memory: 300Mi
+    memory: 500Mi
-  # requests:
+  requests:
-  #   cpu: 100m
+    cpu: 100m
-  #   memory: 300Mi
+    memory: 250Mi
 
 # Affinity for pod assignment
 # Ref: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#affinity-and-anti-affinity

charts/postgres-operator/values.yaml

@@ -108,13 +108,17 @@ configKubernetes:
 # configure resource requests for the Postgres pods
 configPostgresPodResources:
   # CPU limits for the postgres containers
-  default_cpu_limit: "3"
+  default_cpu_limit: "1"
-  # cpu request value for the postgres containers
+  # CPU request value for the postgres containers
   default_cpu_request: 100m
   # memory limits for the postgres containers
-  default_memory_limit: 1Gi
+  default_memory_limit: 500Mi
   # memory request value for the postgres containers
   default_memory_request: 100Mi
+  # hard CPU minimum required to properly run a Postgres cluster
+  min_cpu_limit: 250m
+  # hard memory minimum required to properly run a Postgres cluster
+  min_memory_limit: 250Mi
 
 # timeouts related to some operator actions
 configTimeouts:
@@ -248,13 +252,13 @@ serviceAccount:
 
 priorityClassName: ""
 
-resources: {}
+resources:
-  # limits:
+  limits:
-  #   cpu: 100m
+    cpu: 500m
-  #   memory: 300Mi
+    memory: 500Mi
-  # requests:
+  requests:
-  #   cpu: 100m
+    cpu: 100m
-  #   memory: 300Mi
+    memory: 250Mi
 
 # Affinity for pod assignment
 # Ref: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#affinity-and-anti-affinity

docs/reference/operator_parameters.md

@@ -318,11 +318,19 @@ CRD-based configuration.
 
 * **default_cpu_limit**
   CPU limits for the Postgres containers, unless overridden by cluster-specific
-  settings. The default is `3`.
+  settings. The default is `1`.
 
 * **default_memory_limit**
   memory limits for the Postgres containers, unless overridden by cluster-specific
-  settings. The default is `1Gi`.
+  settings. The default is `500Mi`.
+
+* **min_cpu_limit**
+  hard CPU minimum what we consider to be required to properly run Postgres
+  clusters with Patroni on Kubernetes. The default is `250m`.
+
+* **min_memory_limit**
+  hard memory minimum what we consider to be required to properly run Postgres
+  clusters with Patroni on Kubernetes. The default is `250Mi`.
 
 ## Operator timeouts
 
@@ -579,4 +587,4 @@ scalyr sidecar. In the CRD-based configuration they are grouped under the
   CPU limit value for the Scalyr sidecar. The default is `1`.
 
 * **scalyr_memory_limit**
-  Memory limit value for the Scalyr sidecar. The default is `1Gi`.
+  Memory limit value for the Scalyr sidecar. The default is `500Mi`.

docs/user.md

@@ -232,11 +232,11 @@ spec:
       memory: 300Mi
 ```
 
-The minimum limit to properly run the `postgresql` resource is `256m` for `cpu`
+The minimum limits to properly run the `postgresql` resource are configured to
-and `256Mi` for `memory`. If a lower value is set in the manifest the operator
+`250m` for `cpu` and `250Mi` for `memory`. If a lower value is set in the
-will cancel ADD or UPDATE events on this resource with an error. If no
+manifest the operator will raise the limits to the configured minimum values.
-resources are defined in the manifest the operator will obtain the configured
+If no resources are defined in the manifest they will be obtained from the
-[default requests](reference/operator_parameters.md#kubernetes-resource-requests).
+configured [default requests](reference/operator_parameters.md#kubernetes-resource-requests).
 
 ## Use taints and tolerations for dedicated PostgreSQL nodes
 

e2e/tests/test_e2e.py

@@ -58,6 +58,57 @@ class EndToEndTestCase(unittest.TestCase):
         k8s.create_with_kubectl("manifests/minimal-postgres-manifest.yaml")
         k8s.wait_for_pod_start('spilo-role=master')
 
+    @timeout_decorator.timeout(TEST_TIMEOUT_SEC)
+    def test_min_resource_limits(self):
+        '''
+        Lower resource limits below configured minimum and let operator fix it
+        '''
+        k8s = self.k8s
+        cluster_label = 'version=acid-minimal-cluster'
+        _, failover_targets = k8s.get_pg_nodes(cluster_label)
+
+        # configure minimum boundaries for CPU and memory limits
+        minCPULimit = '250m'
+        minMemoryLimit = '250Mi'
+        patch_min_resource_limits = {
+            "data": {
+                "min_cpu_limit": minCPULimit,
+                "min_memory_limit": minMemoryLimit
+            }
+        }
+        k8s.update_config(patch_min_resource_limits)
+
+        # lower resource limits below minimum
+        pg_patch_resources = {
+            "spec": {
+                "resources": {
+                    "requests": {
+                        "cpu": "10m",
+                        "memory": "50Mi"
+                    },
+                    "limits": {
+                        "cpu": "200m",
+                        "memory": "200Mi"
+                    }
+                }
+            }
+        }
+        k8s.api.custom_objects_api.patch_namespaced_custom_object(
+            "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_resources)
+        k8s.wait_for_master_failover(failover_targets)
+
+        pods = k8s.api.core_v1.list_namespaced_pod(
+            'default', label_selector='spilo-role=master,' + cluster_label).items
+        self.assert_master_is_unique()
+        masterPod = pods[0]
+
+        self.assertEqual(masterPod.spec.containers[0].resources.limits['cpu'], minCPULimit,
+                         "Expected CPU limit {}, found {}"
+                         .format(minCPULimit, masterPod.spec.containers[0].resources.limits['cpu']))
+        self.assertEqual(masterPod.spec.containers[0].resources.limits['memory'], minMemoryLimit,
+                         "Expected memory limit {}, found {}"
+                         .format(minMemoryLimit, masterPod.spec.containers[0].resources.limits['memory']))
+
     @timeout_decorator.timeout(TEST_TIMEOUT_SEC)
     def test_multi_namespace_support(self):
         '''
@@ -76,10 +127,9 @@ class EndToEndTestCase(unittest.TestCase):
 
     @timeout_decorator.timeout(TEST_TIMEOUT_SEC)
     def test_scaling(self):
-        """
+        '''
            Scale up from 2 to 3 and back to 2 pods by updating the Postgres manifest at runtime.
-        """
+        '''
-
         k8s = self.k8s
         labels = "version=acid-minimal-cluster"
 
@@ -93,9 +143,9 @@ class EndToEndTestCase(unittest.TestCase):
 
     @timeout_decorator.timeout(TEST_TIMEOUT_SEC)
     def test_taint_based_eviction(self):
-        """
+        '''
            Add taint "postgres=:NoExecute" to node with master. This must cause a failover.
-        """
+        '''
         k8s = self.k8s
         cluster_label = 'version=acid-minimal-cluster'
 
@@ -145,7 +195,7 @@ class EndToEndTestCase(unittest.TestCase):
 
     @timeout_decorator.timeout(TEST_TIMEOUT_SEC)
     def test_logical_backup_cron_job(self):
-        """
+        '''
         Ensure we can (a) create the cron job at user request for a specific PG cluster
                       (b) update the cluster-wide image for the logical backup pod
                       (c) delete the job at user request
@@ -153,7 +203,7 @@ class EndToEndTestCase(unittest.TestCase):
         Limitations:
         (a) Does not run the actual batch job because there is no S3 mock to upload backups to
         (b) Assumes 'acid-minimal-cluster' exists as defined in setUp
-        """
+        '''
 
         k8s = self.k8s
 
@@ -208,10 +258,10 @@ class EndToEndTestCase(unittest.TestCase):
                          "Expected 0 logical backup jobs, found {}".format(len(jobs)))
 
     def assert_master_is_unique(self, namespace='default', version="acid-minimal-cluster"):
-        """
+        '''
            Check that there is a single pod in the k8s cluster with the label "spilo-role=master"
            To be called manually after operations that affect pods
-        """
+        '''
 
         k8s = self.k8s
         labels = 'spilo-role=master,version=' + version

manifests/complete-postgres-manifest.yaml

@@ -42,8 +42,8 @@ spec:
       cpu: 10m
       memory: 100Mi
     limits:
-      cpu: 300m
+      cpu: 500m
-      memory: 300Mi
+      memory: 500Mi
   patroni:
     initdb:
       encoding: "UTF8"

manifests/configmap.yaml

@@ -15,9 +15,9 @@ data:
   # custom_pod_annotations: "keya:valuea,keyb:valueb"
   db_hosted_zone: db.example.com
   debug_logging: "true"
-  # default_cpu_limit: "3"
+  # default_cpu_limit: "1"
   # default_cpu_request: 100m
-  # default_memory_limit: 1Gi
+  # default_memory_limit: 500Mi
   # default_memory_request: 100Mi
   docker_image: registry.opensource.zalan.do/acid/spilo-cdp-12:1.6-p16
   # enable_admin_role_for_users: "true"
@@ -48,6 +48,8 @@ data:
   # master_pod_move_timeout: 10m
   # max_instances: "-1"
   # min_instances: "-1"
+  # min_cpu_limit: 250m
+  # min_memory_limit: 250Mi
   # node_readiness_label: ""
   # oauth_token_secret_name: postgresql-operator
   # pam_configuration: |

manifests/operatorconfiguration.crd.yaml

@@ -155,6 +155,12 @@ spec:
                 default_memory_request:
                   type: string
                   pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$'
+                min_cpu_limit:
+                  type: string
+                  pattern: '^(\d+m|\d+(\.\d{1,3})?)$'
+                min_memory_limit:
+                  type: string
+                  pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$'
             timeouts:
               type: object
               properties:

manifests/postgres-operator.yaml

@@ -19,10 +19,10 @@ spec:
         imagePullPolicy: IfNotPresent
         resources:
           requests:
-            cpu: 500m
+            cpu: 100m
             memory: 250Mi
           limits:
-            cpu: 2000m
+            cpu: 500m
             memory: 500Mi
         securityContext:
           runAsUser: 1000

manifests/postgresql-operator-default-configuration.yaml

@@ -54,10 +54,12 @@ configuration:
     # toleration: {}
     # watched_namespace: ""
   postgres_pod_resources:
-    default_cpu_limit: "3"
+    default_cpu_limit: "1"
     default_cpu_request: 100m
-    default_memory_limit: 1Gi
+    default_memory_limit: 500Mi
     default_memory_request: 100Mi
+    # min_cpu_limit: 250m
+    # min_memory_limit: 250Mi
   timeouts:
     pod_label_wait_timeout: 10m
     pod_deletion_wait_timeout: 10m
@@ -115,6 +117,6 @@ configuration:
     scalyr_cpu_limit: "1"
     scalyr_cpu_request: 100m
     # scalyr_image: ""
-    scalyr_memory_limit: 1Gi
+    scalyr_memory_limit: 500Mi
     scalyr_memory_request: 50Mi
     # scalyr_server_url: ""

pkg/apis/acid.zalan.do/v1/crds.go

@@ -810,6 +810,14 @@ var OperatorConfigCRDResourceValidation = apiextv1beta1.CustomResourceValidation
 								Type:    "string",
 								Pattern: "^(\\d+(e\\d+)?|\\d+(\\.\\d+)?(e\\d+)?[EPTGMK]i?)$",
 							},
+							"min_cpu_limit": {
+								Type:    "string",
+								Pattern: "^(\\d+m|\\d+(\\.\\d{1,3})?)$",
+							},
+							"min_memory_limit": {
+								Type:    "string",
+								Pattern: "^(\\d+(e\\d+)?|\\d+(\\.\\d+)?(e\\d+)?[EPTGMK]i?)$",
+							},
 						},
 					},
 					"timeouts": {

pkg/apis/acid.zalan.do/v1/operator_configuration_type.go

@@ -79,6 +79,8 @@ type PostgresPodResourcesDefaults struct {
 	DefaultMemoryRequest string `json:"default_memory_request,omitempty"`
 	DefaultCPULimit      string `json:"default_cpu_limit,omitempty"`
 	DefaultMemoryLimit   string `json:"default_memory_limit,omitempty"`
+	MinCPULimit          string `json:"min_cpu_limit,omitempty"`
+	MinMemoryLimit       string `json:"min_memory_limit,omitempty"`
 }
 
 // OperatorTimeouts defines the timeout of ResourceCheck, PodWait, ReadyWait

pkg/cluster/cluster.go

@@ -227,8 +227,8 @@ func (c *Cluster) Create() error {
 
 	c.setStatus(acidv1.ClusterStatusCreating)
 
-	if err = c.validateResources(&c.Spec); err != nil {
+	if err = c.enforceMinResourceLimits(&c.Spec); err != nil {
-		return fmt.Errorf("insufficient resource limits specified: %v", err)
+		return fmt.Errorf("could not enforce minimum resource limits: %v", err)
 	}
 
 	for _, role := range []PostgresRole{Master, Replica} {
@@ -495,38 +495,38 @@ func compareResourcesAssumeFirstNotNil(a *v1.ResourceRequirements, b *v1.Resourc
 
 }
 
-func (c *Cluster) validateResources(spec *acidv1.PostgresSpec) error {
+func (c *Cluster) enforceMinResourceLimits(spec *acidv1.PostgresSpec) error {
-
-	// setting limits too low can cause unnecessary evictions / OOM kills
-	const (
-		cpuMinLimit    = "256m"
-		memoryMinLimit = "256Mi"
-	)
 
 	var (
 		isSmaller bool
 		err       error
 	)
 
+	// setting limits too low can cause unnecessary evictions / OOM kills
+	minCPULimit := c.OpConfig.MinCPULimit
+	minMemoryLimit := c.OpConfig.MinMemoryLimit
+
 	cpuLimit := spec.Resources.ResourceLimits.CPU
 	if cpuLimit != "" {
-		isSmaller, err = util.IsSmallerQuantity(cpuLimit, cpuMinLimit)
+		isSmaller, err = util.IsSmallerQuantity(cpuLimit, minCPULimit)
 		if err != nil {
-			return fmt.Errorf("error validating CPU limit: %v", err)
+			return fmt.Errorf("could not compare defined CPU limit %s with configured minimum value %s: %v", cpuLimit, minCPULimit, err)
 		}
 		if isSmaller {
-			return fmt.Errorf("defined CPU limit %s is below required minimum %s to properly run postgresql resource", cpuLimit, cpuMinLimit)
+			c.logger.Warningf("defined CPU limit %s is below required minimum %s and will be set to it", cpuLimit, minCPULimit)
+			spec.Resources.ResourceLimits.CPU = minCPULimit
 		}
 	}
 
 	memoryLimit := spec.Resources.ResourceLimits.Memory
 	if memoryLimit != "" {
-		isSmaller, err = util.IsSmallerQuantity(memoryLimit, memoryMinLimit)
+		isSmaller, err = util.IsSmallerQuantity(memoryLimit, minMemoryLimit)
 		if err != nil {
-			return fmt.Errorf("error validating memory limit: %v", err)
+			return fmt.Errorf("could not compare defined memory limit %s with configured minimum value %s: %v", memoryLimit, minMemoryLimit, err)
 		}
 		if isSmaller {
-			return fmt.Errorf("defined memory limit %s is below required minimum %s to properly run postgresql resource", memoryLimit, memoryMinLimit)
+			c.logger.Warningf("defined memory limit %s is below required minimum %s and will be set to it", memoryLimit, minMemoryLimit)
+			spec.Resources.ResourceLimits.Memory = minMemoryLimit
 		}
 	}
 
@@ -543,7 +543,6 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	oldStatus := c.Status
 	c.setStatus(acidv1.ClusterStatusUpdating)
 	c.setSpec(newSpec)
 
@@ -555,22 +554,6 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 		}
 	}()
 
-	if err := c.validateResources(&newSpec.Spec); err != nil {
-		err = fmt.Errorf("insufficient resource limits specified: %v", err)
-
-		// cancel update only when (already too low) pod resources were edited
-		// if cluster was successfully running before the update, continue but log a warning
-		isCPULimitSmaller, err2 := util.IsSmallerQuantity(newSpec.Spec.Resources.ResourceLimits.CPU, oldSpec.Spec.Resources.ResourceLimits.CPU)
-		isMemoryLimitSmaller, err3 := util.IsSmallerQuantity(newSpec.Spec.Resources.ResourceLimits.Memory, oldSpec.Spec.Resources.ResourceLimits.Memory)
-
-		if oldStatus.Running() && !isCPULimitSmaller && !isMemoryLimitSmaller && err2 == nil && err3 == nil {
-			c.logger.Warning(err)
-		} else {
-			updateFailed = true
-			return err
-		}
-	}
-
 	if oldSpec.Spec.PgVersion != newSpec.Spec.PgVersion { // PG versions comparison
 		c.logger.Warningf("postgresql version change(%q -> %q) has no effect", oldSpec.Spec.PgVersion, newSpec.Spec.PgVersion)
 		//we need that hack to generate statefulset with the old version
@@ -616,6 +599,12 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 
 	// Statefulset
 	func() {
+		if err := c.enforceMinResourceLimits(&c.Spec); err != nil {
+			c.logger.Errorf("could not sync resources: %v", err)
+			updateFailed = true
+			return
+		}
+
 		oldSs, err := c.generateStatefulSet(&oldSpec.Spec)
 		if err != nil {
 			c.logger.Errorf("could not generate old statefulset spec: %v", err)
@@ -623,6 +612,9 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 			return
 		}
 
+		// update newSpec to for latter comparison with oldSpec
+		c.enforceMinResourceLimits(&newSpec.Spec)
+
 		newSs, err := c.generateStatefulSet(&newSpec.Spec)
 		if err != nil {
 			c.logger.Errorf("could not generate new statefulset spec: %v", err)

pkg/cluster/sync.go

@@ -23,7 +23,6 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	oldStatus := c.Status
 	c.setSpec(newSpec)
 
 	defer func() {
@@ -35,16 +34,6 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error {
 		}
 	}()
 
-	if err = c.validateResources(&c.Spec); err != nil {
-		err = fmt.Errorf("insufficient resource limits specified: %v", err)
-		if oldStatus.Running() {
-			c.logger.Warning(err)
-			err = nil
-		} else {
-			return err
-		}
-	}
-
 	if err = c.initUsers(); err != nil {
 		err = fmt.Errorf("could not init users: %v", err)
 		return err
@@ -76,6 +65,11 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error {
 		return err
 	}
 
+	if err = c.enforceMinResourceLimits(&c.Spec); err != nil {
+		err = fmt.Errorf("could not enforce minimum resource limits: %v", err)
+		return err
+	}
+
 	c.logger.Debugf("syncing statefulsets")
 	if err = c.syncStatefulSet(); err != nil {
 		if !k8sutil.ResourceAlreadyExists(err) {

pkg/controller/operator_config.go

@@ -75,6 +75,8 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur
 	result.DefaultMemoryRequest = fromCRD.PostgresPodResources.DefaultMemoryRequest
 	result.DefaultCPULimit = fromCRD.PostgresPodResources.DefaultCPULimit
 	result.DefaultMemoryLimit = fromCRD.PostgresPodResources.DefaultMemoryLimit
+	result.MinCPULimit = fromCRD.PostgresPodResources.MinCPULimit
+	result.MinMemoryLimit = fromCRD.PostgresPodResources.MinMemoryLimit
 
 	// timeout config
 	result.ResourceCheckInterval = time.Duration(fromCRD.Timeouts.ResourceCheckInterval)

pkg/util/config/config.go

@@ -37,8 +37,10 @@ type Resources struct {
 	PodToleration           map[string]string `name:"toleration" default:""`
 	DefaultCPURequest       string            `name:"default_cpu_request" default:"100m"`
 	DefaultMemoryRequest    string            `name:"default_memory_request" default:"100Mi"`
-	DefaultCPULimit         string            `name:"default_cpu_limit" default:"3"`
+	DefaultCPULimit         string            `name:"default_cpu_limit" default:"1"`
-	DefaultMemoryLimit      string            `name:"default_memory_limit" default:"1Gi"`
+	DefaultMemoryLimit      string            `name:"default_memory_limit" default:"500Mi"`
+	MinCPULimit             string            `name:"min_cpu_limit" default:"250m"`
+	MinMemoryLimit          string            `name:"min_memory_limit" default:"250Mi"`
 	PodEnvironmentConfigMap string            `name:"pod_environment_configmap" default:""`
 	NodeReadinessLabel      map[string]string `name:"node_readiness_label" default:""`
 	MaxInstances            int32             `name:"max_instances" default:"-1"`
@@ -66,7 +68,7 @@ type Scalyr struct {
 	ScalyrCPURequest    string `name:"scalyr_cpu_request" default:"100m"`
 	ScalyrMemoryRequest string `name:"scalyr_memory_request" default:"50Mi"`
 	ScalyrCPULimit      string `name:"scalyr_cpu_limit" default:"1"`
-	ScalyrMemoryLimit   string `name:"scalyr_memory_limit" default:"1Gi"`
+	ScalyrMemoryLimit   string `name:"scalyr_memory_limit" default:"500Mi"`
 }
 
 // LogicalBackup defines configuration for logical backup