Skip to content

Commit d52296c

Browse files
RafiaSabihRafia SabihFxKu
authored
Propagate annotations to the StatefulSet (#932)
* Initial commit * Corrections - set the type of the new configuration parameter to be array of strings - propagate the annotations to statefulset at sync * Enable regular expression matching * Improvements -handle rollingUpdate flag -modularize code -rename config parameter name * fix merge error * Pass annotations to connection pooler deployment * update code-gen * Add documentation and update manifests * add e2e test and introduce option in configmap * fix service annotations test * Add unit test * fix e2e tests * better key lookup of annotations tests * add debug message for annotation tests * Fix typos * minor fix for looping * Handle update path and renaming - handle the update path to update sts and connection pooler deployment. This way no need to wait for sync - rename the parameter to downscaler_annotations - handle other review comments * another try to fix python loops * Avoid unneccessary update events * Update manifests * some final polishing * fix cluster_test after polishing Co-authored-by: Rafia Sabih <[email protected]> Co-authored-by: Felix Kunde <[email protected]>
1 parent be208b6 commit d52296c

20 files changed

+205
-31
lines changed

‎charts/postgres-operator/crds/operatorconfigurations.yaml

+4
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,10 @@ spec:
117117
type: object
118118
additionalProperties:
119119
type: string
120+
downscaler_annotations:
121+
type: array
122+
items:
123+
type: string
120124
enable_init_containers:
121125
type: boolean
122126
enable_pod_antiaffinity:

‎charts/postgres-operator/values-crd.yaml

+7-2
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ configKubernetes:
6767
# keya: valuea
6868
# keyb: valueb
6969

70+
# list of annotations propagated from cluster manifest to statefulset and deployment
71+
# downscaler_annotations:
72+
# - deployment-time
73+
# - downscaler/*
74+
7075
# enables initContainers to run actions before Spilo is started
7176
enable_init_containers: true
7277
# toggles pod anti affinity on the Postgres pods
@@ -262,11 +267,11 @@ configConnectionPooler:
262267
# docker image
263268
connection_pooler_image: "registry.opensource.zalan.do/acid/pgbouncer"
264269
# max db connections the pooler should hold
265-
connection_pooler_max_db_connections: "60"
270+
connection_pooler_max_db_connections: 60
266271
# default pooling mode
267272
connection_pooler_mode: "transaction"
268273
# number of pooler instances
269-
connection_pooler_number_of_instances: "2"
274+
connection_pooler_number_of_instances: 2
270275
# default resources
271276
connection_pooler_default_cpu_request: 500m
272277
connection_pooler_default_memory_request: 100Mi

‎charts/postgres-operator/values.yaml

+3
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ configKubernetes:
6363
# annotations attached to each database pod
6464
# custom_pod_annotations: "keya:valuea,keyb:valueb"
6565

66+
# list of annotations propagated from cluster manifest to statefulset and deployment
67+
# downscaler_annotations: "deployment-time,downscaler/*"
68+
6669
# enables initContainers to run actions before Spilo is started
6770
enable_init_containers: "true"
6871
# toggles pod anti affinity on the Postgres pods

‎docs/reference/cluster_manifest.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ These parameters are grouped directly under the `spec` key in the manifest.
165165
If `targetContainers` is empty, additional volumes will be mounted only in the `postgres` container.
166166
If you set the `all` special item, it will be mounted in all containers (postgres + sidecars).
167167
Else you can set the list of target containers in which the additional volumes will be mounted (eg : postgres, telegraf)
168-
168+
169169
## Postgres parameters
170170

171171
Those parameters are grouped under the `postgresql` top-level key, which is

‎docs/reference/operator_parameters.md

+6
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,12 @@ configuration they are grouped under the `kubernetes` key.
200200
of a database created by the operator. If the annotation key is also provided
201201
by the database definition, the database definition value is used.
202202

203+
* **downscaler_annotations**
204+
An array of annotations that should be passed from Postgres CRD on to the
205+
statefulset and, if exists, to the connection pooler deployment as well.
206+
Regular expressions like `downscaler/*` etc. are also accepted. Can be used
207+
with [kube-downscaler](https://github.com/hjacobs/kube-downscaler).
208+
203209
* **watched_namespace**
204210
The operator watches for Postgres objects in the given namespace. If not
205211
specified, the value is taken from the operator namespace. A special `*`

‎e2e/tests/test_e2e.py

+54-9
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ def test_node_readiness_label(self):
428428
k8s.api.core_v1.patch_node(current_master_node, patch_readiness_label)
429429

430430
# wait a little before proceeding with the pod distribution test
431-
time.sleep(k8s.RETRY_TIMEOUT_SEC)
431+
time.sleep(30)
432432

433433
# toggle pod anti affinity to move replica away from master node
434434
self.assert_distributed_pods(new_master_node, new_replica_nodes, cluster_label)
@@ -465,21 +465,24 @@ def test_service_annotations(self):
465465
pg_patch_custom_annotations = {
466466
"spec": {
467467
"serviceAnnotations": {
468-
"annotation.key": "value"
468+
"annotation.key": "value",
469+
"foo": "bar",
469470
}
470471
}
471472
}
472473
k8s.api.custom_objects_api.patch_namespaced_custom_object(
473474
"acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_custom_annotations)
474475

476+
# wait a little before proceeding
477+
time.sleep(30)
475478
annotations = {
476479
"annotation.key": "value",
477480
"foo": "bar",
478481
}
479482
self.assertTrue(k8s.check_service_annotations(
480-
"cluster-name=acid-service-annotations,spilo-role=master", annotations))
483+
"cluster-name=acid-minimal-cluster,spilo-role=master", annotations))
481484
self.assertTrue(k8s.check_service_annotations(
482-
"cluster-name=acid-service-annotations,spilo-role=replica", annotations))
485+
"cluster-name=acid-minimal-cluster,spilo-role=replica", annotations))
483486

