refactor GenerateResourceRequirements and provide unit tests

charts/postgres-operator/crds/postgresqls.yaml

@@ -150,15 +150,9 @@ spec:
                     minimum: 1
                   resources:
                     type: object
-                    required:
-                      - requests
-                      - limits
                     properties:
                       limits:
                         type: object
-                        required:
-                          - cpu
-                          - memory
                         properties:
                           cpu:
                             type: string
@@ -168,9 +162,6 @@ spec:
                             pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$'
                       requests:
                         type: object
-                        required:
-                          - cpu
-                          - memory
                         properties:
                           cpu:
                             type: string
@@ -406,15 +397,9 @@ spec:
                 description: deprecated
               resources:
                 type: object
-                required:
-                  - requests
-                  - limits
                 properties:
                   limits:
                     type: object
-                    required:
-                      - cpu
-                      - memory
                     properties:
                       cpu:
                         type: string
@@ -443,9 +428,6 @@ spec:
                         # than the corresponding limit.
                   requests:
                     type: object
-                    required:
-                      - cpu
-                      - memory
                     properties:
                       cpu:
                         type: string

docs/reference/cluster_manifest.md

@@ -322,9 +322,7 @@ explanation of `ttl` and `loop_wait` parameters.
 
 Those parameters define [CPU and memory requests and limits](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/)
 for the Postgres container. They are grouped under the `resources` top-level
-key with subgroups `requests` and `limits`. The whole section is optional,
-however if you specify a request or limit you have to define everything
-(unless you are not modifying the default CRD schema validation). 
+key with subgroups `requests` and `limits`. 
 
 ### Requests
 

docs/reference/operator_parameters.md

@@ -159,9 +159,9 @@ Those are top-level keys, containing both leaf keys and groups.
   at the cost of overprovisioning memory and potential scheduling problems for
   containers with high memory limits due to the lack of memory on Kubernetes
   cluster nodes. This affects all containers created by the operator (Postgres,
-  Scalyr sidecar, and other sidecars except **sidecars** defined in the operator
-  configuration); to set resources for the operator's own container, change the
-  [operator deployment manually](https://github.com/zalando/postgres-operator/blob/master/manifests/postgres-operator.yaml#L20).
+  connection pooler, logical backup, scalyr sidecar, and other sidecars except 
+  **sidecars** defined in the operator configuration); to set resources for the
+  operator's own container, change the [operator deployment manually](https://github.com/zalando/postgres-operator/blob/master/manifests/postgres-operator.yaml#L20).
   The default is `false`.
 
 ## Postgres users

manifests/postgresql.crd.yaml

@@ -148,15 +148,9 @@ spec:
                     minimum: 1
                   resources:
                     type: object
-                    required:
-                      - requests
-                      - limits
                     properties:
                       limits:
                         type: object
-                        required:
-                          - cpu
-                          - memory
                         properties:
                           cpu:
                             type: string
@@ -166,9 +160,6 @@ spec:
                             pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$'
                       requests:
                         type: object
-                        required:
-                          - cpu
-                          - memory
                         properties:
                           cpu:
                             type: string
@@ -404,15 +395,9 @@ spec:
                 description: deprecated
               resources:
                 type: object
-                required:
-                  - requests
-                  - limits
                 properties:
                   limits:
                     type: object
-                    required:
-                      - cpu
-                      - memory
                     properties:
                       cpu:
                         type: string
@@ -441,9 +426,6 @@ spec:
                         # than the corresponding limit.
                   requests:
                     type: object
-                    required:
-                      - cpu
-                      - memory
                     properties:
                       cpu:
                         type: string

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

