operator-framework / operator-sdk

SDK for building Kubernetes applications. Provides high level APIs, useful abstractions, and project scaffolding.
https://sdk.operatorframework.io
Apache License 2.0
7.2k stars 1.74k forks source link

Add Watch method to pkg/test/client.go #2655

Closed phoracek closed 4 years ago

phoracek commented 4 years ago

Feature Request

Is your feature request related to a problem? Please describe.

When testing my operator, I want to verify that status on my resource moves from A to B to C. Currently I do it with polling and Get, however, sometimes there is a race if the state transforms from A to C to quickly.

I can use Watch method of client-go directly, however, it would be easier and cleaner if this functionality was included in operator-sdk's test helpers.

Describe the solution you'd like

I'd like pkg/test/client.go:FrameworkClient to include Watch and ListWatch methods enabling users to monitor object transitions.

camilamacedo86 commented 4 years ago

Hi @phoracek,

To check changes in the resource has the Watch feature. Following an example:

err := c.Watch(&source.Kind{Type: &appsv1.Deployment{}}, &handler.EnqueueRequestForOwner{
    IsController: true,
    OwnerType:    &cachev1alpha1.Memcached{},
  })

See the. doc: https://github.com/operator-framework/operator-sdk/blob/master/doc/user-guide.md#resources-watched-by-the-controller

You do not will watch the client, but the resources. (the client is reasonable to the requests in the API only). Then, when the watched resource be created/delete/updated the reconcile will be re-trigged. Note that the recommended approach is you implement an implement a solution where the reconcile will be kept running until all resources are created and be in the desired state. I mean with the wished values and status.

Also, I'd like to recommend you check the blog post Getting started with Golang Operators by using Operator SDK for a better explanation with examples.

Please, let us if we can close this one or what still missing to help you with.

phoracek commented 4 years ago

Hello @camilamacedo86, thanks!

The issue is, I don't want to implement a controller. I want to test what the controller does. My controller is already running, reconciling and setting status on its CR. In my tests, I validate that the status reaches expected state with github.com/operator-framework/operator-sdk/pkg/test's test.Global.Client.Get (https://github.com/kubevirt/cluster-network-addons-operator/blob/master/test/check/check.go#L74).

I would like to use something like test.Global.Client.Watch to monitor the transition, compared to mere polling I do now.

phoracek commented 4 years ago

I see that operator-sdk just uses controller-runtime's client which does not have Watch either. I should first open an RFE on them, shouldn't I.

camilamacedo86 commented 4 years ago

HI @phoracek,

To do the tests you will use a fake-client and check the expected results. See the doc.

Also, you can check an example in my personal project; So, you will create the controller_test as here. And then, you will use the fake client as implemented 'here.

Note that in this way, you will be able to check id the spec and status after the reconciliation is with the expected results.

Example

    //Mock Replicas wrong size
    size := int32(3)
    deployment.Spec.Replicas = &size

    // Update
    err = r.client.Update(context.TODO(), deployment)
    if err != nil {
        t.Fatalf("fails when try to update deployment replicas: (%v)", err)
    }

    res, err = r.Reconcile(req)
    if err != nil {
        t.Fatalf("reconcile: (%v)", err)
    }

    dep, err := service.FetchDeployment(dbInstanceWithoutSpec.Name, dbInstanceWithoutSpec.Namespace, r.client)
    if err != nil {
        t.Fatalf("get deployment: (%v)", err)
    }

    if *dep.Spec.Replicas != 1 {
        t.Errorf("Replicas size was not respected got (%v), when is expected (%v)", *dep.Spec.Replicas, 1)
    }

So, in the controller, you do the implementation and use the watch feature. Then, to test it you will implement your tests and use the fake client to check the results.

Please, let us if we can close this one or what still missing to help you with.

camilamacedo86 commented 4 years ago

HI @phoracek,

Could you please let us know if the above information attends your needs? If not, because for example you really check the status of the transition in the tests, could you please share with us why? I mean, could you illustrate a scenario that it would be required to be done in the tests?

phoracek commented 4 years ago

Thanks for the suggestion!

If I understand it correctly, fake client is only useful for unit tests. That does not fit my needs, I need to monitor the transition on a real cluster.

My operator is deploying set of components on the cluster. One controller is applying desired configuration why another is monitoring all deployed pods and updating the status based on their lifecycle.

In this test, I'm deploying one of these components and want to verify that status on my CR turns from Available to Progressing and back to Available. Without a possibility to Watch changes on the state, I need to use polling and sometimes miss the Progressing part.

Since the updating of the status is really done by concurrently running controller based on other components that are dynamically changing in the cluster, I don't want to test this flow via mocking and the fake client.

camilamacedo86 commented 4 years ago

HI @phoracek,

Could you check the doc Using the Operator SDK's Test Framework to Write E2E Tests?

So, I understand that what you are requesting here would be something like a few helpers, as it has the WaitForDeployment such as WaitForStatus in order to attend your need. Am I right? Could you please confirm my understanding? @joelanford WDYT? Should we move forward with an RFE for it? Have you a better idea over how to address this need?

Also, in order to share some options. See how we do few e2e tests as well for the SDK project using shell script which may help you address your needs for currently: https://github.com/operator-framework/operator-sdk/blob/master/hack/tests/e2e-helm.sh#L39

jberkhahn commented 4 years ago

Hey, just checking in on this issue. We would like if you could post more specific information about your exact usecase. If you could share the exact state change you're hoping to monitor, or provide us with some code examples for what you're doing that would be great.

openshift-bot commented 4 years ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. Exclude this issue from closing by commenting /lifecycle frozen.

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

/lifecycle stale

openshift-bot commented 4 years ago

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity. Exclude this issue from closing by commenting /lifecycle frozen.

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

/lifecycle rotten /remove-lifecycle stale

openshift-bot commented 4 years ago

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen. Mark the issue as fresh by commenting /remove-lifecycle rotten. Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci-robot commented 4 years ago

@openshift-bot: Closing this issue.

In response to [this](https://github.com/operator-framework/operator-sdk/issues/2655#issuecomment-686786687): >Rotten issues close after 30d of inactivity. > >Reopen the issue by commenting `/reopen`. >Mark the issue as fresh by commenting `/remove-lifecycle rotten`. >Exclude this issue from closing again by commenting `/lifecycle frozen`. > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.