arangodb-operater annotation ensurer deletion of cloudprovider annotiatons leads to race condition

[waynr]

Howdy!

I am an engineer working on the Digitalocean (DO) Kubernetes product. Today our team got pinged about a huge number of duplicate DO LoadBalancer updates coming from two different customers' clusters; these updates were being sent by our open source Cloud Controller Manager. We're working on our backend to reduce the impact of these spurious requests, but in the meantime I thought it would be a good idea to report our findings here as to what we have identified as the source of these duplicate updates.

Specifically, both clusters in question have a arangodb-operator deployments running that appear to be repeatedly stripping critical annotations from the Service objects managed by our CCM component -- specifically, kubernetes.digitalocean.com/load-balancer-id, which our CCM component uses to maintain the relationship between the Service object and the underlying DO LoadBalancer; the removal of this annotation results in CCM detecting a change on the Service that results in two behaviors:

• we add the annotation back almost immediately because as I mentioned earlier it is necessary to maintain the identity of the Service as corresponding to the DO LoadBalancer
• because the Service object is changed we push its current state through the DO API to update the LoadBalancer (even though the only thing that has changed is the annotation, our CCM isn't sophisticated enough to diff the current state of the backend LoadBalancer with what would be asserted based on the Service object, nor is it obvious that we should put that kind of effort into making it do this kind of check)

At this point we have been able to reproduce the problem with the following workflow:

$ doctl k8s cluster create reproduce-arangodb-race --version 1.18 --region sfo2
$ doctl k8s cluster kubeconfig save reproduce-arangodb-race
$ export URLPREFIX=https://raw.githubusercontent.com/arangodb/kube-arangodb/1.0.3/manifests
$ kubectl apply -f $URLPREFIX/arango-crd.yaml
$ kubectl apply -f $URLPREFIX/arango-deployment.yaml
$ export URLPREFIX=https://raw.githubusercontent.com/arangodb/kube-arangodb/1.0.3/examples
$ kubectl apply -f $URLPREFIX/simple-cluster.yaml

(doctl is a command line tool for interacting with DO's product APIs)

Anyway, It seems like an antipattern to arbitrarily strip annotations from Service objects so I'm curious if it's really necessary to begin with; and if it is, would y'all be open to us opening a PR to add our annotation namespace to your list of secured namespaces? Thanks for your consideration!

[ajanikow]

Hello!

In Operator 1.1.0 you will be able to set list of ignored annotations, will update this issue with Pull Request details.

Best Regards,
Adam Janikowski.

[waynr]

Hi @ajanikow.

To be clear, the situation I am dealing with involves DO customers who are ArangoDB users that are presumably following some ArangoDB tutorial to deploy ArangoDB to DO Kubernetes clusters. Unless your documentation changes to indicate how to protect DO-specific annotations on Service objects, simply making it configurable doesn't solve the problem described here.

In my view, the arangodb operator should not be clearing annotations it's not responsible for in the first place. For what it's worth I brought that behavior up at a SIG Cloud Provider meeting last week and encountered similar skepticism on their part, especially given that not all cloud providers are going use a annotation that matches the ".*kubernetes\\.io/.*" regex y'all use to exclude some annotations from being stripped.

The ability to configure the set of ignored annotations is a non-solution because it doesn't fix the default behavior which is obviously (to me anyway) incorrect and has the potential to result in a degraded experience not only for cloud providers who have to work around it but for users who are at risk of encountering degraded apiserver performance on some cloud providers whose cloud controller manager components end up fighting with this operator to ensure annotations are set correctly.

So in order of best to worst fixes:

• don't blindly strip annotations off kubernetes resources
• allow me to add a built-in regex value that causes this operator to ignore DO-specific annotations

Thanks for your time and consideration!

[ajanikow]

Hello!

I have attached PR here - in 1.0.5 / 1.1.0 you will be able to:

• Define mode in which annotation should be updated - Disabled/Replace/Append
• Add list of ignored annotations/labels (regexp)

If you wont specify any annotations on deployment or group level, default mode will be "disabled", otherwise it will use "replace" mode.

Will this cover all requirements?

Best Regards,
Adam Janikowski.

[waynr]

Hey @ajanikow, thanks for addressing this issue! Apologies for not getting back to you sooner, your response seems to have escaped my attention back in 2020.