Optimization: Reuse HTTPClient / DynamicClient in reconcile

[justinsb]

Should be slightly more efficient via better HTTP connection reuse.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb

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:

• OWNERS [justinsb]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[justinsb]

cc @cheftako - as requested re reusing the DynamicClient; I think the answer is that the caller should try to provide a cached DynamicClient, because client-go is more efficient if we can reuse the http.Client. Added a comment and ensured that k-d-p itself passes the dynamic client (which we actually already have!)

[atoato88]

First of all, this looks good.

BTW, I am not sure about my comment is related with this PR. 🤔
Current direct applier uses the dynamicClient at here.
This always uses the dynamicClient from Factory.
Should we update the direct.go to use the opt.DynamicClient?

On the other hand, dynamicClient in applylib.go takes care of the existance of opt.DynamicClient.
It's OK as-is, I think.

[justinsb]

Great point @atoato88, and you're absolutely right. I added a commit that should also reuse the dynamic-client in the direct applier.

(I don't think we can practically reuse it in the exec applier.)

I also noticed that we probably should be trying to reuse the http client here, but I think I'll have to send a separate PR for that one!

[atoato88]

@justinsb
Thank you for the response and update.

(I don't think we can practically reuse it in the exec applier.)

I think so too.

I also noticed that we probably should be trying to reuse the http client here, but I think I'll have to send a separate PR for that one!

OK!
We can go forward. 🚀

[atoato88]

/lgtm