Cleanup manual conversions

[thockin]

These might have been needed in the past, but no longer. The ones that are left are a bad situation.

When we do codegen we: a) remove generated files; b) set a build tag to ignore generated files. Then conversion-gen tool searches for specially-named conversion functions. When there is a cross-API dependency, it can't find the functions (because (a) and (b) above) so they generate a compile error. We should do better. For now, docs++.

/kind cleanup

NONE

[thockin]

/retest

[alexzielenski]

When there is a cross-API dependency, it can't find the functions (because (a) and (b) above) so they generate a compile error.

I haven't looked deeply into conversion-gen, so here are some assumptions I'm making (forgive me if its an oversimplification):

1. Generates a list of all GVKs (and their runtime type)
2. Pair all GVKs with all other GVKs of the same GroupKind (to find the list of necessary conversions)
3. Generate autoConvert_* and registration for all GVK pairs
4. Find all manually implemented conversion functions
5. Filter out all manually implemented conversions functions from the list of GVK pairs
6. Generate Convert_* for remaining GVK pairs

IIUC the issue is that to determine # 4 the codebase needs to be compilable. This fails due to missing symbol references after removing generated code and enabling a build tag.

I'm surprised that we need a valid full build for this. I would expect it to be possible to perform a best-effort typecheck (where type check/symbol errors are repalced with an Error AST node, or similar).
Is that not possible in gengo

[alexzielenski]

/lgtm

can confirm PR is a no-op: removes manually written top level conversions functions to replace them with autogenerated ones with identical content.

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 9b50a1beae8bae297b935201a03dcf9be5824355

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexzielenski, thockin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• pkg/apis/OWNERS [thockin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[thockin]

IIUC the issue is that to determine # 4 the codebase needs to be compilable. This fails due to missing symbol references after removing generated code and enabling a build tag.

More than that, we always exclude generated code when generating code. Fundamentally, it's (IMO) sloppy and horrible that we just search for functions with the right name and fingerprint. I think it would be better to do something like:

• Types which have manual conversions say so with a tag.
• Conversion-gen doesn't need the function to exist in order to "find it" - it just emits calls to the manual function or the named-by-convention function.

Edit: a few notes

Consider the case of generating for (e.g.) storage.VolumeAttachmentSource, which contains a reference to core.PersistentVolumeSpec. genConversion.GenerateType() calls generateConversion(t, peerType) and generateConversion(peerType, t). At that point in time we know which package we are generating into (storage) and which packages the types in question come from (core and core/v1). We do not actually know which of those two names is the "internal" name, except by pattern-matching (gross).

We can't keep a lookaside to know which types have already been generated to track which are internal, because we can't (I think) be sure that the dep will already have been handled - the sort is alphabetic, not topological.

So, in the absence of extra information, this is harder than I made it sound.

[ritazh]

/triage accepted