change Clone attribute of PostgresSpec to *CloneDescription (#1020)

* change Clone attribute of PostgresSpec to *ConnectionPooler * update go.mod from master * fix TestConnectionPoolerSynchronization() * Update pkg/apis/acid.zalan.do/v1/postgresql_type.go Co-authored-by: Felix Kunde <felix-kunde@gmx.de> Co-authored-by: Pavlo Golub <pavlo.golub@gmail.com> Co-authored-by: Felix Kunde <felix-kunde@gmx.de>
2020-07-30 16:31:29 +02:00 · 2020-07-30 16:31:29 +02:00 · 47b11f7f89
parent 3bee590d43
commit 47b11f7f89
8 changed files with 50 additions and 41 deletions
--- a/.gitignore
+++ b/.gitignore
@ -30,6 +30,7 @@ _testmain.go
 /docker/build/
 /github.com/
 .idea
+.vscode

 scm-source.json

--- a/pkg/apis/acid.zalan.do/v1/marshal.go
+++ b/pkg/apis/acid.zalan.do/v1/marshal.go
@ -112,8 +112,9 @@ func (p *Postgresql) UnmarshalJSON(data []byte) error {

 	if clusterName, err := extractClusterName(tmp2.ObjectMeta.Name, tmp2.Spec.TeamID); err != nil {
 		tmp2.Error = err.Error()
-		tmp2.Status.PostgresClusterStatus = ClusterStatusInvalid
-	} else if err := validateCloneClusterDescription(&tmp2.Spec.Clone); err != nil {
+		tmp2.Status = PostgresStatus{PostgresClusterStatus: ClusterStatusInvalid}
+	} else if err := validateCloneClusterDescription(tmp2.Spec.Clone); err != nil {
+
 		tmp2.Error = err.Error()
 		tmp2.Status.PostgresClusterStatus = ClusterStatusInvalid
 	} else {
--- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go
+++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go
@ -53,7 +53,7 @@ type PostgresSpec struct {
 	NumberOfInstances     int32                       `json:"numberOfInstances"`
 	Users                 map[string]UserFlags        `json:"users"`
 	MaintenanceWindows    []MaintenanceWindow         `json:"maintenanceWindows,omitempty"`
-	Clone                 CloneDescription            `json:"clone"`
+	Clone                 *CloneDescription           `json:"clone"`
 	ClusterName           string                      `json:"-"`
 	Databases             map[string]string           `json:"databases,omitempty"`
 	PreparedDatabases     map[string]PreparedDatabase `json:"preparedDatabases,omitempty"`
--- a/pkg/apis/acid.zalan.do/v1/util.go
+++ b/pkg/apis/acid.zalan.do/v1/util.go
@ -72,7 +72,7 @@ func extractClusterName(clusterName string, teamName string) (string, error) {

 func validateCloneClusterDescription(clone *CloneDescription) error {
 	// when cloning from the basebackup (no end timestamp) check that the cluster name is a valid service name
-	if clone.ClusterName != "" && clone.EndTimestamp == "" {
+	if clone != nil && clone.ClusterName != "" && clone.EndTimestamp == "" {
 		if !serviceNameRegex.MatchString(clone.ClusterName) {
 			return fmt.Errorf("clone cluster name must confirm to DNS-1035, regex used for validation is %q",
 				serviceNameRegexString)
--- a/pkg/apis/acid.zalan.do/v1/util_test.go
+++ b/pkg/apis/acid.zalan.do/v1/util_test.go
@ -163,7 +163,7 @@ var unmarshalCluster = []struct {
 				"kind": "Postgresql","apiVersion": "acid.zalan.do/v1",
 				"metadata": {"name": "acid-testcluster1"}, "spec": {"teamId": 100}}`), &tmp).Error(),
 		},
-		marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"resources":{"requests":{"cpu":"","memory":""},"limits":{"cpu":"","memory":""}},"teamId":"","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":{}},"status":"Invalid"}`),
+		marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"resources":{"requests":{"cpu":"","memory":""},"limits":{"cpu":"","memory":""}},"teamId":"","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":null},"status":"Invalid"}`),
 		err:     nil},
 	{
 		about: "example with /status subresource",
@ -184,7 +184,7 @@ var unmarshalCluster = []struct {
 				"kind": "Postgresql","apiVersion": "acid.zalan.do/v1",
 				"metadata": {"name": "acid-testcluster1"}, "spec": {"teamId": 100}}`), &tmp).Error(),
 		},
-		marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"resources":{"requests":{"cpu":"","memory":""},"limits":{"cpu":"","memory":""}},"teamId":"","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":{}},"status":{"PostgresClusterStatus":"Invalid"}}`),
+		marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"resources":{"requests":{"cpu":"","memory":""},"limits":{"cpu":"","memory":""}},"teamId":"","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":null},"status":{"PostgresClusterStatus":"Invalid"}}`),
 		err:     nil},
 	{
 		about: "example with detailed input manifest and deprecated pod_priority_class_name -> podPriorityClassName",
@ -327,7 +327,7 @@ var unmarshalCluster = []struct {
 						EndTime:   mustParseTime("05:15"),
 					},
 				},
