generate spilo config can return error (with test) (zalando#570)

* fix: raise explicit error when failing to generate spilo config
Signed-off-by: Stephane Tang <[email protected]>

Author: FxKu

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(

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{