Skip to content

Conversation

jonathanbeber
Copy link
Contributor

The operator parameters 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 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. 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.

Also, it updates the documentation and creates the necessary unit and e2e tests to the above-described feature.

@jonathanbeber jonathanbeber force-pushed the serviceannotations branch 2 times, most recently from 446c410 to 17a58de Compare January 28, 2020 12:47
@jonathanbeber
Copy link
Contributor Author

@FxKu thanks for your review. I addressed these points.

@FxKu
Copy link
Member

FxKu commented Jan 28, 2020

Thanks, tested it myself and worked. Not sure, if I really like the extra manifest. But if you are planning another PR, where it can be removed again, it's fine with me.

Just one more thing: Could you revert the changes in go.mod and go.sum. They probably got introduced due to running e2e tests. That's something we should fix soon.

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
@jonathanbeber
Copy link
Contributor Author

Thanks, tested it myself and worked. Not sure, if I really like the extra manifest. But if you are planning another PR, where it can be removed again, it's fine with me.

Yes, definitely we can remove it soon on my next PR.

Just one more thing: Could you revert the changes in go.mod and go.sum. They probably got introduced due to running e2e tests. That's something we should fix soon.

Done.

thanks again for your review @FxKu

@FxKu
Copy link
Member

FxKu commented Jan 29, 2020

👍

@jonathanbeber
Copy link
Contributor Author

👍

@FxKu FxKu merged commit ba60e15 into zalando:master Feb 10, 2020
@jonathanbeber jonathanbeber deleted the serviceannotations branch February 10, 2020 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants