From cfcae65f4cfb8e15cdc3c3b19eae86162d621a48 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 1 Jun 2018 13:29:14 -0700 Subject: [PATCH 01/13] trying to make a more generic sidecar --- manifests/minimal-postgres-manifest.yaml | 11 +++ pkg/cluster/k8sres.go | 86 ++++++++++++++++++++++-- pkg/spec/postgresql.go | 10 +++ 3 files changed, 103 insertions(+), 4 deletions(-) diff --git a/manifests/minimal-postgres-manifest.yaml b/manifests/minimal-postgres-manifest.yaml index c8f486201..771b3ff7c 100644 --- a/manifests/minimal-postgres-manifest.yaml +++ b/manifests/minimal-postgres-manifest.yaml @@ -21,3 +21,14 @@ spec: foo: zalando postgresql: version: "10" + + sideCars: + - name: "pghero" + dockerImage: "ankane/pghero" + resources: + requests: + cpu: 10m + memory: 100Mi + limits: + cpu: 300m + memory: 3000Mi diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index f216797c0..aaad1f107 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -318,8 +318,9 @@ func (c *Cluster) generatePodTemplate( patroniParameters *spec.Patroni, cloneDescription *spec.CloneDescription, dockerImage *string, - customPodEnvVars map[string]string, -) *v1.PodTemplateSpec { + sidecars *[]spec.Sidecar, + customPodEnvVars map[string]string +) (*v1.PodTemplateSpec, error) { spiloConfiguration := c.generateSpiloJSONConfiguration(pgParameters, patroniParameters) envVars := []v1.EnvVar{ @@ -525,6 +526,16 @@ func (c *Cluster) generatePodTemplate( ) } + if sidecars != nil && len(*sidecars) > 0 { + for index, sidecar := range *sidecars { + sc, err := c.getSidecarContainer(sidecar, index, volumeMounts) + if err != nil { + return nil, err + } + podSpec.Containers = append(podSpec.Containers, *sc) + } + } + template := v1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: c.labelsSet(true), @@ -536,7 +547,71 @@ func (c *Cluster) generatePodTemplate( template.Annotations = map[string]string{constants.KubeIAmAnnotation: c.OpConfig.KubeIAMRole} } - return &template + return &template, nil +} + +func (c *Cluster) getSidecarContainer(sidecar spec.Sidecar, index int, volumeMounts []v1.VolumeMount) (*v1.Container, error) { + name := sidecar.Name + if name == "" { + name = fmt.Sprintf("sidecar-%d", index) + } + resources, err := c.resourceRequirements( + makeResources( + sidecar.Resources.ResourceRequest.CPU, + sidecar.Resources.ResourceRequest.Memory, + sidecar.Resources.ResourceLimits.CPU, + sidecar.Resources.ResourceLimits.Memory, + ), + ) + if err != nil { + return nil, err + } + env := []v1.EnvVar{ + { + Name: "POD_NAME", + ValueFrom: &v1.EnvVarSource{ + FieldRef: &v1.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.name", + }, + }, + }, + { + Name: "POD_NAMESPACE", + ValueFrom: &v1.EnvVarSource{ + FieldRef: &v1.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.namespace", + }, + }, + }, + { + Name: "POSTGRES_USER", + Value: c.OpConfig.SuperUsername, + }, + { + Name: "POSTGRES_PASSWORD", + ValueFrom: &v1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: c.credentialSecretName(c.OpConfig.SuperUsername), + }, + Key: "password", + }, + }, + }, + } + if len(sidecar.Env) > 0 { + env = append(env, sidecar.Env...) + } + return &v1.Container{ + Name: name, + Image: sidecar.DockerImage, + ImagePullPolicy: v1.PullIfNotPresent, + Resources: *resources, + VolumeMounts: volumeMounts, + Env: env, + }, nil } func getBucketScopeSuffix(uid string) string { @@ -583,7 +658,10 @@ func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.Statefu customPodEnvVars = cm.Data } } - podTemplate := c.generatePodTemplate(c.Postgresql.GetUID(), resourceRequirements, resourceRequirementsScalyrSidecar, &spec.Tolerations, &spec.PostgresqlParam, &spec.Patroni, &spec.Clone, &spec.DockerImage, customPodEnvVars) + podTemplate, err := c.generatePodTemplate(c.Postgresql.GetUID(), resourceRequirements, resourceRequirementsScalyrSidecar, &spec.Tolerations, &spec.PostgresqlParam, &spec.Patroni, &spec.Clone, &spec.DockerImage, &spec.Sidecars customPodEnvVars) + if err != nil { + return nil, fmt.Errorf("could not generate pod template: %v", err) + } volumeClaimTemplate, err := generatePersistentVolumeClaimTemplate(spec.Volume.Size, spec.Volume.StorageClass) if err != nil { return nil, fmt.Errorf("could not generate volume claim template: %v", err) diff --git a/pkg/spec/postgresql.go b/pkg/spec/postgresql.go index bd5d06127..70d7788ba 100644 --- a/pkg/spec/postgresql.go +++ b/pkg/spec/postgresql.go @@ -61,6 +61,15 @@ type CloneDescription struct { EndTimestamp string `json:"timestamp,omitempty"` } +// Sidecar defines a container to be run in the same pod as the Postgres container. +type Sidecar struct { + Resources `json:"resources,omitempty"` + Name string `json:"name,omitempty"` + DockerImage string `json:"dockerImage,omitempty"` + Ports []v1.ContainerPort `json:"ports,omitempty"` + Env []v1.EnvVar `json:"env,omitempty"` +} + type UserFlags []string // PostgresStatus contains status of the PostgreSQL cluster (running, creation failed etc.) @@ -124,6 +133,7 @@ type PostgresSpec struct { ClusterName string `json:"-"` Databases map[string]string `json:"databases,omitempty"` Tolerations []v1.Toleration `json:"tolerations,omitempty"` + Sidecars []Sidecar `json:"sideCars,omitempty"` } // PostgresqlList defines a list of PostgreSQL clusters. From 940b3c9e2bdfd021ab68cca795533b06a7d86a54 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 1 Jun 2018 21:06:12 +0000 Subject: [PATCH 02/13] fix compilation errors --- pkg/cluster/k8sres.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index aaad1f107..0ed222a0e 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -319,7 +319,7 @@ func (c *Cluster) generatePodTemplate( cloneDescription *spec.CloneDescription, dockerImage *string, sidecars *[]spec.Sidecar, - customPodEnvVars map[string]string + customPodEnvVars map[string]string, ) (*v1.PodTemplateSpec, error) { spiloConfiguration := c.generateSpiloJSONConfiguration(pgParameters, patroniParameters) @@ -605,12 +605,12 @@ func (c *Cluster) getSidecarContainer(sidecar spec.Sidecar, index int, volumeMou env = append(env, sidecar.Env...) } return &v1.Container{ - Name: name, - Image: sidecar.DockerImage, - ImagePullPolicy: v1.PullIfNotPresent, - Resources: *resources, - VolumeMounts: volumeMounts, - Env: env, + Name: name, + Image: sidecar.DockerImage, + ImagePullPolicy: v1.PullIfNotPresent, + Resources: *resources, + VolumeMounts: volumeMounts, + Env: env, }, nil } @@ -658,7 +658,7 @@ func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.Statefu customPodEnvVars = cm.Data } } - podTemplate, err := c.generatePodTemplate(c.Postgresql.GetUID(), resourceRequirements, resourceRequirementsScalyrSidecar, &spec.Tolerations, &spec.PostgresqlParam, &spec.Patroni, &spec.Clone, &spec.DockerImage, &spec.Sidecars customPodEnvVars) + podTemplate, err := c.generatePodTemplate(c.Postgresql.GetUID(), resourceRequirements, resourceRequirementsScalyrSidecar, &spec.Tolerations, &spec.PostgresqlParam, &spec.Patroni, &spec.Clone, &spec.DockerImage, &spec.Sidecars, customPodEnvVars) if err != nil { return nil, fmt.Errorf("could not generate pod template: %v", err) } From 20a8348f95a0250171e2d0c3290add1895ffd32a Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 1 Jun 2018 21:41:31 +0000 Subject: [PATCH 03/13] update for new docs --- docs/user.md | 32 ++++++++++++++++++++++++++++++++ pkg/spec/postgresql.go | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/docs/user.md b/docs/user.md index d0ebcc1b0..53a260220 100644 --- a/docs/user.md +++ b/docs/user.md @@ -242,3 +242,35 @@ metadata: Note that timezone required for `timestamp` (offset relative to UTC, see RFC 3339 section 5.6) + + +## Sidecar Support + +Each cluster can specify arbitrary sidecars to run. These containers could be used for +log aggregation, monitoring, backups or other tasks. A sidecar can be specified like this: + +```yaml +apiVersion: "acid.zalan.do/v1" +kind: postgresql + +metadata: + name: acid-minimal-cluster +spec: + ... + sidecars: + - name: "container-name" + image: "company/image:tag" + env: + - name: "ENV_VAR_NAME" + value: "any-k8s-env-things" +``` + +In addition to any environment variables you specify, the following environment variables +are always passed to sidecars: + + - `POD_NAME` - field reference to `metadata.name` + - `POD_NAMESPACE` - field reference to `metadata.namespace` + - `POSTGRES_USER` - the superuser that can be used to connect to the database + - `POSTGRES_PASSWORD` - the password for the superuser + +The PostgreSQL volume is shared with sidecars and is mounted at `/home/postgres/pgdata`. diff --git a/pkg/spec/postgresql.go b/pkg/spec/postgresql.go index 70d7788ba..76999c673 100644 --- a/pkg/spec/postgresql.go +++ b/pkg/spec/postgresql.go @@ -65,7 +65,7 @@ type CloneDescription struct { type Sidecar struct { Resources `json:"resources,omitempty"` Name string `json:"name,omitempty"` - DockerImage string `json:"dockerImage,omitempty"` + DockerImage string `json:"image,omitempty"` Ports []v1.ContainerPort `json:"ports,omitempty"` Env []v1.EnvVar `json:"env,omitempty"` } From 232359d2629ce2c18f931dbb96a41d7896b75938 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Sat, 2 Jun 2018 04:37:13 +0000 Subject: [PATCH 04/13] forgot ports --- pkg/cluster/k8sres.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 0ed222a0e..9ac7d30ce 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -611,6 +611,7 @@ func (c *Cluster) getSidecarContainer(sidecar spec.Sidecar, index int, volumeMou Resources: *resources, VolumeMounts: volumeMounts, Env: env, + Posts: sidecar.Ports, }, nil } From beec6e20e19719cdf4fc3ebc568a22f096435fcc Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Mon, 4 Jun 2018 21:44:08 +0000 Subject: [PATCH 05/13] ports not posts silly --- pkg/cluster/k8sres.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 9ac7d30ce..d6e249708 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -611,7 +611,7 @@ func (c *Cluster) getSidecarContainer(sidecar spec.Sidecar, index int, volumeMou Resources: *resources, VolumeMounts: volumeMounts, Env: env, - Posts: sidecar.Ports, + Ports: sidecar.Ports, }, nil } From 6b2a02bf40cdd1ce7a54d10aea7bec6e21e45a6a Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Wed, 6 Jun 2018 01:33:52 +0000 Subject: [PATCH 06/13] sidecars not sideCars --- manifests/minimal-postgres-manifest.yaml | 11 ----------- pkg/spec/postgresql.go | 2 +- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/manifests/minimal-postgres-manifest.yaml b/manifests/minimal-postgres-manifest.yaml index 771b3ff7c..c8f486201 100644 --- a/manifests/minimal-postgres-manifest.yaml +++ b/manifests/minimal-postgres-manifest.yaml @@ -21,14 +21,3 @@ spec: foo: zalando postgresql: version: "10" - - sideCars: - - name: "pghero" - dockerImage: "ankane/pghero" - resources: - requests: - cpu: 10m - memory: 100Mi - limits: - cpu: 300m - memory: 3000Mi diff --git a/pkg/spec/postgresql.go b/pkg/spec/postgresql.go index 76999c673..d59ccd22c 100644 --- a/pkg/spec/postgresql.go +++ b/pkg/spec/postgresql.go @@ -133,7 +133,7 @@ type PostgresSpec struct { ClusterName string `json:"-"` Databases map[string]string `json:"databases,omitempty"` Tolerations []v1.Toleration `json:"tolerations,omitempty"` - Sidecars []Sidecar `json:"sideCars,omitempty"` + Sidecars []Sidecar `json:"sidecars,omitempty"` } // PostgresqlList defines a list of PostgreSQL clusters. From 1e4f046797275de4679461085ae985975eef8120 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 14 Jun 2018 22:20:24 +0200 Subject: [PATCH 07/13] Define sidecars in the operator configuration. Right now only the name and the docker image can be defined, but with the help of the pod_environment_configmap parameter arbitrary environment variables can be passed to the sidecars. --- docs/reference/operator_parameters.md | 5 ++++ pkg/cluster/k8sres.go | 36 ++++++++++++++++++++++++--- pkg/util/config/config.go | 7 +++--- 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index 115cb055e..c13da2317 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -15,6 +15,11 @@ words. your own Spilo image from the [github repository](https://github.com/zalando/spilo). +* **sidecars** + a map of sidecar names to docker images for the containers to run alongside + Spilo. In case of the name conflict with the definition in the cluster + manifest the cluster-specific one is preferred. + * **workers** number of working routines the operator spawns to process requests to create/update/delete/sync clusters concurrently. The default is `4`. diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index d6e249708..a6f40b77b 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -318,7 +318,7 @@ func (c *Cluster) generatePodTemplate( patroniParameters *spec.Patroni, cloneDescription *spec.CloneDescription, dockerImage *string, - sidecars *[]spec.Sidecar, + sidecars []spec.Sidecar, customPodEnvVars map[string]string, ) (*v1.PodTemplateSpec, error) { spiloConfiguration := c.generateSpiloJSONConfiguration(pgParameters, patroniParameters) @@ -526,8 +526,8 @@ func (c *Cluster) generatePodTemplate( ) } - if sidecars != nil && len(*sidecars) > 0 { - for index, sidecar := range *sidecars { + if sidecars != nil && len(sidecars) > 0 { + for index, sidecar := range sidecars { sc, err := c.getSidecarContainer(sidecar, index, volumeMounts) if err != nil { return nil, err @@ -659,7 +659,10 @@ func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.Statefu customPodEnvVars = cm.Data } } - podTemplate, err := c.generatePodTemplate(c.Postgresql.GetUID(), resourceRequirements, resourceRequirementsScalyrSidecar, &spec.Tolerations, &spec.PostgresqlParam, &spec.Patroni, &spec.Clone, &spec.DockerImage, &spec.Sidecars, customPodEnvVars) + + mergedSidecars := c.mergeSidecars(spec.Sidecars) + + podTemplate, err := c.generatePodTemplate(c.Postgresql.GetUID(), resourceRequirements, resourceRequirementsScalyrSidecar, &spec.Tolerations, &spec.PostgresqlParam, &spec.Patroni, &spec.Clone, &spec.DockerImage, mergedSidecars, customPodEnvVars) if err != nil { return nil, fmt.Errorf("could not generate pod template: %v", err) } @@ -689,6 +692,31 @@ func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.Statefu return statefulSet, nil } +// mergeSidecar merges globally-defined sidecars with those defined in the cluster manifest +func (c *Cluster) mergeSidecars(sidecars []spec.Sidecar) []spec.Sidecar { + globalSidecarsToSkip := map[string]bool{} + result := make([]spec.Sidecar, 0) + + for i, sidecar := range sidecars { + dockerImage, ok := c.OpConfig.Sidecars[sidecar.Name] + if ok { + if dockerImage != sidecar.DockerImage { + c.logger.Warningf("merging definitions for sidecar %q: "+ + "ignoring %q in the global scope in favor of %q defined in the cluster", + sidecar.Name, dockerImage, sidecar.DockerImage) + } + globalSidecarsToSkip[sidecar.Name] = true + } + result = append(result, sidecars[i]) + } + for name, dockerImage := range c.OpConfig.Sidecars { + if !globalSidecarsToSkip[name] { + result = append(result, spec.Sidecar{Name: name, DockerImage: dockerImage}) + } + } + return result +} + func (c *Cluster) getNumberOfInstances(spec *spec.PostgresSpec) (newcur int32) { min := c.OpConfig.MinInstances max := c.OpConfig.MaxInstances diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 182bb077c..11ed77a60 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -68,9 +68,10 @@ type Config struct { Auth Scalyr - WatchedNamespace string `name:"watched_namespace"` // special values: "*" means 'watch all namespaces', the empty string "" means 'watch a namespace where operator is deployed to' - EtcdHost string `name:"etcd_host" default:""` // special values: the empty string "" means Patroni will use k8s as a DCS - DockerImage string `name:"docker_image" default:"registry.opensource.zalan.do/acid/spilo-cdp-10:1.4-p8"` + WatchedNamespace string `name:"watched_namespace"` // special values: "*" means 'watch all namespaces', the empty string "" means 'watch a namespace where operator is deployed to' + EtcdHost string `name:"etcd_host" default:""` // special values: the empty string "" means Patroni will use k8s as a DCS + DockerImage string `name:"docker_image" default:"registry.opensource.zalan.do/acid/spilo-cdp-10:1.4-p8"` + Sidecars map[string]string `name:"sidecars"` // default name `operator` enables backward compatibility with the older ServiceAccountName field PodServiceAccountName string `name:"pod_service_account_name" default:"operator"` // value of this string must be valid JSON or YAML; see initPodServiceAccount From 0940d3668ef14e37fdc2a14bb5f36076d15c6d91 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Fri, 15 Jun 2018 18:54:22 +0200 Subject: [PATCH 08/13] Refactoring around generatePodTemplate. --- pkg/cluster/k8sres.go | 470 +++++++++++++++++++++++++----------------- 1 file changed, 282 insertions(+), 188 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index a6f40b77b..346b12a93 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -5,6 +5,7 @@ import ( "fmt" "sort" + "github.com/Sirupsen/logrus" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -15,6 +16,7 @@ import ( "github.com/zalando-incubator/postgres-operator/pkg/spec" "github.com/zalando-incubator/postgres-operator/pkg/util/constants" + "k8s.io/apimachinery/pkg/labels" ) const ( @@ -79,25 +81,30 @@ func (c *Cluster) podDisruptionBudgetName() string { return c.OpConfig.PDBNameFormat.Format("cluster", c.Name) } -func (c *Cluster) resourceRequirements(resources spec.Resources) (*v1.ResourceRequirements, error) { - var err error - - specRequests := resources.ResourceRequest - specLimits := resources.ResourceLimits +func (c *Cluster) makeDefaultResources() spec.Resources { config := c.OpConfig defaultRequests := spec.ResourceDescription{CPU: config.DefaultCPURequest, Memory: config.DefaultMemoryRequest} defaultLimits := spec.ResourceDescription{CPU: config.DefaultCPULimit, Memory: config.DefaultMemoryLimit} + return spec.Resources{defaultRequests, defaultLimits} +} + +func generateResourceRequirements(resources spec.Resources, defaultResources spec.Resources) (*v1.ResourceRequirements, error) { + var err error + + specRequests := resources.ResourceRequest + specLimits := resources.ResourceLimits + result := v1.ResourceRequirements{} - result.Requests, err = fillResourceList(specRequests, defaultRequests) + result.Requests, err = fillResourceList(specRequests, defaultResources.ResourceRequest) if err != nil { return nil, fmt.Errorf("could not fill resource requests: %v", err) } - result.Limits, err = fillResourceList(specLimits, defaultLimits) + result.Limits, err = fillResourceList(specLimits, defaultResources.ResourceLimits) if err != nil { return nil, fmt.Errorf("could not fill resource limits: %v", err) } @@ -135,7 +142,7 @@ func fillResourceList(spec spec.ResourceDescription, defaults spec.ResourceDescr return requests, nil } -func (c *Cluster) generateSpiloJSONConfiguration(pg *spec.PostgresqlParam, patroni *spec.Patroni) string { +func generateSpiloJSONConfiguration(pg *spec.PostgresqlParam, patroni *spec.Patroni, pamRoleName string, logger *logrus.Entry) string { config := spiloConfiguration{} config.Bootstrap = pgBootstrap{} @@ -178,7 +185,7 @@ PatroniInitDBParams: } } default: - c.logger.Warningf("unsupported type for initdb configuration item %s: %T", defaultParam, defaultParam) + logger.Warningf("unsupported type for initdb configuration item %s: %T", defaultParam, defaultParam) continue PatroniInitDBParams } } @@ -201,7 +208,7 @@ PatroniInitDBParams: } else { config.Bootstrap.PgHBA = []string{ "hostnossl all all all reject", - fmt.Sprintf("hostssl all +%s all pam", c.OpConfig.PamRoleName), + fmt.Sprintf("hostssl all +%s all pam", pamRoleName), "hostssl all all all md5", } } @@ -240,25 +247,25 @@ PatroniInitDBParams: } } config.Bootstrap.Users = map[string]pgUser{ - c.OpConfig.PamRoleName: { + pamRoleName: { Password: "", Options: []string{constants.RoleFlagCreateDB, constants.RoleFlagNoLogin}, }, } result, err := json.Marshal(config) if err != nil { - c.logger.Errorf("cannot convert spilo configuration into JSON: %v", err) + logger.Errorf("cannot convert spilo configuration into JSON: %v", err) return "" } return string(result) } -func (c *Cluster) nodeAffinity() *v1.Affinity { +func nodeAffinity(nodeReadinessLabel map[string]string) *v1.Affinity { matchExpressions := make([]v1.NodeSelectorRequirement, 0) - if len(c.OpConfig.NodeReadinessLabel) == 0 { + if len(nodeReadinessLabel) == 0 { return nil } - for k, v := range c.OpConfig.NodeReadinessLabel { + for k, v := range nodeReadinessLabel { matchExpressions = append(matchExpressions, v1.NodeSelectorRequirement{ Key: k, Operator: v1.NodeSelectorOpIn, @@ -275,13 +282,12 @@ func (c *Cluster) nodeAffinity() *v1.Affinity { } } -func (c *Cluster) tolerations(tolerationsSpec *[]v1.Toleration) []v1.Toleration { +func tolerations(tolerationsSpec *[]v1.Toleration, podToleration map[string]string) []v1.Toleration { // allow to override tolerations by postgresql manifest if len(*tolerationsSpec) > 0 { return *tolerationsSpec } - podToleration := c.Config.OpConfig.PodToleration if len(podToleration["key"]) > 0 || len(podToleration["operator"]) > 0 || len(podToleration["value"]) > 0 || len(podToleration["effect"]) > 0 { return []v1.Toleration{ { @@ -309,20 +315,123 @@ func isBootstrapOnlyParameter(param string) bool { param == "track_commit_timestamp" } -func (c *Cluster) generatePodTemplate( - uid types.UID, +func generateVolumeMounts() []v1.VolumeMount { + return []v1.VolumeMount{ + { + Name: constants.DataVolumeName, + MountPath: constants.PostgresDataMount, //TODO: fetch from manifest + }, + } +} + +func generateSpiloContainer( + name string, + dockerImage *string, resourceRequirements *v1.ResourceRequirements, - resourceRequirementsScalyrSidecar *v1.ResourceRequirements, + envVars []v1.EnvVar, + volumeMounts []v1.VolumeMount, +) *v1.Container { + + privilegedMode := true + return &v1.Container{ + Name: name, + Image: *dockerImage, + ImagePullPolicy: v1.PullIfNotPresent, + Resources: *resourceRequirements, + Ports: []v1.ContainerPort{ + { + ContainerPort: 8008, + Protocol: v1.ProtocolTCP, + }, + { + ContainerPort: 5432, + Protocol: v1.ProtocolTCP, + }, + { + ContainerPort: 8080, + Protocol: v1.ProtocolTCP, + }, + }, + VolumeMounts: volumeMounts, + Env: envVars, + SecurityContext: &v1.SecurityContext{ + Privileged: &privilegedMode, + }, + } +} + +func generateSidecarContainers(sidecars []spec.Sidecar, + volumeMounts []v1.VolumeMount, defaultResources spec.Resources, superUserName string, credentialsSecretName string) ([]v1.Container, error) { + + if sidecars != nil && len(sidecars) > 0 { + result := make([]v1.Container, 0) + for index, sidecar := range sidecars { + + resources, err := generateResourceRequirements( + makeResources( + sidecar.Resources.ResourceRequest.CPU, + sidecar.Resources.ResourceRequest.Memory, + sidecar.Resources.ResourceLimits.CPU, + sidecar.Resources.ResourceLimits.Memory, + ), + defaultResources, + ) + if err != nil { + return nil, err + } + + sc := getSidecarContainer(sidecar, index, volumeMounts, resources, superUserName, credentialsSecretName) + result = append(result, *sc) + } + return result, nil + } + return nil, nil +} + +func generatePodTemplate( + namespace string, + labels labels.Set, + spiloContainer *v1.Container, + sidecarContainers []v1.Container, + volumeMounts []v1.VolumeMount, tolerationsSpec *[]v1.Toleration, - pgParameters *spec.PostgresqlParam, - patroniParameters *spec.Patroni, - cloneDescription *spec.CloneDescription, - dockerImage *string, - sidecars []spec.Sidecar, - customPodEnvVars map[string]string, + nodeAffinity *v1.Affinity, + terminateGracePeriod int64, + podServiceAccountName string, + kubeIAMRole string, ) (*v1.PodTemplateSpec, error) { - spiloConfiguration := c.generateSpiloJSONConfiguration(pgParameters, patroniParameters) + terminateGracePeriodSeconds := terminateGracePeriod + containers := []v1.Container{*spiloContainer} + containers = append(containers, sidecarContainers...) + + podSpec := v1.PodSpec{ + ServiceAccountName: podServiceAccountName, + TerminationGracePeriodSeconds: &terminateGracePeriodSeconds, + Containers: containers, + Tolerations: *tolerationsSpec, + } + + if nodeAffinity != nil { + podSpec.Affinity = nodeAffinity + } + + template := v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: labels, + Namespace: namespace, + }, + Spec: podSpec, + } + if kubeIAMRole != "" { + template.Annotations = map[string]string{constants.KubeIAmAnnotation: kubeIAMRole} + } + + return &template, nil +} + +// generatePodEnvVars generates environment variables for the Spilo Pod +func (c *Cluster) generateSpiloPodEnvVars(uid types.UID, spiloConfiguration string, cloneDescription *spec.CloneDescription, customPodEnvVarsList []v1.EnvVar) []v1.EnvVar { envVars := []v1.EnvVar{ { Name: "SCOPE", @@ -410,162 +519,41 @@ func (c *Cluster) generatePodTemplate( envVars = append(envVars, c.generateCloneEnvironment(cloneDescription)...) } - var names []string - // handle environment variables from the PodEnvironmentConfigMap. We don't use envSource here as it is impossible - // to track any changes to the object envSource points to. In order to emulate the envSource behavior, however, we - // need to make sure that PodConfigMap variables doesn't override those we set explicitly from the configuration - // parameters - envVarsMap := make(map[string]string) - for _, envVar := range envVars { - envVarsMap[envVar.Name] = envVar.Value - } - for name := range customPodEnvVars { - if _, ok := envVarsMap[name]; !ok { - names = append(names, name) - } else { - c.logger.Warningf("variable %q value from %q is ignored: conflict with the definition from the operator", - name, c.OpConfig.PodEnvironmentConfigMap) - } - } - sort.Strings(names) - for _, name := range names { - envVars = append(envVars, v1.EnvVar{Name: name, Value: customPodEnvVars[name]}) + if len(customPodEnvVarsList) > 0 { + envVars = append(envVars, customPodEnvVarsList...) } - privilegedMode := true - containerImage := c.OpConfig.DockerImage - if dockerImage != nil && *dockerImage != "" { - containerImage = *dockerImage - } - volumeMounts := []v1.VolumeMount{ - { - Name: constants.DataVolumeName, - MountPath: constants.PostgresDataMount, //TODO: fetch from manifest - }, - } - container := v1.Container{ - Name: c.containerName(), - Image: containerImage, - ImagePullPolicy: v1.PullIfNotPresent, - Resources: *resourceRequirements, - Ports: []v1.ContainerPort{ - { - ContainerPort: 8008, - Protocol: v1.ProtocolTCP, - }, - { - ContainerPort: 5432, - Protocol: v1.ProtocolTCP, - }, - { - ContainerPort: 8080, - Protocol: v1.ProtocolTCP, - }, - }, - VolumeMounts: volumeMounts, - Env: envVars, - SecurityContext: &v1.SecurityContext{ - Privileged: &privilegedMode, - }, - } - terminateGracePeriodSeconds := int64(c.OpConfig.PodTerminateGracePeriod.Seconds()) - - podSpec := v1.PodSpec{ - ServiceAccountName: c.OpConfig.PodServiceAccountName, - TerminationGracePeriodSeconds: &terminateGracePeriodSeconds, - Containers: []v1.Container{container}, - Tolerations: c.tolerations(tolerationsSpec), - } - - if affinity := c.nodeAffinity(); affinity != nil { - podSpec.Affinity = affinity - } - - if c.OpConfig.ScalyrAPIKey != "" && c.OpConfig.ScalyrImage != "" { - podSpec.Containers = append( - podSpec.Containers, - v1.Container{ - Name: "scalyr-sidecar", - Image: c.OpConfig.ScalyrImage, - ImagePullPolicy: v1.PullIfNotPresent, - Resources: *resourceRequirementsScalyrSidecar, - VolumeMounts: volumeMounts, - Env: []v1.EnvVar{ - { - Name: "POD_NAME", - ValueFrom: &v1.EnvVarSource{ - FieldRef: &v1.ObjectFieldSelector{ - APIVersion: "v1", - FieldPath: "metadata.name", - }, - }, - }, - { - Name: "POD_NAMESPACE", - ValueFrom: &v1.EnvVarSource{ - FieldRef: &v1.ObjectFieldSelector{ - APIVersion: "v1", - FieldPath: "metadata.namespace", - }, - }, - }, - { - Name: "SCALYR_API_KEY", - Value: c.OpConfig.ScalyrAPIKey, - }, - { - Name: "SCALYR_SERVER_HOST", - Value: c.Name, - }, - { - Name: "SCALYR_SERVER_URL", - Value: c.OpConfig.ScalyrServerURL, - }, - }, - }, - ) - } + return envVars +} - if sidecars != nil && len(sidecars) > 0 { - for index, sidecar := range sidecars { - sc, err := c.getSidecarContainer(sidecar, index, volumeMounts) - if err != nil { - return nil, err - } - podSpec.Containers = append(podSpec.Containers, *sc) +// deduplicateEnvVars makes sure there are no duplicate in the target envVar array. While Kubernetes already does, it +// leaves the last definition in the list and is not documented, which means that the behavior can be reversed at some +// point (it may also start producing an error). Therefore, the merge is done by the operator, the entries that are ahead +// in the passed list take priority over those that are behind, and only the name is considered in order to eliminate +// duplicates. +func deduplicateEnvVars(input []v1.EnvVar, containerName string, logger *logrus.Entry) []v1.EnvVar { + result := make([]v1.EnvVar, 0) + names := make(map[string]int) + + for i, va := range input { + if names[va.Name] == 0 { + names[va.Name] += 1 + result = append(result, input[i]) + } else if names[va.Name] == 1 { + names[va.Name] += 1 + logger.Warningf("variable %q is defined in %q more than once, the subsequent definitions are ignored", + va.Name, containerName) } } - - template := v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: c.labelsSet(true), - Namespace: c.Namespace, - }, - Spec: podSpec, - } - if c.OpConfig.KubeIAMRole != "" { - template.Annotations = map[string]string{constants.KubeIAmAnnotation: c.OpConfig.KubeIAMRole} - } - - return &template, nil + return result } -func (c *Cluster) getSidecarContainer(sidecar spec.Sidecar, index int, volumeMounts []v1.VolumeMount) (*v1.Container, error) { +func getSidecarContainer(sidecar spec.Sidecar, index int, volumeMounts []v1.VolumeMount, resources *v1.ResourceRequirements, superUserName string, credentialsSecretName string) *v1.Container { name := sidecar.Name if name == "" { name = fmt.Sprintf("sidecar-%d", index) } - resources, err := c.resourceRequirements( - makeResources( - sidecar.Resources.ResourceRequest.CPU, - sidecar.Resources.ResourceRequest.Memory, - sidecar.Resources.ResourceLimits.CPU, - sidecar.Resources.ResourceLimits.Memory, - ), - ) - if err != nil { - return nil, err - } + env := []v1.EnvVar{ { Name: "POD_NAME", @@ -587,14 +575,14 @@ func (c *Cluster) getSidecarContainer(sidecar spec.Sidecar, index int, volumeMou }, { Name: "POSTGRES_USER", - Value: c.OpConfig.SuperUsername, + Value: superUserName, }, { Name: "POSTGRES_PASSWORD", ValueFrom: &v1.EnvVarSource{ SecretKeyRef: &v1.SecretKeySelector{ LocalObjectReference: v1.LocalObjectReference{ - Name: c.credentialSecretName(c.OpConfig.SuperUsername), + Name: credentialsSecretName, }, Key: "password", }, @@ -612,7 +600,7 @@ func (c *Cluster) getSidecarContainer(sidecar spec.Sidecar, index int, volumeMou VolumeMounts: volumeMounts, Env: env, Ports: sidecar.Ports, - }, nil + } } func getBucketScopeSuffix(uid string) string { @@ -636,33 +624,89 @@ func makeResources(cpuRequest, memoryRequest, cpuLimit, memoryLimit string) spec } func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.StatefulSet, error) { - resourceRequirements, err := c.resourceRequirements(spec.Resources) + + defaultResources := c.makeDefaultResources() + + resourceRequirements, err := generateResourceRequirements(spec.Resources, defaultResources) if err != nil { return nil, fmt.Errorf("could not generate resource requirements: %v", err) } - resourceRequirementsScalyrSidecar, err := c.resourceRequirements( - makeResources( - c.OpConfig.ScalyrCPURequest, - c.OpConfig.ScalyrMemoryRequest, - c.OpConfig.ScalyrCPULimit, - c.OpConfig.ScalyrMemoryLimit, - ), - ) + if err != nil { return nil, fmt.Errorf("could not generate Scalyr sidecar resource requirements: %v", err) } - var customPodEnvVars map[string]string + customPodEnvVarsList := make([]v1.EnvVar, 0) + if c.OpConfig.PodEnvironmentConfigMap != "" { if cm, err := c.KubeClient.ConfigMaps(c.Namespace).Get(c.OpConfig.PodEnvironmentConfigMap, metav1.GetOptions{}); err != nil { return nil, fmt.Errorf("could not read PodEnvironmentConfigMap: %v", err) } else { - customPodEnvVars = cm.Data + for k, v := range cm.Data { + customPodEnvVarsList = append(customPodEnvVarsList, v1.EnvVar{Name: k, Value: v}) + } + sort.Slice(customPodEnvVarsList, + func(i, j int) bool { return customPodEnvVarsList[i].Name < customPodEnvVarsList[j].Name }) } } - mergedSidecars := c.mergeSidecars(spec.Sidecars) + spiloConfiguration := generateSpiloJSONConfiguration(&spec.PostgresqlParam, &spec.Patroni, c.OpConfig.PamRoleName, c.logger) + + // generate environment variables for the spilo container + spiloEnvVars := deduplicateEnvVars( + c.generateSpiloPodEnvVars(c.Postgresql.GetUID(), spiloConfiguration, &spec.Clone, customPodEnvVarsList), + c.containerName(), c.logger) + + // pickup the docker image for the spilo container + effectiveDockerImage := getEffectiveDockerImage(c.OpConfig.DockerImage, spec.DockerImage) + + volumeMounts := generateVolumeMounts() + + // generate the spilo container + spiloContainer := generateSpiloContainer(c.containerName(), &effectiveDockerImage, resourceRequirements, spiloEnvVars, volumeMounts) + + // resolve conflicts between operator-global and per-cluster sidecards + sideCars := c.mergeSidecars(spec.Sidecars) + + resourceRequirementsScalyrSidecar := makeResources( + c.OpConfig.ScalyrCPURequest, + c.OpConfig.ScalyrMemoryRequest, + c.OpConfig.ScalyrCPULimit, + c.OpConfig.ScalyrMemoryLimit, + ) + + // generate scalyr sidecar container + // TODO: avoid hardcoding scalyr container and use the sidecar mechansim + if scalarSidecar, present := + generateScalarSidecarSpec(c.Name, + c.OpConfig.ScalyrAPIKey, + c.OpConfig.ScalyrServerURL, + c.OpConfig.ScalyrImage, + &resourceRequirementsScalyrSidecar); present { + sideCars = append(sideCars, *scalarSidecar) + } + + // generate sidecar containers + sidecarContainers, err := generateSidecarContainers(sideCars, volumeMounts, defaultResources, + c.OpConfig.SuperUsername, c.credentialSecretName(c.OpConfig.SuperUsername)) + if err != nil { + return nil, fmt.Errorf("could not generate sidecar containers: %v", err) + } + + tolerationSpec := tolerations(&spec.Tolerations, c.OpConfig.PodToleration) + + // generate pod template for the statefulset, based on the spilo container and sidecards + podTemplate, err := generatePodTemplate( + c.Namespace, + c.labelsSet(true), + spiloContainer, + sidecarContainers, + volumeMounts, + &tolerationSpec, + nodeAffinity(c.OpConfig.NodeReadinessLabel), + int64(c.OpConfig.PodTerminateGracePeriod.Seconds()), + c.OpConfig.PodServiceAccountName, + c.OpConfig.KubeIAMRole) - podTemplate, err := c.generatePodTemplate(c.Postgresql.GetUID(), resourceRequirements, resourceRequirementsScalyrSidecar, &spec.Tolerations, &spec.PostgresqlParam, &spec.Patroni, &spec.Clone, &spec.DockerImage, mergedSidecars, customPodEnvVars) if err != nil { return nil, fmt.Errorf("could not generate pod template: %v", err) } @@ -692,6 +736,56 @@ func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.Statefu return statefulSet, nil } +func getEffectiveDockerImage(globalDockerImage, clusterDockerImage string) string { + if clusterDockerImage == "" { + return globalDockerImage + } + return clusterDockerImage +} + +func generateScalarSidecarSpec(clusterName, APIKey, serverURL, dockerImage string, containerResources *spec.Resources) (sidecar *spec.Sidecar, present bool) { + if APIKey == "" || serverURL == "" || dockerImage == "" { + return nil, false + } + return &spec.Sidecar{ + Name: "scalyr-sidecar", + DockerImage: dockerImage, + Env: []v1.EnvVar{ + { + Name: "POD_NAME", + ValueFrom: &v1.EnvVarSource{ + FieldRef: &v1.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.name", + }, + }, + }, + { + Name: "POD_NAMESPACE", + ValueFrom: &v1.EnvVarSource{ + FieldRef: &v1.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.namespace", + }, + }, + }, + { + Name: "SCALYR_API_KEY", + Value: APIKey, + }, + { + Name: "SCALYR_SERVER_HOST", + Value: clusterName, + }, + { + Name: "SCALYR_SERVER_URL", + Value: serverURL, + }, + }, + Resources: *containerResources, + }, true +} + // mergeSidecar merges globally-defined sidecars with those defined in the cluster manifest func (c *Cluster) mergeSidecars(sidecars []spec.Sidecar) []spec.Sidecar { globalSidecarsToSkip := map[string]bool{} From 57297dc16ba5b030a106e8c900bb6e5347d87751 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 26 Jun 2018 12:44:21 +0200 Subject: [PATCH 09/13] Deduplicate variables for sidecars. --- pkg/cluster/k8sres.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 346b12a93..e6bc10161 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -361,7 +361,8 @@ func generateSpiloContainer( } func generateSidecarContainers(sidecars []spec.Sidecar, - volumeMounts []v1.VolumeMount, defaultResources spec.Resources, superUserName string, credentialsSecretName string) ([]v1.Container, error) { + volumeMounts []v1.VolumeMount, defaultResources spec.Resources, + superUserName string, credentialsSecretName string, logger *logrus.Entry) ([]v1.Container, error) { if sidecars != nil && len(sidecars) > 0 { result := make([]v1.Container, 0) @@ -380,7 +381,7 @@ func generateSidecarContainers(sidecars []spec.Sidecar, return nil, err } - sc := getSidecarContainer(sidecar, index, volumeMounts, resources, superUserName, credentialsSecretName) + sc := getSidecarContainer(sidecar, index, volumeMounts, resources, superUserName, credentialsSecretName, logger) result = append(result, *sc) } return result, nil @@ -393,7 +394,6 @@ func generatePodTemplate( labels labels.Set, spiloContainer *v1.Container, sidecarContainers []v1.Container, - volumeMounts []v1.VolumeMount, tolerationsSpec *[]v1.Toleration, nodeAffinity *v1.Affinity, terminateGracePeriod int64, @@ -548,7 +548,8 @@ func deduplicateEnvVars(input []v1.EnvVar, containerName string, logger *logrus. return result } -func getSidecarContainer(sidecar spec.Sidecar, index int, volumeMounts []v1.VolumeMount, resources *v1.ResourceRequirements, superUserName string, credentialsSecretName string) *v1.Container { +func getSidecarContainer(sidecar spec.Sidecar, index int, volumeMounts []v1.VolumeMount, + resources *v1.ResourceRequirements, superUserName string, credentialsSecretName string, logger *logrus.Entry) *v1.Container { name := sidecar.Name if name == "" { name = fmt.Sprintf("sidecar-%d", index) @@ -598,7 +599,7 @@ func getSidecarContainer(sidecar spec.Sidecar, index int, volumeMounts []v1.Volu ImagePullPolicy: v1.PullIfNotPresent, Resources: *resources, VolumeMounts: volumeMounts, - Env: env, + Env: deduplicateEnvVars(env, name, logger), Ports: sidecar.Ports, } } @@ -674,8 +675,7 @@ func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.Statefu c.OpConfig.ScalyrMemoryLimit, ) - // generate scalyr sidecar container - // TODO: avoid hardcoding scalyr container and use the sidecar mechansim + // generate scalyr sidecar containers if scalarSidecar, present := generateScalarSidecarSpec(c.Name, c.OpConfig.ScalyrAPIKey, @@ -687,7 +687,7 @@ func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.Statefu // generate sidecar containers sidecarContainers, err := generateSidecarContainers(sideCars, volumeMounts, defaultResources, - c.OpConfig.SuperUsername, c.credentialSecretName(c.OpConfig.SuperUsername)) + c.OpConfig.SuperUsername, c.credentialSecretName(c.OpConfig.SuperUsername), c.logger) if err != nil { return nil, fmt.Errorf("could not generate sidecar containers: %v", err) } @@ -700,7 +700,6 @@ func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.Statefu c.labelsSet(true), spiloContainer, sidecarContainers, - volumeMounts, &tolerationSpec, nodeAffinity(c.OpConfig.NodeReadinessLabel), int64(c.OpConfig.PodTerminateGracePeriod.Seconds()), From 3a2c570697d6f2e56c62a23bc0456a03c7e7571f Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Wed, 27 Jun 2018 13:40:32 +0200 Subject: [PATCH 10/13] Document the per-cluster sidecar options --- docs/reference/cluster_manifest.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/reference/cluster_manifest.md b/docs/reference/cluster_manifest.md index c349e1631..b046f0493 100644 --- a/docs/reference/cluster_manifest.md +++ b/docs/reference/cluster_manifest.md @@ -213,3 +213,21 @@ properties of the persistent storage that stores postgres data. See [Kubernetes documentation](https://kubernetes.io/docs/concepts/storage/storage-classes/) for the details on storage classes. Optional. + +### Sidecar definitions + +Those parameters are defined under the `sidecars` key. They consist of a list +of dictionaries, each defining one sidecar (an extra container running +along the main postgres container on the same pod). The following keys can be +defined in the sidecar dictionary: + +* **name** + name of the sidecar. Required. + +* **image** + docker image of the sidecar. Required. + +* **env** + a dictionary of environment variables. Use usual Kubernetes definition + (https://kubernetes.io/docs/tasks/inject-data-application/environment-variable-expose-pod-information/) + for environment variables. Optional. From fa1259f0e68588ad0fed8da54bfae8b71b657745 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Wed, 27 Jun 2018 14:15:59 +0200 Subject: [PATCH 11/13] Rename the sidecars operator parameter. Make it more explicit that this parameter only controls the docker images, not the YAML manifest definiting other aspect of sidecar configuration (such as environment variables.) --- docs/reference/operator_parameters.md | 2 +- pkg/util/config/config.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index c13da2317..f02f4cac8 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -15,7 +15,7 @@ words. your own Spilo image from the [github repository](https://github.com/zalando/spilo). -* **sidecars** +* **sidecar_docker_images** a map of sidecar names to docker images for the containers to run alongside Spilo. In case of the name conflict with the definition in the cluster manifest the cluster-specific one is preferred. diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 11ed77a60..9b19b894d 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -71,7 +71,7 @@ type Config struct { WatchedNamespace string `name:"watched_namespace"` // special values: "*" means 'watch all namespaces', the empty string "" means 'watch a namespace where operator is deployed to' EtcdHost string `name:"etcd_host" default:""` // special values: the empty string "" means Patroni will use k8s as a DCS DockerImage string `name:"docker_image" default:"registry.opensource.zalan.do/acid/spilo-cdp-10:1.4-p8"` - Sidecars map[string]string `name:"sidecars"` + Sidecars map[string]string `name:"sidecar_docker_images"` // default name `operator` enables backward compatibility with the older ServiceAccountName field PodServiceAccountName string `name:"pod_service_account_name" default:"operator"` // value of this string must be valid JSON or YAML; see initPodServiceAccount From e4f60e89d34df6864ac5bf3864534bbabff8d047 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 2 Jul 2018 15:57:11 +0200 Subject: [PATCH 12/13] Fix a few typos and improve the error messags. Per review by @zerg-junior --- pkg/cluster/k8sres.go | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index e6bc10161..d93d09e84 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -526,11 +526,11 @@ func (c *Cluster) generateSpiloPodEnvVars(uid types.UID, spiloConfiguration stri return envVars } -// deduplicateEnvVars makes sure there are no duplicate in the target envVar array. While Kubernetes already does, it -// leaves the last definition in the list and is not documented, which means that the behavior can be reversed at some -// point (it may also start producing an error). Therefore, the merge is done by the operator, the entries that are ahead -// in the passed list take priority over those that are behind, and only the name is considered in order to eliminate -// duplicates. +// deduplicateEnvVars makes sure there are no duplicate in the target envVar array. While Kubernetes already +// deduplicates variables defined in a container, it leaves the last definition in the list and this behavior is not +// well-documented, which means that the behavior can be reversed at some point (it may also start producing an error). +// Therefore, the merge is done by the operator, the entries that are ahead in the passed list take priority over those +// that are behind, and only the name is considered in order to eliminate duplicates. func deduplicateEnvVars(input []v1.EnvVar, containerName string, logger *logrus.Entry) []v1.EnvVar { result := make([]v1.EnvVar, 0) names := make(map[string]int) @@ -675,14 +675,20 @@ func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.Statefu c.OpConfig.ScalyrMemoryLimit, ) - // generate scalyr sidecar containers - if scalarSidecar, present := - generateScalarSidecarSpec(c.Name, + // generate scalyr sidecar container + // TODO: avoid hardcoding scalyr container and use the sidecar mechansim + if scalyrSidecar, defined := + generateScalyrSidecarSpec(c.Name, c.OpConfig.ScalyrAPIKey, c.OpConfig.ScalyrServerURL, c.OpConfig.ScalyrImage, - &resourceRequirementsScalyrSidecar); present { - sideCars = append(sideCars, *scalarSidecar) + &resourceRequirementsScalyrSidecar); defined { + if scalyrSidecar != nil { + sideCars = append(sideCars, *scalyrSidecar) + } else { + c.logger.Warningf("Incomplete configuration for the Scalyr sidecar: " + + "all of SCALYR_API_KEY, SCALYR_SERVER_HOST and SCALYR_SERVER_URL must be defined") + } } // generate sidecar containers @@ -742,9 +748,9 @@ func getEffectiveDockerImage(globalDockerImage, clusterDockerImage string) strin return clusterDockerImage } -func generateScalarSidecarSpec(clusterName, APIKey, serverURL, dockerImage string, containerResources *spec.Resources) (sidecar *spec.Sidecar, present bool) { +func generateScalyrSidecarSpec(clusterName, APIKey, serverURL, dockerImage string, containerResources *spec.Resources) (sidecar *spec.Sidecar, defined bool) { if APIKey == "" || serverURL == "" || dockerImage == "" { - return nil, false + return nil, (APIKey != "" || serverURL != "" || dockerImage != "") } return &spec.Sidecar{ Name: "scalyr-sidecar", From 98da75a60a83d335c6125442aba2976d06316eb5 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 2 Jul 2018 16:14:35 +0200 Subject: [PATCH 13/13] Minor refactoring Per review by @zerg-junior --- pkg/cluster/k8sres.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index d93d09e84..49bd9fe84 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -676,19 +676,13 @@ func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.Statefu ) // generate scalyr sidecar container - // TODO: avoid hardcoding scalyr container and use the sidecar mechansim - if scalyrSidecar, defined := + if scalyrSidecar := generateScalyrSidecarSpec(c.Name, c.OpConfig.ScalyrAPIKey, c.OpConfig.ScalyrServerURL, c.OpConfig.ScalyrImage, - &resourceRequirementsScalyrSidecar); defined { - if scalyrSidecar != nil { - sideCars = append(sideCars, *scalyrSidecar) - } else { - c.logger.Warningf("Incomplete configuration for the Scalyr sidecar: " + - "all of SCALYR_API_KEY, SCALYR_SERVER_HOST and SCALYR_SERVER_URL must be defined") - } + &resourceRequirementsScalyrSidecar, c.logger); scalyrSidecar != nil { + sideCars = append(sideCars, *scalyrSidecar) } // generate sidecar containers @@ -748,9 +742,14 @@ func getEffectiveDockerImage(globalDockerImage, clusterDockerImage string) strin return clusterDockerImage } -func generateScalyrSidecarSpec(clusterName, APIKey, serverURL, dockerImage string, containerResources *spec.Resources) (sidecar *spec.Sidecar, defined bool) { +func generateScalyrSidecarSpec(clusterName, APIKey, serverURL, dockerImage string, + containerResources *spec.Resources, logger *logrus.Entry) *spec.Sidecar { if APIKey == "" || serverURL == "" || dockerImage == "" { - return nil, (APIKey != "" || serverURL != "" || dockerImage != "") + if APIKey != "" || serverURL != "" || dockerImage != "" { + logger.Warningf("Incomplete configuration for the Scalyr sidecar: " + + "all of SCALYR_API_KEY, SCALYR_SERVER_HOST and SCALYR_SERVER_URL must be defined") + } + return nil } return &spec.Sidecar{ Name: "scalyr-sidecar", @@ -788,7 +787,7 @@ func generateScalyrSidecarSpec(clusterName, APIKey, serverURL, dockerImage strin }, }, Resources: *containerResources, - }, true + } } // mergeSidecar merges globally-defined sidecars with those defined in the cluster manifest