Endpoint Slice Error "resourceVersion should not be set on objects to be created" subsys=ces-controller"

[sandeepksingla123]

Is there an existing issue for this?

• I have searched the existing issues

What happened?

Steps
a) 900 Node AKS Cluster with BYO CNI
b) Cilium CNI version 1.12 installed with on AKS
c) Networking Default mode VXLAN

Error:
PATCH:https://xxxxxx:443/api/v1/nodes/xxxxx/status" subsys=klog
level=info msg="Starting CNP derivative handler" subsys=cilium-operator-generic
level=info msg="Starting CCNP derivative handler" subsys=cilium-operator-generic
level=info msg="Initialization complete" subsys=cilium-operator-generic
level=info msg="Unable to create CiliumEndpointSlice in k8s-apiserver" ciliumEndpointSliceName=ces-zjlsmyyzb-q8766 error="resourceVersion should not be set on objects to be created" subsys=ces-controller

Cilium Version

1.12

Kernel Version

5.4.0-1086-azure #91~18.04.1-Ubuntu

Kubernetes Version

1.23.8

Sysdump

No response

Relevant log output

PATCH:https://xxxxxx:443/api/v1/nodes/xxxxx/status" subsys=klog
level=info msg="Starting CNP derivative handler" subsys=cilium-operator-generic
level=info msg="Starting CCNP derivative handler" subsys=cilium-operator-generic
level=info msg="Initialization complete" subsys=cilium-operator-generic
level=info msg="Unable to create CiliumEndpointSlice in k8s-apiserver" ciliumEndpointSliceName=ces-zjlsmyyzb-q8766 error="resourceVersion should not be set on objects to be created" subsys=ces-controller

Anything else?

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[sandeepksingla123]

Discussed over slack Please assign to @Weil0ng

[Weil0ng]

Thanks for reporting the issue!

@dlapcevic will work on a fix. Somehow cannot assign this to him...

[christarazi]

@Weil0ng Any updates here?

[dlapcevic]

Hi, @christarazi

This can hardly be categorized as a bug, because it has no negative effect beside the occasional error logs.
I don't see an obvious way to get rid of it.

I investigated the issue and it turns out that when a new CES is being created while two CEPs get immediately assigned to it, both try to create the new CES.
This happens because syncCES uses k8s store to check if the CES exists (ciliumEndpointSliceStore.GetByKey(key)), and it is not present quickly enough from the previous processed item (syncCES) that already created the CES.

We get the error resourceVersion should not be set on objects to be created because it is issuing a second CREATE request instead of UPDATE.

When I changed that reconciler makes ResourceVersion field of new CES to be empty before sending a request (as it always should be) reconcileCESCreate(), it again fails to create CES:
already exists","level":"info","msg":"Unable to create CiliumEndpointSlice in k8s-apiserver"

I also verified that there is no negative impact beside the error log.
Creating 2 pod deployments in different namespaces (so they need to be added into a new CES), triggered this issue about once in 10 deployment creations.
Even if the creation failed for the second call, the first call already had 2 CEPs included, so everything works correctly even when there are no subsequent updates for it.

[Weil0ng]

Thanks @dlapcevic for the investigation!

This happens because syncCES uses k8s store to check if the CES exists (ciliumEndpointSliceStore.GetByKey(key)), and it is not present quickly enough from the previous processed item (syncCES) that already created the CES.

Curious, shouldn't the operator look up in the local cache instead before creating the CES?

[dlapcevic]

It happens in this order:

1. CEP watcher receives events for CEP-A and CEP-B one after another, practically at the same time.
2. It runs in parallel InsertCEPInCache for both CEPs, where it tries to find a slice for them.
3. For CEP-A it gets a little faster to the point where it figures out that there is no matching CES in cache, so it creates a new CES in cache.
4. For CEP-B it sees the new CES and assigns it to the new slice created when CEP-A was being added.
5. One CES update is enqueued for CEP-A, and then one for CEP-B.
6. Workqueue has one worker only and processes the items sequentially.
7. First CES update doesn’t see this CES in store, so it creates the CES by calling the API server.
8. Second CES update also doesn’t see this CES in the store, which appears to be because it happens too quickly after the previous update which created it. So the watcher may take 10ms to get it from the API server, but the CES update happens faster.

Basically the time between the CES creation call to the API server and the next event getting to the point when it checks the store is less than the time it takes for this new CES to appear in the store.

Maybe I’m overlooking a simple way to fix it.

[Weil0ng]

I think having a already exists","level":"info","msg":"Unable to create CiliumEndpointSlice in k8s-apiserver" is much better and clearer than the existing error msg and the code shouldn't specify resourceVersion to begin with as you said, good to fix if you got some bandwidth :)

[dlapcevic]

This will be fixed by #23615 - "Reduce number of CES updates sent to API server in short time for the same CES".
I verified manually that no Unable to create CES logs appeared when I created 100 deployments (CESs) with this change, while it was common to happen at least once for every 10 CESs, without the change.

Otherwise, to fix the message, we would have to remove ResourceVersion in reconcileCESCreate() before sending it to be created, because copying it is actually required for updates. I verified that if we don't copy over ResourceVersion, the updates would fail then.

[Weil0ng]

Otherwise, to fix the message, we would have to remove ResourceVersion in reconcileCESCreate() before sending it to be created, because copying it is actually required for updates.

Hmm, maybe I'm missing something but I don't think the client should specify what ResourceVersion value to use when updating an obj? I thought ResourceVersion should always be populated by the api-server...

[dlapcevic]

You are right. I was just saying what would be the shortest fix in the current state, which is not the best. Otherwise it would require more refactoring, which we in long term plan to do. This is because before the fix, a race could happen with two very close consecutive updates for a new CES, and CES copy is expected to be used only for already created CESs that are also present in the cache (store), which sometimes didn't happen.