-
Notifications
You must be signed in to change notification settings - Fork 87
Unprocessable Entity #309
Comments
Hello. Thanks for this interesting use-case. I'm quite surprised that it worked with 0.24 and before — it should also fail. There were no changes that could avoid this error. There is a code for a similar case already — when 404 is returned from patching (see code). However, in your case, it is not 404, but 422. We could catch "422 Unprocessable Entity" the same way — in case it is indeed the case from the Kubernetes API point of view. It would also be useful to see the full response body from this PATCH request — but this definitely should not be put on the logs. I will take some time to dive deep into the docs to understand why it is 422. Maybe I can reproduce it locally with the same use-case. |
Hi, I also ran this issue with 0.25 and not with release 0.24. When deleting an Ingress object, I get an error : aiohttp.client_exceptions.ClientResponseError: 422, message='Unprocessable Entity' from k8s api when I delete the Ingress object. It's look like kopf try to patch or read an object that I've just deleted. |
Hi @nolar, I'm seeing the same thing with an on.delete handler for pods. Do you need help reproducing it? It seems to happen every time the pod is deleted. Here is a sample:
|
@nolar / @brutus333 I think I see what's happening. Like the OP, I have a controller for a CRD that creates pods. I think what should happen is kopf would add the finalizer to those pods after they're created, but I don't see that happening. Instead, when the pod is deleted kopf tries to add the finalizer:
This returns a 422 error code because the pod is already in the terminating state, and this can be reproduced using kubectl as well. I couldn't find the general rule of when kopf is supposed to add the finalizers, but I would think it's before the deletion. |
A bit more information: it looks like the pod did indeed have the finalizer handle to begin with. The on.delete handler is being called when the pod is deleted, and the finalizer is removed (correctly). But for some reason there's a second event getting fired that triggers a re-tag of the pod's finalizer, which fails since it's no longer here. Here's an example:
|
@cliffburdick Thank you for this investigation! Can you please verify this with 0.27rc6 in an isolated environment (because it is a release candidate yet)? The logic for finalizer addition/removal is located in The finalizer decisioning logic was significantly reworked in 0.27 RCs (due to a special type of handlers added: daemons & timers), but it is hard to say which cases were or were not solved as a side-effect compared to 0.25. However, thanks to your investigation, I can make a hypothesis, that in 0.25, the finalizer was added because it used 2 criteria only: a finalizer is needed (there are deletion handlers) AND the finalizer is absent on the object — as seen in the link 3. It could only work normally if the object is removed instantly after the finalizer is removed, and there are no additional cycles, e.g. with other controllers with their own finalziers. In 0.27, an additional 3rd criterion was added (as seen in the link 1): So, with some above-zero probability, the issue is solved. But this needs to be verified. |
@nolar sure! I'll try it out and report back. For what it's worth, when this happens |
@nolar I can confirm that 0.27rc6 indeed fixes the problem! I did notice a lot more aiohttp traffic to the k8s server while the pod was active compared to 0.25, but I am no longer seeing the 422 error code. I think this one can likely be closed. |
@cliffburdick Regarding the traffic: Can you please create a separate issue with some excerpts and data? Is it in bytes or in rps? The byte-measured traffic can increase due to double-storage of Kopf's own status: annotations PLUS status — for smooth transitioning. Previously, it was only in status, but Kubernetes's "structural schemas" broke that since K8s 1.16+. This aspect can be configured. The rps-measured traffic should not be higher than before. In theory. This is worth checking out. Anyway, I never tested Kopf for performance yet. Maybe, the time comes to start collecting some data & issues for this. |
@nolar sure. It was rps -- the bytes didn't increase much. I had some debug print statements in the aio library from trying to debug this issue, and saw those increase. I'll try to write up more detail. |
@cliffburdick PS: 0.27rc6 is going to be 0.27 in a few days — I have finally finished testing it in action. But test it carefully before upgrading anyway — 0.27 is a huge change, and therefore it is risky (despite all backward compatibility and stability attempted) — and 6 (!) release candidates kind of suggest that it wasn't an easy release. |
Great! Luckily I'm still in the testing phase and it's not officially released anyways, so it shouldn't break anything on my end. |
I still see this issue in 0.27, but only when the operator uses a custom finalizer of its own. Whenever the delete handler removes a finalizer, the 422 exception is thrown, after the handler returns. It's mostly harmless because the delete handler finishes fine and since handlers are supposed to be idempotent anyway, nothing bad happens from the retried delete handler (plus, the extra finalizer is already gone, otherwise I suppose the retries would keep on forever). Still, it would be nice if the exception didn't happen, since that prevents clean test runs and can be confusing when troubleshooting. Let me know if you'd like me to provide a minimal test case. |
Long story short
I've tried to qualify 0.25 based on existing tests built with KopfRunner context. These tests worked well with all versions from 0.21 to 0.24. However, using 0.25 will raise a framework error.
Description
One of the simplest tests creates a custom object, lets the operator create a pod based on custom object definition and delete the custom object (which by owner cascaded deletion deletes the pod too).
Environment
The text was updated successfully, but these errors were encountered: