Reactor Usage

[pcj]

Asking a question and adding reactor example for some future poor schmuck like myself that spent the afternoon trying to figure out why my reactor was not called.

The key is understanding that that most generated / fake clientsets add in a "catch-all" reactor for the ObjectTracker that will always terminate the ReactorChain before it gets to yours. So, although you'd think .AddReactor would work, in practice it's never called.

Consider this snippet in which the ReactorFunc will never be called:

import (
	"k8s.io/client-go/kubernetes/fake"
	fakebatchv1 "k8s.io/client-go/kubernetes/typed/batch/v1/fake"
	k8stesting "k8s.io/client-go/testing"
)
fakeClient := fake.NewSimpleClientset()
fakeClient.BatchV1().(*fakebatchv1.FakeBatchV1).Fake.AddReactor("*", "*", func(action k8stesting.Action) (bool, runtime.Object, error) {
    t.Logf("I'm never triggered :(")
    return true, nil, nil
})

Looking at https://github.com/kubernetes/client-go/blob/master/kubernetes/fake/clientset_generated.go, we see this code:

// NewSimpleClientset returns a clientset that will respond with the provided objects.
// It's backed by a very simple object tracker that processes creates, updates and deletions as-is,
// without applying any validations and/or defaults. It shouldn't be considered a replacement
// for a real clientset and is mostly useful in simple unit tests.
func NewSimpleClientset(objects ...runtime.Object) *Clientset {
	o := testing.NewObjectTracker(scheme, codecs.UniversalDecoder())

        ...

	fakePtr := testing.Fake{}
	fakePtr.AddReactor("*", "*", testing.ObjectReaction(o))

        ...

	return &Clientset{fakePtr, &fakediscovery.FakeDiscovery{Fake: &fakePtr}}
}

In the testing/fake.go base clientset, one finds the Invokes function that is called by all the derived fake client implementations:

// Invokes records the provided Action and then invokes the ReactionFunc that
// handles the action if one exists. defaultReturnObj is expected to be of the
// same type a normal call would return.
func (c *Fake) Invokes(action Action, defaultReturnObj runtime.Object) (runtime.Object, error) {
	c.Lock()
	defer c.Unlock()

	c.actions = append(c.actions, action)
	for _, reactor := range c.ReactionChain {

		if !reactor.Handles(action) {
			continue
		}

		handled, ret, err := reactor.React(action)
		if !handled {
			continue
		}

		return ret, err
	}

	return defaultReturnObj, nil
}

So the first ReactorFunc that handles the action will be called. Since there is always a *, * in the front if the reactor chain, any other one added via the AddReactor function will never be called.

The workaround is to use PrependReactor instead:

fakeClient := fake.NewSimpleClientset()
fakeClient.BatchV1().(*fakebatchv1.FakeBatchV1).Fake.PrependReactor("*", "*", func(action k8stesting.Action) (bool, runtime.Object, error) {
    t.Logf("This works :)")
    return true, nil, nil
})

My question is, am I doing this wrong? How is the AddReactor function ever useful? I would like to write idiomatic tests but unclear on the reactor usage.

[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

[pcj]

/remove-lifecycle rotten

Still awaiting a response here. Thanks.

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

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

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

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

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[antoineco]

/reopen

[k8s-ci-robot]

@antoineco: Reopened this issue.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[antoineco]

I just faced the same issue and was wondering whether this behaviour is by design. The most confusing part is that there doesn't seem to be a way to chain ReactionFuncs, although those functions are stored in a field called ReactionChain within testing.Fake.

@pcj suggests to use fake.ClientSet.PrependReactor(), but even this is flawed because the chain is broken after the first "handled" ReactionFunc. As a result, it is impossible to both apply transformations to an object and create this object in the testing.ObjectTracker (which is not returned by fake.NewSimpleClientset(), to make things even harder).

Would proposed changes still be considered at this point?

• The simplest thing that comes to my mind: a ReactionFunc can return whether it should be the last function in the chain.

  type ReactionFunc func(action Action) (handled, last bool, ret runtime.Object, err error)

• An alternative proposal: testing.Fake could provide an accessor to "its" testing.ObjectTracker so that custom ReactionFuncs can use it.

  // Fake implements client.Interface. Meant to be embedded into a struct to get
  // a default implementation. This makes faking out just the method you want to
  // test easier.
  type Fake struct {
  	sync.RWMutex
  	actions []Action // these may be castable to other types, but "Action" is the minimum
  
  	tracker ObjectTracker  // added
  
  	// ReactionChain is the list of reactors that will be attempted for every
  	// request in the order they are tried.
  	ReactionChain []Reactor
  	// WatchReactionChain is the list of watch reactors that will be attempted
  	// for every request in the order they are tried.
  	WatchReactionChain []WatchReactor
  	// ProxyReactionChain is the list of proxy reactors that will be attempted
  	// for every request in the order they are tried.
  	ProxyReactionChain []ProxyReactor
  
  	Resources []*metav1.APIResourceList
  }
  
  // GetObjectTracker returns the ObjectTracker used by the fake client.
  func (c *Fake) GetObjectTracker() ObjectTracker {
  	return c.tracker
  }

[liggitt]

@pcj suggests to use fake.ClientSet.PrependReactor(), but even this is flawed because the chain is broken after the first "handled" ReactionFunc. As a result, it is impossible to both apply transformations to an object and create this object in the testing.ObjectTracker (which is not returned by fake.NewSimpleClientset(), to make things even harder).

as of kubernetes/kubernetes#73601, released in client-go v11.0.0, reactors can mutate an object, then return false, and the mutations are carried along to the subsequent reactors

[antoineco]

Aha! One can now mutate actionCopy directly and the state is shared. That's a great improvement, thanks for the pointer @liggitt and sorry for missing the update in v11.

/close

[k8s-ci-robot]

@antoineco: Closing this issue.

In response to this:

Aha! One can now mutate actionCopy directly and the state is shared. That's a great improvement, thanks for the pointer @liggitt and sorry for missing the update in v11.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[louyiping]

I've been working on this for days...... Thanks ! @pcj