vrg: use reader instead of client for check hooks

[raghavendra-talur]

When using the cached client, we fail to find the resource if it is recently created.

[BenamarMk]

@raghavendra-talur It looks good to me, but I have a question about the cache. Are we potentially undermining the purpose of skipping the usage of the cache? If it's used selectively, that would be fine, but consistently hitting etcd instead of the cache feels like an expensive operation. Also, what’s the downside of not finding the "just created resource" in the cache? The next reconciliation cycle should pick it up and things should be fine. Why is it so important that we find it right after we create it. One reason for it would be to not try to recreate it again -- ok, that would be a valid reason. But is that the case?

[raghavendra-talur]

@raghavendra-talur It looks good to me, but I have a question about the cache. Are we potentially undermining the purpose of skipping the usage of the cache? If it's used selectively, that would be fine, but consistently hitting etcd instead of the cache feels like an expensive operation. Also, what’s the downside of not finding the "just created resource" in the cache? The next reconciliation cycle should pick it up and things should be fine. Why is it so important that we find it right after we create it. One reason for it would be to not try to recreate it again -- ok, that would be a valid reason. But is that the case?

There were two different issues we saw:

1. Delay in getting the data and multiple reconciles required
2. The cached client never filled the cache for a certain type of object. IIRC something like pods were in the cache but deployment was not. In case of pod the next reconcile succeeded but in case of deployment, it never did.

Using the reader solves both the problems.

There are pros and cons to using the reader instead of using the cached client. One downside is that we hit the api-server for every backup or restore job, like you said. The pro is that we don't have to maintain the cache for pods, deployment and stateful set.

Comparing the two, I really think the pro of not having to keep the cache about pods,deployment,statefulset in the ramen process memory wins. These resources are not something that ramen relies on in most of the cases(managed apps and discovered with no hooks). It is only in the case of discovered apps with hooks in the recipe that we need this information.

I need to write a unit test that proves this but haven't been able to complete that in this PR. Will do it in a follow up.

[asn1809]

Lgtm

[BenamarMk]

There were two different issues we saw:

Delay in getting the data and multiple reconciles required
The cached client never filled the cache for a certain type of object. IIRC something like pods were in the cache but deployment was not. In case of pod the next reconcile succeeded but in case of deployment, it never did.

Thanks @raghavendra-talur. Based on what you’ve said, it’s strange that the client-go cache behaves that way, but I’ll leave it at that.