Avoid "bulk-comparing" pod resources during sync. (#109)

* Avoid "bulk-comparing" pod resources during sync.

First attempt to fix bogus restarts due to the reported mismatch
of container resources where one of the resources is an empty struct,
while the other has all fields set to nil.

In addition, add an ability to set limits and requests per pod, as well as the operator-level defaults.

manifests/testpostgresql.yaml

@@ -23,8 +23,12 @@ spec:
       max_connections: "10"
       log_statement: "all"
   resources:
+    requests:
       cpu: 10m
       memory: 100Mi
+    limits:
+      cpu: 300m
+      memory: 3000Mi
   patroni:
     initdb:
       encoding: "UTF8"

pkg/cluster/cluster.go

@@ -278,7 +278,7 @@ func (c Cluster) compareStatefulSetWith(statefulSet *v1beta1.StatefulSet) (match
 		return
 	}
 
-	if !reflect.DeepEqual(container1.Resources, container2.Resources) {
+	if !compareResources(&container1.Resources, &container2.Resources) {
 		match = false
 		needsRollUpdate = true
 		reason = "new statefulset's container resources don't match the current ones"
@@ -293,6 +293,35 @@ func (c Cluster) compareStatefulSetWith(statefulSet *v1beta1.StatefulSet) (match
 	return
 }
 
+func compareResources(a *v1.ResourceRequirements, b *v1.ResourceRequirements) (equal bool) {
+	equal = true
+	if a != nil {
+		equal = compareResoucesAssumeFirstNotNil(a, b)
+	}
+	if equal && (b != nil) {
+		equal = compareResoucesAssumeFirstNotNil(b, a)
+	}
+	return
+}
+
+func compareResoucesAssumeFirstNotNil(a *v1.ResourceRequirements, b *v1.ResourceRequirements) bool {
+	if b == nil || (len(b.Requests) == 0) {
+		return (len(a.Requests) == 0)
+	}
+	for k, v := range a.Requests {
+		if (&v).Cmp(b.Requests[k]) != 0 {
+			return false
+		}
+	}
+	for k, v := range a.Limits {
+		if (&v).Cmp(b.Limits[k]) != 0 {
+			return false
+		}
+	}
+	return true
+
+}
+
 func (c *Cluster) Update(newSpec *spec.Postgresql) error {
 	c.logger.Infof("Cluster update from version %s to %s",
 		c.Metadata.ResourceVersion, newSpec.Metadata.ResourceVersion)

pkg/cluster/k8sres.go

@@ -12,20 +12,41 @@ import (
 	"github.bus.zalan.do/acid/postgres-operator/pkg/util/constants"
 )
 
-func resourceList(resources spec.Resources) *v1.ResourceList {
+func (c *Cluster) resourceRequirements(resources spec.Resources) *v1.ResourceRequirements {
-	resourceList := v1.ResourceList{}
+	specRequests := resources.ResourceRequest
-	if resources.Cpu != "" {
+	specLimits := resources.ResourceLimits
-		resourceList[v1.ResourceCPU] = resource.MustParse(resources.Cpu)
-	}
 
-	if resources.Memory != "" {
+	config := c.OpConfig
-		resourceList[v1.ResourceMemory] = resource.MustParse(resources.Memory)
-	}
 
-	return &resourceList
+	defaultRequests := spec.ResourceDescription{Cpu: config.DefaultCpuRequest, Memory: config.DefaultMemoryRequest}
+	defaultLimits := spec.ResourceDescription{Cpu: config.DefaultCpuLimit, Memory: config.DefaultMemoryLimit}
+
+	result := v1.ResourceRequirements{}
+
+	result.Requests = fillResourceList(specRequests, defaultRequests)
+	result.Limits = fillResourceList(specLimits, defaultLimits)
+
+	return &result
 }
 
-func (c *Cluster) genPodTemplate(resourceList *v1.ResourceList, pgVersion string) *v1.PodTemplateSpec {
+func fillResourceList(spec spec.ResourceDescription, defaults spec.ResourceDescription) v1.ResourceList {
+	requests := v1.ResourceList{}
+
+	if spec.Cpu != "" {
+		requests[v1.ResourceCPU] = resource.MustParse(spec.Cpu)
+	} else {
+		requests[v1.ResourceCPU] = resource.MustParse(defaults.Cpu)
+	}
+
+	if spec.Memory != "" {
+		requests[v1.ResourceMemory] = resource.MustParse(spec.Memory)
+	} else {
+		requests[v1.ResourceMemory] = resource.MustParse(defaults.Memory)
+	}
+	return requests
+}
+
+func (c *Cluster) genPodTemplate(resourceRequirements *v1.ResourceRequirements, pgVersion string) *v1.PodTemplateSpec {
 	envVars := []v1.EnvVar{
 		{
 			Name:  "SCOPE",
@@ -112,9 +133,7 @@ bootstrap:
 		Name:            c.Metadata.Name,
 		Image:           c.OpConfig.DockerImage,
 		ImagePullPolicy: v1.PullAlways,
-		Resources: v1.ResourceRequirements{
+		Resources:       *resourceRequirements,
-			Requests: *resourceList,
-		},
 		Ports: []v1.ContainerPort{
 			{
 				ContainerPort: 8008,
@@ -163,8 +182,8 @@ bootstrap:
 }
 
 func (c *Cluster) genStatefulSet(spec spec.PostgresSpec) *v1beta1.StatefulSet {
-	resourceList := resourceList(spec.Resources)
+	resourceRequirements := c.resourceRequirements(spec.Resources)
-	podTemplate := c.genPodTemplate(resourceList, spec.PgVersion)
+	podTemplate := c.genPodTemplate(resourceRequirements, spec.PgVersion)
 	volumeClaimTemplate := persistentVolumeClaimTemplate(spec.Volume.Size, spec.Volume.StorageClass)
 
 	statefulSet := &v1beta1.StatefulSet{

pkg/spec/postgresql.go

@@ -32,11 +32,16 @@ type PostgresqlParam struct {
 	Parameters map[string]string `json:"parameters"`
 }
 
-type Resources struct {
+type ResourceDescription struct {
 	Cpu    string `json:"cpu"`
 	Memory string `json:"memory"`
 }
 
+type Resources struct {
+	ResourceRequest ResourceDescription `json:"requests,omitempty""`
+	ResourceLimits  ResourceDescription `json:"limits,omitempty""`
+}
+
 type Patroni struct {
 	InitDB               map[string]string `json:"initdb"`
 	PgHba                []string          `json:"pg_hba"`

pkg/util/config/config.go

@@ -23,6 +23,10 @@ type Resources struct {
 	ClusterLabels          map[string]string `name:"cluster_labels" default:"application:spilo"`
 	ClusterNameLabel       string            `name:"cluster_name_label" default:"cluster-name"`
 	PodRoleLabel           string            `name:"pod_role_label" default:"spilo-role"`
+	DefaultCpuRequest      string            `name:"default_cpu_request" default:"100m"`
+	DefaultMemoryRequest   string            `name:"default_memory_request" default:"100Mi"`
+	DefaultCpuLimit        string            `name:"default_cpu_limit" default:"3"`
+	DefaultMemoryLimit     string            `name:"default_memory_limit" default:"1Gi"`
 }
 
 type Auth struct {