Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing for Plugins #5724

Closed
everettraven opened this issue May 3, 2022 · 13 comments
Closed

Testing for Plugins #5724

everettraven opened this issue May 3, 2022 · 13 comments
Assignees
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs discussion priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Milestone

Comments

@everettraven
Copy link
Contributor

Feature Request

Describe the problem you need a feature to resolve.

There is currently not an easy way to e2e test a plugin without writing all the tests within operator-sdk. This makes it difficult to e2e test a plugin that is in an external repository.

Describe the solution you'd like.

First thought was to create a Go library that could be used to test a plugin, however this may be something that would need to be implemented upstream in Kubebuilder. This approach also will likely not be viable in the future with the introduction of Phase 2 Plugins as plugins will be able to be written in any language.

This issue can be used as a place of discussion as to how we may want to approach this, or with the introduction of Phase 2 Plugins if this is something worth pursuing or leaving it up to the developers of an external plugin to implement their own form of e2e testing.

@everettraven
Copy link
Contributor Author

Testing strategy

So with Phase 2 Plugins on the way, it seems like there is the potential to have 2 kinds of plugins:

  1. Plugins written in Go that are a library that is imported by Operator SDK and are built in with Operator SDK
  2. Plugins written in any language that will use the new Phase 2 Plugins methods of communication.

Traditional plugins

With the traditional plugins (ones that have to be pulled in and built by operator-sdk like Helm/Ansible/Java), the e2e testing strategy is much more difficult because it is a library that is being imported by operator-sdk and then run.

I believe a potential solution for this could be to create a plugin testing package that these traditional plugins can use to test with an in memory filesystem. This would allow for these traditional plugins to be able to be e2e tested programmatically and the in-memory file system can be checked to make sure everything was scaffolded as expected.

I think this method would require some upstream changes in Kubebuilder to allow the modification of the filesystem used as part of the CLI.

Phase 2 Plugins

With the Phase 2 Plugins, it will be much easier to e2e test these types of plugins because they will be binaries and with the architecture of the Phase 2 Plugins they will simply read JSON in via stdin and output JSON via stdout.

Because these will be much easier to test, is this something that we would want to create some form of tooling to make the testing a bit easier or leave this up to the developers of Phase 2 Plugins to handle themselves?

One potential way we could help is by creating a few libraries for common programming languages that essentially make it easier to create inputs and parse outputs for the Phase 2 Plugins by mapping the Phase 2 Plugin input/output schemas to objects.


@asmacdo @jmrodri what do you think?

@camilamacedo86
Copy link
Contributor

camilamacedo86 commented May 6, 2022

Hi @everettraven,

It exists already. You can check the lib: https://github.com/kubernetes-sigs/kubebuilder/tree/master/test/e2e/utils

Then, you can check some examples:

Note that any external plugins must have a way to be executed by themselves, which means using the Kubebuilder lib and doing the CLI implementation. Therefore, the external plugin should have its own binary and can indeed have a testdata dir with samples such as we build in both projects.

However, (IMHO) would be nice to document it and add examples here: https://book.kubebuilder.io/plugins/plugins.html. We could create there a new section "Writing tests for your own plugins" then we also could have 2 subsections: generating testdata samples and writing e2e tests. WDYT?

But I think that would be also a task for Kubebuilder not SDK.

@everettraven
Copy link
Contributor Author

@camilamacedo86 interesting....

I raised this issue because of a conversation had in the OSDK community meeting and a separate offline conversation.

@jmrodri was a part of that offline conversation and brought up that testing scaffolds was a problem for the java-operator-plugins repo and the helm-operator-plugins repo. Unfortunately there was a lot of other information in that conversation so I have some limited notes regarding the actual testing portion of the conversation.

@jmrodri Would you mind elaborating a bit more on the testing issue?

@everettraven
Copy link
Contributor Author

After digging around a bit I think I am understanding a bit more what the actual point of this issue should be.

It seems like the issue is that it is difficult to write lightweight tests to see exactly what files and their contents are being scaffolded when the plugin is run via something like kubebuilder init or operator-sdk init rather than actually e2e testing an operator built with the plugin.

