[code-generator] Breaking gen_client() function in kube_codegen script

[ary1992]

What would you like to be added?

gen_client() function in kube_codegen script has following issues:

• gen_client() function only supports types.go file https://github.com/kubernetes/code-generator/blob/e4611069dfb4b0c04c7751afae1b9fef64828964/kube_codegen.sh#L539-L544 but we have multiple types file with name as type_*.go in our repo. Hence this function is not working in our case.
  For the mitigation there is already an open PR. I have asked for including our changes as well.
• We have multiple types_*.go files under a pkg for example types_managedseed.go and types_managedseedset.go. Because of this --input-dirs github.com/gardener/gardener/pkg/apis/seedmanagement/v1alpha1 for informer-gen appears twice and it generates duplicate informers

// Group=seedmanagement.gardener.cloud, Version=v1alpha1
case v1alpha1.SchemeGroupVersion.WithResource("managedseeds"):
	return &genericInformer{resource: resource.GroupResource(), informer: f.Seedmanagement().V1alpha1().ManagedSeeds().Informer()}, nil
case v1alpha1.SchemeGroupVersion.WithResource("managedseeds"):
	return &genericInformer{resource: resource.GroupResource(), informer: f.Seedmanagement().V1alpha1().ManagedSeeds().Informer()}, nil
case v1alpha1.SchemeGroupVersion.WithResource("managedseedsets"):
	return &genericInformer{resource: resource.GroupResource(), informer: f.Seedmanagement().V1alpha1().ManagedSeedSets().Informer()}, nil
case v1alpha1.SchemeGroupVersion.WithResource("managedseedsets"):
	return &genericInformer{resource: resource.GroupResource(), informer: f.Seedmanagement().V1alpha1().ManagedSeedSets().Informer()}, nil

We only need unique elements before passing it to informer-gen. I followed this approach for unique elements:

input_pkgs=($(echo "${input_pkgs1[@]}" | tr ' ' '\n' | sort -u | tr '\n' ' '))
group_versions=($(echo "${group_versions1[@]}" | tr ' ' '\n' | sort -u | tr '\n' ' '))

• We have packages under pkg/apis/<package_name> refer . Earlier we were generating client only for pkg/apis/core and pkg/apis/seedmanagement. But with the new kube-codegen I am not able to do that.
  I am using the following call to gen_client function:

kube::codegen::gen_client \
    --with-watch \
    --input-pkg-root github.com/gardener/gardener/pkg/apis/seedmanagement \
    --output-pkg-root github.com/gardener/gardener/pkg/client/seedmanagement \
    --output-base "$(dirname "${BASH_SOURCE[0]}")/../../../.." \
    --boilerplate "${PROJECT_ROOT}/hack/LICENSE_BOILERPLATE.txt"

It results in following flags to client-gen

--input-base : github.com/gardener/gardener/pkg/apis/seedmanagement
--output-base : github.com/gardener/gardener/hack/../../../..
--output-package : github.com/gardener/gardener/pkg/client/seedmanagement/clientset
inputs : --input seedmanagement/v1alpha1 --input seedmanagement/v1alpha1

As there is no directory as github.com/gardener/gardener/pkg/apis/seedmanagement/seedmanagement/v1alpha1, it doesnot generate anything. Is it still possible to call client-gen on specific folder as it was done with earlier scripts?

One mitigation I can think of is to call gen_client function on whole github.com/gardener/gardener/pkg/apis and then remove the +genclient marker from the types for which genclient is not needed. But the issue is we won't be able to pass the output-pkg-root flag for each pkg.

Why is this needed?

gen_client() function in kube_codegen script is not working as expected.

[neolit123]

/sig api-machinery

[ary1992]

/cc @thockin

[Jefftree]

/triage accepted

[thockin]

Hi @ary1992 . I knew that there were all sorts of clients out there doing all sorts of things with these scripts, so I am not surprised to find you. :)

Do you have a PR which attempts to use kube_codegen, so I can repro your issues directly?

[thockin]

As there is no directory as github.com/gardener/gardener/pkg/apis/seedmanagement/seedmanagement/v1alpha1, it doesnot generate anything

I think you have one too many .. in there

Aside from that, the real problem here is that these old codegen tools make some deeeeeeep assumptions about the ability to string-join various arguments and get the right results. It's awful. I was trying to make it simpler in this script, which I did for all of the in-tree consumers, but out of tree people apparently don't always follow the same conventions. For example, we generate clients for every directory that requests it.

For a reason I do not know, you have files like pkg/apis/authentication/types_adminkubeconfigrequest.go which say // +genclient but you don't actually want a client?

If you specify --input-pkg-root github.com/gardener/gardener/pkg/apis then it t

[thockin]

So this code expects that you generate an omnibus client. What if I add a flag, like --one-input-api which you can use to call it for each API to make distinct clients?

In truth the codegen tools need to be fixed, but that has SO MUCH ripple I just can't right now.

[thockin]

#121411

[ary1992]

Do you have a PR which attempts to use kube_codegen, so I can repro your issues directly?

As the codegen was breaking us at several step, hence didn't prepared the PR.

[ary1992]

#121411

Thanks for the PR.
I have one more concern, is it possible to call versioned and internalversion client-gen on the same pkg? With the current script, that is not possible because of the line

kubernetes/staging/src/k8s.io/code-generator/kube_codegen.sh

Lines 590 to 594 in 8453eb0

	( kube::codegen::internal::git_grep -l --null \
	-e '^// Code generated by client-gen. DO NOT EDIT.$' \
	":(glob)${out_root}/${clientset_subdir}"/'**/*.go' \
	|| true \
	) | xargs -0 rm -f

It removes all the generated files first and then calls the client-gen. Hence the later clientset gen will remove the earlier generated clients.

[thockin]

Why are you generating a client for an internal version that isn't served?

…

On Wed, Oct 25, 2023, 2:12 AM Ashish Ranjan Yadav ***@***.***> wrote: #121411 <#121411> Thanks for the PR. I have one more concern, is it possible to call versioned and internalversion client-gen on the same pkg? With the current script, that is not possible because of the line https://github.com/kubernetes/kubernetes/blob/8453eb0c24ea61794157b451e4442ea570abce45/staging/src/k8s.io/code-generator/kube_codegen.sh#L590-L594 It removes all the generated files first and then calls the client-gen. Hence the later clientset gen will remove the earlier generated clients. — Reply to this email directly, view it on GitHub <#121247 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKWAVGMR2TAJKX2DJBSL6DYBDJZFAVCNFSM6AAAAAA6BUOBPOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZYHA2DENZUHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[ary1992]

Why are you generating a client for an internal version that isn't served?

My mistake. We need to stop using clients for internal versions.