From 5578dc99e37086dad047bb99af71ed6b2b235b27 Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Thu, 7 May 2015 13:46:54 +0200 Subject: [PATCH] Improvements for conversions generator --- cmd/kube-conversion/conversion.go | 4 +- pkg/api/v1beta3/conversion.go | 24 +++++++-- pkg/api/v1beta3/conversion_generated.go | 4 +- .../conversion_generation_test.go} | 6 +-- .../conversion_generator.go} | 52 ++++++++++++++++--- 5 files changed, 71 insertions(+), 19 deletions(-) rename pkg/{conversion/code_generation_test.go => runtime/conversion_generation_test.go} (97%) rename pkg/{conversion/generator.go => runtime/conversion_generator.go} (92%) diff --git a/cmd/kube-conversion/conversion.go b/cmd/kube-conversion/conversion.go index f69406b6d95..051e1e6bbcb 100644 --- a/cmd/kube-conversion/conversion.go +++ b/cmd/kube-conversion/conversion.go @@ -25,7 +25,7 @@ import ( _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta2" _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta3" - "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" + pkg_runtime "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/golang/glog" flag "github.com/spf13/pflag" @@ -64,7 +64,7 @@ func main() { nameOut = file } - generator := conversion.NewGenerator(api.Scheme.Raw()) + generator := pkg_runtime.NewGenerator(api.Scheme.Raw()) // TODO(wojtek-t): Change the overwrites to a flag. generator.OverwritePackage(*version, "") generator.OverwritePackage("api", "newer") diff --git a/pkg/api/v1beta3/conversion.go b/pkg/api/v1beta3/conversion.go index 22de80e4333..bc0b3a17052 100644 --- a/pkg/api/v1beta3/conversion.go +++ b/pkg/api/v1beta3/conversion.go @@ -131,6 +131,9 @@ func init() { } func convert_v1beta3_Container_To_api_Container(in *Container, out *newer.Container, s conversion.Scope) error { + if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found { + defaulting.(func(*Container))(in) + } out.Name = in.Name out.Image = in.Image if in.Command != nil { @@ -212,13 +215,21 @@ func convert_v1beta3_Container_To_api_Container(in *Container, out *newer.Contai } } } - if err := s.Convert(&in.SecurityContext, &out.SecurityContext, 0); err != nil { - return err + if in.SecurityContext != nil { + out.SecurityContext = new(newer.SecurityContext) + if err := convert_v1beta3_SecurityContext_To_api_SecurityContext(in.SecurityContext, out.SecurityContext, s); err != nil { + return err + } + } else { + out.SecurityContext = nil } return nil } func convert_api_Container_To_v1beta3_Container(in *newer.Container, out *Container, s conversion.Scope) error { + if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found { + defaulting.(func(*newer.Container))(in) + } out.Name = in.Name out.Image = in.Image if in.Command != nil { @@ -287,8 +298,13 @@ func convert_api_Container_To_v1beta3_Container(in *newer.Container, out *Contai } out.TerminationMessagePath = in.TerminationMessagePath out.ImagePullPolicy = PullPolicy(in.ImagePullPolicy) - if err := s.Convert(&in.SecurityContext, &out.SecurityContext, 0); err != nil { - return err + if in.SecurityContext != nil { + out.SecurityContext = new(SecurityContext) + if err := convert_api_SecurityContext_To_v1beta3_SecurityContext(in.SecurityContext, out.SecurityContext, s); err != nil { + return err + } + } else { + out.SecurityContext = nil } // now that we've converted set the container field from security context if out.SecurityContext != nil && out.SecurityContext.Privileged != nil { diff --git a/pkg/api/v1beta3/conversion_generated.go b/pkg/api/v1beta3/conversion_generated.go index 9086345e268..01b6ed08b8e 100644 --- a/pkg/api/v1beta3/conversion_generated.go +++ b/pkg/api/v1beta3/conversion_generated.go @@ -2614,7 +2614,7 @@ func convert_v1beta3_PodSpec_To_api_PodSpec(in *PodSpec, out *newer.PodSpec, s c if in.Containers != nil { out.Containers = make([]newer.Container, len(in.Containers)) for i := range in.Containers { - if err := s.Convert(&in.Containers[i], &out.Containers[i], 0); err != nil { + if err := convert_v1beta3_Container_To_api_Container(&in.Containers[i], &out.Containers[i], s); err != nil { return err } } @@ -2659,7 +2659,7 @@ func convert_api_PodSpec_To_v1beta3_PodSpec(in *newer.PodSpec, out *PodSpec, s c if in.Containers != nil { out.Containers = make([]Container, len(in.Containers)) for i := range in.Containers { - if err := s.Convert(&in.Containers[i], &out.Containers[i], 0); err != nil { + if err := convert_api_Container_To_v1beta3_Container(&in.Containers[i], &out.Containers[i], s); err != nil { return err } } diff --git a/pkg/conversion/code_generation_test.go b/pkg/runtime/conversion_generation_test.go similarity index 97% rename from pkg/conversion/code_generation_test.go rename to pkg/runtime/conversion_generation_test.go index d8d11a004f0..6bdbc230e41 100644 --- a/pkg/conversion/code_generation_test.go +++ b/pkg/runtime/conversion_generation_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package conversion_test +package runtime_test import ( "bufio" @@ -26,13 +26,13 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/golang/glog" ) func generateConversions(t *testing.T, version string) (bytes.Buffer, bytes.Buffer) { - g := conversion.NewGenerator(api.Scheme.Raw()) + g := runtime.NewGenerator(api.Scheme.Raw()) g.OverwritePackage(version, "") g.OverwritePackage("api", "newer") for _, knownType := range api.Scheme.KnownTypes(version) { diff --git a/pkg/conversion/generator.go b/pkg/runtime/conversion_generator.go similarity index 92% rename from pkg/conversion/generator.go rename to pkg/runtime/conversion_generator.go index e3ef67faea9..722f33b257d 100644 --- a/pkg/conversion/generator.go +++ b/pkg/runtime/conversion_generator.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package conversion +package runtime import ( "fmt" @@ -22,6 +22,8 @@ import ( "reflect" "sort" "strings" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" ) type Generator interface { @@ -31,7 +33,7 @@ type Generator interface { OverwritePackage(pkg, overwrite string) } -func NewGenerator(scheme *Scheme) Generator { +func NewGenerator(scheme *conversion.Scheme) Generator { return &generator{ scheme: scheme, convertibles: make(map[reflect.Type]reflect.Type), @@ -42,7 +44,7 @@ func NewGenerator(scheme *Scheme) Generator { var complexTypes []reflect.Kind = []reflect.Kind{reflect.Map, reflect.Ptr, reflect.Slice, reflect.Interface, reflect.Struct} type generator struct { - scheme *Scheme + scheme *conversion.Scheme convertibles map[reflect.Type]reflect.Type // If pkgOverwrites is set for a given package name, that package name // will be replaced while writing conversion function. If empty, package @@ -449,7 +451,7 @@ func (g *generator) writeConversionForSlice(w io.Writer, inField, outField refle } if !assigned { assignStmt := "" - if g.existsConvertionFunction(inField.Type.Elem(), outField.Type.Elem()) { + if g.existsDedicatedConversionFunction(inField.Type.Elem(), outField.Type.Elem()) { assignFormat := "if err := %s(&in.%s[i], &out.%s[i], s); err != nil {\n" funcName := conversionFunctionName(inField.Type.Elem(), outField.Type.Elem()) assignStmt = fmt.Sprintf(assignFormat, funcName, inField.Name, outField.Name) @@ -539,7 +541,7 @@ func (g *generator) writeConversionForPtr(w io.Writer, inField, outField reflect return err } assignStmt := "" - if g.existsConvertionFunction(inField.Type.Elem(), outField.Type.Elem()) { + if g.existsDedicatedConversionFunction(inField.Type.Elem(), outField.Type.Elem()) { newFormat := "out.%s = new(%s)\n" newStmt := fmt.Sprintf(newFormat, outField.Name, g.typeName(outField.Type.Elem())) if err := writeLine(w, indent+1, newStmt); err != nil { @@ -580,7 +582,8 @@ func (g *generator) writeConversionForStruct(w io.Writer, inType, outType reflec inField := inType.Field(i) outField, _ := outType.FieldByName(inField.Name) - if g.scheme.Converter().HasConversionFunc(inField.Type, outField.Type) { + existsConversion := g.scheme.Converter().HasConversionFunc(inField.Type, outField.Type) + if existsConversion && !g.existsDedicatedConversionFunction(inField.Type, outField.Type) { // Use the conversion method that is already defined. assignFormat := "if err := s.Convert(&in.%s, &out.%s, 0); err != nil {\n" assignStmt := fmt.Sprintf(assignFormat, inField.Name, outField.Name) @@ -644,7 +647,7 @@ func (g *generator) writeConversionForStruct(w io.Writer, inType, outType reflec } assignStmt := "" - if g.existsConvertionFunction(inField.Type, outField.Type) { + if g.existsDedicatedConversionFunction(inField.Type, outField.Type) { assignFormat := "if err := %s(&in.%s, &out.%s, s); err != nil {\n" funcName := conversionFunctionName(inField.Type, outField.Type) assignStmt = fmt.Sprintf(assignFormat, funcName, inField.Name, outField.Name) @@ -690,7 +693,7 @@ func (g *generator) writeConversionForType(w io.Writer, inType, outType reflect. return nil } -func (g *generator) existsConvertionFunction(inType, outType reflect.Type) bool { +func (g *generator) existsConversionFunction(inType, outType reflect.Type) bool { if val, found := g.convertibles[inType]; found && val == outType { return true } @@ -700,6 +703,39 @@ func (g *generator) existsConvertionFunction(inType, outType reflect.Type) bool return false } +// TODO(wojtek-t): We should somehow change the conversion methods registered under: +// pkg/runtime/scheme.go to implement the naming convention for conversion functions +// and get rid of this hack. +type typePair struct { + inType reflect.Type + outType reflect.Type +} + +var defaultConversions []typePair = []typePair{ + {reflect.TypeOf([]RawExtension{}), reflect.TypeOf([]Object{})}, + {reflect.TypeOf([]Object{}), reflect.TypeOf([]RawExtension{})}, + {reflect.TypeOf(RawExtension{}), reflect.TypeOf(EmbeddedObject{})}, + {reflect.TypeOf(EmbeddedObject{}), reflect.TypeOf(RawExtension{})}, +} + +func (g *generator) existsDedicatedConversionFunction(inType, outType reflect.Type) bool { + if inType == outType { + // Assume that conversion are not defined for "deep copies". + return false + } + + if g.existsConversionFunction(inType, outType) { + return true + } + + for _, conv := range defaultConversions { + if conv.inType == inType && conv.outType == outType { + return false + } + } + return g.scheme.Converter().HasConversionFunc(inType, outType) +} + func (g *generator) OverwritePackage(pkg, overwrite string) { g.pkgOverwrites[pkg] = overwrite }