@jmrodri does this sound correct?

@camilamacedo86
Copy link
Contributor

camilamacedo86 commented May 8, 2022

Hi @everettraven,

It seems like the issue is that it is difficult to write lightweight tests to see exactly what files and their contents are being scaffolded when the plugin is run via something like kubebuilder init or operator-sdk init rather than actually e2e testing and operator built with the plugin.

So, if you are looking for creating samples under the test data directory. Then, see that we use the KB test utils and other common utils provided by KB to generate the samples on SDK: https://github.com/operator-framework/operator-sdk/tree/master/hack/generate/samples/internal.

Note that we call the tool binary and afterwords we use the utils provided to insert/replace content on the scaffold.
In Kubebuilder, we do not do things on top of the samples generated. Because of this, the implementation is simpler and we use a shell script to do that, see: https://github.com/kubernetes-sigs/kubebuilder/blob/master/test/testdata/generate.sh. However, we could improve the Kubebuilder samples as well and began to do nicer samples. We have a task to use the same idea over generate the samples with things on top as we do in SDL for we begin to generate the samples for the docs and keep them updated automatically, see: kubernetes-sigs/kubebuilder#2510

In both projects, the samples are updated with the code changes made and checked via the CI to ensure that they will always reflect the latest state/version of what is implemented in the master. We update then when we call the target make generate

Regarding the goal of this issue, wdyt about add here its acceptance criteria here? So that we can easier understand what is expected to get done.

