From 1c4bce86df7b3e75e3e72c564a50db33a1c55c54 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Wed, 26 Apr 2017 18:22:26 +0200 Subject: [PATCH] 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 | 8 ++++-- pkg/cluster/cluster.go | 31 +++++++++++++++++++++- pkg/cluster/k8sres.go | 49 ++++++++++++++++++++++++----------- pkg/spec/postgresql.go | 7 ++++- pkg/util/config/config.go | 4 +++ 5 files changed, 80 insertions(+), 19 deletions(-) diff --git a/manifests/testpostgresql.yaml b/manifests/testpostgresql.yaml index cd3db5ddf..44f1fad78 100644 --- a/manifests/testpostgresql.yaml +++ b/manifests/testpostgresql.yaml @@ -23,8 +23,12 @@ spec: max_connections: "10" log_statement: "all" resources: - cpu: 10m - memory: 100Mi + requests: + cpu: 10m + memory: 100Mi + limits: + cpu: 300m + memory: 3000Mi patroni: initdb: encoding: "UTF8" diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index f36ff79a8..4cbeb0166 100644 --- a/pkg/cluster/cluster.go +++ b/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) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 87b6aa261..f4e65455b 100644 --- a/pkg/cluster/k8sres.go +++ b/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 { - resourceList := v1.ResourceList{} - if resources.Cpu != "" { - resourceList[v1.ResourceCPU] = resource.MustParse(resources.Cpu) - } +func (c *Cluster) resourceRequirements(resources spec.Resources) *v1.ResourceRequirements { + specRequests := resources.ResourceRequest + specLimits := resources.ResourceLimits - if resources.Memory != "" { - resourceList[v1.ResourceMemory] = resource.MustParse(resources.Memory) - } + config := c.OpConfig - 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{ - Requests: *resourceList, - }, + Resources: *resourceRequirements, Ports: []v1.ContainerPort{ { ContainerPort: 8008, @@ -163,8 +182,8 @@ bootstrap: } func (c *Cluster) genStatefulSet(spec spec.PostgresSpec) *v1beta1.StatefulSet { - resourceList := resourceList(spec.Resources) - podTemplate := c.genPodTemplate(resourceList, spec.PgVersion) + resourceRequirements := c.resourceRequirements(spec.Resources) + podTemplate := c.genPodTemplate(resourceRequirements, spec.PgVersion) volumeClaimTemplate := persistentVolumeClaimTemplate(spec.Volume.Size, spec.Volume.StorageClass) statefulSet := &v1beta1.StatefulSet{ diff --git a/pkg/spec/postgresql.go b/pkg/spec/postgresql.go index 4d8e4b534..a49573751 100644 --- a/pkg/spec/postgresql.go +++ b/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"` diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 69aa76e7a..d260fe71e 100644 --- a/pkg/util/config/config.go +++ b/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 {