Use generated DeepCopy methods.

lavalamp · May 28, 2015 · 33318f0 · 33318f0
1 parent b7b89c4
commit 33318f0
Show file tree

Hide file tree

Showing 10 changed files with 36 additions and 131 deletions.
diff --git a/hooks/pre-commit b/hooks/pre-commit
@@ -111,4 +111,16 @@ else
 fi
 echo "${reset}"
 
+echo -ne "Checking for deep-copies that need updating... "
+if ! hack/verify-generated-deep-copies.sh > /dev/null; then
+  echo "${red}ERROR!"
+  echo "Some deep-copy functions need regeneration."
+  echo "To regenerate deep-copies, run:"
+  echo "  hack/update-generated-deep-copies.sh"
+  exit_code=1
+else
+  echo "${green}OK"
+fi
+echo "${reset}"
+
 exit $exit_code
diff --git a/pkg/api/copy_test.go b/pkg/api/copy_test.go
@@ -24,7 +24,6 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/testapi"
 	apitesting "github.com/GoogleCloudPlatform/kubernetes/pkg/api/testing"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/conversion"
 )
 
 func TestDeepCopyApiObjects(t *testing.T) {
@@ -37,7 +36,7 @@ func TestDeepCopyApiObjects(t *testing.T) {
 					t.Fatalf("Could not create a %s: %s", kind, err)
 				}
 				f.Fuzz(item)
-				itemCopy, err := conversion.DeepCopy(item)
+				itemCopy, err := api.Scheme.DeepCopy(item)
 				if err != nil {
 					t.Errorf("Could not deep copy a %s: %s", kind, err)
 					continue

diff --git a/pkg/controller/framework/fake_controller_source.go b/pkg/controller/framework/fake_controller_source.go
@@ -23,7 +23,6 @@ import (
 	"sync"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/conversion"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/watch"
@@ -130,7 +129,7 @@ func (f *FakeControllerSource) List() (runtime.Object, error) {
 		// Otherwise, if they make a change and write it back, they
 		// will inadvertently change the our canonical copy (in
 		// addition to racing with other clients).
-		objCopy, err := conversion.DeepCopy(obj)
+		objCopy, err := api.Scheme.DeepCopy(obj)
 		if err != nil {
 			return nil, err
 		}
@@ -167,7 +166,7 @@ func (f *FakeControllerSource) Watch(resourceVersion string) (watch.Interface, e
 			// it back, they will inadvertently change the our
 			// canonical copy (in addition to racing with other
 			// clients).
-			objCopy, err := conversion.DeepCopy(c.Object)
+			objCopy, err := api.Scheme.DeepCopy(c.Object)
 			if err != nil {
 				return nil, err
 			}

diff --git a/pkg/conversion/cloner.go b/pkg/conversion/cloner.go
@@ -43,8 +43,12 @@ func NewCloner() *Cloner {
 
 // Prevent recursing into every byte...
 func byteSliceDeepCopy(in []byte, out *[]byte, c *Cloner) error {
-	*out = make([]byte, len(in))
-	copy(*out, in)
+	if in != nil {
+		*out = make([]byte, len(in))
+		copy(*out, in)
+	} else {
+		*out = nil
+	}
 	return nil
 }
 

diff --git a/pkg/conversion/deep_copy.go b/pkg/conversion/deep_copy.go
diff --git a/pkg/conversion/deep_copy_test.go b/pkg/conversion/deep_copy_test.go
@@ -40,7 +40,7 @@ func TestDeepCopy(t *testing.T) {
 		}{},
 	}
 	for _, obj := range table {
-		obj2, err := DeepCopy(obj)
+		obj2, err := NewCloner().DeepCopy(obj)
 		if err != nil {
 			t.Errorf("Error: couldn't copy %#v", obj)
 			continue
@@ -51,7 +51,7 @@ func TestDeepCopy(t *testing.T) {
 
 		obj3 := reflect.New(reflect.TypeOf(obj)).Interface()
 		f.Fuzz(obj3)
-		obj4, err := DeepCopy(obj3)
+		obj4, err := NewCloner().DeepCopy(obj3)
 		if err != nil {
 			t.Errorf("Error: couldn't copy %#v", obj)
 			continue
@@ -64,7 +64,7 @@ func TestDeepCopy(t *testing.T) {
 }
 
 func copyOrDie(t *testing.T, in interface{}) interface{} {
-	out, err := DeepCopy(in)
+	out, err := NewCloner().DeepCopy(in)
 	if err != nil {
 		t.Fatalf("DeepCopy failed: %#q: %v", in, err)
 	}
@@ -154,7 +154,7 @@ func BenchmarkDeepCopy(b *testing.B) {
 	var r interface{}
 	for i := 0; i < b.N; i++ {
 		for j := range table {
-			r, _ = DeepCopy(table[j])
+			r, _ = NewCloner().DeepCopy(table[j])
 		}
 	}
 	result = r

diff --git a/pkg/registry/service/rest_test.go b/pkg/registry/service/rest_test.go
@@ -25,7 +25,6 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest/resttest"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/conversion"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest"
@@ -60,7 +59,7 @@ func makeIPNet(t *testing.T) *net.IPNet {
 }
 
 func deepCloneService(svc *api.Service) *api.Service {
-	value, err := conversion.DeepCopy(svc)
+	value, err := api.Scheme.DeepCopy(svc)
 	if err != nil {
 		panic("couldn't copy service")
 	}

diff --git a/pkg/runtime/deep_copy_generator.go b/pkg/runtime/deep_copy_generator.go
@@ -27,6 +27,11 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 )
 
+// TODO(wojtek-t): As suggested in #8320, we should consider the strategy
+// to first do the shallow copy and then recurse into things that need a
+// deep copy (maps, pointers, slices). That sort of copy function would
+// need one parameter - a pointer to the thing it's supposed to expand,
+// and it would involve a lot less memory copying.
 type DeepCopyGenerator interface {
 	// Adds a type to a generator.
 	// If the type is non-struct, it will return an error, otherwise deep-copy

diff --git a/pkg/runtime/embedded_test.go b/pkg/runtime/embedded_test.go
@@ -21,7 +21,7 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/conversion"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 )
@@ -224,7 +224,7 @@ func TestDeepCopyOfEmbeddedObject(t *testing.T) {
 	}
 	t.Logf("originalRole = %v\n", string(originalData))
 
-	copyOfOriginal, err := conversion.DeepCopy(original)
+	copyOfOriginal, err := api.Scheme.DeepCopy(original)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}

diff --git a/pkg/tools/etcd_helper.go b/pkg/tools/etcd_helper.go
@@ -27,6 +27,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/conversion"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
@@ -238,7 +239,7 @@ func (h *EtcdHelper) getFromCache(index uint64) (runtime.Object, bool) {
 	if found {
 		// We should not return the object itself to avoid poluting the cache if someone
 		// modifies returned values.
-		objCopy, err := conversion.DeepCopy(obj)
+		objCopy, err := api.Scheme.DeepCopy(obj)
 		trace.Step("Deep copied")
 		if err != nil {
 			glog.Errorf("Error during DeepCopy of cached object: %q", err)
@@ -256,7 +257,7 @@ func (h *EtcdHelper) addToCache(index uint64, obj runtime.Object) {
 	defer func() {
 		cacheAddLatency.Observe(float64(time.Since(startTime) / time.Microsecond))
 	}()
-	objCopy, err := conversion.DeepCopy(obj)
+	objCopy, err := api.Scheme.DeepCopy(obj)
 	if err != nil {
 		glog.Errorf("Error during DeepCopy of cached object: %q", err)
 		return