Open akutz opened 2 months ago
Hi @akutz! I think your solution is the only one that comes to mind for me.
Thanks @ncdc. That is unfortunate, as it really is a hacky hack. Still, it is better than not having a solution at all I suppose. I will leave this issue open for the time being in case anyone else wants to add their thoughts. If no one has spoken up by next week, I will close this. I may also file a PR against kubebuilder and update docs to reflect the above.
It looks like --input-dirs
was dropped in the latest release?!
Apparently https://github.com/kubernetes/code-generator/commit/d697340b658daf8072223c1dc0bd9c628c0ecc69 removed --input-dirs
when k8s.io/gengo
args were removed. cc @thockin -- you authored the referenced commit -- it broke the use of --input-dirs
for the use case this issue describes. Thoughts?
gengo and all the tools which use it underwent a massive overhaul to make Go workspaces work, which necessarily included breaking CLI changes. If you are using a newer version of the tools, then you can drop the --input-dirs
part and just pass the dirs directly as arguments, but you may need to make other changes to get the flags right. Looking at conversion-gen, it may have been one of the less impacted tools - others got hit harder.
As for this bug, I don't mean to make excuses, but subdirs are just not something that was used/tested in k/k, so it's not a surprise it breaks. The way we detect conversion functions is, IMO, awful (for reasons like this, among others) and I don't think there's a "clean" fix because the naming convention is baked pretty deep. I think the "real" fix would be to have explicit tags indictating "this function converts from X to Y". It sounds like a fun project, but isn't something I personally can tackle any time soon.
Thanks @thockin. I will try your suggestion about passing in the arguments directly.
Thanks @thockin, a variation on your suggestion worked like a charm, thanks again!
/Users/akutz/Projects/vmop/vmop/hack/tools/bin/darwin_arm64/conversion-gen \
--output-file=zz_generated.conversion.go \
--go-header-file=/Users/akutz/Projects/vmop/vmop/hack/boilerplate/boilerplate.generatego.txt \
--extra-peer-dirs='./v1alpha2/sysprep/conversion/v1alpha2,./v1alpha2/sysprep/conversion/v1alpha3' \
./v1alpha1 ./v1alpha2
@thockin I am reluctant to close this issue, however, as your response indicates my hack is just that, a hack. A proper solution would be to do what you said re: tags. Perhaps I will close this issue and open a new one for an enhancement request that can track your proposed solution. The new issue can point to this one for reference.
Let's just retitle this one. I don't have super-access to this repo but something like "conversion should allow explicit declaration of conversion funcs, beyond just naming convention"
This issue was filed based on this discussion in the #kubebuilder channel from Kubernetes Slack.
:wave: I encountered an oddity with controller-gen where I have two types,
Fu
, from two packages,./v1alpha2/subdir
and./v1alpha3/subdir
, and the signature controller-gen expects for both of the functions is:Convert_subdir_Fu_To_subdir_Fu
. Obviously two functions cannot share the same name in the same package, so this is an issue.I have tried looking through the GitHub issues, and the closest I can find is https://github.com/kubernetes/code-generator/issues/94 (hi @ncdc!), but that only applies to other types that are of type
runtime.Object
and are conversion hubs. In my case,Fu
is a data structure shared by different packages in each schema version. For example,./api/v1alpha2
and./api/v1alpha2/hello
might both import./api/v1alpha2/subdir
.I have experimented with several different approaches, including defining the Convert functions for the parent type that includes
Fu
, but I would still receive compileOnError warnings in the generated output about the expected function. The only solution I found is the following:In
./v1alpha2/conversion/v1alpha2/fu_conversion.go
I have oneConvert_subdir_Fu_To_subdir_Fu
, and I have the same signature in the other file, with the arg types reversed. With the above, I add the two directories to the--input-dirs
param, and it works.Is the above solution really the only option?
cc @bryanv