Allow conversion-gen to take types that are not in the output path #45057

caesarxuchao · 2017-04-27T23:50:52Z

Part of fixing #44065.

Partially address kubernetes/enhancements#282.

The first commit is the changes to the conversion-gen (and vendor).

The second commit moves the api/v1 types to staging, and the third commit shows the resulted changes in the generated conversions.

k8s-reviewable · 2017-04-27T23:50:58Z

This change is

caesarxuchao · 2017-04-27T23:51:07Z

/release-note-none

caesarxuchao · 2017-05-15T20:01:10Z

/assign @mbohlool

k8s-ci-robot · 2017-05-15T20:01:11Z

@caesarxuchao: GitHub didn't allow me to assign the following users: mehdy.

Note that only kubernetes members can be assigned..

In response to this:

/assign @mehdy

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. I understand the commands that are listed here.

caesarxuchao · 2017-05-15T20:03:15Z

@mbohlool the first commit is the code-gen change, the second and third commits are just sample outputs.

mbohlool

generally looks good. some nits.

mbohlool · 2017-05-18T21:47:03Z

Makefile.generated_files

@@ -35,7 +35,7 @@ SHELL := /bin/bash
 # This rule collects all the generated file sets into a single rule.  Other
 # rules should depend on this to ensure generated files are rebuilt.
 .PHONY: generated_files
-generated_files: gen_deepcopy gen_defaulter gen_conversion gen_openapi


I assume this change suppose to be temporary, right? can you revert it?

Sure, sorry. The PR is extracted from many local commits.

The two makefile changes are from commit 2 and 3, they will be removed before merge :)

mbohlool · 2017-05-18T21:48:02Z

Makefile.generated_files

@@ -222,7 +222,7 @@ RUN_GEN_DEEPCOPY =                                                          \
                --v $(KUBE_VERBOSE)                                         \
                --logtostderr                                               \
                -i $$(cat $(META_DIR)/$(DEEPCOPY_GEN).todo | paste -sd, -)  \
-                --bounding-dirs $(PRJ_SRC_PATH)                             \
+                --bounding-dirs $(PRJ_SRC_PATH),k8s.io/api                             \


why manually adding this? and why it is not a full path?

Will be removed before merge. Only the first commit is relevant.

mbohlool · 2017-05-18T21:51:53Z

cmd/libs/go2idl/conversion-gen/generators/conversion.go

