Fake clientset not sending events #54075

JosephSalisbury · 2017-10-17T16:08:50Z

Is this a BUG REPORT or FEATURE REQUEST?:

Uncomment only one, leave it on its own line:

/kind bug

/kind feature

What happened:
executing the following code:

package main

import (
	"log"
	"time"

	"k8s.io/api/core/v1"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	"k8s.io/client-go/kubernetes/fake"
)

func main() {
	client := fake.NewSimpleClientset()

	go func() {
		time.Sleep(3 * time.Second)

		client.CoreV1().Services("default").Create(&v1.Service{
			ObjectMeta: metav1.ObjectMeta{
				Namespace: "default",
				Name:      "foo",
			},
		})
		log.Printf("service created!")
	}()

	watch, _ := client.CoreV1().Services("default").Watch(metav1.ListOptions{})

	c := watch.ResultChan()

	for {
		select {
		case <-c:
			log.Println("an event occurred!")
		case <-time.After(10 * time.Second):
			log.Fatalf("timeout!")
		}
	}
}

prints

$ ./fake-watch-test
2017/10/17 16:57:06 service created!
2017/10/17 16:57:13 timeout!

What you expected to happen:
i would expect the "an event occurred" line to be printed, when the watch channel receives the event for the service creation

How to reproduce it (as minimally and precisely as possible):
go build the above, execute it.

Anything else we need to know?:

Environment:

Kubernetes version (use kubectl version):
Latest client-go (076e344c86e52f088b78615f815b245f6d613537)
Cloud provider or hardware configuration**:
OS (e.g. from /etc/os-release):
Kernel (e.g. uname -a):
Install tools:
Others:

The text was updated successfully, but these errors were encountered:

JosephSalisbury · 2017-10-17T16:10:17Z

/sig api-machinery

JosephSalisbury · 2017-10-17T16:12:41Z

@kubernetes/sig-api-machinery-bugs

k8s-ci-robot · 2017-10-17T16:17:34Z

@JosephSalisbury: Reiterating the mentions to trigger a notification:
@kubernetes/sig-api-machinery-bugs

In response to this:

@kubernetes/sig-api-machinery-bugs

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.

ncdc · 2017-10-17T17:04:39Z

The fake clientset has a WatchReactor for testing watch events. You typically do something like this:

// setup
client := fake.NewSimpleClientset()
watcher := watch.NewFake()
// note, testcore is "k8s.io/client-go/testing"
client.PrependWatchReactor("pods", testcore.DefaultWatchReactor(watcher, nil))

// simulate add/update/delete watch events
watcher.Add(myPod)

// test something that uses the client to do a watch
someController.doSomething(client)

@caesarxuchao @deads2k @sttts do you think it's worth trying to make the fake clientset send out watch events in response to CRUD operations?

sttts · 2017-10-17T17:40:21Z

Isn't a fake object always a compromise in terms of completeness? It will never be a full apiserver. Would it make tests much easier if it sent out watch events?

ncdc · 2017-10-17T18:04:40Z

@sttts I'm happy with the watch reactor semantics we have now. I'd vote that if one wants to test the client CRUD -> watch event flow, it should be an integration test with an apiserver in the middle.

JosephSalisbury · 2017-10-18T09:48:26Z

For context, I'm trying to test a controller/operator that uses a watch internally. Passing in a fake clientset is straightforward, having to simulate the watch events is a bit trickier.

I would agree that a larger integration test is probably the 'better' way, but I would like to be able to unit-test watches.

Happy to provide some PRs, if there consensus on the value here.

deads2k · 2017-10-18T13:12:55Z

I think it would be cool, but I suspect the cost will be significant in terms of updating all the usage locations. I wouldn't undertake it myself, but I wouldn't object to seeing it.

@ncdc @sttts do you actually object or just think its not worth the cost?

ncdc · 2017-10-18T13:21:42Z

