conversion-gen is broken in 1.20 and 1.21 release

[tamalsaha]

What happened:

conversion-gen tool is broken in 1.20 and 1.21 release. This is blocking us and projects like cert-manager from migrating to the latest k8s tool chain.

As for context, everything was ok in k8s 1.18. In k8s 1.18.9, the auto generated code will look like the following:

In k8s 1.20 and 1.21, the auto generated code look like the following:

This is obviously incorrect, since the k8s.io/apiextensions-apiserver package does have an auto generated conversion for these types:

Now, digging into the code of conversion-gen I see that the only way conversion-gen tool can know about the existence of this method will be that it can be found in the manual_conversions map.

ref: https://github.com/kubernetes/code-generator/blob/v0.21.0/cmd/conversion-gen/generators/conversion.go#L1053-L1078

First I thought that I needed to pass the --extra-dirs k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1 flag to conversion-gen tool so that those files are parsed for conversion methods. But that did not fix it.

Now, further digging I found that the reason the auto-generated https://github.com/kubernetes/apiextensions-apiserver/blob/master/pkg/apis/apiextensions/v1/zz_generated.conversion.go file is not parsed by the Go Builder in conversion-gen tool because the auto generated file has the build tag // +build !ignore_autogenerated and the Go source parser is configured with ignore_autogenerated build tag.

ref: https://github.com/kubernetes/gengo/blob/master/args/args.go#L132-L138

As a result, the auto generated (files with zz_ prefix) are never parsed and we get that compileErrorOnMissingConversion() error.

Proposed Solution:

In my view, the main problem here is that the same variable g.GeneratedBuildTag variable is used to add the build tag to the generated zz_conversion.go file and to parse existing auto generated conversion files. So, I was able to get this fixed using the following approach:

(1) Use separate flags for adding build tag to the generated conversion file and for importing/parsing existing generated conversion files. Here is my pr: kubernetes/gengo#200

I initialize them to the same default value so that the behavior of the code generators other than conversion-gen remains unchanged. But I also added a new --import-tag flag for the new ImportBuildTag .

(2) Now, in the conversion-gen code, I set the ImportBuildTag to empty string so that by default it parses all the generated conversion functions from the --extra-dirs and --peer-dirs.

pr: #101568

If users need old behavior for some reason, they can set the --import-tag=ignore_autogenerated to avoid parsing the existing generated conversion files.

Testing Fix

If you want to test the fix, you need to use my forked conversion-gen binary.

replace k8s.io/code-generator => github.com/kmodules/code-generator v0.21.1-rc.0.0.20210428003838-7eafae069eb0

replace k8s.io/gengo => github.com/kmodules/gengo v0.0.0-20210428002657-a8850da697c2

What you expected to happen:

How to reproduce it (as minimally and precisely as possible):

Anything else we need to know?:

Environment:

• Kubernetes version (use kubectl version):
• Cloud provider or hardware configuration:
• OS (e.g: cat /etc/os-release):
• Kernel (e.g. uname -a):
• Install tools:
• Network plugin and version (if this is a network-related bug):
• Others:

[tamalsaha]

/sig api-machinery

[dims]

cc @palnabarun @nikhita

[liggitt]

cc @wojtek-t

likely related to #90018 in 1.19, which intentionally removed the fallback to reflect-based conversion, because it is not known to be correct and masks conversion bugs

based on what was generated previously, is it possible you weren't actually picking up the generated conversions before and were using the reflective conversion path?

[tamalsaha]

is it possible you weren't actually picking up the generated conversions before and were using the reflective conversion path?

Yes. It was using reflective conversions. As you can see here with k8s 1.18.9

https://github.com/kmodules/resource-metadata/blob/78a10662e33585c6b06d7e0c07c8c70e111ab4d3/apis/meta/v1alpha1/zz_generated.conversion.go#L1140-L1149

[fedebongio]

/cc @sttts since we assigned the PR to Stefan. Thanks!
/triage accepted

[k8s-triage-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[tamalsaha]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[sozercan]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[sozercan]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

[k8s-ci-robot]

@k8s-triage-robot: Closing this issue.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[sozercan]

/reopen

[k8s-ci-robot]

@sozercan: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.