Skip to content

Commit 33c21b7

Browse files
Merge pull request #17444 from mfojtik/fix-deep-copy
Automatic merge from submit-queue. Switch to use .DeepCopy() instead of kapi.Scheme.DeepCopy() The later will not work in k8s 1.9 (@sttts to confirm). The first commit is just mechanical update. The second commit is needed because image trigger controller used DeepCopy on interface{}. @smarterclayton I switched that to be `runtime.Object` and nothing seems to be complaining. PTAL.
2 parents 2d4b47a + 0b49deb commit 33c21b7

File tree

45 files changed

+142
-362
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+142
-362
lines changed

pkg/apps/controller/deployer/deployer_controller.go

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -490,16 +490,7 @@ func (c *DeploymentController) setDeployerPodsOwnerRef(deployment *v1.Replicatio
490490
continue
491491
}
492492
glog.V(4).Infof("setting ownerRef for pod %s/%s to deployment %s/%s", pod.Namespace, pod.Name, deployment.Namespace, deployment.Name)
493-
objCopy, err := kapi.Scheme.DeepCopy(pod)
494-
if err != nil {
495-
errors = append(errors, err)
496-
continue
497-
}
498-
newPod, ok := objCopy.(*v1.Pod)
499-
if !ok {
500-
errors = append(errors, fmt.Errorf("object %#+v is not a pod", objCopy))
501-
continue
502-
}
493+
newPod := pod.DeepCopy()
503494
newPod.SetOwnerReferences([]metav1.OwnerReference{{
504495
APIVersion: "v1",
505496
Name: deployment.Name,

pkg/apps/strategy/support/lifecycle.go

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -381,20 +381,8 @@ func makeHookPod(hook *deployapi.LifecycleHook, rc *kapi.ReplicationController,
381381
}
382382

383383
gracePeriod := int64(10)
384-
385-
var podSecurityContextCopy *kapi.PodSecurityContext
386-
if ctx, err := kapi.Scheme.DeepCopy(rc.Spec.Template.Spec.SecurityContext); err != nil {
387-
return nil, fmt.Errorf("unable to copy pod securityContext: %v", err)
388-
} else {
389-
podSecurityContextCopy = ctx.(*kapi.PodSecurityContext)
390-
}
391-
392-
var securityContextCopy *kapi.SecurityContext
393-
if ctx, err := kapi.Scheme.DeepCopy(baseContainer.SecurityContext); err != nil {
394-
return nil, fmt.Errorf("unable to copy securityContext: %v", err)
395-
} else {
396-
securityContextCopy = ctx.(*kapi.SecurityContext)
397-
}
384+
podSecurityContextCopy := rc.Spec.Template.Spec.SecurityContext.DeepCopy()
385+
securityContextCopy := baseContainer.SecurityContext.DeepCopy()
398386

399387
pod := &kapi.Pod{
400388
ObjectMeta: metav1.ObjectMeta{

pkg/apps/strategy/support/lifecycle_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,7 @@ func TestHookExecutor_executeExecNewPodSucceeded(t *testing.T) {
109109
go func() {
110110
<-podCreated
111111
podsWatch.Add(createdPod)
112-
podCopy, _ := kapi.Scheme.Copy(createdPod)
113-
updatedPod := podCopy.(*kapi.Pod)
112+
updatedPod := createdPod.DeepCopy()
114113
updatedPod.Status.Phase = kapi.PodSucceeded
115114
podsWatch.Modify(updatedPod)
116115
}()
@@ -175,8 +174,7 @@ func TestHookExecutor_executeExecNewPodFailed(t *testing.T) {
175174
go func() {
176175
<-podCreated
177176
podsWatch.Add(createdPod)
178-
podCopy, _ := kapi.Scheme.Copy(createdPod)
179-
updatedPod := podCopy.(*kapi.Pod)
177+
updatedPod := createdPod.DeepCopy()
180178
updatedPod.Status.Phase = kapi.PodFailed
181179
podsWatch.Modify(updatedPod)
182180
}()

pkg/authorization/rulevalidation/compact_rules_test.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77

88
"k8s.io/apimachinery/pkg/runtime/schema"
99
"k8s.io/apimachinery/pkg/util/sets"
10-
kapi "k8s.io/kubernetes/pkg/api"
1110

1211
authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
1312
)
@@ -106,10 +105,9 @@ func TestCompactRules(t *testing.T) {
106105

107106
for k, tc := range testcases {
108107
rules := tc.Rules
109-
originalRules, err := kapi.Scheme.DeepCopy(tc.Rules)
110-
if err != nil {
111-
t.Errorf("%s: couldn't copy rules: %v", k, err)
112-
continue
108+
originalRules := make([]authorizationapi.PolicyRule, len(tc.Rules))
109+
for i, r := range tc.Rules {
110+
r.DeepCopyInto(&originalRules[i])
113111
}
114112
compacted, err := CompactRules(tc.Rules)
115113
if err != nil {

pkg/build/apis/build/validation/validation.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,7 @@ func ValidateBuildUpdate(build *buildapi.Build, older *buildapi.Build) field.Err
4444
}
4545

4646
// lie about the old build's pushsecret value so we can allow it to be updated.
47-
olderCopy, err := buildutil.BuildDeepCopy(older)
48-
if err != nil {
49-
glog.V(2).Infof("Error copying build for update validation: %v", err)
50-
allErrs = append(allErrs, field.InternalError(field.NewPath(""), fmt.Errorf("Unable to copy build for update validation: %v", err)))
51-
}
47+
olderCopy := older.DeepCopy()
5248
olderCopy.Spec.Output.PushSecret = build.Spec.Output.PushSecret
5349

5450
if !kapihelper.Semantic.DeepEqual(build.Spec, olderCopy.Spec) {

pkg/build/controller/build/build_controller.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -748,10 +748,9 @@ func (bc *BuildController) createBuildPod(build *buildapi.Build) (*buildUpdate,
748748

749749
// image reference resolution requires a copy of the build
750750
var err error
751-
build, err = buildutil.BuildDeepCopy(build)
752-
if err != nil {
753-
return nil, fmt.Errorf("unable to copy build %s: %v", buildDesc(build), err)
754-
}
751+
752+
// TODO: Rename this to buildCopy
753+
build = build.DeepCopy()
755754

756755
// Resolve all Docker image references to valid values.
757756
if err := bc.resolveImageReferences(build, update); err != nil {
@@ -1085,10 +1084,7 @@ func (bc *BuildController) handleBuildConfig(bcNamespace string, bcName string)
10851084
// and applies that patch using the REST client
10861085
func (bc *BuildController) patchBuild(build *buildapi.Build, update *buildUpdate) (*buildapi.Build, error) {
10871086
// Create a patch using the buildUpdate object
1088-
updatedBuild, err := buildutil.BuildDeepCopy(build)
1089-
if err != nil {
1090-
return nil, fmt.Errorf("cannot create a deep copy of build %s: %v", buildDesc(build), err)
1091-
}
1087+
updatedBuild := build.DeepCopy()
10921088
update.apply(updatedBuild)
10931089

10941090
patch, err := validation.CreateBuildPatch(build, updatedBuild)

pkg/build/controller/build/build_controller_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -371,10 +371,7 @@ func TestHandleBuild(t *testing.T) {
371371
t.Errorf("%s: did not get an update. Expected: %v", tc.name, tc.expectUpdate)
372372
return
373373
}
374-
expectedBuild, err := buildutil.BuildDeepCopy(tc.build)
375-
if err != nil {
376-
t.Fatalf("unexpected: %v", err)
377-
}
374+
expectedBuild := tc.build.DeepCopy()
378375
tc.expectUpdate.apply(expectedBuild)
379376

380377
// For start/completion/duration fields, simply validate that they are set/not set

pkg/build/controller/policy/policy.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package policy
33
import (
44
"github.com/golang/glog"
55

6-
kapi "k8s.io/kubernetes/pkg/api"
7-
86
buildapi "github.com/openshift/origin/pkg/build/apis/build"
97
buildclient "github.com/openshift/origin/pkg/build/client"
108
buildlister "github.com/openshift/origin/pkg/build/generated/listers/build/internalversion"
@@ -107,11 +105,3 @@ func GetNextConfigBuild(lister buildlister.BuildLister, namespace, buildConfigNa
107105
}
108106
return nextBuilds, hasRunningBuilds, nil
109107
}
110-
111-
func copyOrDie(build *buildapi.Build) *buildapi.Build {
112-
obj, err := kapi.Scheme.Copy(build)
113-
if err != nil {
114-
panic(err)
115-
}
116-
return obj.(*buildapi.Build)
117-
}

pkg/build/controller/policy/serial_latest_only.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func (s *SerialLatestOnlyPolicy) cancelPreviousBuilds(build *buildapi.Build) []e
8080
var result = []error{}
8181
for _, b := range builds {
8282
err := wait.Poll(500*time.Millisecond, 5*time.Second, func() (bool, error) {
83-
b = copyOrDie(b)
83+
b = b.DeepCopy()
8484
b.Status.Cancelled = true
8585
err := s.BuildUpdater.Update(b.Namespace, b)
8686
if err != nil && errors.IsConflict(err) {

pkg/build/generator/generator.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -458,8 +458,7 @@ func (g *BuildGenerator) generateBuildFromConfig(ctx apirequest.Context, bc *bui
458458
// Need to copy the buildConfig here so that it doesn't share pointers with
459459
// the build object which could be (will be) modified later.
460460
buildName := getNextBuildName(bc)
461-
obj, _ := kapi.Scheme.Copy(bc)
462-
bcCopy := obj.(*buildapi.BuildConfig)
461+
bcCopy := bc.DeepCopy()
463462
serviceAccount := getServiceAccount(bcCopy, g.DefaultServiceAccountName)
464463
t := true
465464
build := &buildapi.Build{
@@ -791,8 +790,7 @@ func UpdateCustomImageEnv(strategy *buildapi.CustomBuildStrategy, newImage strin
791790

792791
// generateBuildFromBuild creates a new build based on a given Build.
793792
func generateBuildFromBuild(build *buildapi.Build, buildConfig *buildapi.BuildConfig) *buildapi.Build {
794-
obj, _ := kapi.Scheme.Copy(build)
795-
buildCopy := obj.(*buildapi.Build)
793+
buildCopy := build.DeepCopy()
796794

797795
newBuild := &buildapi.Build{
798796
Spec: buildCopy.Spec,

0 commit comments

Comments
 (0)