Merge pull request #50684 from sttts/sttts-deepcopy-calls-registry

Automatic merge from submit-queue (batch tested with PRs 50626, 50683, 50679, 50684, 50460)

registries: simplify deepcopy calls

pkg/registry/core/replicationcontroller/strategy_test.go

@@ -186,14 +186,8 @@ func TestValidateUpdate(t *testing.T) {
 	oldController.Annotations[api.NonConvertibleAnnotationPrefix+"/"+"spec.selector"] = "no way"
 
 	// Deep-copy so we won't mutate both selectors.
-	objCopy, err := api.Scheme.DeepCopy(oldController)
+	newController := oldController.DeepCopy()
-	if err != nil {
+
-		t.Fatalf("unexpected deep-copy error: %v", err)
-	}
-	newController, ok := objCopy.(*api.ReplicationController)
-	if !ok {
-		t.Fatalf("unexpected object: %#v", objCopy)
-	}
 	// Irrelevant (to the selector) update for the replication controller.
 	newController.Spec.Replicas = 5
 

pkg/registry/core/service/rest_test.go

@@ -76,14 +76,6 @@ func makeIPNet(t *testing.T) *net.IPNet {
 	return net
 }
 
-func deepCloneService(svc *api.Service) *api.Service {
-	value, err := api.Scheme.DeepCopy(svc)
-	if err != nil {
-		panic("couldn't copy service")
-	}
-	return value.(*api.Service)
-}
-
 func TestServiceRegistryCreate(t *testing.T) {
 	storage, registry := NewTestREST(t, nil)
 
@@ -505,14 +497,14 @@ func TestServiceRegistryUpdateExternalService(t *testing.T) {
 	}
 
 	// Modify load balancer to be external.
-	svc2 := deepCloneService(svc1)
+	svc2 := svc1.DeepCopy()
 	svc2.Spec.Type = api.ServiceTypeLoadBalancer
 	if _, _, err := storage.Update(ctx, svc2.Name, rest.DefaultUpdatedObjectInfo(svc2, api.Scheme)); err != nil {
 		t.Fatalf("Unexpected error: %v", err)
 	}
 
 	// Change port.
-	svc3 := deepCloneService(svc2)
+	svc3 := svc2.DeepCopy()
 	svc3.Spec.Ports[0].Port = 6504
 	if _, _, err := storage.Update(ctx, svc3.Name, rest.DefaultUpdatedObjectInfo(svc3, api.Scheme)); err != nil {
 		t.Fatalf("Unexpected error: %v", err)
@@ -548,7 +540,7 @@ func TestServiceRegistryUpdateMultiPortExternalService(t *testing.T) {
 	}
 
 	// Modify ports
-	svc2 := deepCloneService(svc1)
+	svc2 := svc1.DeepCopy()
 	svc2.Spec.Ports[1].Port = 8088
 	if _, _, err := storage.Update(ctx, svc2.Name, rest.DefaultUpdatedObjectInfo(svc2, api.Scheme)); err != nil {
 		t.Fatalf("Unexpected error: %v", err)
@@ -886,7 +878,7 @@ func TestServiceRegistryIPUpdate(t *testing.T) {
 		t.Errorf("Unexpected ClusterIP: %s", created_service.Spec.ClusterIP)
 	}
 
-	update := deepCloneService(created_service)
+	update := created_service.DeepCopy()
 	update.Spec.Ports[0].Port = 6503
 
 	updated_svc, _, _ := storage.Update(ctx, update.Name, rest.DefaultUpdatedObjectInfo(update, api.Scheme))
@@ -904,7 +896,7 @@ func TestServiceRegistryIPUpdate(t *testing.T) {
 		}
 	}
 
-	update = deepCloneService(created_service)
+	update = created_service.DeepCopy()
 	update.Spec.Ports[0].Port = 6503
 	update.Spec.ClusterIP = testIP // Error: Cluster IP is immutable
 
@@ -940,7 +932,7 @@ func TestServiceRegistryIPLoadBalancer(t *testing.T) {
 		t.Errorf("Unexpected ClusterIP: %s", created_service.Spec.ClusterIP)
 	}
 
-	update := deepCloneService(created_service)
+	update := created_service.DeepCopy()
 
 	_, _, err := storage.Update(ctx, update.Name, rest.DefaultUpdatedObjectInfo(update, api.Scheme))
 	if err != nil {

pkg/registry/rbac/reconciliation/BUILD

@@ -29,6 +29,7 @@ go_library(
         "reconcile_rolebindings.go",
         "role_interfaces.go",
         "rolebinding_interfaces.go",
+        "zz_generated.deepcopy.go",
     ],
     deps = [
         "//pkg/api:go_default_library",
@@ -38,6 +39,7 @@ go_library(
         "//pkg/registry/rbac/validation:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/conversion:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
     ],
 )

pkg/registry/rbac/reconciliation/clusterrole_interfaces.go

@@ -22,6 +22,9 @@ import (
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/rbac/internalversion"
 )
 
+// +k8s:deepcopy-gen=true
+// +k8s:deepcopy-gen:interfaces=k8s.io/kubernetes/pkg/registry/rbac/reconciliation.RuleOwner
+// +k8s:deepcopy-gen:nonpointer-interfaces=true
 type ClusterRoleRuleOwner struct {
 	ClusterRole *rbac.ClusterRole
 }

pkg/registry/rbac/reconciliation/clusterrolebinding_interfaces.go

@@ -23,6 +23,9 @@ import (
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/rbac/internalversion"
 )
 
+// +k8s:deepcopy-gen=true
+// +k8s:deepcopy-gen:interfaces=k8s.io/kubernetes/pkg/registry/rbac/reconciliation.RoleBinding
+// +k8s:deepcopy-gen:nonpointer-interfaces=true
 type ClusterRoleBindingAdapter struct {
 	ClusterRoleBinding *rbac.ClusterRoleBinding
 }

pkg/registry/rbac/reconciliation/reconcile_role.go

@@ -21,7 +21,6 @@ import (
 	"reflect"
 
 	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/apis/rbac"
 	"k8s.io/kubernetes/pkg/registry/rbac/validation"
 )
@@ -50,6 +49,7 @@ type RuleOwner interface {
 	SetAnnotations(map[string]string)
 	GetRules() []rbac.PolicyRule
 	SetRules([]rbac.PolicyRule)
+	DeepCopyRuleOwner() RuleOwner
 }
 
 type ReconcileRoleOptions struct {
@@ -165,11 +165,7 @@ func computeReconciledRole(existing, expected RuleOwner, removeExtraPermissions
 	result.Protected = (existing.GetAnnotations()[rbac.AutoUpdateAnnotationKey] == "false")
 
 	// Start with a copy of the existing object
-	changedObj, err := api.Scheme.DeepCopy(existing)
+	result.Role = existing.DeepCopyRuleOwner()
-	if err != nil {
-		return nil, err
-	}
-	result.Role = changedObj.(RuleOwner)
 
 	// Merge expected annotations and labels
 	result.Role.SetAnnotations(merge(expected.GetAnnotations(), result.Role.GetAnnotations()))

pkg/registry/rbac/reconciliation/reconcile_rolebindings.go

@@ -22,7 +22,6 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/apis/rbac"
 )
 
@@ -44,6 +43,7 @@ type RoleBinding interface {
 	GetRoleRef() rbac.RoleRef
 	GetSubjects() []rbac.Subject
 	SetSubjects([]rbac.Subject)
+	DeepCopyRoleBinding() RoleBinding
 }
 
 // ReconcileRoleBindingOptions holds options for running a role binding reconciliation
@@ -184,11 +184,7 @@ func computeReconciledRoleBinding(existing, expected RoleBinding, removeExtraSub
 	}
 
 	// Start with a copy of the existing object
-	changedObj, err := api.Scheme.DeepCopy(existing)
+	result.RoleBinding = existing.DeepCopyRoleBinding()
-	if err != nil {
-		return nil, err
-	}
-	result.RoleBinding = changedObj.(RoleBinding)
 
 	// Merge expected annotations and labels
 	result.RoleBinding.SetAnnotations(merge(expected.GetAnnotations(), result.RoleBinding.GetAnnotations()))

pkg/registry/rbac/reconciliation/role_interfaces.go

@@ -25,6 +25,9 @@ import (
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/rbac/internalversion"
 )
 
+// +k8s:deepcopy-gen=true
+// +k8s:deepcopy-gen:interfaces=k8s.io/kubernetes/pkg/registry/rbac/reconciliation.RuleOwner
+// +k8s:deepcopy-gen:nonpointer-interfaces=true
 type RoleRuleOwner struct {
 	Role *rbac.Role
 }

pkg/registry/rbac/reconciliation/rolebinding_interfaces.go

@@ -26,6 +26,9 @@ import (
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/rbac/internalversion"
 )
 
+// +k8s:deepcopy-gen=true
+// +k8s:deepcopy-gen:interfaces=k8s.io/kubernetes/pkg/registry/rbac/reconciliation.RoleBinding
+// +k8s:deepcopy-gen:nonpointer-interfaces=true
 type RoleBindingAdapter struct {
 	RoleBinding *rbac.RoleBinding
 }

pkg/registry/rbac/reconciliation/zz_generated.deepcopy.go

@@ -0,0 +1,171 @@
+// +build !ignore_autogenerated
+
+/*
+Copyright 2017 The Kubernetes Authors.
+
+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.
+*/
+
+// This file was autogenerated by deepcopy-gen. Do not edit it manually!
+
+package reconciliation
+
+import (
+	conversion "k8s.io/apimachinery/pkg/conversion"
+	rbac "k8s.io/kubernetes/pkg/apis/rbac"
+	reflect "reflect"
+)
+
+// GetGeneratedDeepCopyFuncs returns the generated funcs, since we aren't registering them.
+//
+// Deprecated: deepcopy registration will go away when static deepcopy is fully implemented.
+func GetGeneratedDeepCopyFuncs() []conversion.GeneratedDeepCopyFunc {
+	return []conversion.GeneratedDeepCopyFunc{
+		{Fn: func(in interface{}, out interface{}, c *conversion.Cloner) error {
+			in.(*ClusterRoleBindingAdapter).DeepCopyInto(out.(*ClusterRoleBindingAdapter))
+			return nil
+		}, InType: reflect.TypeOf(&ClusterRoleBindingAdapter{})},
+		{Fn: func(in interface{}, out interface{}, c *conversion.Cloner) error {
+			in.(*ClusterRoleRuleOwner).DeepCopyInto(out.(*ClusterRoleRuleOwner))
+			return nil
+		}, InType: reflect.TypeOf(&ClusterRoleRuleOwner{})},
+		{Fn: func(in interface{}, out interface{}, c *conversion.Cloner) error {
+			in.(*RoleBindingAdapter).DeepCopyInto(out.(*RoleBindingAdapter))
+			return nil
+		}, InType: reflect.TypeOf(&RoleBindingAdapter{})},
+		{Fn: func(in interface{}, out interface{}, c *conversion.Cloner) error {
+			in.(*RoleRuleOwner).DeepCopyInto(out.(*RoleRuleOwner))
+			return nil
+		}, InType: reflect.TypeOf(&RoleRuleOwner{})},
+	}
+}
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *ClusterRoleBindingAdapter) DeepCopyInto(out *ClusterRoleBindingAdapter) {
+	*out = *in
+	if in.ClusterRoleBinding != nil {
+		in, out := &in.ClusterRoleBinding, &out.ClusterRoleBinding
+		if *in == nil {
+			*out = nil
+		} else {
+			*out = new(rbac.ClusterRoleBinding)
+			(*in).DeepCopyInto(*out)
+		}
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterRoleBindingAdapter.
+func (in *ClusterRoleBindingAdapter) DeepCopy() *ClusterRoleBindingAdapter {
+	if in == nil {
+		return nil
+	}
+	out := new(ClusterRoleBindingAdapter)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyRoleBinding is an autogenerated deepcopy function, copying the receiver, creating a new RoleBinding.
+func (in ClusterRoleBindingAdapter) DeepCopyRoleBinding() RoleBinding {
+	return *in.DeepCopy()
+}
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *ClusterRoleRuleOwner) DeepCopyInto(out *ClusterRoleRuleOwner) {
+	*out = *in
+	if in.ClusterRole != nil {
+		in, out := &in.ClusterRole, &out.ClusterRole
+		if *in == nil {
+			*out = nil
+		} else {
+			*out = new(rbac.ClusterRole)
+			(*in).DeepCopyInto(*out)
+		}
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterRoleRuleOwner.
+func (in *ClusterRoleRuleOwner) DeepCopy() *ClusterRoleRuleOwner {
+	if in == nil {
+		return nil
+	}
+	out := new(ClusterRoleRuleOwner)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyRuleOwner is an autogenerated deepcopy function, copying the receiver, creating a new RuleOwner.
+func (in ClusterRoleRuleOwner) DeepCopyRuleOwner() RuleOwner {
+	return *in.DeepCopy()
+}
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *RoleBindingAdapter) DeepCopyInto(out *RoleBindingAdapter) {
+	*out = *in
+	if in.RoleBinding != nil {
+		in, out := &in.RoleBinding, &out.RoleBinding
+		if *in == nil {
+			*out = nil
+		} else {
+			*out = new(rbac.RoleBinding)
+			(*in).DeepCopyInto(*out)
+		}
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoleBindingAdapter.
+func (in *RoleBindingAdapter) DeepCopy() *RoleBindingAdapter {
+	if in == nil {
+		return nil
+	}
+	out := new(RoleBindingAdapter)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyRoleBinding is an autogenerated deepcopy function, copying the receiver, creating a new RoleBinding.
+func (in RoleBindingAdapter) DeepCopyRoleBinding() RoleBinding {
+	return *in.DeepCopy()
+}
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *RoleRuleOwner) DeepCopyInto(out *RoleRuleOwner) {
+	*out = *in
+	if in.Role != nil {
+		in, out := &in.Role, &out.Role
+		if *in == nil {
+			*out = nil
+		} else {
+			*out = new(rbac.Role)
+			(*in).DeepCopyInto(*out)
+		}
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoleRuleOwner.
+func (in *RoleRuleOwner) DeepCopy() *RoleRuleOwner {
+	if in == nil {
+		return nil
+	}
+	out := new(RoleRuleOwner)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyRuleOwner is an autogenerated deepcopy function, copying the receiver, creating a new RuleOwner.
+func (in RoleRuleOwner) DeepCopyRuleOwner() RuleOwner {
+	return *in.DeepCopy()
+}

pkg/registry/rbac/validation/BUILD

@@ -15,7 +15,6 @@ go_test(
     ],
     library = ":go_default_library",
     deps = [
-        "//pkg/api:go_default_library",
         "//pkg/apis/rbac:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library",
@@ -31,7 +30,6 @@ go_library(
         "rule.go",
     ],
     deps = [
-        "//pkg/api:go_default_library",
         "//pkg/apis/rbac:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",

pkg/registry/rbac/validation/policy_compact.go

@@ -17,10 +17,8 @@ limitations under the License.
 package validation
 
 import (
-	"fmt"
 	"reflect"
 
-	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/apis/rbac"
 )
 
@@ -47,17 +45,7 @@ func CompactRules(rules []rbac.PolicyRule) ([]rbac.PolicyRule, error) {
 				existingRule.Verbs = append(existingRule.Verbs, rule.Verbs...)
 			} else {
 				// Copy the rule to accumulate matching simple resource rules into
-				objCopy, err := api.Scheme.DeepCopy(rule)
+				simpleRules[resource] = rule.DeepCopy()
-				if err != nil {
-					// Unit tests ensure this should not ever happen
-					return nil, err
-				}
-				ruleCopy, ok := objCopy.(rbac.PolicyRule)
-				if !ok {
-					// Unit tests ensure this should not ever happen
-					return nil, fmt.Errorf("expected rbac.PolicyRule, got %#v", objCopy)
-				}
-				simpleRules[resource] = &ruleCopy
 			}
 		} else {
 			compacted = append(compacted, rule)

pkg/registry/rbac/validation/policy_compact_test.go

@@ -21,7 +21,6 @@ import (
 	"sort"
 	"testing"
 
-	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/apis/rbac"
 )
 
@@ -113,10 +112,9 @@ func TestCompactRules(t *testing.T) {
 
 	for k, tc := range testcases {
 		rules := tc.Rules
-		originalRules, err := api.Scheme.DeepCopy(tc.Rules)
+		originalRules := make([]rbac.PolicyRule, len(tc.Rules))
-		if err != nil {
+		for i := range tc.Rules {
-			t.Errorf("%s: couldn't copy rules: %v", k, err)
+			originalRules[i] = *tc.Rules[i].DeepCopy()
-			continue
 		}
 		compacted, err := CompactRules(tc.Rules)
 		if err != nil {

pkg/registry/registrytest/service.go

@@ -76,12 +76,7 @@ func (r *ServiceRegistry) CreateService(ctx genericapirequest.Context, svc *api.
 	r.mu.Lock()
 	defer r.mu.Unlock()
 
-	r.Service = new(api.Service)
+	r.Service = svc.DeepCopy()
-	clone, err := api.Scheme.DeepCopy(svc)
-	if err != nil {
-		return nil, err
-	}
-	r.Service = clone.(*api.Service)
 
 	r.List.Items = append(r.List.Items, *svc)
 	return svc, r.Err