+				v = kv[1]
+			}
+			switch k {
+			case "external_types":


caesarxuchao · 2017-05-18T23:51:05Z

cmd/libs/go2idl/conversion-gen/generators/conversion.go

 const tagName = "k8s:conversion-gen"
+const externalTypesTagName = "k8s:conversion-gen-external-types"


@mehdy sorry i made a change after you last time reviewed it. I added the externalTypesTagName for user to specify where the external types are, instead of squeeze it with the existing tagName. This is more consistent with the way i updated the default-gen (kubernetes/gengo#54).

caesarxuchao · 2017-05-18T23:51:29Z

pkg/api/v1/doc.go

@@ -16,6 +16,7 @@ limitations under the License.

 // +k8s:deepcopy-gen=package,register
 // +k8s:conversion-gen=k8s.io/kubernetes/pkg/api
+// +k8s:conversion-gen-external-types=../../../vendor/k8s.io/api/core/v1


@mbohlool the tag looks like this

caesarxuchao · 2017-05-18T23:52:08Z

@mbohlool could you take another look at the first commit? I made a small change to it.

caesarxuchao · 2017-06-01T00:04:01Z

Updated to cope with the latest gengo changes in https://github.com/kubernetes/gengo/pull/54/files

mbohlool · 2017-06-01T01:08:24Z

"fix conversion-gen" commit looks good to me.

caesarxuchao · 2017-06-01T01:11:11Z

Thanks, @mbohlool. I'll rebase the PR once #46742 is merged. I'll apply the label so that we don't miss the code freeze.

caesarxuchao · 2017-06-05T22:27:48Z

Rebased. Adding back the lgtm.

grodrigues3 · 2017-06-06T18:27:19Z

Hey @caesarxuchao if you use /lgtm that will also approve the PR when you are an approver.

lavalamp · 2017-06-07T00:17:06Z

cmd/libs/go2idl/conversion-gen/generators/conversion.go

@@ -46,13 +46,18 @@ type CustomArgs struct {
 	SkipUnsafe bool
 }

-// This is the comment tag that carries parameters for conversion generation.
+// There are the comment tags that carry parameters for conversion generation.
 const tagName = "k8s:conversion-gen"


nit: use a const () block

lavalamp · 2017-06-07T00:18:01Z

cmd/libs/go2idl/conversion-gen/generators/conversion.go

@@ -46,13 +46,18 @@ type CustomArgs struct {
 	SkipUnsafe bool
 }

-// This is the comment tag that carries parameters for conversion generation.
+// There are the comment tags that carry parameters for conversion generation.


s/There/These/

Document what argument these tag names accept and its exact format.

caesarxuchao · 2017-06-07T23:24:10Z

Added the doc. @lavalamp PTAL. Thanks.

lavalamp · 2017-06-09T20:39:19Z

/lgtm
/approve

k8s-github-robot · 2017-06-09T20:47:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: caesarxuchao, lavalamp

Associated issue: 44065

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

Needs approval from an approver in each of these OWNERS Files:

~~cmd/libs/go2idl/OWNERS~~ [lavalamp]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-ci-robot · 2017-06-09T21:53:14Z

@caesarxuchao: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
Jenkins Bazel Build	28f20b9d1459257d01477c7b3e8c7dd58b42ca4b	link	`@k8s-bot bazel test this`
Jenkins GCE Node e2e	28f20b9d1459257d01477c7b3e8c7dd58b42ca4b	link	`@k8s-bot node e2e test this`
Jenkins unit/integration	28f20b9d1459257d01477c7b3e8c7dd58b42ca4b	link	`@k8s-bot unit test this`
Jenkins Kubemark GCE e2e	28f20b9d1459257d01477c7b3e8c7dd58b42ca4b	link	`@k8s-bot kubemark e2e test this`
Jenkins verification	28f20b9d1459257d01477c7b3e8c7dd58b42ca4b	link	`@k8s-bot verify test this`
Jenkins kops AWS e2e	28f20b9d1459257d01477c7b3e8c7dd58b42ca4b	link	`@k8s-bot kops aws e2e test this`
Jenkins GCE etcd3 e2e	28f20b9d1459257d01477c7b3e8c7dd58b42ca4b	link	`@k8s-bot gce etcd3 e2e test this`
pull-kubernetes-federation-e2e-gce	21bba61e0120cc21cf52d10d098ac160579d1bad	link	`@k8s-bot pull-kubernetes-federation-e2e-gce test this`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

caesarxuchao · 2017-06-09T22:01:19Z

@k8s-bot pull-kubernetes-e2e-gce-etcd3 test this

k8s-github-robot · 2017-06-09T22:16:41Z

Automatic merge from submit-queue (batch tested with PRs 45057, 47259)

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 27, 2017

k8s-github-robot assigned wojtek-t and timothysc Apr 27, 2017

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Apr 27, 2017

caesarxuchao assigned lavalamp and unassigned timothysc and wojtek-t Apr 27, 2017

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Apr 27, 2017

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 28, 2017

caesarxuchao force-pushed the fix-conversion-for-pr branch from 0c0ac8d to 0f91d24 Compare April 28, 2017 05:36

k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 28, 2017

mbohlool reviewed May 18, 2017

View reviewed changes

mbohlool self-assigned this May 18, 2017

caesarxuchao force-pushed the fix-conversion-for-pr branch from 0f91d24 to 28f20b9 Compare May 18, 2017 23:47

caesarxuchao commented May 18, 2017

View reviewed changes

caesarxuchao force-pushed the fix-conversion-for-pr branch from 28f20b9 to 21bba61 Compare June 1, 2017 00:03

caesarxuchao added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 1, 2017

caesarxuchao added this to the v1.7 milestone Jun 1, 2017

caesarxuchao force-pushed the fix-conversion-for-pr branch from 21bba61 to 0868b5b Compare June 5, 2017 22:16

caesarxuchao added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 5, 2017

lavalamp reviewed Jun 7, 2017

View reviewed changes

fix conversion-gen

ab71a6e

caesarxuchao force-pushed the fix-conversion-for-pr branch from 0868b5b to ab71a6e Compare June 7, 2017 23:23

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 7, 2017

marun added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jun 8, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 9, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 9, 2017

k8s-github-robot merged commit e1efc54 into kubernetes:master Jun 9, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow conversion-gen to take types that are not in the output path #45057

Allow conversion-gen to take types that are not in the output path #45057

caesarxuchao commented Apr 27, 2017 •

edited

k8s-reviewable commented Apr 27, 2017

caesarxuchao commented Apr 27, 2017

caesarxuchao commented May 15, 2017 •

edited

k8s-ci-robot commented May 15, 2017

caesarxuchao commented May 15, 2017

mbohlool left a comment

mbohlool May 18, 2017

caesarxuchao May 18, 2017

caesarxuchao May 18, 2017

mbohlool May 18, 2017

caesarxuchao Jun 1, 2017

mbohlool May 18, 2017

caesarxuchao May 18, 2017

caesarxuchao May 18, 2017

caesarxuchao May 18, 2017

caesarxuchao commented May 18, 2017

caesarxuchao commented Jun 1, 2017

mbohlool commented Jun 1, 2017

caesarxuchao commented Jun 1, 2017

caesarxuchao commented Jun 5, 2017

grodrigues3 commented Jun 6, 2017

lavalamp Jun 7, 2017

lavalamp Jun 7, 2017

caesarxuchao commented Jun 7, 2017

lavalamp commented Jun 9, 2017

k8s-github-robot commented Jun 9, 2017

k8s-ci-robot commented Jun 9, 2017 •

edited

caesarxuchao commented Jun 9, 2017

k8s-github-robot commented Jun 9, 2017

		const tagName = "k8s:conversion-gen"
		const externalTypesTagName = "k8s:conversion-gen-external-types"

Allow conversion-gen to take types that are not in the output path #45057

Allow conversion-gen to take types that are not in the output path #45057

Conversation

caesarxuchao commented Apr 27, 2017 • edited

k8s-reviewable commented Apr 27, 2017

caesarxuchao commented Apr 27, 2017

caesarxuchao commented May 15, 2017 • edited

k8s-ci-robot commented May 15, 2017

caesarxuchao commented May 15, 2017

mbohlool left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caesarxuchao commented May 18, 2017

caesarxuchao commented Jun 1, 2017

mbohlool commented Jun 1, 2017

caesarxuchao commented Jun 1, 2017

caesarxuchao commented Jun 5, 2017

grodrigues3 commented Jun 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caesarxuchao commented Jun 7, 2017

lavalamp commented Jun 9, 2017

k8s-github-robot commented Jun 9, 2017

k8s-ci-robot commented Jun 9, 2017 • edited

caesarxuchao commented Jun 9, 2017

k8s-github-robot commented Jun 9, 2017

caesarxuchao commented Apr 27, 2017 •

edited

caesarxuchao commented May 15, 2017 •

edited

k8s-ci-robot commented Jun 9, 2017 •

edited