I would be fine if someone undertook the effort to make the fake clientset generate watch events, but I think we have other avenues we can use right now that make it possible to test (e.g. making the sync handler a function pointer so it's easier to unit test, using the fake watch + reactor pattern).

@JosephSalisbury maybe you could expand a bit on how your code is structured and we can see if we can help you determine a way to unit test it?

sttts · 2017-10-18T15:54:20Z

do you actually object or just think its not worth the cost?

I don't object. If added complexity and effort to implement it pays off enough in test coverage & simplicity of tests.

caesarxuchao · 2017-10-19T00:02:05Z

I would be fine if someone undertook the effort to make the fake clientset generate watch events, but I think we have other avenues we can use right now that make it possible to test (e.g. making the sync handler a function pointer so it's easier to unit test, using the fake watch + reactor pattern).

I couldn't have said it any better.

deads2k · 2017-10-19T14:40:16Z

Happy to provide some PRs, if there consensus on the value here.

@JosephSalisbury sounds like everyone agrees it would be fine to do, but that it's likely to be a lot of work and we're not prepared to do it ourselves. If you want to undertake the effort, I'd suggest starting with converting just a few exemplar packages and having one (or more) of us four look at your approach before tackling the entire codebase.

yue9944882 · 2017-12-26T16:02:33Z

@deads2k @sttts @ncdc @JosephSalisbury @caesarxuchao
I've implemented a very simple watch method for fake client.#57504
And these implementation will effect nothing seriously currently. WDYT?

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. feat(fakeclient): push event on watched channel on add/update/delete **What this PR does / why we need it**: This PR enables watch function for kubernetes [fakeclient](https://github.com/kubernetes/kubernetes/blob/1bcf0b0a227d57057bde1f6fd50f26224483b324/staging/src/k8s.io/client-go/kubernetes/fake/clientset_generated.go#L88). This fake client add watchReactorFunction by wrapping [watch.NewFake](https://github.com/kubernetes/kubernetes/blob/1bcf0b0a227d57057bde1f6fd50f26224483b324/staging/src/k8s.io/client-go/kubernetes/fake/clientset_generated.go#L98) which is a `chan Event` but actually nothing pushes objects into this channel. So all watch function called by fake client will never return or never receive any object. This PR intercepts ReactionFunc of `Create / Update / DeleteActionImpl` and will push the requested object to channel. Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged): Fixes #54075 **Special notes for your reviewer**: **Release note**: ```dev-release-note enable watch function for fake client ```

k8s-github-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Oct 17, 2017

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Oct 17, 2017

k8s-github-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Oct 17, 2017

k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Oct 17, 2017

yue9944882 mentioned this issue Dec 24, 2017

feat(fakeclient): push event on watched channel on add/update/delete #57504

Merged

earlgreyz mentioned this issue Dec 25, 2017

ClusterController tests grtl/mysql-operator#22

Merged

wallrj mentioned this issue Jan 5, 2018

Integration tests for sample-controller #57839

Closed

yue9944882 mentioned this issue Jan 7, 2018

limit/continue paging of fakeclient doesn't work #57927

Closed

k8s-github-robot closed this as completed in #57504 Jan 16, 2018

grantr mentioned this issue Feb 24, 2018

Informers in unit tests do not handle events after they've started knative/serving#214

Closed

ffilippopoulos mentioned this issue Mar 27, 2018

Fix ingresses watch test utilitywarehouse/ingress53#15

Merged

cofyc mentioned this issue Dec 26, 2018

Use fake client real watch mechanism in PV controller tests #72327

Closed

gfichtenholt mentioned this issue Jun 29, 2021

Add caching for fluxv2 plugin (first step toward ##3032) vmware-tanzu/kubeapps#3044

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fake clientset not sending events #54075

Fake clientset not sending events #54075

JosephSalisbury commented Oct 17, 2017

JosephSalisbury commented Oct 17, 2017

JosephSalisbury commented Oct 17, 2017

k8s-ci-robot commented Oct 17, 2017

ncdc commented Oct 17, 2017

sttts commented Oct 17, 2017

ncdc commented Oct 17, 2017

JosephSalisbury commented Oct 18, 2017

deads2k commented Oct 18, 2017

ncdc commented Oct 18, 2017

sttts commented Oct 18, 2017

caesarxuchao commented Oct 19, 2017

deads2k commented Oct 19, 2017

yue9944882 commented Dec 26, 2017

Fake clientset not sending events #54075

Fake clientset not sending events #54075

Comments

JosephSalisbury commented Oct 17, 2017

JosephSalisbury commented Oct 17, 2017

JosephSalisbury commented Oct 17, 2017

k8s-ci-robot commented Oct 17, 2017

ncdc commented Oct 17, 2017

sttts commented Oct 17, 2017

ncdc commented Oct 17, 2017

JosephSalisbury commented Oct 18, 2017

deads2k commented Oct 18, 2017

ncdc commented Oct 18, 2017

sttts commented Oct 18, 2017

caesarxuchao commented Oct 19, 2017

deads2k commented Oct 19, 2017

yue9944882 commented Dec 26, 2017