Ps.: I raised an issue for Kubebuilder (kubernetes-sigs/kubebuilder#2677 ) for we doc how to write tests for the plugins. It was a good point identified for you (IHMO).

@everettraven
Copy link
Contributor Author

Hi @camilamacedo86 ,

So, if you are looking for creating samples under the test data directory. Then, see that we use the KB test utils and other common utils provided by KB to generate the samples on SDK: https://github.com/operator-framework/operator-sdk/tree/master/hack/generate/samples/internal.

Note that we call the tool binary and afterwords we use the utils provided to insert/replace content on the scaffold.

In Kubebuilder, we do not do things on top of the samples generated. Because of this, the implementation is simpler and we use a shell script to do that, see: https://github.com/kubernetes-sigs/kubebuilder/blob/master/test/testdata/generate.sh. However, we could improve the Kubebuilder samples as well and began to do nicer samples. We have a task to use the same idea over generate the samples with things on top as we do in SDL for we begin to generate the samples for the docs and keep them updated automatically, see: kubernetes-sigs/kubebuilder#2510

In both projects, the samples are updated with the code changes made and checked via the CI to ensure that they will always reflect the latest state/version of what is implemented in the master. We update then when we call the target make generate

Regarding the goal of this issue, wdyt about add here its acceptance criteria here? So that we can easier understand what is expected to get done.

Ps.: I raised an issue for Kubebuilder (kubernetes-sigs/kubebuilder#2677 ) for we doc how to write tests for the plugins. It was a good point identified for you (IHMO).

Thanks for all this information!

At the moment, I'm not sure what the acceptance criteria would be for this. I think it may be best to address this again during the SDK community meeting on May 9th and get more thoughts on this.

@jmrodri had mentioned something about e2e testing for plugins to me and I think more thoughts on this from him would help with understanding what the problem is. I should have spoke with him more regarding it before raising this issue to understand the problem a little bit better, but hopefully during the next community meeting we can get a clear direction for this issue.

@rashmigottipati rashmigottipati added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label May 9, 2022
@everettraven
Copy link
Contributor Author

More information was provided during the community meeting. The goal of this issue is to work on getting the e2e tests for the plugins that are in external repositories to be able to run the e2e tests themselves instead of the e2e tests for them residing within the operator-sdk repository.

@camilamacedo86 provided some good resources to look into for determining the best way to do this.

This issue will continue to be used for discussion of different points and ideas until we come to a solution that defines the path that is going to be taken to get this accomplished.

@everettraven
Copy link
Contributor Author

everettraven commented May 13, 2022

So I spent some time looking around at the current ways that testdata is generated and how e2e tests are written. The Kubebuilder library for writing the e2e tests and generating test data are nice, but I feel there is some room for some improvements that I think would make it easier to implement testdata generation and e2e tests in the future.

I have created a PoC to test and validate some of these ideas. It can be found at:
https://github.com/everettraven/plugin-testing-poc


Pain points

Test Data Generation

Currently to create test data new struct is created that implements a few methods that are used to generate a sample project and then make modifications to the files. In the Operator SDK test data generation this process of creating a struct for generating a sample is repeated for each plugin supported and tested by the Operator SDK's e2e tests.

This can be seen in the following files:

They all share a very similar structure and way of creating a sample project for testing purposes, albeit having slightly different internal implementations.

To address this I propose that we create some kind of interface that allows for an easier way to generate sample projects without having to write a new struct and generation method for every sample project.

An interface for these sample projects could be something like:

type Sample interface {
	CommandContext() command.CommandContext
	Name() string
	GenerateInit() error
	GenerateApi() error
	GenerateWebhook() error
}

This Sample interface would allow for the creation of a more generic sample implementation that could apply to many scenarios but also allow for customization by creating an implementation of this interface that does more specialized things.
A PoC for a generic Sample implementation that is fairly versatile can be found here:
https://github.com/everettraven/plugin-testing-poc/blob/04f59e269bd089f1449f5643d537d099fb094ab7/pkg/samples/samples.go#L21-L217

Then there can be a generator tool that takes a list of these Samples in and generates each of them by running the various generate functions on the sample. Something like:

type Generator interface {
	GenerateSamples(samples ...samples.Sample) error
}

A PoC for a generic Generator that allows for some flexibility in the running of the generation logic can be found here:
https://github.com/everettraven/plugin-testing-poc/blob/04f59e269bd089f1449f5643d537d099fb094ab7/pkg/generator/generator.go#L13-L79

A couple examples showing how this could simplify the process of test data generation:

A single sample generated:

	simpleSample := samples.NewGenericSample(
		samples.WithBinary("/usr/local/bin/operator-sdk"),
		samples.WithDomain("sample.com"),
		samples.WithGvk(schema.GroupVersionKind{
			Group:   "simple",
			Version: "v1alpha1",
			Kind:    "Sample",
		}),
		samples.WithName("simple-sample"),
		samples.WithExtraApiOptions("--resource", "--controller"),
	)

	generator := generator.NewGenericGenerator(
		generator.WithNoWebhook(),
	)

	err := generator.GenerateSamples(simpleSample)

Full runable example go file for above code can be found at: https://github.com/everettraven/plugin-testing-poc/blob/main/examples/simple-generate/main.go

Multiple samples generated:

simpleGoSample := samples.NewGenericSample(
		samples.WithBinary("/usr/local/bin/operator-sdk"),
		samples.WithDomain("simple.go.com"),
		samples.WithGvk(schema.GroupVersionKind{
			Group:   "simplego",
			Version: "v1alpha1",
			Kind:    "GoSample",
		}),
		samples.WithName("go-simple-sample"),
		samples.WithExtraApiOptions("--resource", "--controller"),
	)

	simpleHelmSample := samples.NewGenericSample(
		samples.WithBinary("/usr/local/bin/operator-sdk"),
		samples.WithDomain("simple.helm.com"),
		samples.WithGvk(schema.GroupVersionKind{
			Group:   "simplehelm",
			Version: "v1alpha1",
			Kind:    "HelmSample",
		}),
		samples.WithName("helm-simple-sample"),
		samples.WithPlugins("helm"),
	)

	simpleAnsibleSample := samples.NewGenericSample(
		samples.WithBinary("/usr/local/bin/operator-sdk"),
		samples.WithDomain("simple.ansible.com"),
		samples.WithGvk(schema.GroupVersionKind{
			Group:   "simpleansible",
			Version: "v1alpha1",
			Kind:    "AnsibleSample",
		}),
		samples.WithName("ansible-simple-sample"),
		samples.WithPlugins("ansible"),
	)

	generator := generator.NewGenericGenerator(
		generator.WithNoWebhook(),
	)

	err := generator.GenerateSamples(simpleGoSample, simpleHelmSample, simpleAnsibleSample)

Full runable example go file for above code can be found at: https://github.com/everettraven/plugin-testing-poc/blob/main/examples/multiple-samples/main.go

E2E Testing

There seems to be a lot of reliance of the Kubebuilder E2E TestContext struct for performing e2e testing and I noticed that in order to extend it in the Operator SDK for different scenarios that it was required to create a new struct that included the composition of the TestContext and then creating extensions for it.

I feel like this limits the ability to extend e2e tests to relying on using and creating a new separate implementation of the TestContext and couples e2e logic to relying on the creation of a TestContext. I also felt like it would be much easier for extension of e2e testing capabilities by creating a couple interfaces for the most common things that are used within the TestContext

One of the common fields that is used from the TestContext is the ProjectName. This value can be retrieved from using the Sample interface mentioned previously and using the Sample.Name() function to get the name of the sample project created.

Another common field that is used is the TestContext.Dir field. This is actually retrieved from the nested struct CmdContext which I think could be fairly easily converted into an interface like:

type CommandContext interface {
	Env() []string
	Dir() string
	Stdin() io.Reader
	Run(cmd *exec.Cmd, path ...string) ([]byte, error)
}

An example general implementation can be found at: https://github.com/everettraven/plugin-testing-poc/blob/04f59e269bd089f1449f5643d537d099fb094ab7/pkg/command/command.go#L19-L89

The other most common field I noticed was being used was TestContext.Kubectl. This could also be converted to an interface like so:

type Kubectl interface {
	CommandContext() command.CommandContext
	Namespace() string
	ServiceAccount() string

	// Actual functions
	Command(options ...string) (string, error)
	CommandInNamespace(options ...string) (string, error)
	Apply(inNamespace bool, options ...string) (string, error)
	Get(inNamespace bool, options ...string) (string, error)
	Delete(inNamespace bool, options ...string) (string, error)
	Logs(inNamespace bool, options ...string) (string, error)
	Wait(inNamespace bool, options ...string) (string, error)
	Version() (KubernetesVersion, error)
}

This would allow for multiple different implementations of this to exist, not limiting e2e tests to using kubectl. A kubectl implementation:https://github.com/everettraven/plugin-testing-poc/blob/dc67cdc8dbd874bee59c402f345b42f11d7a7a10/pkg/kubernetes/kubectl.go#L28-L147

The TestContext struct that has numerous helper functions and the helper functions could become regular functions in a helper library. These helper functions could accept as parameters the interfaces that they use to perform their various logic, allowing them to be used with varying configurations but still performing the same actions they did when they were a function on the TestContext struct.

An example of this suggestion can be found at: https://github.com/everettraven/plugin-testing-poc/blob/main/pkg/e2e/helpers.go

An example e2e test with these suggested changes was created and mimics the current Operator SDK e2e test for a Go operator. It can be found here: https://github.com/everettraven/plugin-testing-poc/blob/main/examples/e2e/go/e2e_test.go


I feel like these changes would make it significantly easier to write e2e tests for plugins that are external from the Operator SDK or Kubebuilder repo while making the e2e tests easier to extend than they are currently. I also believe it would allow Phase 2 Plugins that are written to have e2e tests written using this library (assuming the e2e tests are written with Go).

I think if it was deemed necessary multiple versions of this library could be created to help with e2e testing for Phase 2 Plugins written in a language other than Go.


@asmacdo @jmrodri @camilamacedo86 I would love to get your thoughts on this!

@everettraven
Copy link
Contributor Author

Created a draft PR to show a PoC of this testing library/package update here: #5813

@jberkhahn jberkhahn added this to the Backlog milestone Jun 6, 2022
@openshift-bot
Copy link

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-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 5, 2022
@openshift-bot
Copy link

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-ci openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 5, 2022
@openshift-bot
Copy link

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 openshift-ci bot closed this as completed Nov 5, 2022
@openshift-ci
Copy link

openshift-ci bot commented Nov 5, 2022

@openshift-bot: Closing this issue.

In response to this:

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. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs discussion priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Development

No branches or pull requests

6 participants