Skip to content

Commit cf829df

Browse files
FxKuFxKu
authored
define ownership between operator and clusters via annotation (zalando#802)
* define ownership between operator and postgres clusters * add documentation * add unit test
1 parent d666c52 commit cf829df

File tree

10 files changed

+160
-43
lines changed

10 files changed

+160
-43
lines changed

‎cmd/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func main() {
7777
log.Fatalf("couldn't get REST config: %v", err)
7878
}
7979

80-
c := controller.NewController(&config)
80+
c := controller.NewController(&config, "")
8181

8282
c.Run(stop, wg)
8383

‎docs/administrator.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,34 @@ lacks access rights to any of them (except K8s system namespaces like
9595
'list pods' execute at the cluster scope and fail at the first violation of
9696
access rights.
9797

98+
## Operators with defined ownership of certain Postgres clusters
99+
100+
By default, multiple operators can only run together in one K8s cluster when
101+
isolated into their [own namespaces](administrator.md#specify-the-namespace-to-watch).
102+
But, it is also possible to define ownership between operator instances and
103+
Postgres clusters running all in the same namespace or K8s cluster without
104+
interfering.
105+
106+
First, define the [`CONTROLLER_ID`](../../manifests/postgres-operator.yaml#L38)
107+
environment variable in the operator deployment manifest. Then specify the ID
108+
in every Postgres cluster manifest you want this operator to watch using the
109+
`"acid.zalan.do/controller"` annotation:
110+
111+
```yaml
112+
apiVersion: "acid.zalan.do/v1"
113+
kind: postgresql
114+
metadata:
115+
name: demo-cluster
116+
annotations:
117+
"acid.zalan.do/controller": "second-operator"
118+
spec:
119+
...
120+
```
121+
122+
Every other Postgres cluster which lacks the annotation will be ignored by this
123+
operator. Conversely, operators without a defined `CONTROLLER_ID` will ignore
124+
clusters with defined ownership of another operator.
125+
98126
## Role-based access control for the operator
99127

100128
The manifest [`operator-service-account-rbac.yaml`](../manifests/operator-service-account-rbac.yaml)

‎manifests/complete-postgres-manifest.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ metadata:
44
name: acid-test-cluster
55
# labels:
66
# environment: demo
7+
# annotations:
8+
# "acid.zalan.do/controller": "second-operator"
79
spec:
810
dockerImage: registry.opensource.zalan.do/acid/spilo-12:1.6-p2
911
teamId: "acid"

‎manifests/postgres-operator.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,6 @@ spec:
3535
# In order to use the CRD OperatorConfiguration instead, uncomment these lines and comment out the two lines above
3636
# - name: POSTGRES_OPERATOR_CONFIGURATION_OBJECT
3737
# value: postgresql-operator-default-configuration
38+
# Define an ID to isolate controllers from each other
39+
# - name: CONTROLLER_ID
40+
# value: "second-operator"

‎pkg/controller/controller.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"k8s.io/client-go/kubernetes/scheme"
1414
"k8s.io/client-go/tools/cache"
1515

16+
acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1"
1617
"github.com/zalando/postgres-operator/pkg/apiserver"
1718
"github.com/zalando/postgres-operator/pkg/cluster"
1819
"github.com/zalando/postgres-operator/pkg/spec"
@@ -36,6 +37,7 @@ type Controller struct {
3637

3738
stopCh chan struct{}
3839

40+
controllerID string
3941
curWorkerID uint32 //initialized with 0
4042
curWorkerCluster sync.Map
4143
clusterWorkers map[spec.NamespacedName]uint32
@@ -61,13 +63,14 @@ type Controller struct {
6163
}
6264

6365
// NewController creates a new controller
64-
func NewController(controllerConfig *spec.ControllerConfig) *Controller {
66+
func NewController(controllerConfig *spec.ControllerConfig, controllerId string) *Controller {
6567
logger := logrus.New()
6668

6769
c := &Controller{
6870
config: *controllerConfig,
6971
opConfig: &config.Config{},
7072
logger: logger.WithField("pkg", "controller"),
73+
controllerID: controllerId,
7174
curWorkerCluster: sync.Map{},
7275
clusterWorkers: make(map[spec.NamespacedName]uint32),
7376
clusters: make(map[spec.NamespacedName]*cluster.Cluster),
@@ -239,6 +242,7 @@ func (c *Controller) initRoleBinding() {
239242

240243
func (c *Controller) initController() {
241244
c.initClients()
245+
c.controllerID = os.Getenv("CONTROLLER_ID")
242246

243247
if configObjectName := os.Getenv("POSTGRES_OPERATOR_CONFIGURATION_OBJECT"); configObjectName != "" {
244248
if err := c.createConfigurationCRD(c.opConfig.EnableCRDValidation); err != nil {
@@ -412,3 +416,16 @@ func (c *Controller) getEffectiveNamespace(namespaceFromEnvironment, namespaceFr
412416

413417
return namespace
414418
}
419+
420+
// hasOwnership returns true if the controller is the "owner" of the postgresql.
421+
// Whether it's owner is determined by the value of 'acid.zalan.do/controller'
422+
// annotation. If the value matches the controllerID then it owns it, or if the
423+
// controllerID is "" and there's no annotation set.
424+
func (c *Controller) hasOwnership(postgresql *acidv1.Postgresql) bool {
425+
if postgresql.Annotations != nil {
426+
if owner, ok := postgresql.Annotations[constants.PostgresqlControllerAnnotationKey]; ok {
427+
return owner == c.controllerID
428+
}
429+
}
430+
return c.controllerID == ""
431+
}

‎pkg/controller/node_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import (
44
"testing"
55

66
"github.com/zalando/postgres-operator/pkg/spec"
7-
"k8s.io/api/core/v1"
7+
v1 "k8s.io/api/core/v1"
88
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
99
)
1010

@@ -13,10 +13,10 @@ const (
1313
readyValue = "ready"
1414
)
1515

16-
func initializeController() *Controller {
17-
var c = NewController(&spec.ControllerConfig{})
18-
c.opConfig.NodeReadinessLabel = map[string]string{readyLabel: readyValue}
19-
return c
16+
func newNodeTestController() *Controller {
17+
var controller = NewController(&spec.ControllerConfig{}, "node-test")
18+
controller.opConfig.NodeReadinessLabel = map[string]string{readyLabel: readyValue}
19+
return controller
2020
}
2121

2222
func makeNode(labels map[string]string, isSchedulable bool) *v1.Node {
@@ -31,7 +31,7 @@ func makeNode(labels map[string]string, isSchedulable bool) *v1.Node {
3131
}
3232
}
3333

34-
var c = initializeController()
34+
var nodeTestController = newNodeTestController()
3535

3636
func TestNodeIsReady(t *testing.T) {
3737
testName := "TestNodeIsReady"
@@ -57,7 +57,7 @@ func TestNodeIsReady(t *testing.T) {
5757
},
5858
}
5959
for _, tt := range testTable {
60-
if isReady := c.nodeIsReady(tt.in); isReady != tt.out {
60+
if isReady := nodeTestController.nodeIsReady(tt.in); isReady != tt.out {
6161
t.Errorf("%s: expected response %t doesn't match the actual %t for the node %#v",
6262
testName, tt.out, isReady, tt.in)
6363
}

‎pkg/controller/postgresql.go

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,23 @@ func (c *Controller) clusterResync(stopCh <-chan struct{}, wg *sync.WaitGroup) {
4040

4141
// clusterListFunc obtains a list of all PostgreSQL clusters
4242
func (c *Controller) listClusters(options metav1.ListOptions) (*acidv1.PostgresqlList, error) {
43+
var pgList acidv1.PostgresqlList
44+
4345
// TODO: use the SharedInformer cache instead of quering Kubernetes API directly.
4446
list, err := c.KubeClient.AcidV1ClientSet.AcidV1().Postgresqls(c.opConfig.WatchedNamespace).List(options)
4547
if err != nil {
4648
c.logger.Errorf("could not list postgresql objects: %v", err)
4749
}
48-
return list, err
50+
if c.controllerID != "" {
51+
c.logger.Debugf("watch only clusters with controllerID %q", c.controllerID)
52+
}
53+
for _, pg := range list.Items {
54+
if pg.Error == "" && c.hasOwnership(&pg) {
55+
pgList.Items = append(pgList.Items, pg)
56+
}
57+
}
58+
59+
return &pgList, err
4960
}
5061

5162
// clusterListAndSync lists all manifests and decides whether to run the sync or repair.
@@ -455,41 +466,48 @@ func (c *Controller) queueClusterEvent(informerOldSpec, informerNewSpec *acidv1.
455466
}
456467

457468
func (c *Controller) postgresqlAdd(obj interface{}) {
458-
pg, ok := obj.(*acidv1.Postgresql)
459-
if !ok {
460-
c.logger.Errorf("could not cast to postgresql spec")
461-
return
469+
pg := c.postgresqlCheck(obj)
470+
if pg != nil {
471+
// We will not get multiple Add events for the same cluster
472+
c.queueClusterEvent(nil, pg, EventAdd)
462473
}
463474

464-
// We will not get multiple Add events for the same cluster
465-
c.queueClusterEvent(nil, pg, EventAdd)
475+
return
466476
}
467477

468478
func (c *Controller) postgresqlUpdate(prev, cur interface{}) {
469-
pgOld, ok := prev.(*acidv1.Postgresql)
470-
if !ok {
471-
c.logger.Errorf("could not cast to postgresql spec")
472-
}
473-
pgNew, ok := cur.(*acidv1.Postgresql)
474-
if !ok {
475-
c.logger.Errorf("could not cast to postgresql spec")
476-
}
477-
// Avoid the inifinite recursion for status updates
478-
if reflect.DeepEqual(pgOld.Spec, pgNew.Spec) {
479-
return
479+
pgOld := c.postgresqlCheck(prev)
480+
pgNew := c.postgresqlCheck(cur)
481+
if pgOld != nil && pgNew != nil {
482+
// Avoid the inifinite recursion for status updates
483+
if reflect.DeepEqual(pgOld.Spec, pgNew.Spec) {
484+
return
485+
}
486+
c.queueClusterEvent(pgOld, pgNew, EventUpdate)
480487
}
481488

482-
c.queueClusterEvent(pgOld, pgNew, EventUpdate)
489+
return
483490
}
484491

485492
func (c *Controller) postgresqlDelete(obj interface{}) {
493+
pg := c.postgresqlCheck(obj)
494+
if pg != nil {
495+
c.queueClusterEvent(pg, nil, EventDelete)
496+
}
497+
498+
return
499+
}
500+
501+
func (c *Controller) postgresqlCheck(obj interface{}) *acidv1.Postgresql {
486502
pg, ok := obj.(*acidv1.Postgresql)
487503
if !ok {
488504
c.logger.Errorf("could not cast to postgresql spec")
489-
return
505+
return nil
490506
}
491-
492-
c.queueClusterEvent(pg, nil, EventDelete)
507+
if !c.hasOwnership(pg) {
508+
return nil
509+
}
510+
return pg
493511
}
494512

495513
/*

‎pkg/controller/postgresql_test.go

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,68 @@
11
package controller
22

33
import (
4-
acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1"
5-
"github.com/zalando/postgres-operator/pkg/spec"
64
"reflect"
75
"testing"
6+
7+
acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1"
8+
"github.com/zalando/postgres-operator/pkg/spec"
9+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
810
)
911

1012
var (
1113
True = true
1214
False = false
1315
)
1416

15-
func TestMergeDeprecatedPostgreSQLSpecParameters(t *testing.T) {
16-
c := NewController(&spec.ControllerConfig{})
17+
func newPostgresqlTestController() *Controller {
18+
controller := NewController(&spec.ControllerConfig{}, "postgresql-test")
19+
return controller
20+
}
21+
22+
var postgresqlTestController = newPostgresqlTestController()
23+
24+
func TestControllerOwnershipOnPostgresql(t *testing.T) {
25+
tests := []struct {
26+
name string
27+
pg *acidv1.Postgresql
28+
owned bool
29+
error string
30+
}{
31+
{
32+
"Postgres cluster with defined ownership of mocked controller",
33+
&acidv1.Postgresql{
34+
ObjectMeta: metav1.ObjectMeta{
35+
Annotations: map[string]string{"acid.zalan.do/controller": "postgresql-test"},
36+
},
37+
},
38+
True,
39+
"Postgres cluster should be owned by operator, but controller says no",
40+
},
41+
{
42+
"Postgres cluster with defined ownership of another controller",
43+
&acidv1.Postgresql{
44+
ObjectMeta: metav1.ObjectMeta{
45+
Annotations: map[string]string{"acid.zalan.do/controller": "stups-test"},
46+
},
47+
},
48+
False,
49+
"Postgres cluster should be owned by another operator, but controller say yes",
50+
},
51+
{
52+
"Test Postgres cluster without defined ownership",
53+
&acidv1.Postgresql{},
54+
False,
55+
"Postgres cluster should be owned by operator with empty controller ID, but controller says yes",
56+
},
57+
}
58+
for _, tt := range tests {
59+
if postgresqlTestController.hasOwnership(tt.pg) != tt.owned {
60+
t.Errorf("%s: %v", tt.name, tt.error)
61+
}
62+
}
63+
}
1764

65+
func TestMergeDeprecatedPostgreSQLSpecParameters(t *testing.T) {
1866
tests := []struct {
1967
name string
2068
in *acidv1.PostgresSpec
@@ -36,7 +84,7 @@ func TestMergeDeprecatedPostgreSQLSpecParameters(t *testing.T) {
3684
},
3785
}
3886
for _, tt := range tests {
39-
result := c.mergeDeprecatedPostgreSQLSpecParameters(tt.in)
87+
result := postgresqlTestController.mergeDeprecatedPostgreSQLSpecParameters(tt.in)
4088
if !reflect.DeepEqual(result, tt.out) {
4189
t.Errorf("%s: %v", tt.name, tt.error)
4290
}

‎pkg/controller/util_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ const (
1717
testInfrastructureRolesSecretName = "infrastructureroles-test"
1818
)
1919

20-
func newMockController() *Controller {
21-
controller := NewController(&spec.ControllerConfig{})
20+
func newUtilTestController() *Controller {
21+
controller := NewController(&spec.ControllerConfig{}, "util-test")
2222
controller.opConfig.ClusterNameLabel = "cluster-name"
2323
controller.opConfig.InfrastructureRolesSecretName =
2424
spec.NamespacedName{Namespace: v1.NamespaceDefault, Name: testInfrastructureRolesSecretName}
@@ -27,7 +27,7 @@ func newMockController() *Controller {
2727
return controller
2828
}
2929

30-
var mockController = newMockController()
30+
var utilTestController = newUtilTestController()
3131

3232
func TestPodClusterName(t *testing.T) {
3333
var testTable = []struct {
@@ -43,15 +43,15 @@ func TestPodClusterName(t *testing.T) {
4343
ObjectMeta: metav1.ObjectMeta{
4444
Namespace: v1.NamespaceDefault,
4545
Labels: map[string]string{
46-
mockController.opConfig.ClusterNameLabel: "testcluster",
46+
utilTestController.opConfig.ClusterNameLabel: "testcluster",
4747
},
4848
},
4949
},
5050
spec.NamespacedName{Namespace: v1.NamespaceDefault, Name: "testcluster"},
5151
},
5252
}
5353
for _, test := range testTable {
54-
resp := mockController.podClusterName(test.in)
54+
resp := utilTestController.podClusterName(test.in)
5555
if resp != test.expected {
5656
t.Errorf("expected response %v does not match the actual %v", test.expected, resp)
5757
}
@@ -73,7 +73,7 @@ func TestClusterWorkerID(t *testing.T) {
7373
},
7474
}
7575
for _, test := range testTable {
76-
resp := mockController.clusterWorkerID(test.in)
76+
resp := utilTestController.clusterWorkerID(test.in)
7777
if resp != test.expected {
7878
t.Errorf("expected response %v does not match the actual %v", test.expected, resp)
7979
}
@@ -116,7 +116,7 @@ func TestGetInfrastructureRoles(t *testing.T) {
116116
},
117117
}
118118
for _, test := range testTable {
119-
roles, err := mockController.getInfrastructureRoles(&test.secretName)
119+
roles, err := utilTestController.getInfrastructureRoles(&test.secretName)
120120
if err != test.expectedError {
121121
if err != nil && test.expectedError != nil && err.Error() == test.expectedError.Error() {
122122
continue

‎pkg/util/constants/annotations.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ const (
77
ElbTimeoutAnnotationValue = "3600"
88
KubeIAmAnnotation = "iam.amazonaws.com/role"
99
VolumeStorateProvisionerAnnotation = "pv.kubernetes.io/provisioned-by"
10+
PostgresqlControllerAnnotationKey = "acid.zalan.do/controller"
1011
)

0 commit comments

Comments
 (0)