[Bug] Operator continuously update resources in AKS

[vutkin]

Describe the bug
In our case we noticed that when we trying to create internal LB in AKS managed by Rancher (this is important) using annotation:

externalBootstrapService:
  metadata:
    annotations:
      service.beta.kubernetes.io/azure-load-balancer-internal: "true"
perPodService:
  metadata:
    annotations:
      service.beta.kubernetes.io/azure-load-balancer-internal: "true"

we see that time needed for creating LB is ~15 min.
The root cause is that Strimzi operator and Rancher (which adds custom annotations like /metadata/annotations/field.cattle.io~1publicEndpoints) simultaneously try to change the same services of LoadBalancer type. Rancher adds some labels and annotations to services and Strimzi operator removes that data. That process caused continuously updating Azure load balancer configuration. As a result, Azure load balancer stays in an "updating" state with a long processing queue.

To Reproduce
Steps to reproduce the behavior:

1. Create internal LB for external listener
2. Create AKS cluster managed by Rancher (https://rancher.com/docs/rancher/v2.x/en/cluster-provisioning/hosted-kubernetes-clusters/aks/)
3. Create Internal LB by adding annotations: creating takes more than 15 min

Expected behavior
LB creation takes less than 2 min.

Environment (please complete the following information):

• Strimzi version: 0.18
• Installation method: Helm chart
• Kubernetes cluster: Kubernetes 1.18
• Infrastructure: Azure AKS/Rancher

YAML files and logs

Additional context
Suggestion: add ability ignore changes for annotations/labels for resources watched by operator:

example:
ignore_changes = [
      metadata[0].annotations["cattle.io/*"],
      metadata[0].annotations["field.cattle.io~1publicEndpoints"]
    ]

[vutkin]

Same behavior I saw in this issue: #2558

[scholzj]

Rancher seems to be the only distro I met so far doing all kind of weird stuff like this which nobody else does :-o.

I'm not sure we can so easily just ignore them. What we do, is we really just patch the service which removes these unexpected annotations. If needed, we can detect that these annotations are the only difference and not patch it in that case. But we still need to patch it if something changes and we need to update the service. That should decrease the frequency of it happening. But not remove it completely. So I wonder what is the meaning of these annotations and how important it is and whether this would still be causing issues. But I guess we can give it a try and see if it helps.

[vutkin]

Rancher seems to be the only distro I met so far doing all kind of weird stuff like this which nobody else does :-o.

I think tools like FluxCD/ArgoCD do same: add internal annotations.

Sounds good, how we could do this?

[scholzj]

If you are interested in looking into it and contributing it, I can try to guide you. If not, I will try to find some time for it and fix it my self.

[vutkin]

Hi @scholzj, Unfortunately I am not strong in Java...

[mluiten]

@scholzj if you can point me in the right direction, I would be eager to take a look if I can try and make an improvement.

Why does Strimzi want to remove annotations that have nothing to do with Strimzi? Is there a whitelist of annotations it should care about?

[scholzj]

@mluiten Strimzi does not remove the annotation per se. It just reconciles the resource => i.e. applies our version of it which removes these annotations because it does not have them. We do not have any whitelists or blacklists because so far it was never needed until this case. But for this particular usecase I would assume one can add it here:

strimzi-kafka-operator/operator-common/src/main/java/io/strimzi/operator/common/operator/resource/ServiceOperator.java

Line 54 in 3e76f37

protected Future<ReconcileResult<Service>> internalPatch(String namespace, String name, Service current, Service desired) {

There we already handle some similar things in the service spec section such as assigned node ports or ipFamily etc. So I assume here we can have some allow-list for annotations which would be back ported from the original service to not get them deleted in the patch. That is at least where I planned to start ... but of course contributions are always welcomed, so if you wanna look into it you are more then welcomed.