@@ -238,12 +238,10 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{
 								Minimum: &min1,
 							},
 							"resources": {
-								Type:     "object",
-								Required: []string{"requests", "limits"},
+								Type: "object",
 								Properties: map[string]apiextv1.JSONSchemaProps{
 									"limits": {
-										Type:     "object",
-										Required: []string{"cpu", "memory"},
+										Type: "object",
 										Properties: map[string]apiextv1.JSONSchemaProps{
 											"cpu": {
 												Type:    "string",
@@ -256,8 +254,7 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{
 										},
 									},
 									"requests": {
-										Type:     "object",
-										Required: []string{"cpu", "memory"},
+										Type: "object",
 										Properties: map[string]apiextv1.JSONSchemaProps{
 											"cpu": {
 												Type:    "string",
@@ -648,12 +645,10 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{
 						Description: "deprecated",
 					},
 					"resources": {
-						Type:     "object",
-						Required: []string{"requests", "limits"},
+						Type: "object",
 						Properties: map[string]apiextv1.JSONSchemaProps{
 							"limits": {
-								Type:     "object",
-								Required: []string{"cpu", "memory"},
+								Type: "object",
 								Properties: map[string]apiextv1.JSONSchemaProps{
 									"cpu": {
 										Type:    "string",
@@ -666,8 +661,7 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{
 								},
 							},
 							"requests": {
-								Type:     "object",
-								Required: []string{"cpu", "memory"},
+								Type: "object",
 								Properties: map[string]apiextv1.JSONSchemaProps{
 									"cpu": {
 										Type:    "string",

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

@@ -167,7 +167,7 @@ type Patroni struct {
 	Slots                 map[string]map[string]string `json:"slots,omitempty"`
 	SynchronousMode       bool                         `json:"synchronous_mode,omitempty"`
 	SynchronousModeStrict bool                         `json:"synchronous_mode_strict,omitempty"`
-	SynchronousNodeCount  uint32                       `json:"synchronous_node_count,omitempty" defaults:1`
+	SynchronousNodeCount  uint32                       `json:"synchronous_node_count,omitempty" defaults:"1"`
 }
 
 // StandbyDescription contains s3 wal path

pkg/cluster/cluster.go

@@ -256,10 +256,6 @@ func (c *Cluster) Create() error {
 	c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusCreating)
 	c.eventRecorder.Event(c.GetReference(), v1.EventTypeNormal, "Create", "Started creation of new cluster resources")
 
-	if err = c.enforceMinResourceLimits(&c.Spec); err != nil {
-		return fmt.Errorf("could not enforce minimum resource limits: %v", err)
-	}
-
 	for _, role := range []PostgresRole{Master, Replica} {
 
 		if c.Endpoints[role] != nil {
@@ -676,50 +672,6 @@ func comparePorts(a, b []v1.ContainerPort) bool {
 	return true
 }
 
-func (c *Cluster) enforceMinResourceLimits(spec *acidv1.PostgresSpec) error {
-
-	var (
-		isSmaller bool
-		err       error
-	)
-
-	if spec.Resources == nil {
-		return nil
-	}
-
-	// 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, minCPULimit)
-		if err != nil {
-			return fmt.Errorf("could not compare defined CPU limit %s with configured minimum value %s: %v", cpuLimit, minCPULimit, err)
-		}
-		if isSmaller {
-			c.logger.Warningf("defined CPU limit %s is below required minimum %s and will be increased", cpuLimit, minCPULimit)
-			c.eventRecorder.Eventf(c.GetReference(), v1.EventTypeWarning, "ResourceLimits", "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, minMemoryLimit)
-		if err != nil {
-			return fmt.Errorf("could not compare defined memory limit %s with configured minimum value %s: %v", memoryLimit, minMemoryLimit, err)
-		}
-		if isSmaller {
-			c.logger.Warningf("defined memory limit %s is below required minimum %s and will be increased", memoryLimit, minMemoryLimit)
-			c.eventRecorder.Eventf(c.GetReference(), v1.EventTypeWarning, "ResourceLimits", "defined memory limit %s is below required minimum %s and will be set to it", memoryLimit, minMemoryLimit)
-			spec.Resources.ResourceLimits.Memory = minMemoryLimit
-		}
-	}
-
-	return nil
-}
-
 // Update changes Kubernetes objects according to the new specification. Unlike the sync case, the missing object
 // (i.e. service) is treated as an error
 // logical backup cron jobs are an exception: a user-initiated Update can enable a logical backup job
@@ -799,22 +751,13 @@ 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)
 			updateFailed = true
 			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/connection_pooler.go

@@ -210,9 +210,10 @@ func (c *Cluster) generateConnectionPoolerPodTemplate(role PostgresRole) (
 		connectionPoolerSpec = &acidv1.ConnectionPooler{}
 	}
 	gracePeriod := int64(c.OpConfig.PodTerminateGracePeriod.Seconds())
-	resources, err := generateResourceRequirements(
+	resources, err := c.generateResourceRequirements(
 		connectionPoolerSpec.Resources,
-		makeDefaultConnectionPoolerResources(&c.OpConfig))
+		makeDefaultConnectionPoolerResources(&c.OpConfig),
+		connectionPoolerContainer)
 
 	effectiveDockerImage := util.Coalesce(
 		connectionPoolerSpec.DockerImage,
@@ -627,8 +628,9 @@ func (c *Cluster) needSyncConnectionPoolerDefaults(Config *Config, spec *acidv1.
 		reasons = append(reasons, msg)
 	}
 
-	expectedResources, err := generateResourceRequirements(spec.Resources,
-		makeDefaultConnectionPoolerResources(&Config.OpConfig))
+	expectedResources, err := c.generateResourceRequirements(spec.Resources,
+		makeDefaultConnectionPoolerResources(&Config.OpConfig),
+		connectionPoolerContainer)
 
 	// An error to generate expected resources means something is not quite
 	// right, but for the purpose of robustness do not panic here, just report