From 0b49deb753aa98d5a3b2a0d638ad354f4b096391 Mon Sep 17 00:00:00 2001 From: Michal Fojtik Date: Fri, 24 Nov 2017 13:04:35 +0100 Subject: [PATCH] replace usage of kapi.Scheme.Copy() with .DeepCopy() --- pkg/apps/strategy/support/lifecycle_test.go | 6 ++---- pkg/build/controller/policy/policy.go | 10 --------- .../controller/policy/serial_latest_only.go | 2 +- pkg/build/generator/generator.go | 6 ++---- pkg/diagnostics/cluster/route_validation.go | 9 +------- .../trigger/image_trigger_controller_test.go | 7 +------ .../deploymentconfigs/deploymentconfigs.go | 6 +----- .../clusterresourcequota/accessor.go | 8 ++----- .../clusterquotamapping_test.go | 10 ++------- .../reconciliation_controller.go | 6 +----- .../apis/route/validation/validation_test.go | 7 +------ pkg/route/registry/route/strategy_test.go | 11 +++++----- ...amespace_security_allocation_controller.go | 21 +++++++------------ 13 files changed, 27 insertions(+), 82 deletions(-) diff --git a/pkg/apps/strategy/support/lifecycle_test.go b/pkg/apps/strategy/support/lifecycle_test.go index 1dc0301d7817..678df089e3cf 100644 --- a/pkg/apps/strategy/support/lifecycle_test.go +++ b/pkg/apps/strategy/support/lifecycle_test.go @@ -109,8 +109,7 @@ func TestHookExecutor_executeExecNewPodSucceeded(t *testing.T) { go func() { <-podCreated podsWatch.Add(createdPod) - podCopy, _ := kapi.Scheme.Copy(createdPod) - updatedPod := podCopy.(*kapi.Pod) + updatedPod := createdPod.DeepCopy() updatedPod.Status.Phase = kapi.PodSucceeded podsWatch.Modify(updatedPod) }() @@ -175,8 +174,7 @@ func TestHookExecutor_executeExecNewPodFailed(t *testing.T) { go func() { <-podCreated podsWatch.Add(createdPod) - podCopy, _ := kapi.Scheme.Copy(createdPod) - updatedPod := podCopy.(*kapi.Pod) + updatedPod := createdPod.DeepCopy() updatedPod.Status.Phase = kapi.PodFailed podsWatch.Modify(updatedPod) }() diff --git a/pkg/build/controller/policy/policy.go b/pkg/build/controller/policy/policy.go index 655ace9e73f1..13818c5b5705 100644 --- a/pkg/build/controller/policy/policy.go +++ b/pkg/build/controller/policy/policy.go @@ -3,8 +3,6 @@ package policy import ( "github.com/golang/glog" - kapi "k8s.io/kubernetes/pkg/api" - buildapi "github.com/openshift/origin/pkg/build/apis/build" buildclient "github.com/openshift/origin/pkg/build/client" buildlister "github.com/openshift/origin/pkg/build/generated/listers/build/internalversion" @@ -107,11 +105,3 @@ func GetNextConfigBuild(lister buildlister.BuildLister, namespace, buildConfigNa } return nextBuilds, hasRunningBuilds, nil } - -func copyOrDie(build *buildapi.Build) *buildapi.Build { - obj, err := kapi.Scheme.Copy(build) - if err != nil { - panic(err) - } - return obj.(*buildapi.Build) -} diff --git a/pkg/build/controller/policy/serial_latest_only.go b/pkg/build/controller/policy/serial_latest_only.go index 04ac4f4cfcd7..83ed8941963f 100644 --- a/pkg/build/controller/policy/serial_latest_only.go +++ b/pkg/build/controller/policy/serial_latest_only.go @@ -80,7 +80,7 @@ func (s *SerialLatestOnlyPolicy) cancelPreviousBuilds(build *buildapi.Build) []e var result = []error{} for _, b := range builds { err := wait.Poll(500*time.Millisecond, 5*time.Second, func() (bool, error) { - b = copyOrDie(b) + b = b.DeepCopy() b.Status.Cancelled = true err := s.BuildUpdater.Update(b.Namespace, b) if err != nil && errors.IsConflict(err) { diff --git a/pkg/build/generator/generator.go b/pkg/build/generator/generator.go index 16af6fd4b548..76581816541c 100644 --- a/pkg/build/generator/generator.go +++ b/pkg/build/generator/generator.go @@ -458,8 +458,7 @@ func (g *BuildGenerator) generateBuildFromConfig(ctx apirequest.Context, bc *bui // Need to copy the buildConfig here so that it doesn't share pointers with // the build object which could be (will be) modified later. buildName := getNextBuildName(bc) - obj, _ := kapi.Scheme.Copy(bc) - bcCopy := obj.(*buildapi.BuildConfig) + bcCopy := bc.DeepCopy() serviceAccount := getServiceAccount(bcCopy, g.DefaultServiceAccountName) t := true build := &buildapi.Build{ @@ -791,8 +790,7 @@ func UpdateCustomImageEnv(strategy *buildapi.CustomBuildStrategy, newImage strin // generateBuildFromBuild creates a new build based on a given Build. func generateBuildFromBuild(build *buildapi.Build, buildConfig *buildapi.BuildConfig) *buildapi.Build { - obj, _ := kapi.Scheme.Copy(build) - buildCopy := obj.(*buildapi.Build) + buildCopy := build.DeepCopy() newBuild := &buildapi.Build{ Spec: buildCopy.Spec, diff --git a/pkg/diagnostics/cluster/route_validation.go b/pkg/diagnostics/cluster/route_validation.go index e9c8555d8f54..a1ff04b9b5b1 100644 --- a/pkg/diagnostics/cluster/route_validation.go +++ b/pkg/diagnostics/cluster/route_validation.go @@ -8,7 +8,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/client-go/rest" - kapi "k8s.io/kubernetes/pkg/api" kapihelper "k8s.io/kubernetes/pkg/api/helper" "k8s.io/kubernetes/pkg/apis/authorization" authorizationtypedclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/authorization/internalversion" @@ -79,13 +78,7 @@ func (d *RouteCertificateValidation) Check() types.DiagnosticResult { } for _, route := range routes.Items { - copied, err := kapi.Scheme.Copy(&route) - if err != nil { - r.Error("DRouCert2003", err, fmt.Errorf("unable to copy route: %v", err).Error()) - return r - } - original := copied.(*routeapi.Route) - + original := route.DeepCopy() errs := validation.ExtendedValidateRoute(&route) if len(errs) == 0 { diff --git a/pkg/image/controller/trigger/image_trigger_controller_test.go b/pkg/image/controller/trigger/image_trigger_controller_test.go index 8bf2b9f44d75..f2dca852b4fb 100644 --- a/pkg/image/controller/trigger/image_trigger_controller_test.go +++ b/pkg/image/controller/trigger/image_trigger_controller_test.go @@ -956,12 +956,7 @@ func updateBuildConfigImages(bc *buildapi.BuildConfig, tagRetriever trigger.TagR continue } if updated == nil { - copied, err := kapi.Scheme.Copy(bc) - if err != nil { - return nil, err - } - bc = copied.(*buildapi.BuildConfig) - updated = bc + updated = bc.DeepCopy() } p = bc.Spec.Triggers[i].ImageChange p.LastTriggeredImageID = latest diff --git a/pkg/image/trigger/deploymentconfigs/deploymentconfigs.go b/pkg/image/trigger/deploymentconfigs/deploymentconfigs.go index d875b77b2a8a..7845e38df6d8 100644 --- a/pkg/image/trigger/deploymentconfigs/deploymentconfigs.go +++ b/pkg/image/trigger/deploymentconfigs/deploymentconfigs.go @@ -164,11 +164,7 @@ func UpdateDeploymentConfigImages(dc *appsapi.DeploymentConfig, tagRetriever tri if updated != nil { return } - copied, err := kapi.Scheme.Copy(dc) - if err != nil { - return - } - dc = copied.(*appsapi.DeploymentConfig) + dc = dc.DeepCopy() updated = dc } diff --git a/pkg/quota/admission/clusterresourcequota/accessor.go b/pkg/quota/admission/clusterresourcequota/accessor.go index 962daaf90503..ffc70a3f312c 100644 --- a/pkg/quota/admission/clusterresourcequota/accessor.go +++ b/pkg/quota/admission/clusterresourcequota/accessor.go @@ -63,13 +63,9 @@ func (e *clusterQuotaAccessor) UpdateQuotaStatus(newQuota *kapi.ResourceQuota) e } clusterQuota = e.checkCache(clusterQuota) - // make a copy - obj, err := kapi.Scheme.Copy(clusterQuota) - if err != nil { - return err - } // re-assign objectmeta - clusterQuota = obj.(*quotaapi.ClusterResourceQuota) + // make a copy + clusterQuota = clusterQuota.DeepCopy() clusterQuota.ObjectMeta = newQuota.ObjectMeta clusterQuota.Namespace = "" diff --git a/pkg/quota/controller/clusterquotamapping/clusterquotamapping_test.go b/pkg/quota/controller/clusterquotamapping/clusterquotamapping_test.go index 879bfd872288..da40df64c4cb 100644 --- a/pkg/quota/controller/clusterquotamapping/clusterquotamapping_test.go +++ b/pkg/quota/controller/clusterquotamapping/clusterquotamapping_test.go @@ -110,10 +110,7 @@ func runFuzzer(t *testing.T) { quota := NewQuota(name) finalQuotas[name] = quota - copied, err := kapi.Scheme.Copy(quota) - if err != nil { - t.Fatal(err) - } + copied := quota.DeepCopy() if exists { quotaActions[name] = append(quotaActions[name], fmt.Sprintf("updating %v to %v", name, quota.Spec.Selector)) quotaWatch.Modify(copied) @@ -149,10 +146,7 @@ func runFuzzer(t *testing.T) { ns := NewNamespace(name) finalNamespaces[name] = ns - copied, err := kapi.Scheme.Copy(ns) - if err != nil { - t.Fatal(err) - } + copied := ns.DeepCopy() if exists { namespaceActions[name] = append(namespaceActions[name], fmt.Sprintf("updating %v to %v", name, ns.Labels)) nsWatch.Modify(copied) diff --git a/pkg/quota/controller/clusterquotareconciliation/reconciliation_controller.go b/pkg/quota/controller/clusterquotareconciliation/reconciliation_controller.go index 5d40b2dcc843..0cc1d69c53c0 100644 --- a/pkg/quota/controller/clusterquotareconciliation/reconciliation_controller.go +++ b/pkg/quota/controller/clusterquotareconciliation/reconciliation_controller.go @@ -248,11 +248,7 @@ func (c *ClusterQuotaReconcilationController) worker() { // syncResourceQuotaFromKey syncs a quota key func (c *ClusterQuotaReconcilationController) syncQuotaForNamespaces(originalQuota *quotaapi.ClusterResourceQuota, workItems []workItem) (error, []workItem /* to retry */) { - obj, err := kapi.Scheme.Copy(originalQuota) - if err != nil { - return err, workItems - } - quota := obj.(*quotaapi.ClusterResourceQuota) + quota := originalQuota.DeepCopy() // get the list of namespaces that match this cluster quota matchingNamespaceNamesList, quotaSelector := c.clusterQuotaMapper.GetNamespacesFor(quota.Name) diff --git a/pkg/route/apis/route/validation/validation_test.go b/pkg/route/apis/route/validation/validation_test.go index efe2550e8b4b..9d9c266d4db8 100644 --- a/pkg/route/apis/route/validation/validation_test.go +++ b/pkg/route/apis/route/validation/validation_test.go @@ -8,7 +8,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/intstr" - kapi "k8s.io/kubernetes/pkg/api" routeapi "github.com/openshift/origin/pkg/route/apis/route" ) @@ -1291,11 +1290,7 @@ func TestValidateRouteUpdate(t *testing.T) { } for i, tc := range tests { - copied, err := kapi.Scheme.Copy(tc.route) - if err != nil { - t.Fatal(err) - } - newRoute := copied.(*routeapi.Route) + newRoute := tc.route.DeepCopy() tc.change(newRoute) errs := ValidateRouteUpdate(newRoute, tc.route) if len(errs) != tc.expectedErrors { diff --git a/pkg/route/registry/route/strategy_test.go b/pkg/route/registry/route/strategy_test.go index 7d677ab8ec7d..d4764a926cce 100644 --- a/pkg/route/registry/route/strategy_test.go +++ b/pkg/route/registry/route/strategy_test.go @@ -9,7 +9,6 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/authentication/user" apirequest "k8s.io/apiserver/pkg/endpoints/request" - kapi "k8s.io/kubernetes/pkg/api" authorizationapi "k8s.io/kubernetes/pkg/apis/authorization" routeapi "github.com/openshift/origin/pkg/route/apis/route" @@ -88,21 +87,21 @@ func TestEmptyDefaultCACertificate(t *testing.T) { }, } for i, testCase := range testCases { - copied, _ := kapi.Scheme.Copy(testCase.route) - if err := DecorateLegacyRouteWithEmptyDestinationCACertificates(copied.(*routeapi.Route)); err != nil { + copied := testCase.route.DeepCopy() + if err := DecorateLegacyRouteWithEmptyDestinationCACertificates(copied); err != nil { t.Errorf("%d: unexpected error: %v", i, err) continue } - routeStrategy{}.PrepareForCreate(nil, copied.(*routeapi.Route)) + routeStrategy{}.PrepareForCreate(nil, copied) if !reflect.DeepEqual(testCase.route, copied) { t.Errorf("%d: unexpected change: %#v", i, copied) continue } - if err := DecorateLegacyRouteWithEmptyDestinationCACertificates(copied.(*routeapi.Route)); err != nil { + if err := DecorateLegacyRouteWithEmptyDestinationCACertificates(copied); err != nil { t.Errorf("%d: unexpected error: %v", i, err) continue } - routeStrategy{}.PrepareForUpdate(nil, copied.(*routeapi.Route), &routeapi.Route{}) + routeStrategy{}.PrepareForUpdate(nil, copied, &routeapi.Route{}) if !reflect.DeepEqual(testCase.route, copied) { t.Errorf("%d: unexpected change: %#v", i, copied) continue diff --git a/pkg/security/controller/namespace_security_allocation_controller.go b/pkg/security/controller/namespace_security_allocation_controller.go index 1aa19406530d..fcb1016b1aa2 100644 --- a/pkg/security/controller/namespace_security_allocation_controller.go +++ b/pkg/security/controller/namespace_security_allocation_controller.go @@ -11,7 +11,6 @@ import ( kcoreclient "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" - kapi "k8s.io/kubernetes/pkg/api" "github.com/golang/glog" "github.com/openshift/origin/pkg/security" @@ -157,14 +156,10 @@ func (c *NamespaceSecurityDefaultsController) allocate(ns *v1.Namespace) error { return nil } - obj, err := kapi.Scheme.Copy(ns) - if err != nil { - return err - } - ns = obj.(*v1.Namespace) + nsCopy := ns.DeepCopy() - if ns.Annotations == nil { - ns.Annotations = make(map[string]string) + if nsCopy.Annotations == nil { + nsCopy.Annotations = make(map[string]string) } // do uid allocation @@ -174,15 +169,15 @@ func (c *NamespaceSecurityDefaultsController) allocate(ns *v1.Namespace) error { } tx.Add(func() error { return c.uidAllocator.Release(block) }) - ns.Annotations[security.UIDRangeAnnotation] = block.String() - ns.Annotations[security.SupplementalGroupsAnnotation] = block.String() - if _, ok := ns.Annotations[security.MCSAnnotation]; !ok { + nsCopy.Annotations[security.UIDRangeAnnotation] = block.String() + nsCopy.Annotations[security.SupplementalGroupsAnnotation] = block.String() + if _, ok := nsCopy.Annotations[security.MCSAnnotation]; !ok { if label := c.mcsAllocator(block); label != nil { - ns.Annotations[security.MCSAnnotation] = label.String() + nsCopy.Annotations[security.MCSAnnotation] = label.String() } } - _, err = c.client.Update(ns) + _, err = c.client.Update(nsCopy) if err == nil { tx.Commit() }