kubernetes / kubernetes

Production-Grade Container Scheduling and Management
https://kubernetes.io
Apache License 2.0
111.15k stars 39.68k forks source link

The Clientset returned from the new NewClientset function does not work for CRDs #126850

Open tpantelis opened 2 months ago

tpantelis commented 2 months ago

What happened?

Calling the Create method fails with, eg

failed to convert new object (/cluster-egressIP; submariner.io/v1, Kind=ClusterGlobalEgressIP) to smd typed: schema error: no type found matching: com.github.submariner-io.submariner.pkg.apis.submariner.io.v1.ClusterGlobalEgressIP

Long story short, It ends up trying to validate a resource using the global schemaYAML var defined in the generated file pkg/client/applyconfiguration/internal/internal.go. However this is not populated with the appropriate types. So NewClientset is unusable even though NewSimpleClientset is now marked as deprecated.

Note that NewClientset works fine with core K8s types because its schemaYAML is populated appropriately. However apiextensions is not.

What did you expect to happen?

I expect the Clientset methods to work for CRDs. It seems the code generator needs to be updated.

Also NewSimpleClientset should not be deprecated unless/until NewClientset works.

How can we reproduce it (as minimally and precisely as possible)?

import fakeclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake"
...
client := fakeclientset.NewClientset()
client.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, crd, metav1.CreateOptions{})

Anything else we need to know?

No response

Kubernetes version

```console $ kubectl version # paste output here ```

Cloud provider

OS version

```console # On Linux: $ cat /etc/os-release # paste output here $ uname -a # paste output here # On Windows: C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture # paste output here ```

Install tools

Container runtime (CRI) and version (if applicable)

Related plugins (CNI, CSI, ...) and versions (if applicable)

skitt commented 2 months ago

/cc @jpbetz

Divyansh200102 commented 2 months ago

What happened?

Calling the Create method fails with, eg

failed to convert new object (/cluster-egressIP; submariner.io/v1, Kind=ClusterGlobalEgressIP) to smd typed: schema error: no type found matching: com.github.submariner-io.submariner.pkg.apis.submariner.io.v1.ClusterGlobalEgressIP

Long story short, It ends up trying to validate a resource using the global schemaYAML var defined in the generated file pkg/client/applyconfiguration/internal/internal.go. However this is not populated with the appropriate types. So NewClientset is unusable even though NewSimpleClientset is now marked as deprecated.

Note that NewClientset works fine with core K8s types because its schemaYAML is populated appropriately. However apiextensions is not.

What did you expect to happen?

I expect the Clientset methods to work for CRDs. It seems the code generator needs to be updated.

Also NewSimpleClientset should not be deprecated unless/until NewClientset works.

How can we reproduce it (as minimally and precisely as possible)?

import fakeclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake"
...
client := fakeclientset.NewClientset()
client.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, crd, metav1.CreateOptions{})

Anything else we need to know?

No response

Kubernetes version

```console $ kubectl version # paste output here ```

Cloud provider

OS version

```console # On Linux: $ cat /etc/os-release # paste output here $ uname -a # paste output here # On Windows: C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture # paste output here ```

Install tools

Container runtime (CRI) and version (if applicable)

Related plugins (CNI, CSI, ...) and versions (if applicable)

Hi, I am new to contributing to this repository can you please tell me what all things I should be familiar with to start contributing in this repository.

neolit123 commented 2 months ago

/sig api-machinery

fedebongio commented 2 months ago

/assign @aaron-prindle

k8s-ci-robot commented 2 months ago

@fedebongio: GitHub didn't allow me to assign the following users: aaron-prindle.

Note that only kubernetes members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. For more information please see the contributor guide

In response to [this](https://github.com/kubernetes/kubernetes/issues/126850#issuecomment-2313429024): >/assign @aaron-prindle 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-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
aaron-prindle commented 2 months ago

/assign @aaron-prindle

aaron-prindle commented 2 months ago

I was able to repro this using the repro example above in the following main.go file: https://gist.github.com/aaron-prindle/8dd89c5fddb9cbb189d71312c1e135bd

Using NewSimpleClientset creating the CRD succeeds

aprindle@aprindle-ssd ~/repro-126850 go run main.go
CRD created successfully

Using NewClientset creating the CRD fails

aprindle@aprindle-ssd ~/repro-126850 go run main.go
Failed to create CRD: failed to convert new object (/testcrds.example.com; apiextensions.k8s.io/v1, Kind=CustomResourceDefinition) to smd typed: schema error: no type found matching: io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.CustomResourceDefinition

Thank you for reporting this issue and repro case @tpantelis. As you noted above the problem seems to be rooted in the code generation process not correctly incorporating CRD schemas into the schemaYAML used by NewClientset. NewSimpleClientset uses testing.NewObjectTracker which handles basic CRUD operations (Create, Update, Delete) without applying logic for field management, validation, defaulting, etc. This allows it to support CRDs as it does not have any associated schema or validation. NewClientset introduces a NewTypeConverter which depends on the global schema (schemaYAML noted above) being fully populated with all necessary types, including CRDs. If the CRD types are not registered in the scheme, the type conversion fails because the NewTypeConverter cannot find a matching type in the schema, leading to errors like the one reported. Currently I am not sure if it is possible to add all possible CRD types to the schemaYAML as CRD schemas are registered dynamically IIUC.

Below are the options to resolve this as I understand them:

  1. Assess whether NewClientset should be enhanced to include CRD schemas or if an alternative solution needs to be proposed. Perhaps we can add a way to register/add schemas to NewClientset or enable all CRD schemas automatically (I am not currently familiar w/ a way to do this).
  2. Consider temporarily un-deprecating NewSimpleClientset until this issue is resolved to ensure no disruption for users working with CRDs.

@jpbetz I believe you recently did some work related to fake clients, do you have any opinions on the preferred option above (or additional alternatives). Assuming NewClientset could be made to support CRDs, do you have any ideas on what might need to be updated there (files to touch, etc.)? Thanks!

fedebongio commented 2 months ago

/triage accepted

jpbetz commented 2 months ago

This is my mistake. We need each generated fake client to use the schemaYAML for that client, not the one for the core API types.

/assign

mbobrovskyi commented 1 week ago

Is any progress here?