Kubebuilder: Include controller startup in generated suite_test.go

[nathanperkins]

What do you want to happen?

In the Test Environment Setup section of the Kubebuilder book, it shows that to test controller behavior, you have to manually add the code to start the controller in the test environment. I ran into some confusion because I missed this part of the tutorial and couldn't tell why my tests were failing with no logs.

One thing that this autogenerated file is missing, however, is a way to actually start your controller. The code above will set up a client for interacting with your custom Kind, but will not be able to test your controller behavior. If you want to test your custom controller logic, you’ll need to add some familiar-looking manager logic to your BeforeSuite() function, so you can register your custom controller to run on this test cluster.

I imagine that most users writing tests are going to be testing their controller's behavior. What do you think if the generated code included this missing section so that the suite is ready to test controller behavior out of the box?

/kind feature

[camilamacedo86]

Hi @nathanperkins,

Thank you for your contact. If you look at the sample in https://github.com/kubernetes-sigs/kubebuilder/blob/master/testdata/project-v3 you are able to check the https://github.com/kubernetes-sigs/kubebuilder/blob/master/testdata/project-v3/controllers/suite_test.go which its latest scaffolded layout.

Could you please let us know what your opinion for example should be scaffold and is not? Could you please describe it with a snippet code and an example? Would be scaffold a test for each controller?

[nathanperkins]

My suggestion is that these lines near the bottom of BeforeSuite in suite_test.go are instead generated with code that starts each of the controllers. The current code doesn't start any of the controllers, which means you can't really test the behavior of the controllers themselves without adding this boilerplate yourself.

Current code:

    //+kubebuilder:scaffold:scheme

    k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
    Expect(err).NotTo(HaveOccurred())
    Expect(k8sClient).NotTo(BeNil())

}, 60)

I think in the testdata/project-v3 it would look something like this.

    //+kubebuilder:scaffold:scheme

    k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{
        Scheme: scheme.Scheme,
    })
    Expect(err).ToNot(HaveOccurred())

    err = (&AdmiralJobReconciler{
        Client: k8sManager.GetClient(),
        Log:    ctrl.Log.WithName("controllers").WithName("Admiral"),
    }).SetupWithManager(k8sManager)
    Expect(err).ToNot(HaveOccurred())

    err = (&CaptainReconciler{
        Client: k8sManager.GetClient(),
        Log:    ctrl.Log.WithName("controllers").WithName("Captain"),
    }).SetupWithManager(k8sManager)
    Expect(err).ToNot(HaveOccurred())

    err = (&FirstMateReconciler{
        Client: k8sManager.GetClient(),
        Log:    ctrl.Log.WithName("controllers").WithName("FirstMate"),
    }).SetupWithManager(k8sManager)
    Expect(err).ToNot(HaveOccurred())

    err = (&LakerReconciler{
        Client: k8sManager.GetClient(),
        Log:    ctrl.Log.WithName("controllers").WithName("Laker"),
    }).SetupWithManager(k8sManager)
    Expect(err).ToNot(HaveOccurred())

    go func() {
        err = k8sManager.Start(ctrl.SetupSignalHandler())
        Expect(err).ToNot(HaveOccurred())
    }()

    k8sClient = k8sManager.GetClient()
    Expect(k8sClient).ToNot(BeNil())

    close(done)
}, 60)

[Adirio]

The last line would not be needed, and we would have to add a marker similar to the one for scheme but this would be totally doable. Do you want to work on it?

(@camilamacedo86 he added the requested info, I think we can pass this to accepted and add the help wanted and first issue as both labels fit very well this issue)

[camilamacedo86]

Really thank you for your collab @nathanperkins and @Adirio .
WDYT?

• We add the marker to scaffold the following code for each controller :

err = (&KindReconciler{
        Client: k8sManager.GetClient(),
        Log:    ctrl.Log.WithName("controllers").WithName("Kind"),
    }).SetupWithManager(k8sManager)
    Expect(err).ToNot(HaveOccurred())

• We start to scaffold a <kind>_controler_test.go for each case scenario as well with:

var _ = Describe("<Kind> controller", func() {

     // your logic here

}

• Then, it would be done only for v3. See here

[nathanperkins]

Thanks for the quick response on this! I'll take a look later to see if it's something that I could find space to handle.

I agree with @Adirio that it seems like the first issue tag would be appropriate. It's a good opportunity for a university student to break in! :)

[Adirio]

@nathanperkins (or anyone else if he doesn't find the time) I will give you a few pointers:

That file's template is located in pkg/plugins/golang/v3/scaffolds/internal/templates/controllers/controller_suitetest.go. It follows two interfaces Template and Inserter.

https://github.com/kubernetes-sigs/kubebuilder/blob/26d7b42314dc8df4369df3594329c3d2401793d2/pkg/model/file/interfaces.go#L40-L47

The Template interface allows to scaffold new files. For this you would only need to update the controllerSuiteTestTemplate const at the end of the file using %s instead of the marker and then insert it inside SetTemplateDefaults the same way the other 2 markers are being inserted. Here you would have to add the manager part, the coroutine part and the client part. The channel is no longer a thing despite still not being updated in the docs.

https://github.com/kubernetes-sigs/kubebuilder/blob/26d7b42314dc8df4369df3594329c3d2401793d2/pkg/model/file/interfaces.go#L49-L56

The Inserter interface allows to add the repetitive part for each controller. You will have to add the new marker to GetMarkers and the code fragment to GetCodeFragments.

About the marker name itself, a very similar setup is found in the main template, so we may want to reuse that marker name: https://github.com/kubernetes-sigs/kubebuilder/blob/26d7b42314dc8df4369df3594329c3d2401793d2/pkg/plugins/golang/v3/scaffolds/internal/templates/main.go#L76-L80

Hope this helps to get introduced in kubebuilder plugin template modification.

[nathanperkins]

That's excellent, thank you so much!

I think with your direction I will be able to make this change. Feel free to assign it to me :)

[Adirio]

/assign @nathanperkins

[Adirio]

Just as a comment, information in #2018 should be relevant. It should use a full client for testing and not the cached client that manager returns.