484487
# clean up
485488
unpatch_custom_service_annotations = {
@@ -489,6 +492,40 @@ def test_service_annotations(self):
489492
}
490493
k8s.update_config(unpatch_custom_service_annotations)
491494

495+
@timeout_decorator.timeout(TEST_TIMEOUT_SEC)
496+
def test_statefulset_annotation_propagation(self):
497+
'''
498+
Inject annotation to Postgresql CRD and check it's propagation to stateful set
499+
'''
500+
k8s = self.k8s
501+
cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster'
502+
503+
patch_sset_propagate_annotations = {
504+
"data": {
505+
"downscaler_annotations": "deployment-time,downscaler/*",
506+
}
507+
}
508+
k8s.update_config(patch_sset_propagate_annotations)
509+
510+
pg_crd_annotations = {
511+
"metadata": {
512+
"annotations": {
513+
"deployment-time": "2020-04-30 12:00:00",
514+
"downscaler/downtime_replicas": "0",
515+
},
516+
}
517+
}
518+
k8s.api.custom_objects_api.patch_namespaced_custom_object(
519+
"acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_crd_annotations)
520+
521+
# wait a little before proceeding
522+
time.sleep(60)
523+
annotations = {
524+
"deployment-time": "2020-04-30 12:00:00",
525+
"downscaler/downtime_replicas": "0",
526+
}
527+
self.assertTrue(k8s.check_statefulset_annotations(cluster_label, annotations))
528+
492529
@timeout_decorator.timeout(TEST_TIMEOUT_SEC)
493530
def test_taint_based_eviction(self):
494531
'''
@@ -528,7 +565,7 @@ def test_taint_based_eviction(self):
528565
k8s.update_config(patch_toleration_config)
529566

530567
# wait a little before proceeding with the pod distribution test
531-
time.sleep(k8s.RETRY_TIMEOUT_SEC)
568+
time.sleep(30)
532569

533570
# toggle pod anti affinity to move replica away from master node
534571
self.assert_distributed_pods(new_master_node, new_replica_nodes, cluster_label)
@@ -694,10 +731,18 @@ def get_service_type(self, svc_labels, namespace='default'):
694731
def check_service_annotations(self, svc_labels, annotations, namespace='default'):
695732
svcs = self.api.core_v1.list_namespaced_service(namespace, label_selector=svc_labels, limit=1).items
696733
for svc in svcs:
697-
if len(svc.metadata.annotations) != len(annotations):
698-
return False
699-
for key in svc.metadata.annotations:
700-
if svc.metadata.annotations[key] != annotations[key]:
734+
for key, value in annotations.items():
735+
if key not in svc.metadata.annotations or svc.metadata.annotations[key] != value:
736+
print("Expected key {} not found in annotations {}".format(key, svc.metadata.annotation))
737+
return False
738+
return True
739+
740+
def check_statefulset_annotations(self, sset_labels, annotations, namespace='default'):
741+
ssets = self.api.apps_v1.list_namespaced_stateful_set(namespace, label_selector=sset_labels, limit=1).items
742+
for sset in ssets:
743+
for key, value in annotations.items():
744+
if key not in sset.metadata.annotations or sset.metadata.annotations[key] != value:
745+
print("Expected key {} not found in annotations {}".format(key, sset.metadata.annotation))
701746
return False
702747
return True
703748

‎manifests/configmap.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ data:
3030
# default_memory_limit: 500Mi
3131
# default_memory_request: 100Mi
3232
docker_image: registry.opensource.zalan.do/acid/spilo-cdp-12:1.6-p115
33+
# downscaler_annotations: "deployment-time,downscaler/*"
3334
# enable_admin_role_for_users: "true"
3435
# enable_crd_validation: "true"
3536
# enable_database_access: "true"

‎manifests/operatorconfiguration.crd.yaml

+4
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ spec:
9393
type: object
9494
additionalProperties:
9595
type: string
96+
downscaler_annotations:
97+
type: array
98+
items:
99+
type: string
96100
enable_init_containers:
97101
type: boolean
98102
enable_pod_antiaffinity:

‎manifests/postgresql-operator-default-configuration.yaml

+3
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ configuration:
3131
# custom_pod_annotations:
3232
# keya: valuea
3333
# keyb: valueb
34+
# downscaler_annotations:
35+
# - deployment-time
36+
# - downscaler/*
3437
enable_init_containers: true
3538
enable_pod_antiaffinity: false
3639
enable_pod_disruption_budget: true

‎pkg/apis/acid.zalan.do/v1/crds.go

+21-13
Original file line numberDiff line numberDiff line change
@@ -21,48 +21,48 @@ const (
2121

2222
// PostgresCRDResourceColumns definition of AdditionalPrinterColumns for postgresql CRD
2323
var PostgresCRDResourceColumns = []apiextv1beta1.CustomResourceColumnDefinition{
24-
apiextv1beta1.CustomResourceColumnDefinition{
24+
{
2525
Name: "Team",
2626
Type: "string",
2727
Description: "Team responsible for Postgres cluster",
2828
JSONPath: ".spec.teamId",
2929
},
30-
apiextv1beta1.CustomResourceColumnDefinition{
30+
{
3131
Name: "Version",
3232
Type: "string",
3333
Description: "PostgreSQL version",
3434
JSONPath: ".spec.postgresql.version",
3535
},
36-
apiextv1beta1.CustomResourceColumnDefinition{
36+
{
3737
Name: "Pods",
3838
Type: "integer",
3939
Description: "Number of Pods per Postgres cluster",
4040
JSONPath: ".spec.numberOfInstances",
4141
},
42-
apiextv1beta1.CustomResourceColumnDefinition{
42+
{
4343
Name: "Volume",
4444
Type: "string",
4545
Description: "Size of the bound volume",
4646
JSONPath: ".spec.volume.size",
4747
},
48-
apiextv1beta1.CustomResourceColumnDefinition{
48+
{
4949
Name: "CPU-Request",
5050
Type: "string",
5151
Description: "Requested CPU for Postgres containers",
5252
JSONPath: ".spec.resources.requests.cpu",
5353
},
54-
apiextv1beta1.CustomResourceColumnDefinition{
54+
{
5555
Name: "Memory-Request",
5656
Type: "string",
5757
Description: "Requested memory for Postgres containers",
5858
JSONPath: ".spec.resources.requests.memory",
5959
},
60-
apiextv1beta1.CustomResourceColumnDefinition{
60+
{
6161
Name: "Age",
6262
Type: "date",
6363
JSONPath: ".metadata.creationTimestamp",
6464
},
65-
apiextv1beta1.CustomResourceColumnDefinition{
65+
{
6666
Name: "Status",
6767
Type: "string",
6868
Description: "Current sync status of postgresql resource",
@@ -72,31 +72,31 @@ var PostgresCRDResourceColumns = []apiextv1beta1.CustomResourceColumnDefinition{
7272

7373
// OperatorConfigCRDResourceColumns definition of AdditionalPrinterColumns for OperatorConfiguration CRD
7474
var OperatorConfigCRDResourceColumns = []apiextv1beta1.CustomResourceColumnDefinition{
75-
apiextv1beta1.CustomResourceColumnDefinition{
75+
{
7676
Name: "Image",
7777
Type: "string",
7878
Description: "Spilo image to be used for Pods",
7979
JSONPath: ".configuration.docker_image",
8080
},
81-
apiextv1beta1.CustomResourceColumnDefinition{
81+
{
8282
Name: "Cluster-Label",
8383
Type: "string",
8484
Description: "Label for K8s resources created by operator",
8585
JSONPath: ".configuration.kubernetes.cluster_name_label",
8686
},
87-
apiextv1beta1.CustomResourceColumnDefinition{
87+
{
8888
Name: "Service-Account",
8989
Type: "string",
9090
Description: "Name of service account to be used",
9191
JSONPath: ".configuration.kubernetes.pod_service_account_name",
9292
},
93-
apiextv1beta1.CustomResourceColumnDefinition{
93+
{
9494
Name: "Min-Instances",
9595
Type: "integer",
9696
Description: "Minimum number of instances per Postgres cluster",
9797
JSONPath: ".configuration.min_instances",
9898
},
99-
apiextv1beta1.CustomResourceColumnDefinition{
99+
{
100100
Name: "Age",
101101
Type: "date",
102102
JSONPath: ".metadata.creationTimestamp",
@@ -888,6 +888,14 @@ var OperatorConfigCRDResourceValidation = apiextv1beta1.CustomResourceValidation
888888
},
889889
},
890890
},
891+
"downscaler_annotations": {
892+
Type: "array",
893+
Items: &apiextv1beta1.JSONSchemaPropsOrArray{
894+
Schema: &apiextv1beta1.JSONSchemaProps{
895+
Type: "string",
896+
},
897+
},
898+
},
891899
"enable_init_containers": {
892900
Type: "boolean",
893901
},

‎pkg/apis/acid.zalan.do/v1/operator_configuration_type.go

+1
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ type KubernetesMetaConfiguration struct {
6262
PodRoleLabel string `json:"pod_role_label,omitempty"`
6363
ClusterLabels map[string]string `json:"cluster_labels,omitempty"`
6464
InheritedLabels []string `json:"inherited_labels,omitempty"`
65+
DownscalerAnnotations []string `json:"downscaler_annotations,omitempty"`
6566
ClusterNameLabel string `json:"cluster_name_label,omitempty"`
6667
NodeReadinessLabel map[string]string `json:"node_readiness_label,omitempty"`
6768
CustomPodAnnotations map[string]string `json:"custom_pod_annotations,omitempty"`

‎pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go

+5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/cluster/cluster.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -711,8 +711,7 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
711711
updateFailed = true
712712
return
713713
}
714-
715-
if !reflect.DeepEqual(oldSs, newSs) {
714+
if !reflect.DeepEqual(oldSs, newSs) || !reflect.DeepEqual(oldSpec.Annotations, newSpec.Annotations) {
716715
c.logger.Debugf("syncing statefulsets")
717716
// TODO: avoid generating the StatefulSet object twice by passing it to syncStatefulSet
718717
if err := c.syncStatefulSet(); err != nil {

‎pkg/cluster/cluster_test.go

+31-2
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,48 @@ var eventRecorder = record.NewFakeRecorder(1)
2828
var cl = New(
2929
Config{
3030
OpConfig: config.Config{
31-
ProtectedRoles: []string{"admin"},
31+
PodManagementPolicy: "ordered_ready",
32+
ProtectedRoles: []string{"admin"},
3233
Auth: config.Auth{
3334
SuperUsername: superUserName,
3435
ReplicationUsername: replicationUserName,
3536
},
37+
Resources: config.Resources{
38+
DownscalerAnnotations: []string{"downscaler/*"},
39+
},
3640
},
3741
},
3842
k8sutil.NewMockKubernetesClient(),
39-
acidv1.Postgresql{ObjectMeta: metav1.ObjectMeta{Name: "acid-test", Namespace: "test"}},
43+
acidv1.Postgresql{ObjectMeta: metav1.ObjectMeta{Name: "acid-test", Namespace: "test", Annotations: map[string]string{"downscaler/downtime_replicas": "0"}}},
4044
logger,
4145
eventRecorder,
4246
)
4347

48+
func TestStatefulSetAnnotations(t *testing.T) {
49+
testName := "CheckStatefulsetAnnotations"
50+
spec := acidv1.PostgresSpec{
51+
TeamID: "myapp", NumberOfInstances: 1,
52+
Resources: acidv1.Resources{
53+
ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
54+
ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
55+
},
56+
Volume: acidv1.Volume{
57+
Size: "1G",
58+
},
59+
}
60+
ss, err := cl.generateStatefulSet(&spec)
61+
if err != nil {
62+
t.Errorf("in %s no statefulset created %v", testName, err)
63+
}
64+
if ss != nil {
65+
annotation := ss.ObjectMeta.GetAnnotations()
66+
if _, ok := annotation["downscaler/downtime_replicas"]; !ok {
67+
t.Errorf("in %s respective annotation not found on sts", testName)
68+
}
69+
}
70+
71+
}
72+
4473
func TestInitRobotUsers(t *testing.T) {
4574
testName := "TestInitRobotUsers"
4675
tests := []struct {

0 commit comments

Comments
 (0)