Load balancing service annotations are applying to services that do not have load balancing enabled

[Kaelten]

Please, answer some short questions which should help us to understand your problem / question better?

• Which image of the operator are you using? registry.opensource.zalan.do/acid/postgres-operator:v1.8.1
• Where do you run it - cloud or metal? Kubernetes or OpenShift? EKS
• Are you running Postgres Operator in production? yes
• Type of issue? bug report

I'm setting up a standby cluster that I wish to be exposed via a load balancer. I'm also using the aws-load-balancer-controller, which requires some special annotations to configure the NLB that's created. Those annotations are setup via the operator config map.

Out of the two cluster configurations in this project, only the standby has enableMasterLoadBalancer: true enabled. However, the annotations defined in the operator config map are then applied to the master and replication services for both clusters. This results in multiple unneeded NLBs being created.

I feel this is a bug in how the annotations are being calculated and to which services they are applying as only the standby's master service is having it's type set to LoadBalancer. Ultimately, I just need a way to limit which services receive these annotations.

[FxKu]

I don't know if the operator should block annotations from triggering actions in the chosen cloud infrastructure. Maybe another option can be added to define which pod and service instances should receive the specified annotations. Master, replica or all (default). Are you using customer_service_annotations option of serviceAnnotations from the manifest?

[alikhanich]

We are having the same problem here. It results in aws-load-balancer controller writing a lot of error logs.

{"level":"error","ts":1655375419.9841306,"logger":"controller-runtime.manager.controller.service","msg":"Reconciler error","name":"dba-test-cluster-postgresql-repl","namespace":"dba-test-ns","error":"unsupported service type \"ClusterIP\" for load balancer target type \"instance\""}

We definitely need a way to specify which services would receive annotations.

[klincool]

Have the same problem. Creates extra annotations -> creates extra load balancers, that are useless.

Has anyone found a way to get around this?

[yanchenko-igor]

Can you guys suggest a solution for your problem without breaking the current behavior? I need to apply annotations for ClusterIP type of the service, so the current behavior is exactly what I expect.

[Kaelten]

I've been able to work around this by upgrading to the latest versions of the aws-load-balancer-controller and enabling an feature flag to only look at loadbalancer types. Unfortunately, I had to fork the helm chart for it since that setting is not exposed through the helm chart.

I consider this a work around, as some people may need to support aws load balancers for non load balancers services.

[Kaelten]

I don't know if the operator should block annotations from triggering actions in the chosen cloud infrastructure. Maybe another option can be added to define which pod and service instances should receive the specified annotations. Master, replica or all (default). Are you using customer_service_annotations option of serviceAnnotations from the manifest?

I think setting it up so the annotations from the load balancer configuration only apply to enabled load balancers would work. Whether that means introducing need settings or not I'm not sure.

I'm also thinking that it's possible that we would want to support a different annotation set on different load balancers since those annotations can configure the NLB in different ways.

[ljcesca]

Hi, we are also running into this issue. I'm happy to work on a PR to improve this behavior, but would like your opinion if you'd prefer to:

• add masterLoadBalancerServiceAnnotations and replicaLoadBalancerServiceAnnotations to the Cluster CRD that get merged with serviceAnnotations
• disable pushing down serviceAnnotations onto the repl service when enableReplicaLoadBalancer is false (which would be a change to existing behavior)
• or another option

[Kaelten]

Hi, we are also running into this issue. I'm happy to work on a PR to improve this behavior, but would like your opinion if you'd prefer to:

• add masterLoadBalancerServiceAnnotations and replicaLoadBalancerServiceAnnotations to the Cluster CRD that get merged with serviceAnnotations
• disable pushing down serviceAnnotations onto the repl service when enableReplicaLoadBalancer is false (which would be a change to existing behavior)
• or another option

I think the serviceAnnotations block in the controller config has it's place, however I'd suggest having something akin to the propposed masterLoadBalancerServiceAnnotations and replicaLoadBalancerServiceAnnotations annocations that could exist in both the controller config as well as the cluster CRD. I suggest having it in both places since sometimes you'd want to have a global config as well as the ability to override things at the cluster level.

I think there is an argument that these settings shouldn't apply to disabled load balancers. However, balancing backward compatibility is a gotcha for sure.

I wonder if something like:

serviceAnnotations:
   setOnDisabled: bool
    global:
        key: value
    master:
        key: value
    slave:
        key: value

could be a sensical approach? (my names are bad, but it's an idea)

[Privatecoder]

+1 for masterLoadBalancerServiceAnnotations and replicaLoadBalancerServiceAnnotations

[owenthereal]

The separation of masterLoadBalancerServiceAnnotations and replicaLoadBalancerServiceAnnotations looks like the best option.

cc: @FxKu

[rishiraj88]

Looks good to me, too.

[owenthereal]

I put together https://github.com/zalando/postgres-operator/pull/2161 to separate the master service annotations & replica service annotations.

[FxKu]

Thanks for continuing the discussion. Two need manifest field seem like a proper solution. I've added a few comments. I want to do a release next week, so please take a look soon so we can include the feature in 1.9.

[rishiraj88]

Great thanks to all.