From 4d5899955e77c7e6a87186ead2fbaa2c9265fcf8 Mon Sep 17 00:00:00 2001 From: Muhammed Uluyol Date: Mon, 10 Aug 2015 13:00:20 -0700 Subject: [PATCH 1/2] Remove redundant tests. We already check that no changes have been made in hack/verify-generated-*.sh. --- pkg/runtime/conversion_generation_test.go | 152 ---------------------- pkg/runtime/deep_copy_generation_test.go | 95 -------------- 2 files changed, 247 deletions(-) delete mode 100644 pkg/runtime/conversion_generation_test.go delete mode 100644 pkg/runtime/deep_copy_generation_test.go diff --git a/pkg/runtime/conversion_generation_test.go b/pkg/runtime/conversion_generation_test.go deleted file mode 100644 index d6960cda549..00000000000 --- a/pkg/runtime/conversion_generation_test.go +++ /dev/null @@ -1,152 +0,0 @@ -/* -Copyright 2015 The Kubernetes Authors All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package runtime_test - -import ( - "bufio" - "bytes" - "fmt" - "io" - "io/ioutil" - "os" - "path" - "testing" - - "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/api/testapi" - _ "k8s.io/kubernetes/pkg/api/v1" - "k8s.io/kubernetes/pkg/runtime" - "k8s.io/kubernetes/pkg/util" -) - -func generateConversions(t *testing.T, version string) bytes.Buffer { - g := runtime.NewConversionGenerator(api.Scheme.Raw(), path.Join("k8s.io/kubernetes/pkg/api", version)) - apiShort := g.AddImport("k8s.io/kubernetes/pkg/api") - g.AddImport("k8s.io/kubernetes/pkg/api/resource") - // TODO(wojtek-t): Change the overwrites to a flag. - g.OverwritePackage(version, "") - for _, knownType := range api.Scheme.KnownTypes(version) { - if err := g.GenerateConversionsForType(version, knownType); err != nil { - t.Fatalf("error while generating conversion functions for %v: %v", knownType, err) - } - } - g.RepackImports(util.NewStringSet()) - var functions bytes.Buffer - functionsWriter := bufio.NewWriter(&functions) - if err := g.WriteImports(functionsWriter); err != nil { - t.Fatalf("error while writing imports: %v", err) - } - if err := g.WriteConversionFunctions(functionsWriter); err != nil { - t.Fatalf("couldn't generate conversion functions: %v", err) - } - if err := g.RegisterConversionFunctions(functionsWriter, fmt.Sprintf("%s.Scheme", apiShort)); err != nil { - t.Fatalf("couldn't generate conversion function names: %v", err) - } - if err := functionsWriter.Flush(); err != nil { - t.Fatalf("error while flushing writer") - } - - return functions -} - -func readLinesUntil(t *testing.T, reader *bufio.Reader, stop string, buffer *bytes.Buffer) error { - for { - line, err := reader.ReadString('\n') - if err == io.EOF { - return fmt.Errorf("'%s' line not found", stop) - } - if err != nil { - t.Fatalf("error while reading file: %v", err) - } - if line == stop { - break - } - if buffer != nil { - if _, err := buffer.WriteString(line); err != nil { - t.Fatalf("error while buffering line") - } - } - } - return nil -} - -func bufferExistingGeneratedCode(t *testing.T, fileName string) bytes.Buffer { - file, err := os.Open(fileName) - if err != nil { - t.Fatalf("couldn't open file %s", fileName) - } - defer file.Close() - - reader := bufio.NewReader(file) - - functionsPrefix := "// AUTO-GENERATED FUNCTIONS START HERE\n" - functionsSuffix := "// AUTO-GENERATED FUNCTIONS END HERE\n" - if err := readLinesUntil(t, reader, functionsPrefix, nil); err != nil { - t.Fatalf("error while parsing file: %v", err) - } - var functions bytes.Buffer - if err := readLinesUntil(t, reader, functionsSuffix, &functions); err != nil { - t.Fatalf("error while parsing file: %v", err) - } - _, err = reader.ReadString('\n') - if err == nil || err != io.EOF { - t.Fatalf("end-of-file expected") - } - return functions -} - -func compareBuffers(t *testing.T, generatedFile string, existing, generated bytes.Buffer) bool { - ok := true - for { - existingLine, existingErr := existing.ReadString('\n') - generatedLine, generatedErr := generated.ReadString('\n') - if existingErr == io.EOF && generatedErr == io.EOF { - break - } - if existingErr != generatedErr { - ok = false - t.Errorf("reading errors: existing %v generated %v", existingErr, generatedErr) - return ok - } - if existingErr != nil { - ok = false - t.Errorf("error while reading: %v", existingErr) - } - if existingLine != generatedLine { - ok = false - diff := fmt.Sprintf("\nexpected: %s\n got: %s", generatedLine, existingLine) - t.Errorf("please update conversion functions; generated: %s; first diff: %s", generatedFile, diff) - return ok - } - } - return ok -} - -func TestNoManualChangesToGenerateConversions(t *testing.T) { - version := testapi.Version() - fileName := fmt.Sprintf("../../pkg/api/%s/conversion_generated.go", version) - - existingFunctions := bufferExistingGeneratedCode(t, fileName) - generatedFunctions := generateConversions(t, version) - - functionsTxt := fmt.Sprintf("%s.functions.txt", version) - ioutil.WriteFile(functionsTxt, generatedFunctions.Bytes(), os.FileMode(0644)) - - if ok := compareBuffers(t, functionsTxt, existingFunctions, generatedFunctions); ok { - os.Remove(functionsTxt) - } -} diff --git a/pkg/runtime/deep_copy_generation_test.go b/pkg/runtime/deep_copy_generation_test.go deleted file mode 100644 index 239b4aa77f9..00000000000 --- a/pkg/runtime/deep_copy_generation_test.go +++ /dev/null @@ -1,95 +0,0 @@ -/* -Copyright 2015 The Kubernetes Authors All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package runtime_test - -import ( - "bufio" - "bytes" - "fmt" - "io/ioutil" - "os" - "path" - "testing" - - "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/api/testapi" - _ "k8s.io/kubernetes/pkg/api/v1" - "k8s.io/kubernetes/pkg/runtime" - "k8s.io/kubernetes/pkg/util" - - "github.com/golang/glog" -) - -func generateDeepCopies(t *testing.T, version string) bytes.Buffer { - testedVersion := version - registerTo := "api.Scheme" - if testedVersion == "api" { - testedVersion = api.Scheme.Raw().InternalVersion - registerTo = "Scheme" - } - - g := runtime.NewDeepCopyGenerator(api.Scheme.Raw(), path.Join("k8s.io/kubernetes/pkg/api", testedVersion), util.NewStringSet("k8s.io/kubernetes")) - g.AddImport("k8s.io/kubernetes/pkg/api") - g.OverwritePackage(version, "") - - for _, knownType := range api.Scheme.KnownTypes(testedVersion) { - if err := g.AddType(knownType); err != nil { - glog.Errorf("error while generating deep-copy functions for %v: %v", knownType, err) - } - } - - var functions bytes.Buffer - functionsWriter := bufio.NewWriter(&functions) - g.RepackImports() - if err := g.WriteImports(functionsWriter); err != nil { - t.Fatalf("couldn't generate deep-copy function imports: %v", err) - } - if err := g.WriteDeepCopyFunctions(functionsWriter); err != nil { - t.Fatalf("couldn't generate deep-copy functions: %v", err) - } - if err := g.RegisterDeepCopyFunctions(functionsWriter, registerTo); err != nil { - t.Fatalf("couldn't generate deep-copy function names: %v", err) - } - if err := functionsWriter.Flush(); err != nil { - t.Fatalf("error while flushing writer") - } - - return functions -} - -func TestNoManualChangesToGenerateDeepCopies(t *testing.T) { - versions := []string{"api", testapi.Version()} - - for _, version := range versions { - fileName := "" - if version == "api" { - fileName = "../../pkg/api/deep_copy_generated.go" - } else { - fileName = fmt.Sprintf("../../pkg/api/%s/deep_copy_generated.go", version) - } - - existingFunctions := bufferExistingGeneratedCode(t, fileName) - generatedFunctions := generateDeepCopies(t, version) - - functionsTxt := fmt.Sprintf("%s.deep_copy.txt", version) - ioutil.WriteFile(functionsTxt, generatedFunctions.Bytes(), os.FileMode(0644)) - - if ok := compareBuffers(t, functionsTxt, existingFunctions, generatedFunctions); ok { - os.Remove(functionsTxt) - } - } -} From 567bb154325d24b94ac286288da7c404b2c4975d Mon Sep 17 00:00:00 2001 From: Muhammed Uluyol Date: Fri, 7 Aug 2015 11:26:44 -0700 Subject: [PATCH 2/2] Generate conversions/deep-copies for experimental. Currently we make (and register) duplicate functions but this is benign. --- .travis.yml | 1 + cmd/genconversion/conversion.go | 26 +- cmd/gendeepcopy/deep_copy.go | 25 +- .../update-generated-conversions.sh | 25 +- .../update-generated-deep-copies.sh | 48 ++-- .../verify-generated-conversions.sh | 42 +-- .../verify-generated-deep-copies.sh | 41 +-- pkg/api/deep_copy_generated.go | 3 +- pkg/api/v1/conversion_generated.go | 3 +- pkg/api/v1/deep_copy_generated.go | 3 +- pkg/expapi/deep_copy.go | 19 -- pkg/expapi/deep_copy_generated.go | 138 ++++++++++ pkg/expapi/v1/conversion_generated.go | 254 ++++++++++++++++++ pkg/expapi/v1/deep_copy.go | 19 -- pkg/expapi/v1/deep_copy_generated.go | 139 ++++++++++ pkg/expapi/v1/register.go | 1 - shippable.yml | 1 + 17 files changed, 661 insertions(+), 127 deletions(-) delete mode 100644 pkg/expapi/deep_copy.go create mode 100644 pkg/expapi/deep_copy_generated.go create mode 100644 pkg/expapi/v1/conversion_generated.go delete mode 100644 pkg/expapi/v1/deep_copy.go create mode 100644 pkg/expapi/v1/deep_copy_generated.go diff --git a/.travis.yml b/.travis.yml index 90bcffe0517..64c2f6f2117 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,6 +8,7 @@ matrix: install: - go get github.com/tools/godep + - go get golang.org/x/tools/cmd/goimports - ./hack/travis/install-etcd.sh - ./hack/build-go.sh diff --git a/cmd/genconversion/conversion.go b/cmd/genconversion/conversion.go index dc178e9a871..461fd10527e 100644 --- a/cmd/genconversion/conversion.go +++ b/cmd/genconversion/conversion.go @@ -22,9 +22,12 @@ import ( "os" "path" "runtime" + "strings" "k8s.io/kubernetes/pkg/api" _ "k8s.io/kubernetes/pkg/api/v1" + _ "k8s.io/kubernetes/pkg/expapi" + _ "k8s.io/kubernetes/pkg/expapi/v1" pkg_runtime "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util" @@ -32,9 +35,11 @@ import ( flag "github.com/spf13/pflag" ) +const pkgBase = "k8s.io/kubernetes/pkg" + var ( functionDest = flag.StringP("funcDest", "f", "-", "Output for conversion functions; '-' means stdout") - version = flag.StringP("version", "v", "v1", "Version for conversion.") + groupVersion = flag.StringP("version", "v", "api/v1", "groupPath/version for conversion.") ) func main() { @@ -53,13 +58,20 @@ func main() { funcOut = file } - generator := pkg_runtime.NewConversionGenerator(api.Scheme.Raw(), path.Join("k8s.io/kubernetes/pkg/api", *version)) - apiShort := generator.AddImport("k8s.io/kubernetes/pkg/api") - generator.AddImport("k8s.io/kubernetes/pkg/api/resource") + group, version := path.Split(*groupVersion) + group = strings.TrimRight(group, "/") + + versionPath := path.Join(pkgBase, group, version) + generator := pkg_runtime.NewConversionGenerator(api.Scheme.Raw(), versionPath) + apiShort := generator.AddImport(path.Join(pkgBase, "api")) + generator.AddImport(path.Join(pkgBase, "api/resource")) // TODO(wojtek-t): Change the overwrites to a flag. - generator.OverwritePackage(*version, "") - for _, knownType := range api.Scheme.KnownTypes(*version) { - if err := generator.GenerateConversionsForType(*version, knownType); err != nil { + generator.OverwritePackage(version, "") + for _, knownType := range api.Scheme.KnownTypes(version) { + if !strings.HasPrefix(knownType.PkgPath(), versionPath) { + continue + } + if err := generator.GenerateConversionsForType(version, knownType); err != nil { glog.Errorf("error while generating conversion functions for %v: %v", knownType, err) } } diff --git a/cmd/gendeepcopy/deep_copy.go b/cmd/gendeepcopy/deep_copy.go index 9aace9757fb..fb7fa12f5ff 100644 --- a/cmd/gendeepcopy/deep_copy.go +++ b/cmd/gendeepcopy/deep_copy.go @@ -25,6 +25,8 @@ import ( "k8s.io/kubernetes/pkg/api" _ "k8s.io/kubernetes/pkg/api/v1" + _ "k8s.io/kubernetes/pkg/expapi" + _ "k8s.io/kubernetes/pkg/expapi/v1" pkg_runtime "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util" @@ -32,9 +34,11 @@ import ( flag "github.com/spf13/pflag" ) +const pkgBase = "k8s.io/kubernetes/pkg" + var ( - functionDest = flag.StringP("func-dest", "f", "-", "Output for deep copy functions; '-' means stdout") - version = flag.StringP("version", "v", "v1", "Version for deep copies.") + functionDest = flag.StringP("funcDest", "f", "-", "Output for deep copy functions; '-' means stdout") + groupVersion = flag.StringP("version", "v", "", "groupPath/version for deep copies.") overwrites = flag.StringP("overwrites", "o", "", "Comma-separated overwrites for package names") ) @@ -54,15 +58,15 @@ func main() { funcOut = file } - knownVersion := *version + group, version := path.Split(*groupVersion) + group = strings.TrimRight(group, "/") registerTo := "api.Scheme" - if knownVersion == "api" { - knownVersion = api.Scheme.Raw().InternalVersion + if *groupVersion == "api/" { registerTo = "Scheme" } - pkgPath := path.Join("k8s.io/kubernetes/pkg/api", knownVersion) - generator := pkg_runtime.NewDeepCopyGenerator(api.Scheme.Raw(), pkgPath, util.NewStringSet("k8s.io/kubernetes")) - generator.AddImport("k8s.io/kubernetes/pkg/api") + versionPath := path.Join(pkgBase, group, version) + generator := pkg_runtime.NewDeepCopyGenerator(api.Scheme.Raw(), versionPath, util.NewStringSet("k8s.io/kubernetes")) + generator.AddImport(path.Join(pkgBase, "api")) if len(*overwrites) > 0 { for _, overwrite := range strings.Split(*overwrites, ",") { @@ -73,7 +77,10 @@ func main() { generator.OverwritePackage(vals[0], vals[1]) } } - for _, knownType := range api.Scheme.KnownTypes(knownVersion) { + for _, knownType := range api.Scheme.KnownTypes(version) { + if !strings.HasPrefix(knownType.PkgPath(), versionPath) { + continue + } if err := generator.AddType(knownType); err != nil { glog.Errorf("error while generating deep copy functions for %v: %v", knownType, err) } diff --git a/hack/after-build/update-generated-conversions.sh b/hack/after-build/update-generated-conversions.sh index 318ef754a86..bd9d5c7c1d2 100755 --- a/hack/after-build/update-generated-conversions.sh +++ b/hack/after-build/update-generated-conversions.sh @@ -29,11 +29,11 @@ function generate_version() { local version=$1 local TMPFILE="/tmp/conversion_generated.$(date +%s).go" - echo "Generating for version ${version}" + echo "Generating for ${version}" sed 's/YEAR/2015/' hack/boilerplate/boilerplate.go.txt > "$TMPFILE" cat >> "$TMPFILE" </dev/null; then + echo "goimports not in path, run go get golang.org/x/tools/cmd/goimports" + exit 1 +fi -# ex: ts=2 sw=2 et filetype=sh +DEFAULT_VERSIONS="api/v1 expapi/v1" +VERSIONS=${VERSIONS:-$DEFAULT_VERSIONS} +for ver in $VERSIONS; do + # Ensure that the version being processed is registered by setting + # KUBE_API_VERSIONS. + KUBE_API_VERSIONS="${ver##*/}" generate_version "${ver}" +done diff --git a/hack/after-build/update-generated-deep-copies.sh b/hack/after-build/update-generated-deep-copies.sh index 38d21677959..b848b5e9a75 100755 --- a/hack/after-build/update-generated-deep-copies.sh +++ b/hack/after-build/update-generated-deep-copies.sh @@ -27,22 +27,25 @@ gendeepcopy=$(kube::util::find-binary "gendeepcopy") function result_file_name() { local version=$1 - if [ "${version}" == "api" ]; then - echo "pkg/api/deep_copy_generated.go" - else - echo "pkg/api/${version}/deep_copy_generated.go" - fi + echo "pkg/${version}/deep_copy_generated.go" } function generate_version() { local version=$1 local TMPFILE="/tmp/deep_copy_generated.$(date +%s).go" - echo "Generating for version ${version}" + echo "Generating for ${version}" + + # version is group/version, so use the version number as the package name unless + # this is an internal version, in which case use the group name. + pkgname=${version##*/} + if [[ -z $pkgname ]]; then + pkgname=${version%/*} + fi sed 's/YEAR/2015/' hack/boilerplate/boilerplate.go.txt > $TMPFILE cat >> $TMPFILE <