Support reactor features in fake client

[grantr]

Reactors are a useful feature of the official client's fake implementation. They allow injecting errors, simulating the actions of other controllers, and tracking the actions of the controller under test instead of just the output. IIUC, none of those are possible with the current fake client.

[droot]

@fanzhangio Can you investigate how hard it will be to add reactors ?

[droot]

@grantr Have you considered using https://github.com/golang/mock.

[grantr]

@droot Can you give an example of how mocks would be used for this? What interface would I mock?

[DirectXMan12]

/kind feature

honestly, reactors might be a decent idea in theory, but in practice Kubernetes reactors tend to yield difficult-to-follow, brittle test code, IMO (as someone who's written or updated a number of reactor-based tests in the Kubernetes and OpenShift codebases)

we should think long and hard on the design before we opt for introducing them.

[DirectXMan12]

They allow injecting errors

Generally possible with a real API server for most errors (not found, conflict, etc), but I can understand a desire to easily simulate others.

simulating the actions of other controllers

Simulating the actions of other controllers generally isn't particularly hard if you have something that can store objects (either an actual API server or something that just tracks objects), since controllers are just updating state through normal API calls.

and tracking the actions of the controller under test instead of just the output

I'd assert we probably want to discourage this behavior -- you should generally be tracking if the controller yielded the desired state, not how it does that (granted, there are times you want to ensure things happen in a certain order, but that's rarer, and often can be worked around by splitting up the actual method that does the sync and/or using requeues)

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[DirectXMan12]

closing for the moment. as outlined about, I don't think we actually want to support this

[khuddlefish]

@DirectXMan12 We ended up writing our own reactive client and wrapping it around the provided fake client to give ourselves the ability to use reactors. We found it pretty difficult to create the error states we wanted to test without the use of reactors. Our implementation is not the cleanest at the moment because we just wrote it for internal use, but we might be interested in polishing it up and sharing it, or helping to work those features into the provided client if there was interest. The option to not use reactors is always there if you don't like them, but I do think they can be a useful feature for testing and it would be nice to make them available.

[fiunchinho]

@khuddlefish do you have a public repo to take a look at? I'm interested on using reactors as well (or find a useful workaround).

[khuddlefish]

@fiunchinho Our implementation is not public right now I'm afraid. We might move it out of our internal repo and share it somewhere if we get the time and if there's enough interest. I can share with you a blog post we wrote about our process developing it though. I hope this is a bit helpful http://engineering.pivotal.io/post/gp4k-kubebuilder-tdd/

[DirectXMan12]

@khuddlefish just as a heads up, the coloring on the syntax highlighting is almost impossible to read for me -- the grey used for the symbols fades into black background.

[iamkirkbater]

@khuddlefish I'm digging into the same scenario you are, where I'm having a problem trying to mock specific errors (at the appropriate time) for our operator. If there's been any progress on opening the implementation you used to be able to mock the fake I would really appreciate if you have the ability to share it.

[iamkirkbater]

Sorry for pinging everyone again, but I wanted to update how we ended up accomplishing this for anyone who comes here in the future.

https://www.bater.io/posts/2020/06/throwing-errors-with-controller-runtime-fake/