Testing with external CRDs is difficult

[JoelSpeed]

I was party to a conversation yesterday in the #kubebuilder channel on Slack which raised an interesting point.

If you are building a controller which interacts with other CRDs, it is not easy to add these (and under-documented as well) into the test environment without adding new CRDDirectoryPaths

t := &envtest.Environment{
        Config:             cfg,
        CRDDirectoryPaths:  []string{filepath.Join("..", "..", "..", "config", "crds")},
    }

Since the Environment struct can also take a list of *apiextensions.CustomResourceDefintion, I think it would be useful if Kubebuilder projects could auto generate the CRDs as runtime objects (as well as the yaml definitions) to allow them to be imported by other test suites.

Given the code is already written for generating the CRDs and dumping them to disk, I don't think it would be too much extra work to add this to the project.

I envisage this looking something like a zz_generated.crds.go file in each of the "version" (v1alpha1) folders within each API with some exported method which returns the CRD.
This would then be used as below:

t := &envtest.Environment{
        Config:             cfg,
        CRDs:  []*apiextensions.CustomResourceDefinition{shipsv1alpha1.SloopCRD()},
    }

A very similar approach can be seen here: zalando/postgres-operator#378

A bonus of this approach would be that, if we expect controllers to auto-inject CRDs into the API server if they don't exist, we have the CRD as a runtime object already.

If adding this auto-generation of CRDs seems like it should be within scope of the project, I'll see about raising a PR when I have time

[jkevlin]

Questions:

1. Would it autogenerate these files everytime it generates?
2. Would it be a flag on the kubebuilder create api --generate-crd?
3. Would it be a separate operation kubebuilder generate-crd?

[JoelSpeed]

@jkev53 I had imagined it being run at the same time as the deepcopy generation (as a go generator) and being a default option (especially if it might be used for injecting CRDs into clusters later). But if you think it should be behind some flag then I'm sure that could be done.

I think at the moment you get the CRD manifests generated by default as well?

[jkevlin]

@fanzhangio have you thought about how it should be implemented. Should we do as @JoelSpeed suggested and generate the files along side of the current generated files and instead of importing the CRD yaml directory as a default we would import the go crd struct directly?

[fanzhangio]

I think it is reasonable to add a feature of generating a runtime objects CRD for controller tests. I think we can make it in zz_generated_crd files along with generated deep copy files in the same path. It is feasible to add a small effort in crd/generator since we have crds already.

The file can be generated by calling Do() function

[fanzhangio]

I added a crdGenerator for generating zz_generated_crd.go files. It will be executed along with creating crd manifest file. Currently, it supports validation and subresource features of CRD, the versioned typed CRDs in this file are exported. We may add further functions as needed, something like getCRD().

Open to discussion.

[droot]

I have one concern about generating the CRD definitions in Go file is that now we have two ways of generating the CRDs. We had that in the previous version of Kubebuilder and we observed it used go out-sync and cause bugs.

If the external project is also a Kubebuilder project, then it will have CRD manifests in its repo and if it is vendor'd in current project, then manifests path could be added to the CRDDirectoryPaths in the tests. Right ? or I am missing something. (For this to work, Gopkg.toml should allow non-go files to be vendored)

Now for a non-kubebuilder project, if they have CRD definitions exported in their pkg/apis, then that can be passed using apiextensions.CustomResourceDefintion field in the envtest.Environment.

Do we really need to generated zz_generated_crd.go ?

[yuzisun]

I got it working with a controller which interacts with other CRDs, but it took me quite sometime to figure out, at least document or examples are missing.

[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

[ryanbrainard]

/remove-lifecycle rotten

Similar to @yuzisun's comment above, at least having better documentation or examples would be very helpful. This issue seems to have gotten into the weeds on implementation details of a potential solution, but the "Testing with external CRDs is difficult" problem is very much an issue, and one that could probably be solved at least partially by better documentation. This was the closest thing that I could find that addresses this in docs, but it doesn't actually even add the type to CRDDirectoryPaths, so perhaps improving that doc (and linking it from the main docs) would be a good first step? I can take a stab at that once I get my own code working, but I'm still slogging through it...

[ryanbrainard]

Ok, I finally figured out my issues. For anyone else running into issues with adding and testing external CRDs, this is what I did:

1. Import the version of the external CRD you want and run dep ensure.

2. Append the AddToScheme of the external CRD to your own AddToSchemesin the pkg/apis/addtoscheme_<type>.go file

AddToSchemes = append(
		AddToSchemes,
		yours.SchemeBuilder.AddToScheme,
		theirs.SchemeBuilder.AddToScheme,
	)

3. Append the CRDs YAML files to the CRDDirectoryPaths in your controller's suite test.

	t := &envtest.Environment{
		CRDDirectoryPaths: []string{
			filepath.Join("..", "..", "..", "config", "crds"),
			filepath.Join("..", "..", "..", "vendor", "github.com", "their-org", "their-project", "config", "crds"),
		},
	}

I tested this out in isolation in two projects where one (kubebuilder-p2) depends on the other ((kubebuilder-p1). Here are where Steps 2 and 3 are implemented.

If this advice is correct, would it be ok if I contribute clearer instructions to this doc?

[DirectXMan12]

. In the case of v2, it's mostly already covered in the cronjob tutorial when we talk about adding to scheYep, those look correct. It needs to be updated for KB v2, but we're always glad to have contributions. You'll need to talk a bit about tests first, though, but we don't talk much about those in the v2 book yet.

[ryanbrainard]

Thanks @DirectXMan12. Since the v2 book doesn't have a tests section yet, I opened a PR (#752) for improving the existing docs for now. That'll at least improve what we have now and that can eventually be incorporated into the v2 book.

[DirectXMan12]

awesome, thanks!

[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

[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.