From 29cfa27c504ff98d710182f8af63ba49dc37db0e Mon Sep 17 00:00:00 2001 From: Stephane Tang Date: Wed, 6 Mar 2019 12:35:23 +0000 Subject: [PATCH 1/3] save initial patroniConfig in annotation Signed-off-by: Stephane Tang --- pkg/cluster/k8sres.go | 17 ++++++++++++++++- pkg/cluster/resources.go | 1 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 87eeee27a..7a83cb579 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -858,12 +858,27 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*v1beta1.State numberOfInstances := c.getNumberOfInstances(spec) + initialPatroniConfig := c.Statefulset.ObjectMeta.Annotations[initialPatroniConfigAnnotationKey] + ann := map[string]string{rollingUpdateStatefulsetAnnotationKey: "false"} + // Save the initial patroni config for later usage. + // Note: if the annotation is empty (during the upgrade path for example), set it to the current specs. + // see: issues#501 + if c.isNewCluster() || initialPatroniConfig == "" { + x, err := json.Marshal(spec.Patroni) + if err != nil { + return nil, fmt.Errorf("could not JSON encode initial patroni config: %v", err) + } + ann[initialPatroniConfigAnnotationKey] = string(x) + } else { + ann[initialPatroniConfigAnnotationKey] = initialPatroniConfig + } + statefulSet := &v1beta1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ Name: c.statefulSetName(), Namespace: c.Namespace, Labels: c.labelsSet(true), - Annotations: map[string]string{rollingUpdateStatefulsetAnnotationKey: "false"}, + Annotations: ann, }, Spec: v1beta1.StatefulSetSpec{ Replicas: &numberOfInstances, diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index 18c295804..7f15c12a8 100644 --- a/pkg/cluster/resources.go +++ b/pkg/cluster/resources.go @@ -19,6 +19,7 @@ import ( const ( rollingUpdateStatefulsetAnnotationKey = "zalando-postgres-operator-rolling-update-required" + initialPatroniConfigAnnotationKey = "v1.acid.zalan.do/initial-patroni-config" ) func (c *Cluster) listResources() error { From 3ca1a76e2a13ec2de5f9acac365809a00625e32a Mon Sep 17 00:00:00 2001 From: Stephane Tang Date: Wed, 6 Mar 2019 13:22:55 +0000 Subject: [PATCH 2/3] fix: do not update 'Slots' configuration in SPILO_CONFIGURATION Fixes #501 Signed-off-by: Stephane Tang --- pkg/cluster/k8sres.go | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 7a83cb579..61c6c4ec4 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -147,7 +147,7 @@ func fillResourceList(spec acidv1.ResourceDescription, defaults acidv1.ResourceD return requests, nil } -func generateSpiloJSONConfiguration(pg *acidv1.PostgresqlParam, patroni *acidv1.Patroni, pamRoleName string, logger *logrus.Entry) string { +func generateSpiloJSONConfiguration(pg *acidv1.PostgresqlParam, patroni *acidv1.Patroni, pamRoleName string, logger *logrus.Entry, isNewCluster bool, initialPatroniConfig string) string { config := spiloConfiguration{} config.Bootstrap = pgBootstrap{} @@ -217,8 +217,27 @@ PatroniInitDBParams: if patroni.TTL != 0 { config.Bootstrap.DCS.TTL = patroni.TTL } - if patroni.Slots != nil { - config.Bootstrap.DCS.Slots = patroni.Slots + + // The SPILO image is taking in account the 'Slots' settings **only** when bootstraping a new cluster. + // Therefore, updating this value in the SPILO_CONFIGURATION of the statefulset is pointless, but still + // required at cluster creation. + if isNewCluster { + if patroni.Slots != nil { + config.Bootstrap.DCS.Slots = patroni.Slots + } + } else { + oldestKnownPatroniCfg := acidv1.Patroni{} + if initialPatroniConfig == "" { + // when upgrading postgres-operator to this new version, existing StatefulSet won't have this annotation, so we're using the current specs + oldestKnownPatroniCfg = *patroni + } else { + if err := json.Unmarshal([]byte(initialPatroniConfig), &oldestKnownPatroniCfg); err != nil { + logger.Errorf("cannot decode initial patroni config from annotations: %s (%v)", err, initialPatroniConfig) + return "" + } + } + + config.Bootstrap.DCS.Slots = oldestKnownPatroniCfg.Slots } config.PgLocalConfiguration = make(map[string]interface{}) @@ -778,7 +797,12 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*v1beta1.State func(i, j int) bool { return customPodEnvVarsList[i].Name < customPodEnvVarsList[j].Name }) } - spiloConfiguration := generateSpiloJSONConfiguration(&spec.PostgresqlParam, &spec.Patroni, c.OpConfig.PamRoleName, c.logger) + // c.Statefulset is not set when called from .createStatefulSet() + initialPatroniConfig := "" + if c.Statefulset != nil { + initialPatroniConfig = c.Statefulset.ObjectMeta.Annotations[initialPatroniConfigAnnotationKey] + } + spiloConfiguration := generateSpiloJSONConfiguration(&spec.PostgresqlParam, &spec.Patroni, c.OpConfig.PamRoleName, c.logger, c.isNewCluster(), initialPatroniConfig) // generate environment variables for the spilo container spiloEnvVars := deduplicateEnvVars( @@ -858,7 +882,6 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*v1beta1.State numberOfInstances := c.getNumberOfInstances(spec) - initialPatroniConfig := c.Statefulset.ObjectMeta.Annotations[initialPatroniConfigAnnotationKey] ann := map[string]string{rollingUpdateStatefulsetAnnotationKey: "false"} // Save the initial patroni config for later usage. // Note: if the annotation is empty (during the upgrade path for example), set it to the current specs. From d4faf3f01b2bd763fbc25a7d181eb06c9791973c Mon Sep 17 00:00:00 2001 From: Stephane Tang Date: Wed, 6 Mar 2019 14:02:14 +0000 Subject: [PATCH 3/3] fix: update patroni config when necessary Fixes #447 Signed-off-by: Stephane Tang --- pkg/cluster/cluster.go | 8 ++++++++ pkg/cluster/sync.go | 33 +++++++++++++++++++++++++++++++++ pkg/util/patroni/patroni.go | 12 ++++++++++++ 3 files changed, 53 insertions(+) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index d4680cec0..1a5e253ab 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -565,6 +565,14 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { c.logger.Errorf("could not sync statefulsets: %v", err) updateFailed = true } + } else { + // patroni configuration is not always reflected on the StatefulSet as they have + // different lifecycle (due to SPILO's implementation - see: issues#501) but they + // still need to be updated via the Patroni REST API. + if err := c.applyPatroniConfig(); err != nil { + c.logger.Errorf("could not apply patroni config: %v", err) + updateFailed = true + } } }() diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index d2157c8a2..1ed87b881 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -303,6 +303,10 @@ func (c *Cluster) syncStatefulSet() error { return fmt.Errorf("could not set cluster-wide PostgreSQL configuration options: %v", err) } + if err := c.applyPatroniConfig(); err != nil { + return fmt.Errorf("could not apply patroni config: %v", err) + } + // if we get here we also need to re-create the pods (either leftovers from the old // statefulset or those that got their configuration from the outdated statefulset) if podsRollingUpdateRequired { @@ -361,6 +365,35 @@ func (c *Cluster) checkAndSetGlobalPostgreSQLConfiguration() error { len(pods)) } +// TODO: merge applyPatroniConfig() and checkAndSetGlobalPostgreSQLConfiguration()? +func (c *Cluster) applyPatroniConfig() error { + var ( + err error + pods []v1.Pod + ) + + if pods, err = c.listPods(); err != nil { + return err + } + if len(pods) == 0 { + return fmt.Errorf("could not call Patroni API: cluster has no pods") + } + // try all pods until the first one that is successful, as it doesn't matter which pod + // carries the request to change configuration through + for _, pod := range pods { + podName := util.NameFromMeta(pod.ObjectMeta) + c.logger.Debugf("calling Patroni API on a pod %s to set the following patroni config: %v", + podName, c.Postgresql.Spec.Patroni) + + if err = c.patroni.ApplyConfig(&pod, c.Postgresql.Spec.Patroni); err == nil { + return nil + } + c.logger.Warningf("could not patch patroni config with on pod %s: %v", podName, err) + } + return fmt.Errorf("could not reach Patroni API to patch patroni config: failed on every pod (%d total)", + len(pods)) +} + func (c *Cluster) syncSecrets() error { var ( err error diff --git a/pkg/util/patroni/patroni.go b/pkg/util/patroni/patroni.go index 23260f277..3e9e94d83 100644 --- a/pkg/util/patroni/patroni.go +++ b/pkg/util/patroni/patroni.go @@ -10,6 +10,8 @@ import ( "github.com/sirupsen/logrus" "k8s.io/api/core/v1" + + acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" ) const ( @@ -23,6 +25,7 @@ const ( type Interface interface { Switchover(master *v1.Pod, candidate string) error SetPostgresParameters(server *v1.Pod, options map[string]string) error + ApplyConfig(server *v1.Pod, config acidv1.Patroni) error } // Patroni API client @@ -102,3 +105,12 @@ func (p *Patroni) SetPostgresParameters(server *v1.Pod, parameters map[string]st } return p.httpPostOrPatch(http.MethodPatch, apiURL(server)+configPath, buf) } + +func (p *Patroni) ApplyConfig(server *v1.Pod, config acidv1.Patroni) error { + buf := &bytes.Buffer{} + err := json.NewEncoder(buf).Encode(config) + if err != nil { + return fmt.Errorf("could not encode json: %v", err) + } + return p.httpPostOrPatch(http.MethodPatch, apiURL(server)+configPath, buf) +}