From ed586da1476a50790b75f6013ab9c679d40b105f Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 6 Oct 2017 13:14:34 +0200 Subject: [PATCH] apimachinery: remove Scheme.DeepCopy --- .../job/jobcontroller.go | 6 +-- federation/test/e2e/job.go | 4 +- pkg/apis/apps/validation/validation_test.go | 9 +--- pkg/kubelet/cm/node_container_manager.go | 12 +---- pkg/kubelet/config/config_test.go | 14 ++---- pkg/kubelet/kubelet_node_status.go | 24 +++------- pkg/kubelet/kubelet_resources.go | 11 +---- pkg/kubelet/status/status_manager.go | 36 +++------------ .../desired_state_of_world_populator.go | 14 +----- pkg/util/taints/taints.go | 12 +---- .../apimachinery/pkg/api/resource/quantity.go | 1 + .../pkg/api/resource/zz_generated.deepcopy.go | 44 +++++++++++++++++++ .../k8s.io/apimachinery/pkg/runtime/scheme.go | 5 --- test/e2e_node/cpu_manager_test.go | 5 +-- 14 files changed, 73 insertions(+), 124 deletions(-) create mode 100644 staging/src/k8s.io/apimachinery/pkg/api/resource/zz_generated.deepcopy.go diff --git a/federation/pkg/federation-controller/job/jobcontroller.go b/federation/pkg/federation-controller/job/jobcontroller.go index d868264b3a..783badfd75 100644 --- a/federation/pkg/federation-controller/job/jobcontroller.go +++ b/federation/pkg/federation-controller/job/jobcontroller.go @@ -392,11 +392,11 @@ func (fjc *FederationJobController) reconcileJob(key string) (reconciliationStat } // Create a copy before modifying the obj to prevent race condition with other readers of obj from store. - obj, err := api.Scheme.DeepCopy(objFromStore) - fjob, ok := obj.(*batchv1.Job) - if err != nil || !ok { + fjob, ok := objFromStore.(*batchv1.Job) + if !ok { return statusError, err } + fjob = fjob.DeepCopy() // delete job if fjob.DeletionTimestamp != nil { diff --git a/federation/test/e2e/job.go b/federation/test/e2e/job.go index 023daeebd7..5e1e3a6792 100644 --- a/federation/test/e2e/job.go +++ b/federation/test/e2e/job.go @@ -34,7 +34,6 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/kubernetes/pkg/api" ) const ( @@ -221,8 +220,7 @@ func waitForJob(c *fedclientset.Clientset, namespace string, jobName string, clu } func verifyJob(fedJob, localJob *batchv1.Job) bool { - localJobObj, _ := api.Scheme.DeepCopy(localJob) - localJob = localJobObj.(*batchv1.Job) + localJob = localJob.DeepCopy() localJob.Spec.ManualSelector = fedJob.Spec.ManualSelector localJob.Spec.Completions = fedJob.Spec.Completions localJob.Spec.Parallelism = fedJob.Spec.Parallelism diff --git a/pkg/apis/apps/validation/validation_test.go b/pkg/apis/apps/validation/validation_test.go index 1ed420a753..306fddc7c4 100644 --- a/pkg/apis/apps/validation/validation_test.go +++ b/pkg/apis/apps/validation/validation_test.go @@ -430,14 +430,7 @@ func TestValidateStatefulSetUpdate(t *testing.T) { }, } - obj, err := api.Scheme.DeepCopy(validPodTemplate) - if err != nil { - t.Errorf("failure during test setup when copying PodTemplate: %v", err) - } - addContainersValidTemplate, ok := obj.(api.PodTemplate) - if !ok { - t.Errorf("failure during test setup, copied pod template is not a pod template") - } + addContainersValidTemplate := validPodTemplate.DeepCopy() addContainersValidTemplate.Template.Spec.Containers = append(addContainersValidTemplate.Template.Spec.Containers, api.Container{Name: "def", Image: "image2", ImagePullPolicy: "IfNotPresent"}) if len(addContainersValidTemplate.Template.Spec.Containers) != len(validPodTemplate.Template.Spec.Containers)+1 { diff --git a/pkg/kubelet/cm/node_container_manager.go b/pkg/kubelet/cm/node_container_manager.go index f587eaa4f2..66e0d82467 100644 --- a/pkg/kubelet/cm/node_container_manager.go +++ b/pkg/kubelet/cm/node_container_manager.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/types" utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/kubernetes/pkg/api" kubefeatures "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/events" evictionapi "k8s.io/kubernetes/pkg/kubelet/eviction/api" @@ -238,16 +237,7 @@ func (cm *containerManagerImpl) validateNodeAllocatable() error { var errors []string nar := cm.GetNodeAllocatableReservation() for k, v := range nar { - capacityClone, err := api.Scheme.DeepCopy(cm.capacity[k]) - if err != nil { - errors = append(errors, fmt.Sprintf("DeepCopy capacity error")) - } - value, ok := capacityClone.(resource.Quantity) - if !ok { - return fmt.Errorf( - "failed to cast object %#v to Quantity", - capacityClone) - } + value := cm.capacity[k].DeepCopy() value.Sub(v) if value.Sign() < 0 { diff --git a/pkg/kubelet/config/config_test.go b/pkg/kubelet/config/config_test.go index b424041c82..1e93e5f2ab 100644 --- a/pkg/kubelet/config/config_test.go +++ b/pkg/kubelet/config/config_test.go @@ -370,12 +370,9 @@ func TestPodUpdateAnnotations(t *testing.T) { pod.Annotations = make(map[string]string, 0) pod.Annotations["kubernetes.io/blah"] = "blah" - clone, err := scheme.Scheme.DeepCopy(pod) - if err != nil { - t.Fatalf("%v", err) - } + clone := pod.DeepCopy() - podUpdate := CreatePodUpdate(kubetypes.SET, TestSource, CreateValidPod("foo1", "new"), clone.(*v1.Pod), CreateValidPod("foo3", "new")) + podUpdate := CreatePodUpdate(kubetypes.SET, TestSource, CreateValidPod("foo1", "new"), clone, CreateValidPod("foo3", "new")) channel <- podUpdate expectPodUpdate(t, ch, CreatePodUpdate(kubetypes.ADD, TestSource, CreateValidPod("foo1", "new"), pod, CreateValidPod("foo3", "new"))) @@ -402,12 +399,9 @@ func TestPodUpdateLabels(t *testing.T) { pod.Labels = make(map[string]string, 0) pod.Labels["key"] = "value" - clone, err := scheme.Scheme.DeepCopy(pod) - if err != nil { - t.Fatalf("%v", err) - } + clone := pod.DeepCopy() - podUpdate := CreatePodUpdate(kubetypes.SET, TestSource, clone.(*v1.Pod)) + podUpdate := CreatePodUpdate(kubetypes.SET, TestSource, clone) channel <- podUpdate expectPodUpdate(t, ch, CreatePodUpdate(kubetypes.ADD, TestSource, pod)) diff --git a/pkg/kubelet/kubelet_node_status.go b/pkg/kubelet/kubelet_node_status.go index 24ac208ffe..a7fce17765 100644 --- a/pkg/kubelet/kubelet_node_status.go +++ b/pkg/kubelet/kubelet_node_status.go @@ -30,7 +30,6 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/conversion" "k8s.io/apimachinery/pkg/types" utilnet "k8s.io/apimachinery/pkg/util/net" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -118,15 +117,9 @@ func (kl *Kubelet) tryRegisterWithAPIServer(node *v1.Node) bool { return false } - clonedNode, err := conversion.NewCloner().DeepCopy(existingNode) - if err != nil { - glog.Errorf("Unable to clone %q node object %#v: %v", kl.nodeName, existingNode, err) - return false - } - - originalNode, ok := clonedNode.(*v1.Node) - if !ok || originalNode == nil { - glog.Errorf("Unable to cast %q node object %#v to v1.Node", kl.nodeName, clonedNode) + originalNode := existingNode.DeepCopy() + if originalNode == nil { + glog.Errorf("Nil %q node object", kl.nodeName) return false } @@ -409,14 +402,9 @@ func (kl *Kubelet) tryUpdateNodeStatus(tryNumber int) error { return fmt.Errorf("error getting node %q: %v", kl.nodeName, err) } - clonedNode, err := conversion.NewCloner().DeepCopy(node) - if err != nil { - return fmt.Errorf("error clone node %q: %v", kl.nodeName, err) - } - - originalNode, ok := clonedNode.(*v1.Node) - if !ok || originalNode == nil { - return fmt.Errorf("failed to cast %q node object %#v to v1.Node", kl.nodeName, clonedNode) + originalNode := node.DeepCopy() + if originalNode == nil { + return fmt.Errorf("nil %q node object", kl.nodeName) } kl.updatePodCIDR(node.Spec.PodCIDR) diff --git a/pkg/kubelet/kubelet_resources.go b/pkg/kubelet/kubelet_resources.go index e8f5525e8b..dc47a85880 100644 --- a/pkg/kubelet/kubelet_resources.go +++ b/pkg/kubelet/kubelet_resources.go @@ -22,7 +22,6 @@ import ( "github.com/golang/glog" "k8s.io/api/core/v1" - "k8s.io/client-go/kubernetes/scheme" "k8s.io/kubernetes/pkg/api/v1/resource" ) @@ -51,15 +50,7 @@ func (kl *Kubelet) defaultPodLimitsForDownwardAPI(pod *v1.Pod, container *v1.Con var outputContainer *v1.Container if container != nil { - containerCopy, err := scheme.Scheme.DeepCopy(container) - if err != nil { - return nil, nil, fmt.Errorf("failed to perform a deep copy of container object: %v", err) - } - var ok bool - outputContainer, ok = containerCopy.(*v1.Container) - if !ok { - return nil, nil, fmt.Errorf("unexpected type returned from deep copy of container object") - } + outputContainer = container.DeepCopy() resource.MergeContainerResourceLimits(outputContainer, allocatable) } return outputPod, outputContainer, nil diff --git a/pkg/kubelet/status/status_manager.go b/pkg/kubelet/status/status_manager.go index 20c751038a..dafd0a0917 100644 --- a/pkg/kubelet/status/status_manager.go +++ b/pkg/kubelet/status/status_manager.go @@ -22,7 +22,6 @@ import ( "time" clientset "k8s.io/client-go/kubernetes" - "k8s.io/client-go/kubernetes/scheme" "github.com/golang/glog" "k8s.io/api/core/v1" @@ -163,10 +162,8 @@ func (m *manager) SetPodStatus(pod *v1.Pod, status v1.PodStatus) { m.podStatusesLock.Lock() defer m.podStatusesLock.Unlock() // Make sure we're caching a deep copy. - status, err := copyStatus(&status) - if err != nil { - return - } + status = *status.DeepCopy() + // Force a status update if deletion timestamp is set. This is necessary // because if the pod is in the non-running state, the pod worker still // needs to be able to trigger an update and/or deletion. @@ -205,10 +202,7 @@ func (m *manager) SetContainerReadiness(podUID types.UID, containerID kubecontai } // Make sure we're not updating the cached version. - status, err := copyStatus(&oldStatus.status) - if err != nil { - return - } + status := *oldStatus.status.DeepCopy() containerStatus, _, _ = findContainerStatus(&status, containerID.String()) containerStatus.Ready = ready @@ -256,10 +250,7 @@ func (m *manager) TerminatePod(pod *v1.Pod) { if cachedStatus, ok := m.podStatuses[pod.UID]; ok { oldStatus = &cachedStatus.status } - status, err := copyStatus(oldStatus) - if err != nil { - return - } + status := *oldStatus.DeepCopy() for i := range status.ContainerStatuses { status.ContainerStatuses[i].State = v1.ContainerState{ Terminated: &v1.ContainerStateTerminated{}, @@ -513,13 +504,10 @@ func (m *manager) needsReconcile(uid types.UID, status v1.PodStatus) bool { pod = mirrorPod } - podStatus, err := copyStatus(&pod.Status) - if err != nil { - return false - } - normalizeStatus(pod, &podStatus) + podStatus := pod.Status.DeepCopy() + normalizeStatus(pod, podStatus) - if isStatusEqual(&podStatus, &status) { + if isStatusEqual(podStatus, &status) { // If the status from the source is the same with the cached status, // reconcile is not needed. Just return. return false @@ -586,13 +574,3 @@ func normalizeStatus(pod *v1.Pod, status *v1.PodStatus) *v1.PodStatus { kubetypes.SortInitContainerStatuses(pod, status.InitContainerStatuses) return status } - -func copyStatus(source *v1.PodStatus) (v1.PodStatus, error) { - clone, err := scheme.Scheme.DeepCopy(source) - if err != nil { - glog.Errorf("Failed to clone status %+v: %v", source, err) - return v1.PodStatus{}, err - } - status := *clone.(*v1.PodStatus) - return status, nil -} diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go index a584d53532..d4a13c58a8 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go @@ -32,7 +32,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" - "k8s.io/client-go/kubernetes/scheme" "k8s.io/kubernetes/pkg/kubelet/config" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" "k8s.io/kubernetes/pkg/kubelet/pod" @@ -386,18 +385,7 @@ func (dswp *desiredStateOfWorldPopulator) createVolumeSpec( } // Do not return the original volume object, since the source could mutate it - clonedPodVolumeObj, err := scheme.Scheme.DeepCopy(&podVolume) - if err != nil || clonedPodVolumeObj == nil { - return nil, "", fmt.Errorf( - "failed to deep copy %q volume object. err=%v", podVolume.Name, err) - } - - clonedPodVolume, ok := clonedPodVolumeObj.(*v1.Volume) - if !ok { - return nil, "", fmt.Errorf( - "failed to cast clonedPodVolume %#v to v1.Volume", - clonedPodVolumeObj) - } + clonedPodVolume := podVolume.DeepCopy() return volume.NewSpecFromVolume(clonedPodVolume), "", nil } diff --git a/pkg/util/taints/taints.go b/pkg/util/taints/taints.go index d8b18eede5..d748686d2a 100644 --- a/pkg/util/taints/taints.go +++ b/pkg/util/taints/taints.go @@ -242,11 +242,7 @@ func DeleteTaint(taints []v1.Taint, taintToDelete *v1.Taint) ([]v1.Taint, bool) // RemoveTaint tries to remove a taint from annotations list. Returns a new copy of updated Node and true if something was updated // false otherwise. func RemoveTaint(node *v1.Node, taint *v1.Taint) (*v1.Node, bool, error) { - objCopy, err := api.Scheme.DeepCopy(node) - if err != nil { - return nil, false, err - } - newNode := objCopy.(*v1.Node) + newNode := node.DeepCopy() nodeTaints := newNode.Spec.Taints if len(nodeTaints) == 0 { return newNode, false, nil @@ -264,11 +260,7 @@ func RemoveTaint(node *v1.Node, taint *v1.Taint) (*v1.Node, bool, error) { // AddOrUpdateTaint tries to add a taint to annotations list. Returns a new copy of updated Node and true if something was updated // false otherwise. func AddOrUpdateTaint(node *v1.Node, taint *v1.Taint) (*v1.Node, bool, error) { - objCopy, err := api.Scheme.DeepCopy(node) - if err != nil { - return nil, false, err - } - newNode := objCopy.(*v1.Node) + newNode := node.DeepCopy() nodeTaints := newNode.Spec.Taints var newTaints []v1.Taint diff --git a/staging/src/k8s.io/apimachinery/pkg/api/resource/quantity.go b/staging/src/k8s.io/apimachinery/pkg/api/resource/quantity.go index 1839d78a34..aac002f6e0 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/resource/quantity.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/resource/quantity.go @@ -93,6 +93,7 @@ import ( // +protobuf.embed=string // +protobuf.options.marshal=false // +protobuf.options.(gogoproto.goproto_stringer)=false +// +k8s:deepcopy-gen=true // +k8s:openapi-gen=true type Quantity struct { // i is the quantity in int64 scaled form, if d.Dec == nil diff --git a/staging/src/k8s.io/apimachinery/pkg/api/resource/zz_generated.deepcopy.go b/staging/src/k8s.io/apimachinery/pkg/api/resource/zz_generated.deepcopy.go new file mode 100644 index 0000000000..118dfca07e --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/api/resource/zz_generated.deepcopy.go @@ -0,0 +1,44 @@ +// +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 resource + +import ( + conversion "k8s.io/apimachinery/pkg/conversion" + 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.(*Quantity).DeepCopyInto(out.(*Quantity)) + return nil + }, InType: reflect.TypeOf(&Quantity{})}, + } +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Quantity) DeepCopyInto(out *Quantity) { + *out = in.DeepCopy() + return +} diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go index abfe6f5394..7200ccea3f 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go @@ -420,11 +420,6 @@ func (s *Scheme) Default(src Object) { } } -// Performs a deep copy of the given object. -func (s *Scheme) DeepCopy(src interface{}) (interface{}, error) { - return s.cloner.DeepCopy(src) -} - // Convert will attempt to convert in into out. Both must be pointers. For easy // testing of conversion functions. Returns an error if the conversion isn't // possible. You can call this with types that haven't been registered (for example, diff --git a/test/e2e_node/cpu_manager_test.go b/test/e2e_node/cpu_manager_test.go index 859c13b7f0..953a04d1e9 100644 --- a/test/e2e_node/cpu_manager_test.go +++ b/test/e2e_node/cpu_manager_test.go @@ -26,7 +26,6 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/scheme" "k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig" "k8s.io/kubernetes/pkg/kubelet/cm/cpumanager" "k8s.io/kubernetes/pkg/kubelet/cm/cpuset" @@ -146,9 +145,7 @@ func enableCPUManagerInKubelet(f *framework.Framework) (oldCfg *kubeletconfig.Ku // Enable CPU Manager in Kubelet with static policy. oldCfg, err := getCurrentKubeletConfig() framework.ExpectNoError(err) - clone, err := scheme.Scheme.DeepCopy(oldCfg) - framework.ExpectNoError(err) - newCfg := clone.(*kubeletconfig.KubeletConfiguration) + newCfg := oldCfg.DeepCopy() // Enable CPU Manager using feature gate. if newCfg.FeatureGates != "" {