Commit 2d5624bb authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

Merge pull request #50681 from sttts/sttts-deepcopy-calls-apiserver

Automatic merge from submit-queue apiserver: simplify deepcopy calls
parents 28a5ecb9 b2442224
...@@ -490,11 +490,7 @@ func (storage *SimpleRESTStorage) Get(ctx request.Context, id string, options *m ...@@ -490,11 +490,7 @@ func (storage *SimpleRESTStorage) Get(ctx request.Context, id string, options *m
if id == "binary" { if id == "binary" {
return storage.stream, storage.errors["get"] return storage.stream, storage.errors["get"]
} }
copied, err := scheme.Copy(&storage.item) return storage.item.DeepCopy(), storage.errors["get"]
if err != nil {
panic(err)
}
return copied, storage.errors["get"]
} }
func (storage *SimpleRESTStorage) checkContext(ctx request.Context) { func (storage *SimpleRESTStorage) checkContext(ctx request.Context) {
...@@ -748,11 +744,7 @@ func (storage *SimpleTypedStorage) New() runtime.Object { ...@@ -748,11 +744,7 @@ func (storage *SimpleTypedStorage) New() runtime.Object {
func (storage *SimpleTypedStorage) Get(ctx request.Context, id string, options *metav1.GetOptions) (runtime.Object, error) { func (storage *SimpleTypedStorage) Get(ctx request.Context, id string, options *metav1.GetOptions) (runtime.Object, error) {
storage.checkContext(ctx) storage.checkContext(ctx)
copied, err := scheme.Copy(storage.item) return storage.item.DeepCopyObject(), storage.errors["get"]
if err != nil {
panic(err)
}
return copied, storage.errors["get"]
} }
func (storage *SimpleTypedStorage) checkContext(ctx request.Context) { func (storage *SimpleTypedStorage) checkContext(ctx request.Context) {
...@@ -3876,11 +3868,7 @@ func (storage *SimpleXGSubresourceRESTStorage) New() runtime.Object { ...@@ -3876,11 +3868,7 @@ func (storage *SimpleXGSubresourceRESTStorage) New() runtime.Object {
} }
func (storage *SimpleXGSubresourceRESTStorage) Get(ctx request.Context, id string, options *metav1.GetOptions) (runtime.Object, error) { func (storage *SimpleXGSubresourceRESTStorage) Get(ctx request.Context, id string, options *metav1.GetOptions) (runtime.Object, error) {
copied, err := scheme.Copy(&storage.item) return storage.item.DeepCopyObject(), nil
if err != nil {
panic(err)
}
return copied, nil
} }
func TestXGSubresource(t *testing.T) { func TestXGSubresource(t *testing.T) {
......
...@@ -16,4 +16,4 @@ limitations under the License. ...@@ -16,4 +16,4 @@ limitations under the License.
// +k8s:deepcopy-gen=package // +k8s:deepcopy-gen=package
package testing // import "k8s.io/apiserver/pkg/endpoints/testing" package testing
...@@ -1048,18 +1048,6 @@ func (e *Store) Delete(ctx genericapirequest.Context, name string, options *meta ...@@ -1048,18 +1048,6 @@ func (e *Store) Delete(ctx genericapirequest.Context, name string, options *meta
return out, true, err return out, true, err
} }
// copyListOptions copies list options for mutation.
func copyListOptions(options *metainternalversion.ListOptions) *metainternalversion.ListOptions {
if options == nil {
return &metainternalversion.ListOptions{}
}
copied, err := metainternalversion.Copier.Copy(options)
if err != nil {
panic(err)
}
return copied.(*metainternalversion.ListOptions)
}
// DeleteCollection removes all items returned by List with a given ListOptions from storage. // DeleteCollection removes all items returned by List with a given ListOptions from storage.
// //
// DeleteCollection is currently NOT atomic. It can happen that only subset of objects // DeleteCollection is currently NOT atomic. It can happen that only subset of objects
...@@ -1071,10 +1059,15 @@ func copyListOptions(options *metainternalversion.ListOptions) *metainternalvers ...@@ -1071,10 +1059,15 @@ func copyListOptions(options *metainternalversion.ListOptions) *metainternalvers
// possibly with storage API, but watch is not delivered correctly then). // possibly with storage API, but watch is not delivered correctly then).
// It will be possible to fix it with v3 etcd API. // It will be possible to fix it with v3 etcd API.
func (e *Store) DeleteCollection(ctx genericapirequest.Context, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { func (e *Store) DeleteCollection(ctx genericapirequest.Context, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) {
if listOptions == nil {
listOptions = &metainternalversion.ListOptions{}
} else {
listOptions = listOptions.DeepCopy()
}
// DeleteCollection must remain backwards compatible with old clients that expect it to // DeleteCollection must remain backwards compatible with old clients that expect it to
// remove all resources, initialized or not, within the type. It is also consistent with // remove all resources, initialized or not, within the type. It is also consistent with
// Delete which does not require IncludeUninitialized // Delete which does not require IncludeUninitialized
listOptions = copyListOptions(listOptions)
listOptions.IncludeUninitialized = true listOptions.IncludeUninitialized = true
listObj, err := e.List(ctx, listOptions) listObj, err := e.List(ctx, listOptions)
......
...@@ -172,10 +172,7 @@ func (i *defaultUpdatedObjectInfo) UpdatedObject(ctx genericapirequest.Context, ...@@ -172,10 +172,7 @@ func (i *defaultUpdatedObjectInfo) UpdatedObject(ctx genericapirequest.Context,
// so we don't return the original. BeforeUpdate can mutate the returned object, doing things like clearing ResourceVersion. // so we don't return the original. BeforeUpdate can mutate the returned object, doing things like clearing ResourceVersion.
// If we're re-called, we need to be able to return the pristine version. // If we're re-called, we need to be able to return the pristine version.
if newObj != nil { if newObj != nil {
newObj, err = i.copier.Copy(newObj) newObj = newObj.DeepCopyObject()
if err != nil {
return nil, err
}
} }
// Allow any configured transformers to update the new object // Allow any configured transformers to update the new object
......
...@@ -543,11 +543,8 @@ func (c *Cacher) GuaranteedUpdate( ...@@ -543,11 +543,8 @@ func (c *Cacher) GuaranteedUpdate(
if elem, exists, err := c.watchCache.GetByKey(key); err != nil { if elem, exists, err := c.watchCache.GetByKey(key); err != nil {
glog.Errorf("GetByKey returned error: %v", err) glog.Errorf("GetByKey returned error: %v", err)
} else if exists { } else if exists {
currObj, copyErr := c.copier.Copy(elem.(*storeElement).Object) currObj := elem.(*storeElement).Object.DeepCopyObject()
if copyErr == nil { return c.storage.GuaranteedUpdate(ctx, key, ptrToType, ignoreNotFound, preconditions, tryUpdate, currObj)
return c.storage.GuaranteedUpdate(ctx, key, ptrToType, ignoreNotFound, preconditions, tryUpdate, currObj)
}
glog.Errorf("couldn't copy object: %v", copyErr)
} }
// If we couldn't get the object, fallback to no-suggestion. // If we couldn't get the object, fallback to no-suggestion.
return c.storage.GuaranteedUpdate(ctx, key, ptrToType, ignoreNotFound, preconditions, tryUpdate) return c.storage.GuaranteedUpdate(ctx, key, ptrToType, ignoreNotFound, preconditions, tryUpdate)
...@@ -877,26 +874,11 @@ func (c *cacheWatcher) sendWatchCacheEvent(event *watchCacheEvent) { ...@@ -877,26 +874,11 @@ func (c *cacheWatcher) sendWatchCacheEvent(event *watchCacheEvent) {
var watchEvent watch.Event var watchEvent watch.Event
switch { switch {
case curObjPasses && !oldObjPasses: case curObjPasses && !oldObjPasses:
object, err := c.copier.Copy(event.Object) watchEvent = watch.Event{Type: watch.Added, Object: event.Object.DeepCopyObject()}
if err != nil {
utilruntime.HandleError(fmt.Errorf("unexpected copy error: %v", err))
return
}
watchEvent = watch.Event{Type: watch.Added, Object: object}
case curObjPasses && oldObjPasses: case curObjPasses && oldObjPasses:
object, err := c.copier.Copy(event.Object) watchEvent = watch.Event{Type: watch.Modified, Object: event.Object.DeepCopyObject()}
if err != nil {
utilruntime.HandleError(fmt.Errorf("unexpected copy error: %v", err))
return
}
watchEvent = watch.Event{Type: watch.Modified, Object: object}
case !curObjPasses && oldObjPasses: case !curObjPasses && oldObjPasses:
object, err := c.copier.Copy(event.PrevObject) watchEvent = watch.Event{Type: watch.Deleted, Object: event.PrevObject.DeepCopyObject()}
if err != nil {
utilruntime.HandleError(fmt.Errorf("unexpected copy error: %v", err))
return
}
watchEvent = watch.Event{Type: watch.Deleted, Object: object}
} }
// We need to ensure that if we put event X to the c.result, all // We need to ensure that if we put event X to the c.result, all
......
...@@ -612,12 +612,7 @@ func (h *etcdHelper) getFromCache(index uint64, filter storage.FilterFunc) (runt ...@@ -612,12 +612,7 @@ func (h *etcdHelper) getFromCache(index uint64, filter storage.FilterFunc) (runt
} }
// We should not return the object itself to avoid polluting the cache if someone // We should not return the object itself to avoid polluting the cache if someone
// modifies returned values. // modifies returned values.
objCopy, err := h.copier.Copy(obj.(runtime.Object)) objCopy := obj.(runtime.Object).DeepCopyObject()
if err != nil {
glog.Errorf("Error during DeepCopy of cached object: %q", err)
// We can't return a copy, thus we report the object as not found.
return nil, false
}
metrics.ObserveCacheHit() metrics.ObserveCacheHit()
return objCopy.(runtime.Object), true return objCopy.(runtime.Object), true
} }
...@@ -630,11 +625,7 @@ func (h *etcdHelper) addToCache(index uint64, obj runtime.Object) { ...@@ -630,11 +625,7 @@ func (h *etcdHelper) addToCache(index uint64, obj runtime.Object) {
defer func() { defer func() {
metrics.ObserveAddCache(startTime) metrics.ObserveAddCache(startTime)
}() }()
objCopy, err := h.copier.Copy(obj) objCopy := obj.DeepCopyObject()
if err != nil {
glog.Errorf("Error during DeepCopy of cached object: %q", err)
return
}
isOverwrite := h.cache.Add(index, objCopy) isOverwrite := h.cache.Add(index, objCopy)
if !isOverwrite { if !isOverwrite {
metrics.ObserveNewEntry() metrics.ObserveNewEntry()
......
...@@ -373,12 +373,7 @@ func TestWatchEtcdState(t *testing.T) { ...@@ -373,12 +373,7 @@ func TestWatchEtcdState(t *testing.T) {
// CAS the previous value // CAS the previous value
updateFn := func(input runtime.Object, res storage.ResponseMeta) (runtime.Object, *uint64, error) { updateFn := func(input runtime.Object, res storage.ResponseMeta) (runtime.Object, *uint64, error) {
newObj, err := scheme.DeepCopy(pod) return pod.DeepCopyObject(), nil, nil
if err != nil {
t.Errorf("unexpected error: %v", err)
return nil, nil, err
}
return newObj.(*example.Pod), nil, nil
} }
err = h.GuaranteedUpdate(context.TODO(), key, &example.Pod{}, false, nil, updateFn) err = h.GuaranteedUpdate(context.TODO(), key, &example.Pod{}, false, nil, updateFn)
if err != nil { if err != nil {
......
...@@ -123,12 +123,7 @@ func makeTestPod(name string) *example.Pod { ...@@ -123,12 +123,7 @@ func makeTestPod(name string) *example.Pod {
func updatePod(t *testing.T, s storage.Interface, obj, old *example.Pod) *example.Pod { func updatePod(t *testing.T, s storage.Interface, obj, old *example.Pod) *example.Pod {
updateFn := func(input runtime.Object, res storage.ResponseMeta) (runtime.Object, *uint64, error) { updateFn := func(input runtime.Object, res storage.ResponseMeta) (runtime.Object, *uint64, error) {
newObj, err := scheme.DeepCopy(obj) return obj.DeepCopyObject(), nil, nil
if err != nil {
t.Errorf("unexpected error: %v", err)
return nil, nil, err
}
return newObj.(*example.Pod), nil, nil
} }
key := "pods/" + obj.Namespace + "/" + obj.Name key := "pods/" + obj.Namespace + "/" + obj.Name
if err := s.GuaranteedUpdate(context.TODO(), key, &example.Pod{}, old == nil, nil, updateFn); err != nil { if err := s.GuaranteedUpdate(context.TODO(), key, &example.Pod{}, old == nil, nil, updateFn); err != nil {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment