Add ServiceAnnotations cluster config (#803)

The [operator parameters][1] already support the
`custom_service_annotations` config.With this parameter is possible to
define custom annotations that will be used on the services created by the
operator. The `custom_service_annotations` as all the other
[operator parameters][1] are defined on the operator level and do not allow
customization on the cluster level. A cluster may require different service
annotations, as for example, set up different cloud load balancers
timeouts, different ingress annotations, and/or enable more customizable
environments.

This commit introduces a new parameter on the cluster level, called
`serviceAnnotations`, responsible for defining custom annotations just for
the services created by the operator to the specifically defined cluster.
It allows a mix of configuration between `custom_service_annotations` and
`serviceAnnotations` where the latest one will have priority. In order to
allow custom service annotations to be used on services without
LoadBalancers (as for example, service mesh services annotations) both
`custom_service_annotations` and `serviceAnnotations` are applied
independently of load-balancing configuration. For retro-compatibility
purposes, `custom_service_annotations` is still under
[Load balancer related options][2]. The two default annotations when using
LoadBalancer services, `external-dns.alpha.kubernetes.io/hostname` and
`service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout` are
still defined by the operator.
`service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout` can
be overridden by `custom_service_annotations` or `serviceAnnotations`,
allowing a more customizable environment.
`external-dns.alpha.kubernetes.io/hostname` can not be overridden once
there is no differentiation between custom service annotations for
replicas and masters.

It updates the documentation and creates the necessary unit and e2e
tests to the above-described feature too.

[1]: https://github.com/zalando/postgres-operator/blob/master/docs/reference/operator_parameters.md
[2]: https://github.com/zalando/postgres-operator/blob/master/docs/reference/operator_parameters.md#load-balancer-related-options
This commit is contained in:
Jonathan Juares Beber
2020-02-10 12:03:25 +01:00
committed by GitHub
parent a660d758a5
commit ba60e15d07
15 changed files with 565 additions and 37 deletions
+8
View File
@@ -383,6 +383,14 @@ var PostgresCRDResourceValidation = apiextv1beta1.CustomResourceValidation{
},
},
},
"serviceAnnotations": {
Type: "object",
AdditionalProperties: &apiextv1beta1.JSONSchemaPropsOrBool{
Schema: &apiextv1beta1.JSONSchemaProps{
Type: "string",
},
},
},
"sidecars": {
Type: "array",
Items: &apiextv1beta1.JSONSchemaPropsOrArray{
@@ -60,6 +60,7 @@ type PostgresSpec struct {
LogicalBackupSchedule string `json:"logicalBackupSchedule,omitempty"`
StandbyCluster *StandbyDescription `json:"standby"`
PodAnnotations map[string]string `json:"podAnnotations"`
ServiceAnnotations map[string]string `json:"serviceAnnotations"`
// deprecated json tags
InitContainersOld []v1.Container `json:"init_containers,omitempty"`
+94 -7
View File
@@ -456,18 +456,84 @@ var postgresqlList = []struct {
PostgresqlList{},
errors.New("unexpected end of JSON input")}}
var annotations = []struct {
var podAnnotations = []struct {
about string
in []byte
annotations map[string]string
err error
}{{
about: "common annotations",
in: []byte(`{"kind": "Postgresql","apiVersion": "acid.zalan.do/v1","metadata": {"name": "acid-testcluster1"}, "spec": {"podAnnotations": {"foo": "bar"},"teamId": "acid", "clone": {"cluster": "team-batman"}}}`),
about: "common annotations",
in: []byte(`{
"kind": "Postgresql",
"apiVersion": "acid.zalan.do/v1",
"metadata": {
"name": "acid-testcluster1"
},
"spec": {
"podAnnotations": {
"foo": "bar"
},
"teamId": "acid",
"clone": {
"cluster": "team-batman"
}
}
}`),
annotations: map[string]string{"foo": "bar"},
err: nil},
}
var serviceAnnotations = []struct {
about string
in []byte
annotations map[string]string
err error
}{
{
about: "common single annotation",
in: []byte(`{
"kind": "Postgresql",
"apiVersion": "acid.zalan.do/v1",
"metadata": {
"name": "acid-testcluster1"
},
"spec": {
"serviceAnnotations": {
"foo": "bar"
},
"teamId": "acid",
"clone": {
"cluster": "team-batman"
}
}
}`),
annotations: map[string]string{"foo": "bar"},
err: nil,
},
{
about: "common two annotations",
in: []byte(`{
"kind": "Postgresql",
"apiVersion": "acid.zalan.do/v1",
"metadata": {
"name": "acid-testcluster1"
},
"spec": {
"serviceAnnotations": {
"foo": "bar",
"post": "gres"
},
"teamId": "acid",
"clone": {
"cluster": "team-batman"
}
}
}`),
annotations: map[string]string{"foo": "bar", "post": "gres"},
err: nil,
},
}
func mustParseTime(s string) metav1.Time {
v, err := time.Parse("15:04", s)
if err != nil {
@@ -517,21 +583,42 @@ func TestWeekdayTime(t *testing.T) {
}
}
func TestClusterAnnotations(t *testing.T) {
for _, tt := range annotations {
func TestPodAnnotations(t *testing.T) {
for _, tt := range podAnnotations {
t.Run(tt.about, func(t *testing.T) {
var cluster Postgresql
err := cluster.UnmarshalJSON(tt.in)
if err != nil {
if tt.err == nil || err.Error() != tt.err.Error() {
t.Errorf("Unable to marshal cluster with annotations: expected %v got %v", tt.err, err)
t.Errorf("Unable to marshal cluster with podAnnotations: expected %v got %v", tt.err, err)
}
return
}
for k, v := range cluster.Spec.PodAnnotations {
found, expected := v, tt.annotations[k]
if found != expected {
t.Errorf("Didn't find correct value for key %v in for podAnnotations: Expected %v found %v", k, expected, found)
t.Errorf("Didn't find correct value for key %v in for podAnnotations: Expected %v found %v", k, expected, found)
}
}
})
}
}
func TestServiceAnnotations(t *testing.T) {
for _, tt := range serviceAnnotations {
t.Run(tt.about, func(t *testing.T) {
var cluster Postgresql
err := cluster.UnmarshalJSON(tt.in)
if err != nil {
if tt.err == nil || err.Error() != tt.err.Error() {
t.Errorf("Unable to marshal cluster with serviceAnnotations: expected %v got %v", tt.err, err)
}
return
}
for k, v := range cluster.Spec.ServiceAnnotations {
found, expected := v, tt.annotations[k]
if found != expected {
t.Errorf("Didn't find correct value for key %v in for serviceAnnotations: Expected %v found %v", k, expected, found)
}
}
})
@@ -514,6 +514,13 @@ func (in *PostgresSpec) DeepCopyInto(out *PostgresSpec) {
(*out)[key] = val
}
}
if in.ServiceAnnotations != nil {
in, out := &in.ServiceAnnotations, &out.ServiceAnnotations
*out = make(map[string]string, len(*in))
for key, val := range *in {
(*out)[key] = val
}
}
if in.InitContainersOld != nil {
in, out := &in.InitContainersOld, &out.InitContainersOld
*out = make([]corev1.Container, len(*in))
+322
View File
@@ -355,6 +355,12 @@ func TestPodAnnotations(t *testing.T) {
database: map[string]string{"foo": "bar"},
merged: map[string]string{"foo": "bar"},
},
{
subTest: "Both Annotations",
operator: map[string]string{"foo": "bar"},
database: map[string]string{"post": "gres"},
merged: map[string]string{"foo": "bar", "post": "gres"},
},
{
subTest: "Database Config overrides Operator Config Annotations",
operator: map[string]string{"foo": "bar", "global": "foo"},
@@ -382,3 +388,319 @@ func TestPodAnnotations(t *testing.T) {
}
}
}
func TestServiceAnnotations(t *testing.T) {
enabled := true
disabled := false
tests := []struct {
about string
role PostgresRole
enableMasterLoadBalancerSpec *bool
enableMasterLoadBalancerOC bool
enableReplicaLoadBalancerSpec *bool
enableReplicaLoadBalancerOC bool
operatorAnnotations map[string]string
clusterAnnotations map[string]string
expect map[string]string
}{
//MASTER
{
about: "Master with no annotations and EnableMasterLoadBalancer disabled on spec and OperatorConfig",
role: "master",
enableMasterLoadBalancerSpec: &disabled,
enableMasterLoadBalancerOC: false,
operatorAnnotations: make(map[string]string),
clusterAnnotations: make(map[string]string),
expect: make(map[string]string),
},
{
about: "Master with no annotations and EnableMasterLoadBalancer enabled on spec",
role: "master",
enableMasterLoadBalancerSpec: &enabled,
enableMasterLoadBalancerOC: false,
operatorAnnotations: make(map[string]string),
clusterAnnotations: make(map[string]string),
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
},
},
{
about: "Master with no annotations and EnableMasterLoadBalancer enabled only on operator config",
role: "master",
enableMasterLoadBalancerSpec: &disabled,
enableMasterLoadBalancerOC: true,
operatorAnnotations: make(map[string]string),
clusterAnnotations: make(map[string]string),
expect: make(map[string]string),
},
{
about: "Master with no annotations and EnableMasterLoadBalancer defined only on operator config",
role: "master",
enableMasterLoadBalancerOC: true,
operatorAnnotations: make(map[string]string),
clusterAnnotations: make(map[string]string),
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
},
},
{
about: "Master with cluster annotations and load balancer enabled",
role: "master",
enableMasterLoadBalancerOC: true,
operatorAnnotations: make(map[string]string),
clusterAnnotations: map[string]string{"foo": "bar"},
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
"foo": "bar",
},
},
{
about: "Master with cluster annotations and load balancer disabled",
role: "master",
enableMasterLoadBalancerSpec: &disabled,
enableMasterLoadBalancerOC: true,
operatorAnnotations: make(map[string]string),
clusterAnnotations: map[string]string{"foo": "bar"},
expect: map[string]string{"foo": "bar"},
},
{
about: "Master with operator annotations and load balancer enabled",
role: "master",
enableMasterLoadBalancerOC: true,
operatorAnnotations: map[string]string{"foo": "bar"},
clusterAnnotations: make(map[string]string),
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
"foo": "bar",
},
},
{
about: "Master with operator annotations override default annotations",
role: "master",
enableMasterLoadBalancerOC: true,
operatorAnnotations: map[string]string{
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
},
clusterAnnotations: make(map[string]string),
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
},
},
{
about: "Master with cluster annotations override default annotations",
role: "master",
enableMasterLoadBalancerOC: true,
operatorAnnotations: make(map[string]string),
clusterAnnotations: map[string]string{
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
},
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
},
},
{
about: "Master with cluster annotations do not override external-dns annotations",
role: "master",
enableMasterLoadBalancerOC: true,
operatorAnnotations: make(map[string]string),
clusterAnnotations: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "wrong.external-dns-name.example.com",
},
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
},
},
{
about: "Master with operator annotations do not override external-dns annotations",
role: "master",
enableMasterLoadBalancerOC: true,
clusterAnnotations: make(map[string]string),
operatorAnnotations: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "wrong.external-dns-name.example.com",
},
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
},
},
// REPLICA
{
about: "Replica with no annotations and EnableReplicaLoadBalancer disabled on spec and OperatorConfig",
role: "replica",
enableReplicaLoadBalancerSpec: &disabled,
enableReplicaLoadBalancerOC: false,
operatorAnnotations: make(map[string]string),
clusterAnnotations: make(map[string]string),
expect: make(map[string]string),
},
{
about: "Replica with no annotations and EnableReplicaLoadBalancer enabled on spec",
role: "replica",
enableReplicaLoadBalancerSpec: &enabled,
enableReplicaLoadBalancerOC: false,
operatorAnnotations: make(map[string]string),
clusterAnnotations: make(map[string]string),
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test-repl.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
},
},
{
about: "Replica with no annotations and EnableReplicaLoadBalancer enabled only on operator config",
role: "replica",
enableReplicaLoadBalancerSpec: &disabled,
enableReplicaLoadBalancerOC: true,
operatorAnnotations: make(map[string]string),
clusterAnnotations: make(map[string]string),
expect: make(map[string]string),
},
{
about: "Replica with no annotations and EnableReplicaLoadBalancer defined only on operator config",
role: "replica",
enableReplicaLoadBalancerOC: true,
operatorAnnotations: make(map[string]string),
clusterAnnotations: make(map[string]string),
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test-repl.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
},
},
{
about: "Replica with cluster annotations and load balancer enabled",
role: "replica",
enableReplicaLoadBalancerOC: true,
operatorAnnotations: make(map[string]string),
clusterAnnotations: map[string]string{"foo": "bar"},
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test-repl.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
"foo": "bar",
},
},
{
about: "Replica with cluster annotations and load balancer disabled",
role: "replica",
enableReplicaLoadBalancerSpec: &disabled,
enableReplicaLoadBalancerOC: true,
operatorAnnotations: make(map[string]string),
clusterAnnotations: map[string]string{"foo": "bar"},
expect: map[string]string{"foo": "bar"},
},
{
about: "Replica with operator annotations and load balancer enabled",
role: "replica",
enableReplicaLoadBalancerOC: true,
operatorAnnotations: map[string]string{"foo": "bar"},
clusterAnnotations: make(map[string]string),
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test-repl.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
"foo": "bar",
},
},
{
about: "Replica with operator annotations override default annotations",
role: "replica",
enableReplicaLoadBalancerOC: true,
operatorAnnotations: map[string]string{
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
},
clusterAnnotations: make(map[string]string),
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test-repl.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
},
},
{
about: "Replica with cluster annotations override default annotations",
role: "replica",
enableReplicaLoadBalancerOC: true,
operatorAnnotations: make(map[string]string),
clusterAnnotations: map[string]string{
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
},
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test-repl.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
},
},
{
about: "Replica with cluster annotations do not override external-dns annotations",
role: "replica",
enableReplicaLoadBalancerOC: true,
operatorAnnotations: make(map[string]string),
clusterAnnotations: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "wrong.external-dns-name.example.com",
},
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test-repl.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
},
},
{
about: "Replica with operator annotations do not override external-dns annotations",
role: "replica",
enableReplicaLoadBalancerOC: true,
clusterAnnotations: make(map[string]string),
operatorAnnotations: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "wrong.external-dns-name.example.com",
},
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test-repl.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
},
},
// COMMON
{
about: "cluster annotations append to operator annotations",
role: "replica",
enableReplicaLoadBalancerOC: false,
operatorAnnotations: map[string]string{"foo": "bar"},
clusterAnnotations: map[string]string{"post": "gres"},
expect: map[string]string{"foo": "bar", "post": "gres"},
},
{
about: "cluster annotations override operator annotations",
role: "replica",
enableReplicaLoadBalancerOC: false,
operatorAnnotations: map[string]string{"foo": "bar", "post": "gres"},
clusterAnnotations: map[string]string{"post": "greSQL"},
expect: map[string]string{"foo": "bar", "post": "greSQL"},
},
}
for _, tt := range tests {
t.Run(tt.about, func(t *testing.T) {
cl.OpConfig.CustomServiceAnnotations = tt.operatorAnnotations
cl.OpConfig.EnableMasterLoadBalancer = tt.enableMasterLoadBalancerOC
cl.OpConfig.EnableReplicaLoadBalancer = tt.enableReplicaLoadBalancerOC
cl.OpConfig.MasterDNSNameFormat = "{cluster}.{team}.{hostedzone}"
cl.OpConfig.ReplicaDNSNameFormat = "{cluster}-repl.{team}.{hostedzone}"
cl.OpConfig.DbHostedZone = "db.example.com"
cl.Postgresql.Spec.ClusterName = "test"
cl.Postgresql.Spec.TeamID = "acid"
cl.Postgresql.Spec.ServiceAnnotations = tt.clusterAnnotations
cl.Postgresql.Spec.EnableMasterLoadBalancer = tt.enableMasterLoadBalancerSpec
cl.Postgresql.Spec.EnableReplicaLoadBalancer = tt.enableReplicaLoadBalancerSpec
got := cl.generateServiceAnnotations(tt.role, &cl.Postgresql.Spec)
if len(tt.expect) != len(got) {
t.Errorf("expected %d annotation(s), got %d", len(tt.expect), len(got))
return
}
for k, v := range got {
if tt.expect[k] != v {
t.Errorf("expected annotation '%v' with value '%v', got value '%v'", k, tt.expect[k], v)
}
}
})
}
}
+37 -23
View File
@@ -1230,14 +1230,6 @@ func (c *Cluster) shouldCreateLoadBalancerForService(role PostgresRole, spec *ac
}
func (c *Cluster) generateService(role PostgresRole, spec *acidv1.PostgresSpec) *v1.Service {
var dnsName string
if role == Master {
dnsName = c.masterDNSName()
} else {
dnsName = c.replicaDNSName()
}
serviceSpec := v1.ServiceSpec{
Ports: []v1.ServicePort{{Name: "postgresql", Port: 5432, TargetPort: intstr.IntOrString{IntVal: 5432}}},
Type: v1.ServiceTypeClusterIP,
@@ -1247,8 +1239,6 @@ func (c *Cluster) generateService(role PostgresRole, spec *acidv1.PostgresSpec)
serviceSpec.Selector = c.roleLabelsSet(false, role)
}
var annotations map[string]string
if c.shouldCreateLoadBalancerForService(role, spec) {
// spec.AllowedSourceRanges evaluates to the empty slice of zero length
@@ -1262,18 +1252,6 @@ func (c *Cluster) generateService(role PostgresRole, spec *acidv1.PostgresSpec)
c.logger.Debugf("final load balancer source ranges as seen in a service spec (not necessarily applied): %q", serviceSpec.LoadBalancerSourceRanges)
serviceSpec.Type = v1.ServiceTypeLoadBalancer
annotations = map[string]string{
constants.ZalandoDNSNameAnnotation: dnsName,
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
}
if len(c.OpConfig.CustomServiceAnnotations) != 0 {
c.logger.Debugf("There are custom annotations defined, creating them.")
for customAnnotationKey, customAnnotationValue := range c.OpConfig.CustomServiceAnnotations {
annotations[customAnnotationKey] = customAnnotationValue
}
}
} else if role == Replica {
// before PR #258, the replica service was only created if allocated a LB
// now we always create the service but warn if the LB is absent
@@ -1285,7 +1263,7 @@ func (c *Cluster) generateService(role PostgresRole, spec *acidv1.PostgresSpec)
Name: c.serviceName(role),
Namespace: c.Namespace,
Labels: c.roleLabelsSet(true, role),
Annotations: annotations,
Annotations: c.generateServiceAnnotations(role, spec),
},
Spec: serviceSpec,
}
@@ -1293,6 +1271,42 @@ func (c *Cluster) generateService(role PostgresRole, spec *acidv1.PostgresSpec)
return service
}
func (c *Cluster) generateServiceAnnotations(role PostgresRole, spec *acidv1.PostgresSpec) map[string]string {
annotations := make(map[string]string)
for k, v := range c.OpConfig.CustomServiceAnnotations {
annotations[k] = v
}
if spec != nil || spec.ServiceAnnotations != nil {
for k, v := range spec.ServiceAnnotations {
annotations[k] = v
}
}
if c.shouldCreateLoadBalancerForService(role, spec) {
var dnsName string
if role == Master {
dnsName = c.masterDNSName()
} else {
dnsName = c.replicaDNSName()
}
// Just set ELB Timeout annotation with default value, if it does not
// have a cutom value
if _, ok := annotations[constants.ElbTimeoutAnnotationName]; !ok {
annotations[constants.ElbTimeoutAnnotationName] = constants.ElbTimeoutAnnotationValue
}
// External DNS name annotation is not customizable
annotations[constants.ZalandoDNSNameAnnotation] = dnsName
}
if len(annotations) == 0 {
return nil
}
return annotations
}
func (c *Cluster) generateEndpoint(role PostgresRole, subsets []v1.EndpointSubset) *v1.Endpoints {
endpoints := &v1.Endpoints{
ObjectMeta: metav1.ObjectMeta{