expapi conversion missing convert_v1_PodSpec_To_api_PodSpec and convert_api_PodSpec_To_v1_PodSpec methods

[soltysh]

That came up during working on #12910. I'm adding new Job resource in pkg/expapi and while generating conversions for that I'm reaching convert_v1_PodSpec_To_api_PodSpec and convert_api_PodSpec_To_v1_PodSpec functions which are hand written in pkg/api/v1/conversion.go which results in compilation errors. What should be the right path to proceed with this? For now I've copied those functions manually to pkg/expapi/v1/conversion.go in aforementioned PR but I'm quite it's not the expected way to proceed with that.
@wojtek-t @smarterclayton

[smarterclayton]

It would probably be to expose that conversion as a public method (create
Convert_v1_PodSpec_To_api_PodSpec that calls the existing one) and then use
the public version from expapi.

On Thu, Aug 20, 2015 at 9:24 AM, Maciej Szulik notifications@github.com
wrote:

That came up during working on #12910
#12910. I'm adding new Job
resource in pkg/expapi and while generating conversions for that I'm
reaching convert_v1_PodSpec_To_api_PodSpec and
convert_api_PodSpec_To_v1_PodSpec functions which are hand written in
pkg/api/v1/conversion.go
https://github.com/kubernetes/kubernetes/blob/master/pkg/api/v1/conversion.go#L230-L361
which results in compilation errors. What should be the right path to
proceed with this? For now I've copied those functions manually to
pkg/expapi/v1/conversion.go in aforementioned PR but I'm quite it's not
the expected way to proceed with that.
@wojtek-t https://github.com/wojtek-t @smarterclayton
https://github.com/smarterclayton

—
Reply to this email directly or view it on GitHub
#12977.

Clayton Coleman | Lead Engineer, OpenShift

[nikhiljindal]

cc @uluyol @caesarxuchao

[caesarxuchao]

I'm not sure if the genconversion tool will properly call the public method. If it does, I think making the method public is the proper way.

[uluyol]

I thought that genconversion should be generating a conversion that calls the handwritten function through s.Convert. I really don't think conversion functions should be made public.

[nikhiljindal]

Yes if there is not much overhead in calling s.Convert, then it seems better to change the generated functions to use s.Convert instead of calling hand-written functions directly. That seems better to me than making the function public.

@wojtek-t WDYT?

[nikhiljindal]

The auto-generated conversion functions are also duplicated in both api/ and expapi/
I see now that @uluyol has a proposal to fix this #12598

[wojtek-t]

@nikhiljindal I agree that changing to "s.Convert" is the cleanest way to do it. But we should measure the impact of that change on performance (and we should wait for watch in apiserver because this can change the result in my opinion - i.e. changing it might have smaller impact and we will be able to change it).

[lavalamp]

I believe this must have been resolved, given that we were able to make the experimental group.

[nikhiljindal]

The functions are duplicated in both pkg/apis/experimental/v1alpha1/conversion.go and pkg/api/v1/conversion.go which as OP said allowed us to proceed but is not the cleanest way.

[soltysh]

Frankly said I'm partially happy with the current solution, but I'm guessing nobody has enough time to work it out in a better way so I'm ok with closing it as is. Once we start moving things out of experimental we'll be able to clean it as well.

[smarterclayton]

This ended up breaking conversion generation in #20847 between Go 1.4 and Go 1.5. Still trying to track which package forces this to get registered first.