-				Clone: CloneDescription{
+				Clone: &CloneDescription{
 					ClusterName: "acid-batman",
 				},
 				ClusterName: "testcluster1",
@ -351,7 +351,7 @@ var unmarshalCluster = []struct {
 			Status: PostgresStatus{PostgresClusterStatus: ClusterStatusInvalid},
 			Error:  errors.New("name must match {TEAM}-{NAME} format").Error(),
 		},
-		marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"teapot-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null} ,"resources":{"requests":{"cpu":"","memory":""},"limits":{"cpu":"","memory":""}},"teamId":"acid","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":{}},"status":{"PostgresClusterStatus":"Invalid"}}`),
+		marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"teapot-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null} ,"resources":{"requests":{"cpu":"","memory":""},"limits":{"cpu":"","memory":""}},"teamId":"acid","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":null},"status":{"PostgresClusterStatus":"Invalid"}}`),
 		err:     nil},
 	{
 		about: "example with clone",
@ -366,7 +366,7 @@ var unmarshalCluster = []struct {
 			},
 			Spec: PostgresSpec{
 				TeamID: "acid",
-				Clone: CloneDescription{
+				Clone: &CloneDescription{
 					ClusterName: "team-batman",
 				},
 				ClusterName: "testcluster1",
@ -405,7 +405,7 @@ var unmarshalCluster = []struct {
 		err:     errors.New("unexpected end of JSON input")},
 	{
 		about:   "expect error on JSON with field's value malformatted",
-		in:      []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster","creationTimestamp":qaz},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"resources":{"requests":{"cpu":"","memory":""},"limits":{"cpu":"","memory":""}},"teamId":"acid","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":{}},"status":{"PostgresClusterStatus":"Invalid"}}`),
+		in:      []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster","creationTimestamp":qaz},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"resources":{"requests":{"cpu":"","memory":""},"limits":{"cpu":"","memory":""}},"teamId":"acid","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":null},"status":{"PostgresClusterStatus":"Invalid"}}`),
 		out:     Postgresql{},
 		marshal: []byte{},
 		err:     errors.New("invalid character 'q' looking for beginning of value"),
--- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go
+++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go
@ -567,7 +567,11 @@ func (in *PostgresSpec) DeepCopyInto(out *PostgresSpec) {
 			(*in)[i].DeepCopyInto(&(*out)[i])
 		}
 	}
-	in.Clone.DeepCopyInto(&out.Clone)
+	if in.Clone != nil {
+		in, out := &in.Clone, &out.Clone
+		*out = new(CloneDescription)
+		(*in).DeepCopyInto(*out)
+	}
 	if in.Databases != nil {
 		in, out := &in.Databases, &out.Databases
 		*out = make(map[string]string, len(*in))
--- a/pkg/cluster/k8sres.go
+++ b/pkg/cluster/k8sres.go
@ -725,7 +725,7 @@ func (c *Cluster) generateSpiloPodEnvVars(uid types.UID, spiloConfiguration stri
 		envVars = append(envVars, v1.EnvVar{Name: "KUBERNETES_USE_CONFIGMAPS", Value: "true"})
 	}

-	if cloneDescription.ClusterName != "" {
+	if cloneDescription != nil && cloneDescription.ClusterName != "" {
 		envVars = append(envVars, c.generateCloneEnvironment(cloneDescription)...)
 	}

@ -1065,7 +1065,7 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef
 	spiloEnvVars := c.generateSpiloPodEnvVars(
 		c.Postgresql.GetUID(),
 		spiloConfiguration,
-		&spec.Clone,
+		spec.Clone,
 		spec.StandbyCluster,
 		customPodEnvVarsList,
 	)
--- a/pkg/cluster/sync_test.go
+++ b/pkg/cluster/sync_test.go
@ -63,23 +63,26 @@ func noEmptySync(cluster *Cluster, err error, reason SyncReason) error {

 func TestConnectionPoolerSynchronization(t *testing.T) {
 	testName := "Test connection pooler synchronization"
-	var cluster = New(
-		Config{
-			OpConfig: config.Config{
-				ProtectedRoles: []string{"admin"},
-				Auth: config.Auth{
-					SuperUsername:       superUserName,
-					ReplicationUsername: replicationUserName,
+	newCluster := func() *Cluster {
+		return New(
+			Config{
+				OpConfig: config.Config{
+					ProtectedRoles: []string{"admin"},
+					Auth: config.Auth{
+						SuperUsername:       superUserName,
+						ReplicationUsername: replicationUserName,
+					},
+					ConnectionPooler: config.ConnectionPooler{
+						ConnectionPoolerDefaultCPURequest:    "100m",
+						ConnectionPoolerDefaultCPULimit:      "100m",
+						ConnectionPoolerDefaultMemoryRequest: "100Mi",
+						ConnectionPoolerDefaultMemoryLimit:   "100Mi",
+						NumberOfInstances:                    int32ToPointer(1),
+					},
 				},
-				ConnectionPooler: config.ConnectionPooler{
-					ConnectionPoolerDefaultCPURequest:    "100m",
-					ConnectionPoolerDefaultCPULimit:      "100m",
-					ConnectionPoolerDefaultMemoryRequest: "100Mi",
-					ConnectionPoolerDefaultMemoryLimit:   "100Mi",
-					NumberOfInstances:                    int32ToPointer(1),
-				},
-			},
-		}, k8sutil.KubernetesClient{}, acidv1.Postgresql{}, logger, eventRecorder)
+			}, k8sutil.KubernetesClient{}, acidv1.Postgresql{}, logger, eventRecorder)
+	}
+	cluster := newCluster()

 	cluster.Statefulset = &appsv1.StatefulSet{
 		ObjectMeta: metav1.ObjectMeta{
@ -87,20 +90,20 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
 		},
 	}

-	clusterMissingObjects := *cluster
+	clusterMissingObjects := newCluster()
 	clusterMissingObjects.KubeClient = k8sutil.ClientMissingObjects()

-	clusterMock := *cluster
+	clusterMock := newCluster()
 	clusterMock.KubeClient = k8sutil.NewMockKubernetesClient()

-	clusterDirtyMock := *cluster
+	clusterDirtyMock := newCluster()
 	clusterDirtyMock.KubeClient = k8sutil.NewMockKubernetesClient()
 	clusterDirtyMock.ConnectionPooler = &ConnectionPoolerObjects{
 		Deployment: &appsv1.Deployment{},
 		Service:    &v1.Service{},
 	}

-	clusterNewDefaultsMock := *cluster
+	clusterNewDefaultsMock := newCluster()
 	clusterNewDefaultsMock.KubeClient = k8sutil.NewMockKubernetesClient()

 	tests := []struct {
@ -124,7 +127,7 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
 					ConnectionPooler: &acidv1.ConnectionPooler{},
 				},
 			},
-			cluster:          &clusterMissingObjects,
+			cluster:          clusterMissingObjects,
 			defaultImage:     "pooler:1.0",
 			defaultInstances: 1,
 			check:            objectsAreSaved,
@ -139,7 +142,7 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
 					EnableConnectionPooler: boolToPointer(true),
 				},
 			},
-			cluster:          &clusterMissingObjects,
+			cluster:          clusterMissingObjects,
 			defaultImage:     "pooler:1.0",
 			defaultInstances: 1,
 			check:            objectsAreSaved,
@ -154,7 +157,7 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
 					ConnectionPooler: &acidv1.ConnectionPooler{},
 				},
 			},
-			cluster:          &clusterMissingObjects,
+			cluster:          clusterMissingObjects,
 			defaultImage:     "pooler:1.0",
 			defaultInstances: 1,
 			check:            objectsAreSaved,
@ -169,7 +172,7 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
 			newSpec: &acidv1.Postgresql{
 				Spec: acidv1.PostgresSpec{},
 			},
-			cluster:          &clusterMock,
+			cluster:          clusterMock,
 			defaultImage:     "pooler:1.0",
 			defaultInstances: 1,
 			check:            objectsAreDeleted,
@ -182,7 +185,7 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
 			newSpec: &acidv1.Postgresql{
 				Spec: acidv1.PostgresSpec{},
 			},
-			cluster:          &clusterDirtyMock,
+			cluster:          clusterDirtyMock,
 			defaultImage:     "pooler:1.0",
 			defaultInstances: 1,
 			check:            objectsAreDeleted,
@ -203,7 +206,7 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
 					},
 				},
 			},
-			cluster:          &clusterMock,
+			cluster:          clusterMock,
 			defaultImage:     "pooler:1.0",
 			defaultInstances: 1,
 			check:            deploymentUpdated,
@ -220,7 +223,7 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
 					ConnectionPooler: &acidv1.ConnectionPooler{},
 				},
 			},
-			cluster:          &clusterNewDefaultsMock,
+			cluster:          clusterNewDefaultsMock,
 			defaultImage:     "pooler:2.0",
 			defaultInstances: 2,
 			check:            deploymentUpdated,
@ -239,7 +242,7 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
 					ConnectionPooler:       &acidv1.ConnectionPooler{},
 				},
 			},
-			cluster:          &clusterMock,
+			cluster:          clusterMock,
 			defaultImage:     "pooler:1.0",
 			defaultInstances: 1,
 			check:            noEmptySync,