From 0fbfbb23bbd5a534bab7565498b6e48e8ffe54d2 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Tue, 7 May 2019 12:01:45 +0200 Subject: [PATCH] Use /status subresource instead of plain manifest field (#534) * turns PostgresStatus type into a struct with field PostgresClusterStatus * setStatus patch target is now /status subresource * unmarshalling PostgresStatus takes care of previous status field convention * new simple bool functions status.Running(), status.Creating() --- .../templates/clusterrole.yaml | 1 + manifests/operator-service-account-rbac.yaml | 3 +- pkg/apis/acid.zalan.do/v1/const.go | 16 +- pkg/apis/acid.zalan.do/v1/marshal.go | 27 +- pkg/apis/acid.zalan.do/v1/postgresql_type.go | 6 +- pkg/apis/acid.zalan.do/v1/util.go | 22 +- pkg/apis/acid.zalan.do/v1/util_test.go | 293 +++++++++++------- .../acid.zalan.do/v1/zz_generated.deepcopy.go | 17 + pkg/cluster/cluster.go | 26 +- pkg/cluster/cluster_test.go | 18 +- pkg/cluster/sync.go | 2 +- pkg/controller/util_test.go | 64 +--- pkg/util/k8sutil/k8sutil.go | 66 +++- 13 files changed, 356 insertions(+), 205 deletions(-) diff --git a/charts/postgres-operator/templates/clusterrole.yaml b/charts/postgres-operator/templates/clusterrole.yaml index cdd58f5ba..93e60c02a 100644 --- a/charts/postgres-operator/templates/clusterrole.yaml +++ b/charts/postgres-operator/templates/clusterrole.yaml @@ -13,6 +13,7 @@ rules: - acid.zalan.do resources: - postgresqls + - postgresqls/status - operatorconfigurations verbs: - "*" diff --git a/manifests/operator-service-account-rbac.yaml b/manifests/operator-service-account-rbac.yaml index 7bd539ac5..4fd4676cc 100644 --- a/manifests/operator-service-account-rbac.yaml +++ b/manifests/operator-service-account-rbac.yaml @@ -14,6 +14,7 @@ rules: - acid.zalan.do resources: - postgresqls + - postgresqls/status - operatorconfigurations verbs: - "*" @@ -137,7 +138,7 @@ rules: - clusterroles verbs: - bind - resourceNames: + resourceNames: - zalando-postgres-operator --- diff --git a/pkg/apis/acid.zalan.do/v1/const.go b/pkg/apis/acid.zalan.do/v1/const.go index 59d6c1406..3cb1c1ade 100644 --- a/pkg/apis/acid.zalan.do/v1/const.go +++ b/pkg/apis/acid.zalan.do/v1/const.go @@ -2,14 +2,14 @@ package v1 // ClusterStatusUnknown etc : status of a Postgres cluster known to the operator const ( - ClusterStatusUnknown PostgresStatus = "" - ClusterStatusCreating PostgresStatus = "Creating" - ClusterStatusUpdating PostgresStatus = "Updating" - ClusterStatusUpdateFailed PostgresStatus = "UpdateFailed" - ClusterStatusSyncFailed PostgresStatus = "SyncFailed" - ClusterStatusAddFailed PostgresStatus = "CreateFailed" - ClusterStatusRunning PostgresStatus = "Running" - ClusterStatusInvalid PostgresStatus = "Invalid" + ClusterStatusUnknown = "" + ClusterStatusCreating = "Creating" + ClusterStatusUpdating = "Updating" + ClusterStatusUpdateFailed = "UpdateFailed" + ClusterStatusSyncFailed = "SyncFailed" + ClusterStatusAddFailed = "CreateFailed" + ClusterStatusRunning = "Running" + ClusterStatusInvalid = "Invalid" ) const ( diff --git a/pkg/apis/acid.zalan.do/v1/marshal.go b/pkg/apis/acid.zalan.do/v1/marshal.go index 823ff0ef2..d180f784c 100644 --- a/pkg/apis/acid.zalan.do/v1/marshal.go +++ b/pkg/apis/acid.zalan.do/v1/marshal.go @@ -8,6 +8,7 @@ import ( ) type postgresqlCopy Postgresql +type postgresStatusCopy PostgresStatus // MarshalJSON converts a maintenance window definition to JSON. func (m *MaintenanceWindow) MarshalJSON() ([]byte, error) { @@ -69,6 +70,26 @@ func (m *MaintenanceWindow) UnmarshalJSON(data []byte) error { return nil } +// UnmarshalJSON converts a JSON to the status subresource definition. +func (ps *PostgresStatus) UnmarshalJSON(data []byte) error { + var ( + tmp postgresStatusCopy + status string + ) + + err := json.Unmarshal(data, &tmp) + if err != nil { + metaErr := json.Unmarshal(data, &status) + if metaErr != nil { + return fmt.Errorf("Could not parse status: %v; err %v", string(data), metaErr) + } + tmp.PostgresClusterStatus = status + } + *ps = PostgresStatus(tmp) + + return nil +} + // UnmarshalJSON converts a JSON into the PostgreSQL object. func (p *Postgresql) UnmarshalJSON(data []byte) error { var tmp postgresqlCopy @@ -81,7 +102,7 @@ func (p *Postgresql) UnmarshalJSON(data []byte) error { } tmp.Error = err.Error() - tmp.Status = ClusterStatusInvalid + tmp.Status = PostgresStatus{PostgresClusterStatus: ClusterStatusInvalid} *p = Postgresql(tmp) @@ -91,10 +112,10 @@ 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 = ClusterStatusInvalid + tmp2.Status = PostgresStatus{PostgresClusterStatus: ClusterStatusInvalid} } else if err := validateCloneClusterDescription(&tmp2.Spec.Clone); err != nil { tmp2.Error = err.Error() - tmp2.Status = ClusterStatusInvalid + tmp2.Status = PostgresStatus{PostgresClusterStatus: ClusterStatusInvalid} } else { tmp2.Spec.ClusterName = clusterName } diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go index ccd7fe08c..6f7a1f857 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go +++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go @@ -16,7 +16,7 @@ type Postgresql struct { metav1.ObjectMeta `json:"metadata,omitempty"` Spec PostgresSpec `json:"spec"` - Status PostgresStatus `json:"status,omitempty"` + Status PostgresStatus `json:"status"` Error string `json:"-"` } @@ -129,4 +129,6 @@ type Sidecar struct { type UserFlags []string // PostgresStatus contains status of the PostgreSQL cluster (running, creation failed etc.) -type PostgresStatus string +type PostgresStatus struct { + PostgresClusterStatus string `json:"PostgresClusterStatus"` +} diff --git a/pkg/apis/acid.zalan.do/v1/util.go b/pkg/apis/acid.zalan.do/v1/util.go index 0a3267972..db6efcd71 100644 --- a/pkg/apis/acid.zalan.do/v1/util.go +++ b/pkg/apis/acid.zalan.do/v1/util.go @@ -85,12 +85,22 @@ func validateCloneClusterDescription(clone *CloneDescription) error { } // Success of the current Status -func (status PostgresStatus) Success() bool { - return status != ClusterStatusAddFailed && - status != ClusterStatusUpdateFailed && - status != ClusterStatusSyncFailed +func (postgresStatus PostgresStatus) Success() bool { + return postgresStatus.PostgresClusterStatus != ClusterStatusAddFailed && + postgresStatus.PostgresClusterStatus != ClusterStatusUpdateFailed && + postgresStatus.PostgresClusterStatus != ClusterStatusSyncFailed } -func (status PostgresStatus) String() string { - return string(status) +// Running status of cluster +func (postgresStatus PostgresStatus) Running() bool { + return postgresStatus.PostgresClusterStatus == ClusterStatusRunning +} + +// Creating status of cluster +func (postgresStatus PostgresStatus) Creating() bool { + return postgresStatus.PostgresClusterStatus == ClusterStatusCreating +} + +func (postgresStatus PostgresStatus) String() string { + return postgresStatus.PostgresClusterStatus } diff --git a/pkg/apis/acid.zalan.do/v1/util_test.go b/pkg/apis/acid.zalan.do/v1/util_test.go index 01be31e88..6b9f3df99 100644 --- a/pkg/apis/acid.zalan.do/v1/util_test.go +++ b/pkg/apis/acid.zalan.do/v1/util_test.go @@ -111,101 +111,139 @@ var maintenanceWindows = []struct { {[]byte(`"Mon:00:00"`), MaintenanceWindow{}, errors.New("incorrect maintenance window format")}, {[]byte(`"Mon:00:00-00:00:00"`), MaintenanceWindow{}, errors.New("could not parse end time: incorrect time format")}} +var postgresStatus = []struct { + in []byte + out PostgresStatus + err error +}{ + {[]byte(`{"PostgresClusterStatus":"Running"}`), + PostgresStatus{PostgresClusterStatus: ClusterStatusRunning}, nil}, + {[]byte(`{"PostgresClusterStatus":""}`), + PostgresStatus{PostgresClusterStatus: ClusterStatusUnknown}, nil}, + {[]byte(`"Running"`), + PostgresStatus{PostgresClusterStatus: ClusterStatusRunning}, nil}, + {[]byte(`""`), + PostgresStatus{PostgresClusterStatus: ClusterStatusUnknown}, nil}} + var unmarshalCluster = []struct { in []byte out Postgresql marshal []byte err error -}{{ - []byte(`{ - "kind": "Postgresql","apiVersion": "acid.zalan.do/v1", - "metadata": {"name": "acid-testcluster1"}, "spec": {"teamId": 100}}`), - Postgresql{ - TypeMeta: metav1.TypeMeta{ - Kind: "Postgresql", - APIVersion: "acid.zalan.do/v1", +}{ + // example with simple status field + { + in: []byte(`{ + "kind": "Postgresql","apiVersion": "acid.zalan.do/v1", + "metadata": {"name": "acid-testcluster1"}, "spec": {"teamId": 100}}`), + out: Postgresql{ + TypeMeta: metav1.TypeMeta{ + Kind: "Postgresql", + APIVersion: "acid.zalan.do/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "acid-testcluster1", + }, + Status: PostgresStatus{PostgresClusterStatus: ClusterStatusInvalid}, + // This error message can vary between Go versions, so compute it for the current version. + Error: json.Unmarshal([]byte(`{"teamId": 0}`), &PostgresSpec{}).Error(), }, - ObjectMeta: metav1.ObjectMeta{ - Name: "acid-testcluster1", + 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"}`), + err: nil}, + // example with /status subresource + { + in: []byte(`{ + "kind": "Postgresql","apiVersion": "acid.zalan.do/v1", + "metadata": {"name": "acid-testcluster1"}, "spec": {"teamId": 100}}`), + out: Postgresql{ + TypeMeta: metav1.TypeMeta{ + Kind: "Postgresql", + APIVersion: "acid.zalan.do/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "acid-testcluster1", + }, + Status: PostgresStatus{PostgresClusterStatus: ClusterStatusInvalid}, + // This error message can vary between Go versions, so compute it for the current version. + Error: json.Unmarshal([]byte(`{"teamId": 0}`), &PostgresSpec{}).Error(), }, - Status: ClusterStatusInvalid, - // This error message can vary between Go versions, so compute it for the current version. - Error: json.Unmarshal([]byte(`{"teamId": 0}`), &PostgresSpec{}).Error(), - }, - []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"}`), nil}, - {[]byte(`{ - "kind": "Postgresql", - "apiVersion": "acid.zalan.do/v1", - "metadata": { - "name": "acid-testcluster1" - }, - "spec": { - "teamId": "ACID", - "volume": { - "size": "5Gi", - "storageClass": "SSD" - }, - "numberOfInstances": 2, - "users": { - "zalando": [ - "superuser", - "createdb" - ] - }, - "allowedSourceRanges": [ - "127.0.0.1/32" - ], - "postgresql": { - "version": "9.6", - "parameters": { - "shared_buffers": "32MB", - "max_connections": "10", - "log_statement": "all" - } - }, - "resources": { - "requests": { - "cpu": "10m", - "memory": "50Mi" - }, - "limits": { - "cpu": "300m", - "memory": "3000Mi" - } - }, - "clone" : { - "cluster": "acid-batman" - }, - "patroni": { - "initdb": { - "encoding": "UTF8", - "locale": "en_US.UTF-8", - "data-checksums": "true" - }, - "pg_hba": [ - "hostssl all all 0.0.0.0/0 md5", - "host all all 0.0.0.0/0 md5" - ], - "ttl": 30, - "loop_wait": 10, - "retry_timeout": 10, - "maximum_lag_on_failover": 33554432, - "slots" : { - "permanent_logical_1" : { - "type" : "logical", - "database" : "foo", - "plugin" : "pgoutput" - } + 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"}}`), + err: nil}, + // example with detailed input manifest + { + in: []byte(`{ + "kind": "Postgresql", + "apiVersion": "acid.zalan.do/v1", + "metadata": { + "name": "acid-testcluster1" + }, + "spec": { + "teamId": "ACID", + "volume": { + "size": "5Gi", + "storageClass": "SSD" + }, + "numberOfInstances": 2, + "users": { + "zalando": [ + "superuser", + "createdb" + ] + }, + "allowedSourceRanges": [ + "127.0.0.1/32" + ], + "postgresql": { + "version": "9.6", + "parameters": { + "shared_buffers": "32MB", + "max_connections": "10", + "log_statement": "all" + } + }, + "resources": { + "requests": { + "cpu": "10m", + "memory": "50Mi" + }, + "limits": { + "cpu": "300m", + "memory": "3000Mi" + } + }, + "clone" : { + "cluster": "acid-batman" + }, + "patroni": { + "initdb": { + "encoding": "UTF8", + "locale": "en_US.UTF-8", + "data-checksums": "true" + }, + "pg_hba": [ + "hostssl all all 0.0.0.0/0 md5", + "host all all 0.0.0.0/0 md5" + ], + "ttl": 30, + "loop_wait": 10, + "retry_timeout": 10, + "maximum_lag_on_failover": 33554432, + "slots" : { + "permanent_logical_1" : { + "type" : "logical", + "database" : "foo", + "plugin" : "pgoutput" + } + } + }, + "maintenanceWindows": [ + "Mon:01:00-06:00", + "Sat:00:00-04:00", + "05:00-05:15" + ] } - }, - "maintenanceWindows": [ - "Mon:01:00-06:00", - "Sat:00:00-04:00", - "05:00-05:15" - ] - } -}`), - Postgresql{ + }`), + out: Postgresql{ TypeMeta: metav1.TypeMeta{ Kind: "Postgresql", APIVersion: "acid.zalan.do/v1", @@ -273,10 +311,12 @@ var unmarshalCluster = []struct { }, Error: "", }, - []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"9.6","parameters":{"log_statement":"all","max_connections":"10","shared_buffers":"32MB"}},"volume":{"size":"5Gi","storageClass":"SSD"},"patroni":{"initdb":{"data-checksums":"true","encoding":"UTF8","locale":"en_US.UTF-8"},"pg_hba":["hostssl all all 0.0.0.0/0 md5","host all all 0.0.0.0/0 md5"],"ttl":30,"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"slots":{"permanent_logical_1":{"database":"foo","plugin":"pgoutput","type":"logical"}}},"resources":{"requests":{"cpu":"10m","memory":"50Mi"},"limits":{"cpu":"300m","memory":"3000Mi"}},"teamId":"ACID","allowedSourceRanges":["127.0.0.1/32"],"numberOfInstances":2,"users":{"zalando":["superuser","createdb"]},"maintenanceWindows":["Mon:01:00-06:00","Sat:00:00-04:00","05:00-05:15"],"clone":{"cluster":"acid-batman"}}}`), nil}, + marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"9.6","parameters":{"log_statement":"all","max_connections":"10","shared_buffers":"32MB"}},"volume":{"size":"5Gi","storageClass":"SSD"},"patroni":{"initdb":{"data-checksums":"true","encoding":"UTF8","locale":"en_US.UTF-8"},"pg_hba":["hostssl all all 0.0.0.0/0 md5","host all all 0.0.0.0/0 md5"],"ttl":30,"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"slots":{"permanent_logical_1":{"database":"foo","plugin":"pgoutput","type":"logical"}}},"resources":{"requests":{"cpu":"10m","memory":"50Mi"},"limits":{"cpu":"300m","memory":"3000Mi"}},"teamId":"ACID","allowedSourceRanges":["127.0.0.1/32"],"numberOfInstances":2,"users":{"zalando":["superuser","createdb"]},"maintenanceWindows":["Mon:01:00-06:00","Sat:00:00-04:00","05:00-05:15"],"clone":{"cluster":"acid-batman"}},"status":{"PostgresClusterStatus":""}}`), + err: nil}, + // example with teamId set in input { - []byte(`{"kind": "Postgresql","apiVersion": "acid.zalan.do/v1","metadata": {"name": "teapot-testcluster1"}, "spec": {"teamId": "acid"}}`), - Postgresql{ + in: []byte(`{"kind": "Postgresql","apiVersion": "acid.zalan.do/v1","metadata": {"name": "teapot-testcluster1"}, "spec": {"teamId": "acid"}}`), + out: Postgresql{ TypeMeta: metav1.TypeMeta{ Kind: "Postgresql", APIVersion: "acid.zalan.do/v1", @@ -285,10 +325,12 @@ var unmarshalCluster = []struct { Name: "teapot-testcluster1", }, Spec: PostgresSpec{TeamID: "acid"}, - Status: ClusterStatusInvalid, + Status: PostgresStatus{PostgresClusterStatus: ClusterStatusInvalid}, Error: errors.New("name must match {TEAM}-{NAME} format").Error(), }, - []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":"Invalid"}`), nil}, + 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"}}`), + err: nil}, + // clone example { in: []byte(`{"kind": "Postgresql","apiVersion": "acid.zalan.do/v1","metadata": {"name": "acid-testcluster1"}, "spec": {"teamId": "acid", "clone": {"cluster": "team-batman"}}}`), out: Postgresql{ @@ -308,22 +350,26 @@ var unmarshalCluster = []struct { }, 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":"acid","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":{"cluster":"team-batman"}}}`), err: nil}, - {[]byte(`{"kind": "Postgresql","apiVersion": "acid.zalan.do/v1"`), - Postgresql{}, - []byte{}, - errors.New("unexpected end of JSON input")}, - {[]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":"Invalid"}`), - Postgresql{}, - []byte{}, - errors.New("invalid character 'q' looking for beginning of value")}} + 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":"acid","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":{"cluster":"team-batman"}},"status":{"PostgresClusterStatus":""}}`), + err: nil}, + // erroneous examples + { + in: []byte(`{"kind": "Postgresql","apiVersion": "acid.zalan.do/v1"`), + out: Postgresql{}, + marshal: []byte{}, + err: errors.New("unexpected end of JSON input")}, + { + 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"}}`), + out: Postgresql{}, + marshal: []byte{}, + err: errors.New("invalid character 'q' looking for beginning of value")}} var postgresqlList = []struct { in []byte out PostgresqlList err error }{ - {[]byte(`{"apiVersion":"v1","items":[{"apiVersion":"acid.zalan.do/v1","kind":"Postgresql","metadata":{"labels":{"team":"acid"},"name":"acid-testcluster42","namespace":"default","resourceVersion":"30446957","selfLink":"/apis/acid.zalan.do/v1/namespaces/default/postgresqls/acid-testcluster42","uid":"857cd208-33dc-11e7-b20a-0699041e4b03"},"spec":{"allowedSourceRanges":["185.85.220.0/22"],"numberOfInstances":1,"postgresql":{"version":"9.6"},"teamId":"acid","volume":{"size":"10Gi"}},"status":"Running"}],"kind":"List","metadata":{},"resourceVersion":"","selfLink":""}`), + {[]byte(`{"apiVersion":"v1","items":[{"apiVersion":"acid.zalan.do/v1","kind":"Postgresql","metadata":{"labels":{"team":"acid"},"name":"acid-testcluster42","namespace":"default","resourceVersion":"30446957","selfLink":"/apis/acid.zalan.do/v1/namespaces/default/postgresqls/acid-testcluster42","uid":"857cd208-33dc-11e7-b20a-0699041e4b03"},"spec":{"allowedSourceRanges":["185.85.220.0/22"],"numberOfInstances":1,"postgresql":{"version":"9.6"},"teamId":"acid","volume":{"size":"10Gi"}},"status":{"PostgresClusterStatus":"Running"}}],"kind":"List","metadata":{},"resourceVersion":"","selfLink":""}`), PostgresqlList{ TypeMeta: metav1.TypeMeta{ Kind: "List", @@ -350,8 +396,10 @@ var postgresqlList = []struct { AllowedSourceRanges: []string{"185.85.220.0/22"}, NumberOfInstances: 1, }, - Status: ClusterStatusRunning, - Error: "", + Status: PostgresStatus{ + PostgresClusterStatus: ClusterStatusRunning, + }, + Error: "", }}, }, nil}, @@ -469,6 +517,25 @@ func TestMarshalMaintenanceWindow(t *testing.T) { } } +func TestUnmarshalPostgresStatus(t *testing.T) { + for _, tt := range postgresStatus { + var ps PostgresStatus + err := ps.UnmarshalJSON(tt.in) + if err != nil { + if tt.err == nil || err.Error() != tt.err.Error() { + t.Errorf("CR status unmarshal expected error: %v, got %v", tt.err, err) + } + continue + //} else if tt.err != nil { + //t.Errorf("Expected error: %v", tt.err) + } + + if !reflect.DeepEqual(ps, tt.out) { + t.Errorf("Expected status: %#v, got: %#v", tt.out, ps) + } + } +} + func TestPostgresUnmarshal(t *testing.T) { for _, tt := range unmarshalCluster { var cluster Postgresql @@ -494,12 +561,26 @@ func TestMarshal(t *testing.T) { continue } + // Unmarshal and marshal example to capture api changes + var cluster Postgresql + err := cluster.UnmarshalJSON(tt.marshal) + if err != nil { + if tt.err == nil || err.Error() != tt.err.Error() { + t.Errorf("Backwards compatibility unmarshal expected error: %v, got: %v", tt.err, err) + } + continue + } + expected, err := json.Marshal(cluster) + if err != nil { + t.Errorf("Backwards compatibility marshal error: %v", err) + } + m, err := json.Marshal(tt.out) if err != nil { t.Errorf("Marshal error: %v", err) } - if !bytes.Equal(m, tt.marshal) { - t.Errorf("Marshal Postgresql \nexpected: %q, \ngot: %q", string(tt.marshal), string(m)) + if !bytes.Equal(m, expected) { + t.Errorf("Marshal Postgresql \nexpected: %q, \ngot: %q", string(expected), string(m)) } } } diff --git a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go index 0f5546f0f..e51b2eaa3 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -479,6 +479,22 @@ func (in *PostgresSpec) DeepCopy() *PostgresSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PostgresStatus) DeepCopyInto(out *PostgresStatus) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresStatus. +func (in *PostgresStatus) DeepCopy() *PostgresStatus { + if in == nil { + return nil + } + out := new(PostgresStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresUsersConfiguration) DeepCopyInto(out *PostgresUsersConfiguration) { *out = *in @@ -501,6 +517,7 @@ func (in *Postgresql) DeepCopyInto(out *Postgresql) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) + out.Status = in.Status return } diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index c319ab5d1..21f039149 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -4,6 +4,7 @@ package cluster import ( "database/sql" + "encoding/json" "fmt" "reflect" "regexp" @@ -19,8 +20,6 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" - "encoding/json" - acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" "github.com/zalando/postgres-operator/pkg/spec" "github.com/zalando/postgres-operator/pkg/util" @@ -149,21 +148,24 @@ func (c *Cluster) setProcessName(procName string, args ...interface{}) { } } -func (c *Cluster) setStatus(status acidv1.PostgresStatus) { - // TODO: eventually switch to updateStatus() for kubernetes 1.11 and above - var ( - err error - b []byte - ) - if b, err = json.Marshal(status); err != nil { +// SetStatus of Postgres cluster +// TODO: eventually switch to updateStatus() for kubernetes 1.11 and above +func (c *Cluster) setStatus(status string) { + var pgStatus acidv1.PostgresStatus + pgStatus.PostgresClusterStatus = status + + patch, err := json.Marshal(struct { + PgStatus interface{} `json:"status"` + }{&pgStatus}) + + if err != nil { c.logger.Errorf("could not marshal status: %v", err) } - patch := []byte(fmt.Sprintf(`{"status": %s}`, string(b))) // we cannot do a full scale update here without fetching the previous manifest (as the resourceVersion may differ), // however, we could do patch without it. In the future, once /status subresource is there (starting Kubernets 1.11) // we should take advantage of it. - newspec, err := c.KubeClient.AcidV1ClientSet.AcidV1().Postgresqls(c.clusterNamespace()).Patch(c.Name, types.MergePatchType, patch) + newspec, err := c.KubeClient.AcidV1ClientSet.AcidV1().Postgresqls(c.clusterNamespace()).Patch(c.Name, types.MergePatchType, patch, "status") if err != nil { c.logger.Errorf("could not update status: %v", err) } @@ -172,7 +174,7 @@ func (c *Cluster) setStatus(status acidv1.PostgresStatus) { } func (c *Cluster) isNewCluster() bool { - return c.Status == acidv1.ClusterStatusCreating + return c.Status.Creating() } // initUsers populates c.systemUsers and c.pgUsers maps. diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 5ff00c7b3..6f10aae22 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -20,10 +20,20 @@ const ( ) var logger = logrus.New().WithField("test", "cluster") -var cl = New(Config{OpConfig: config.Config{ProtectedRoles: []string{"admin"}, - Auth: config.Auth{SuperUsername: superUserName, - ReplicationUsername: replicationUserName}}}, - k8sutil.KubernetesClient{}, acidv1.Postgresql{}, logger) +var cl = New( + Config{ + OpConfig: config.Config{ + ProtectedRoles: []string{"admin"}, + Auth: config.Auth{ + SuperUsername: superUserName, + ReplicationUsername: replicationUserName, + }, + }, + }, + k8sutil.NewMockKubernetesClient(), + acidv1.Postgresql{}, + logger, +) func TestInitRobotUsers(t *testing.T) { testName := "TestInitRobotUsers" diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index d2157c8a2..9ba2ee40a 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -28,7 +28,7 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error { if err != nil { c.logger.Warningf("error while syncing cluster state: %v", err) c.setStatus(acidv1.ClusterStatusSyncFailed) - } else if c.Status != acidv1.ClusterStatusRunning { + } else if !c.Status.Running() { c.setStatus(acidv1.ClusterStatusRunning) } }() diff --git a/pkg/controller/util_test.go b/pkg/controller/util_test.go index cb782904c..c9e16cbd9 100644 --- a/pkg/controller/util_test.go +++ b/pkg/controller/util_test.go @@ -6,82 +6,24 @@ import ( "testing" b64 "encoding/base64" - "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1core "k8s.io/client-go/kubernetes/typed/core/v1" "github.com/zalando/postgres-operator/pkg/spec" "github.com/zalando/postgres-operator/pkg/util/k8sutil" + "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( testInfrastructureRolesSecretName = "infrastructureroles-test" ) -type mockSecret struct { - v1core.SecretInterface -} - -type mockConfigMap struct { - v1core.ConfigMapInterface -} - -func (c *mockSecret) Get(name string, options metav1.GetOptions) (*v1.Secret, error) { - if name != testInfrastructureRolesSecretName { - return nil, fmt.Errorf("NotFound") - } - secret := &v1.Secret{} - secret.Name = mockController.opConfig.ClusterNameLabel - secret.Data = map[string][]byte{ - "user1": []byte("testrole"), - "password1": []byte("testpassword"), - "inrole1": []byte("testinrole"), - "foobar": []byte(b64.StdEncoding.EncodeToString([]byte("password"))), - } - return secret, nil - -} - -func (c *mockConfigMap) Get(name string, options metav1.GetOptions) (*v1.ConfigMap, error) { - if name != testInfrastructureRolesSecretName { - return nil, fmt.Errorf("NotFound") - } - configmap := &v1.ConfigMap{} - configmap.Name = mockController.opConfig.ClusterNameLabel - configmap.Data = map[string]string{ - "foobar": "{}", - } - return configmap, nil -} - -type MockSecretGetter struct { -} - -type MockConfigMapsGetter struct { -} - -func (c *MockSecretGetter) Secrets(namespace string) v1core.SecretInterface { - return &mockSecret{} -} - -func (c *MockConfigMapsGetter) ConfigMaps(namespace string) v1core.ConfigMapInterface { - return &mockConfigMap{} -} - -func newMockKubernetesClient() k8sutil.KubernetesClient { - return k8sutil.KubernetesClient{ - SecretsGetter: &MockSecretGetter{}, - ConfigMapsGetter: &MockConfigMapsGetter{}, - } -} - func newMockController() *Controller { controller := NewController(&spec.ControllerConfig{}) controller.opConfig.ClusterNameLabel = "cluster-name" controller.opConfig.InfrastructureRolesSecretName = spec.NamespacedName{Namespace: v1.NamespaceDefault, Name: testInfrastructureRolesSecretName} controller.opConfig.Workers = 4 - controller.KubeClient = newMockKubernetesClient() + controller.KubeClient = k8sutil.NewMockKubernetesClient() return controller } diff --git a/pkg/util/k8sutil/k8sutil.go b/pkg/util/k8sutil/k8sutil.go index 6f5b15085..dad6a7b8e 100644 --- a/pkg/util/k8sutil/k8sutil.go +++ b/pkg/util/k8sutil/k8sutil.go @@ -2,6 +2,10 @@ package k8sutil import ( "fmt" + "reflect" + + b64 "encoding/base64" + "github.com/zalando/postgres-operator/pkg/util/constants" "k8s.io/api/core/v1" policybeta1 "k8s.io/api/policy/v1beta1" @@ -15,9 +19,9 @@ import ( rbacv1beta1 "k8s.io/client-go/kubernetes/typed/rbac/v1beta1" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" - "reflect" acidv1client "github.com/zalando/postgres-operator/pkg/generated/clientset/versioned" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // KubernetesClient describes getters for Kubernetes objects @@ -41,6 +45,20 @@ type KubernetesClient struct { AcidV1ClientSet *acidv1client.Clientset } +type mockSecret struct { + v1core.SecretInterface +} + +type MockSecretGetter struct { +} + +type mockConfigMap struct { + v1core.ConfigMapInterface +} + +type MockConfigMapsGetter struct { +} + // RestConfig creates REST config func RestConfig(kubeConfig string, outOfCluster bool) (*rest.Config, error) { if outOfCluster { @@ -140,3 +158,49 @@ func SamePDB(cur, new *policybeta1.PodDisruptionBudget) (match bool, reason stri return } + +func (c *mockSecret) Get(name string, options metav1.GetOptions) (*v1.Secret, error) { + if name != "infrastructureroles-test" { + return nil, fmt.Errorf("NotFound") + } + secret := &v1.Secret{} + secret.Name = "testcluster" + secret.Data = map[string][]byte{ + "user1": []byte("testrole"), + "password1": []byte("testpassword"), + "inrole1": []byte("testinrole"), + "foobar": []byte(b64.StdEncoding.EncodeToString([]byte("password"))), + } + return secret, nil + +} + +func (c *mockConfigMap) Get(name string, options metav1.GetOptions) (*v1.ConfigMap, error) { + if name != "infrastructureroles-test" { + return nil, fmt.Errorf("NotFound") + } + configmap := &v1.ConfigMap{} + configmap.Name = "testcluster" + configmap.Data = map[string]string{ + "foobar": "{}", + } + return configmap, nil +} + +// Secrets to be mocked +func (c *MockSecretGetter) Secrets(namespace string) v1core.SecretInterface { + return &mockSecret{} +} + +// ConfigMaps to be mocked +func (c *MockConfigMapsGetter) ConfigMaps(namespace string) v1core.ConfigMapInterface { + return &mockConfigMap{} +} + +// NewMockKubernetesClient for other tests +func NewMockKubernetesClient() KubernetesClient { + return KubernetesClient{ + SecretsGetter: &MockSecretGetter{}, + ConfigMapsGetter: &MockConfigMapsGetter{}, + } +}