[client-go] Fake Dynamic Client

[DirectXMan12]

This commit converts "k8s.io/client-go/dynamic".Client to an interface, and implements fake versions of both ClientPool and Client. This allows components which make uses of these clients to be tested in the same way that clientset-based components can be tested, using the standard testing.Fake machinery.

Release note:

NONE

[k8s-reviewable]

This change is 

[DirectXMan12]

encountered this while trying to write unit tests for the custom metrics adapter for Prometheus. I'm still in the progress of writing those tests and making sure this is sufficient, but I think it should be.

[krousey]

I don't think dynamic client should return an interface just to enable testing. I think the library that wants to test it's interaction with the dynamic client should assign the return value from New to an interface. That being said, I'm going to re-assign this to someone on the api machinery team.

[DirectXMan12]

I don't think dynamic client should return an interface just to enable testing.

That's what the "normal" clients do. I was under the impression that it was a pretty well-established pattern.

[krousey]

That's what the "normal" clients do. I was under the impression that it was a pretty well-established pattern.

That's true, it is a pattern. I have the opinion that it's a mistake, but not a completely terrible one. As it is the current pattern, it would fit it. I'm also not the reviewer (anymore).

[soltysh]

I'd stick with the pattern we already have in place in the generated clientset, where there's a Interface and Client implements that, (see here). Just my $0.02.

[DirectXMan12]

I'd stick with the pattern we already have in place in the generated clientset, where there's a Interface and Client implements that, (see here). Just my $0.02.

I wanted to avoid a mass renaming, and wanted to follow the pattern established with ClientPool itself, which was already an interface.

[DirectXMan12]

ping @kubernetes/sig-api-machinery-pr-reviews when you get a chance

[sttts]

/lgtm

[sttts]

@smarterclayton approved?

[sttts]

/approve no-issue

[deads2k]

/approve no-issue

[deads2k]

/retest

[fejta]

/retest

[DirectXMan12]

This looks like it might actually be an issue with some code having gotten updated in the mean time. I'll rebase shortly.

[DirectXMan12]

ok, this should fix the test issue -- someone updated the signature of DynamicClient in the mean time ;-).

[liggitt]

/retest

[caesarxuchao]

golint failures:

Errors from golint:
W0721 23:23:06.250] staging/src/k8s.io/client-go/dynamic/fake/client.go:36:6: type name will be used as fake.FakeClient by other packages, and that stutters; consider calling this Client
W0721 23:23:06.250] staging/src/k8s.io/client-go/dynamic/fake/client.go:68:6: type name will be used as fake.FakeResourceClient by other packages, and that stutters; consider calling this ResourceClient
W0721 23:23:06.250] staging/src/k8s.io/client-go/dynamic/fake/client_pool.go:30:6: type name will be used as fake.FakeClientPool by other packages, and that stutters; consider calling this ClientPool
W0721 23:23:06.250] staging/src/k8s.io/client-go/dynamic/fake/client_pool.go:34:1: comment on exported method FakeClientPool.ClientForGroupVersionResource should be of the form "ClientForGroupVersionResource ..."
W0721 23:23:06.250] 
W0721 23:23:06.251] Please review the above warnings. You can test via "golint" and commit the result.
W0721 23:23:06.251] If the above warnings do not make sense, you can exempt this package from golint
W0721 23:23:06.251] checking by adding it to hack/.golint_failures (if your reviewer is okay with it).

[DirectXMan12]

wow, golint is supremely nitpicky

[DirectXMan12]

I'm adding this to the golint_failures file, since it follows the pattern in other fake clients

[sttts]

/lgtm

[k8s-github-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DirectXMan12, deads2k, sttts

Associated issue requirement bypassed by: deads2k, sttts

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

• hack/OWNERS [deads2k,sttts]
• pkg/controller/OWNERS [deads2k]
• pkg/printers/OWNERS [deads2k]
• staging/src/k8s.io/apiextensions-apiserver/OWNERS [deads2k,sttts]
• staging/src/k8s.io/client-go/OWNERS [deads2k,sttts]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

[k8s-github-robot]

Automatic merge from submit-queue (batch tested with PRs 48224, 45431, 45946, 48775, 49396)