From 1f4267eb054699ef91374e33e40317355bf8caee Mon Sep 17 00:00:00 2001 From: Stephane T Date: Tue, 21 May 2019 12:49:34 +0100 Subject: [PATCH 1/2] fix: remove headless service config when deleting cluster (#567) see: https://github.com/zalando/postgres-operator/issues/566 Signed-off-by: Stephane Tang --- pkg/cluster/cluster.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 9cbc46e70..e75002a7f 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1004,8 +1004,8 @@ func (c *Cluster) deletePatroniClusterObjects() error { if !c.patroniUsesKubernetes() { c.logger.Infof("not cleaning up Etcd Patroni objects on cluster delete") } - c.logger.Debugf("removing leftover Patroni objects (endpoints or configmaps)") - for _, deleter := range []simpleActionWithResult{c.deletePatroniClusterEndpoints, c.deletePatroniClusterConfigMaps} { + c.logger.Debugf("removing leftover Patroni objects (endpoints, services and configmaps)") + for _, deleter := range []simpleActionWithResult{c.deletePatroniClusterEndpoints, c.deletePatroniClusterServices, c.deletePatroniClusterConfigMaps} { if err := deleter(); err != nil { return err } @@ -1037,6 +1037,19 @@ func (c *Cluster) deleteClusterObject( return nil } +func (c *Cluster) deletePatroniClusterServices() error { + get := func(name string) (spec.NamespacedName, error) { + svc, err := c.KubeClient.Services(c.Namespace).Get(name, metav1.GetOptions{}) + return util.NameFromMeta(svc.ObjectMeta), err + } + + deleteServiceFn := func(name string) error { + return c.KubeClient.Services(c.Namespace).Delete(name, c.deleteOptions) + } + + return c.deleteClusterObject(get, deleteServiceFn, "service") +} + func (c *Cluster) deletePatroniClusterEndpoints() error { get := func(name string) (spec.NamespacedName, error) { ep, err := c.KubeClient.Endpoints(c.Namespace).Get(name, metav1.GetOptions{}) From 24d412a5622114007484dbfb083b57bccbd642fc Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 22 May 2019 17:35:03 +0200 Subject: [PATCH 2/2] generate spilo config can return error (with test) (#570) * fix: raise explicit error when failing to generate spilo config Signed-off-by: Stephane Tang --- pkg/cluster/k8sres.go | 16 ++++----- pkg/cluster/k8sres_test.go | 66 +++++++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 4953fcfe9..3aa5c3f07 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -149,7 +149,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) (string, error) { config := spiloConfiguration{} config.Bootstrap = pgBootstrap{} @@ -249,12 +249,9 @@ PatroniInitDBParams: Options: []string{constants.RoleFlagCreateDB, constants.RoleFlagNoLogin}, }, } - result, err := json.Marshal(config) - if err != nil { - logger.Errorf("cannot convert spilo configuration into JSON: %v", err) - return "" - } - return string(result) + + res, err := json.Marshal(config) + return string(res), err } func getLocalAndBoostrapPostgreSQLParameters(parameters map[string]string) (local, bootstrap map[string]string) { @@ -780,7 +777,10 @@ 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) + spiloConfiguration, err := generateSpiloJSONConfiguration(&spec.PostgresqlParam, &spec.Patroni, c.OpConfig.PamRoleName, c.logger) + if err != nil { + return nil, fmt.Errorf("could not generate Spilo JSON configuration: %v", err) + } // generate environment variables for the spilo container spiloEnvVars := deduplicateEnvVars( diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index dc48c0389..7ccbcb51d 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -3,11 +3,12 @@ package cluster import ( "k8s.io/api/core/v1" + "testing" + acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" "github.com/zalando/postgres-operator/pkg/util/config" "github.com/zalando/postgres-operator/pkg/util/constants" "github.com/zalando/postgres-operator/pkg/util/k8sutil" - "testing" ) func True() *bool { @@ -20,6 +21,69 @@ func False() *bool { return &b } +func TestGenerateSpiloJSONConfiguration(t *testing.T) { + var cluster = New( + Config{ + OpConfig: config.Config{ + ProtectedRoles: []string{"admin"}, + Auth: config.Auth{ + SuperUsername: superUserName, + ReplicationUsername: replicationUserName, + }, + }, + }, k8sutil.KubernetesClient{}, acidv1.Postgresql{}, logger) + + testName := "TestGenerateSpiloConfig" + tests := []struct { + subtest string + pgParam *acidv1.PostgresqlParam + patroni *acidv1.Patroni + role string + opConfig config.Config + result string + }{ + { + subtest: "Patroni default configuration", + pgParam: &acidv1.PostgresqlParam{PgVersion: "9.6"}, + patroni: &acidv1.Patroni{}, + role: "zalandos", + opConfig: config.Config{}, + result: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/9.6/bin"},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"}],"users":{"zalandos":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{}}}`, + }, + { + subtest: "Patroni configured", + pgParam: &acidv1.PostgresqlParam{PgVersion: "11"}, + patroni: &acidv1.Patroni{ + InitDB: map[string]string{ + "encoding": "UTF8", + "locale": "en_US.UTF-8", + "data-checksums": "true", + }, + PgHba: []string{"hostssl all all 0.0.0.0/0 md5", "host all all 0.0.0.0/0 md5"}, + TTL: 30, + LoopWait: 10, + RetryTimeout: 10, + MaximumLagOnFailover: 33554432, + Slots: map[string]map[string]string{"permanent_logical_1": {"type": "logical", "database": "foo", "plugin": "pgoutput"}}, + }, + role: "zalandos", + opConfig: config.Config{}, + result: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/11/bin","pg_hba":["hostssl all all 0.0.0.0/0 md5","host all all 0.0.0.0/0 md5"]},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"},"data-checksums",{"encoding":"UTF8"},{"locale":"en_US.UTF-8"}],"users":{"zalandos":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{"ttl":30,"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"slots":{"permanent_logical_1":{"database":"foo","plugin":"pgoutput","type":"logical"}}}}}`, + }, + } + for _, tt := range tests { + cluster.OpConfig = tt.opConfig + result, err := generateSpiloJSONConfiguration(tt.pgParam, tt.patroni, tt.role, logger) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if tt.result != result { + t.Errorf("%s %s: Spilo Config is %v, expected %v for role %#v and param %#v", + testName, tt.subtest, result, tt.result, tt.role, tt.pgParam) + } + } +} + func TestCreateLoadBalancerLogic(t *testing.T) { var cluster = New( Config{