Use a single dynamic client in the reconciler #87
Conversation
|
|
||
| v1err := r.Get(context.TODO(), key, &v1def) | ||
| v1def, v1err := r.TargetK8sDynamicClient.Resource(crdGVR).Get(context.TODO(), crdName, metav1.GetOptions{}) |
There was a problem hiding this comment.
I think the CRD will be on the hosting cluster, not the target cluster.
Also, I'm wondering if going through the dynamic client removes the need for #88 ? Since I can't tell how the cache settings there would get to the dynamic client (but maybe I'm missing something)
There was a problem hiding this comment.
Sorry about that. I didn't mean to commit this part.
There was a problem hiding this comment.
I was originally going towards the route of using the dynamic client but then realized it did actually need to be a client pointing to the cluster the config-policy-controller is running on. I decided to not have yet another client on the reconciler and figured the caching provided by #88 might be a good thing rather than query for the CRD on every ConfigurationPolicy deletion.
The latest push removes the code that was accidentally committed.
This makes the code simpler and removes the overhead of creating separate copies of API configs and dynamic clients on each object being evaluated. Signed-off-by: mprahl <mprahl@users.noreply.github.com>
mprahl
force-pushed
the
single-dynamic-client
branch
from
6203389
to
c39c096
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JustinKuli, mprahl The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This makes the code simpler and removes the overhead of creating separate copies of API configs and dynamic clients on each object being evaluated.