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

The fake clientset does not work as expected #3517

Closed
1 task done
robertmarsal opened this issue Feb 8, 2023 · 2 comments · Fixed by #3621
Closed
1 task done

The fake clientset does not work as expected #3517

robertmarsal opened this issue Feb 8, 2023 · 2 comments · Fixed by #3621
Labels
bug Something isn't working

Comments

@robertmarsal
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

Trying to use the fake client set to write some unit tests:

kongClientset := kongtestclient.NewSimpleClientset(&kongv1.KongPlugin{
	ObjectMeta: metav1.ObjectMeta{
		Name:      "test-plugin",
		Namespace: "test-ns",
	},
})

Where kongv1 is the following import: kongv1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1"

Calling

plugin, err := kongClientset.ConfigurationV1().KongPlugins("test-ns").Get(ctx, "test-plugin", metav1.GetOptions{})

returns an error that the plugin can not be found. This is because the group of the fake generated resources is wrong: https://github.com/Kong/kubernetes-ingress-controller/blob/main/pkg/clientset/typed/configuration/v1/fake/fake_kongplugin.go#L39. It should be configuration.konghq.com instead of configuration

It looks like client-gen uses the folder structure to determine the group and at the moment it's set to configuration: https://github.com/Kong/kubernetes-ingress-controller/tree/main/pkg/apis/configuration

Renaming the folder to configuration.konghq.com and reruning client-gen generates the correct groups. I am not sure if having the wrong name for the folder has other impacts I've only noticed this when trying to use it for testing.

Renaming that folder seems to break the public API though so I was not sure if it was an acceptable solution. I haven't seen any ways around it with the client-gen options

Expected Behavior

The fake simple client set can be used for testing.

Steps To Reproduce

1. Try to use the `NewSimpleClientSet` in tests as described in the current behaviour

Kong Ingress Controller version

v2.8.1

Kubernetes version

No response

Anything else?

No response

@robertmarsal robertmarsal added the bug Something isn't working label Feb 8, 2023
@pmalek
Copy link
Member

pmalek commented Feb 17, 2023

Thanks for your report @robertmarsal!

This is actually interesting and it seems we have to be doing something not 100% correct. I've looked at other projects that generate clientsets e.g. Gateway API and Knative's serving and they both don't use the group name in their packages import path, e.g.

Yet they do get the full group name generated in the fake clients, e.g.

I've tried playing around with client-gen options and I cannot make client-gen generate the full group name in those fakes.

I've tried setting the --input-base to "" just like knative does it but still no luck.

These are the options I've used with not much of an effect:

	$(CLIENT_GEN) \
		--go-header-file ./hack/boilerplate.go.txt \
		--logtostderr \
		--clientset-name clientset \
		--input-base ""  \
		--input $(REPO_URL)/v2/pkg/apis/configuration/v1,$(REPO_URL)/v2/pkg/apis/configuration/v1beta1,$(REPO_URL)/v2/pkg/apis/configuration/v1alpha1 \
		--output-base pkg/ \
		--output-package $(REPO_URL)/v2/pkg/ \
		--trim-path-prefix pkg/$(REPO_URL)/v2/

What you're proposing would warrant a breaking changes release which we're not looking at right now (but we might at some point). So trying to figure this out just like the other projects did this would be they preferred solution.

@robertmarsal
Copy link
Contributor Author

@pmalek thank you for the feedback! It pointed me in the right direction and I noticed that the gateway-api project used these doc.go files to defined the tags for the package. The tags were defined but in the wrong location. After creating those files the generation works as expected 🎉 .

I've created the PR #3621 with the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants