Repository navigation
Readiness Tracker Deadlock for Terminating Resources聽#660
Description
What
This was brought to my attention and discovered by 馃挭@SimKev2 馃挭
Resources which are marked for termination, on gatekeeper startup, cause gatekeeper to fail its readiness probes indefinitely (assuming gatekeeper is trying to sync those resources). This seems to be because gatekeeper is trying to ensure it has successfully loaded the sync cache before it handles any traffic. However, it "expects" but fails to "observe" terminating resources, resulting in a deadlock for the readiness check.
Steps
- Create the following namespace.
apiVersion: v1
kind: Namespace
metadata:
finalizers:
- rob.test.io/abcdefg
name: rob-test-
Delete it with
kubectl delete ns rob-test(This should hang, since the finalizer won't resolve) The purpose of this is to put a resource permanently into the terminating state. -
Ensure
Namespaceis in the sync config
apiVersion: config.gatekeeper.sh/v1alpha1
kind: Config
metadata:
name: config
namespace: gatekeeper-system
spec:
sync:
syncOnly:
- kind: Namespace
version: v1- Start Gatekeeper
After gatekeeper starts up it should fail its readiness checks indefinitely.
This seems to be because when setting the expectations for the objectTracker we take into consideration the rob-test namespace (even though it's terminating). This becomes a problem later when running the sync controller, since it doesn't observe resources marked for termination.
I think what makes sense is to run Observe on resources which have been marked for termination.
So add this r.tracker.ForData(gvk).Observe(instance) here
FYI @shomron
Environment:
- Gatekeeper version: v3.1.0-beta.9
- Kubernetes version: 1.15
Activity
maxsmythe commentedon Jun 2, 2020
Thanks for finding this! We should also call cancel expectations for any observed deletes. Otherwise there is a race condition where an object is deleted sometime after the initial list is gathered but before the operator begins syncing.
maxsmythe commentedon Jun 2, 2020
This also involves modifying the cancel expectation function to short-circuit if expectations already satisfied to avoid a memory leak.
maxsmythe commentedon Jun 2, 2020
It looks like we are already calling CancelExpect for all deleted constraint templates, so short-circuiting-if-populated would fix a memory leak there
shomron commentedon Jun 9, 2020
@theMagicalKarp thank you again!
@maxsmythe I can pick up the remaining work, need to confirm the scope.
maxsmythe commentedon Jun 9, 2020
ack, lemme know. Happy to help if the scope is too large.
Introduce circuit breaker into objectTracker
Introduce circuit breaker into objectTracker
Introduce circuit breaker into objectTracker (#683)
niroowns commentedon Aug 28, 2020
Hi @maxsmythe @shomron - is there a work around for this without having to update to the latest image? We're running verison v3.1.0-beta.9 of the controller. We're seeing what appears to be a similar issue impacting the pod resource, but I don't see anything in a "Terminating" state. Removing pods from the config file and deleting the controller pod does allow it to come up and the readiness probe to pass. I'm just not sure which is the offending pod within the cluster that's causing the readiness probe to return a 500. Any help would be appreciated.
shomron commentedon Aug 28, 2020
@niroowns without updating the image, your best bet would be to remove the
readinessProbefrom the Gatekeeper manifest. But upgrading would be preferable :)