diff --git a/pkg/api/rest/resttest/resttest.go b/pkg/api/rest/resttest/resttest.go index d565eb0abd8..17e0a584220 100644 --- a/pkg/api/rest/resttest/resttest.go +++ b/pkg/api/rest/resttest/resttest.go @@ -148,7 +148,7 @@ func (t *Tester) TestCreate(valid runtime.Object, setFn SetFunc, getFn GetFunc, // Test updating an object. func (t *Tester) TestUpdate(valid runtime.Object, setFn SetFunc, getFn GetFunc, updateFn UpdateFunc, invalidUpdateFn ...UpdateFunc) { t.testUpdateEquals(copyOrDie(valid), setFn, getFn, updateFn) - t.testUpdateFailsOnVersionTooOld(copyOrDie(valid), setFn) + t.testUpdateFailsOnVersionTooOld(copyOrDie(valid), setFn, getFn) t.testUpdateOnNotFound(copyOrDie(valid)) if !t.clusterScope { t.testUpdateRejectsMismatchedNamespace(copyOrDie(valid), setFn) @@ -426,7 +426,7 @@ func (t *Tester) testUpdateEquals(obj runtime.Object, setFn SetFunc, getFn GetFu } } -func (t *Tester) testUpdateFailsOnVersionTooOld(obj runtime.Object, setFn SetFunc) { +func (t *Tester) testUpdateFailsOnVersionTooOld(obj runtime.Object, setFn SetFunc, getFn GetFunc) { ctx := t.TestContext() foo := copyOrDie(obj) @@ -436,11 +436,16 @@ func (t *Tester) testUpdateFailsOnVersionTooOld(obj runtime.Object, setFn SetFun t.Errorf("unexpected error: %v", err) } - older := copyOrDie(foo) + storedFoo, err := getFn(ctx, foo) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + older := copyOrDie(storedFoo) olderMeta := t.getObjectMetaOrFail(older) olderMeta.ResourceVersion = "1" - _, _, err := t.storage.(rest.Updater).Update(t.TestContext(), older) + _, _, err = t.storage.(rest.Updater).Update(t.TestContext(), older) if err == nil { t.Errorf("Expected an error, but we didn't get one") } else if !errors.IsConflict(err) { diff --git a/pkg/client/unversioned/endpoints.go b/pkg/client/unversioned/endpoints.go index a5df26c5170..c58c88a28d1 100644 --- a/pkg/client/unversioned/endpoints.go +++ b/pkg/client/unversioned/endpoints.go @@ -17,8 +17,6 @@ limitations under the License. package unversioned import ( - "fmt" - "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/watch" ) @@ -92,9 +90,6 @@ func (c *endpoints) Watch(opts api.ListOptions) (watch.Interface, error) { func (c *endpoints) Update(endpoints *api.Endpoints) (*api.Endpoints, error) { result := &api.Endpoints{} - if len(endpoints.ResourceVersion) == 0 { - return nil, fmt.Errorf("invalid update object, missing resource version: %v", endpoints) - } err := c.r.Put(). Namespace(c.ns). Resource("endpoints"). diff --git a/pkg/client/unversioned/events.go b/pkg/client/unversioned/events.go index 882204be693..b882ccdc47e 100644 --- a/pkg/client/unversioned/events.go +++ b/pkg/client/unversioned/events.go @@ -86,9 +86,6 @@ func (e *events) Create(event *api.Event) (*api.Event, error) { // created with the "" namespace. Update also requires the ResourceVersion to be set in the event // object. func (e *events) Update(event *api.Event) (*api.Event, error) { - if len(event.ResourceVersion) == 0 { - return nil, fmt.Errorf("invalid event update object, missing resource version: %#v", event) - } result := &api.Event{} err := e.client.Put(). Namespace(event.Namespace). diff --git a/pkg/client/unversioned/limit_ranges.go b/pkg/client/unversioned/limit_ranges.go index 0ed7e2a25f4..8bc2253da91 100644 --- a/pkg/client/unversioned/limit_ranges.go +++ b/pkg/client/unversioned/limit_ranges.go @@ -17,8 +17,6 @@ limitations under the License. package unversioned import ( - "fmt" - "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/watch" ) @@ -81,10 +79,6 @@ func (c *limitRanges) Create(limitRange *api.LimitRange) (result *api.LimitRange // Update takes the representation of a limitRange to update. Returns the server's representation of the limitRange, and an error, if it occurs. func (c *limitRanges) Update(limitRange *api.LimitRange) (result *api.LimitRange, err error) { result = &api.LimitRange{} - if len(limitRange.ResourceVersion) == 0 { - err = fmt.Errorf("invalid update object, missing resource version: %v", limitRange) - return - } err = c.r.Put().Namespace(c.ns).Resource("limitRanges").Name(limitRange.Name).Body(limitRange).Do().Into(result) return } diff --git a/pkg/client/unversioned/limit_ranges_test.go b/pkg/client/unversioned/limit_ranges_test.go index dec5448f6a0..f6936570ccd 100644 --- a/pkg/client/unversioned/limit_ranges_test.go +++ b/pkg/client/unversioned/limit_ranges_test.go @@ -164,39 +164,6 @@ func TestLimitRangeUpdate(t *testing.T) { c.Validate(t, response, err) } -func TestInvalidLimitRangeUpdate(t *testing.T) { - ns := api.NamespaceDefault - limitRange := &api.LimitRange{ - ObjectMeta: api.ObjectMeta{ - Name: "abc", - }, - Spec: api.LimitRangeSpec{ - Limits: []api.LimitRangeItem{ - { - Type: api.LimitTypePod, - Max: api.ResourceList{ - api.ResourceCPU: resource.MustParse("100"), - api.ResourceMemory: resource.MustParse("10000"), - }, - Min: api.ResourceList{ - api.ResourceCPU: resource.MustParse("0"), - api.ResourceMemory: resource.MustParse("100"), - }, - }, - }, - }, - } - c := &simple.Client{ - Request: simple.Request{Method: "PUT", Path: testapi.Default.ResourcePath(getLimitRangesResourceName(), ns, "abc"), Query: simple.BuildQueryValues(nil)}, - Response: simple.Response{StatusCode: 200, Body: limitRange}, - } - _, err := c.Setup(t).LimitRanges(ns).Update(limitRange) - defer c.Close() - if err == nil { - t.Errorf("Expected an error due to missing ResourceVersion") - } -} - func TestLimitRangeDelete(t *testing.T) { ns := api.NamespaceDefault c := &simple.Client{ diff --git a/pkg/client/unversioned/namespaces.go b/pkg/client/unversioned/namespaces.go index c3158d89d5e..122bcba5085 100644 --- a/pkg/client/unversioned/namespaces.go +++ b/pkg/client/unversioned/namespaces.go @@ -68,10 +68,6 @@ func (c *namespaces) List(opts api.ListOptions) (*api.NamespaceList, error) { // Update takes the representation of a namespace to update. Returns the server's representation of the namespace, and an error, if it occurs. func (c *namespaces) Update(namespace *api.Namespace) (result *api.Namespace, err error) { result = &api.Namespace{} - if len(namespace.ResourceVersion) == 0 { - err = fmt.Errorf("invalid update object, missing resource version: %v", namespace) - return - } err = c.r.Put().Resource("namespaces").Name(namespace.Name).Body(namespace).Do().Into(result) return } diff --git a/pkg/client/unversioned/nodes.go b/pkg/client/unversioned/nodes.go index 1d2990ba684..452a03f1691 100644 --- a/pkg/client/unversioned/nodes.go +++ b/pkg/client/unversioned/nodes.go @@ -17,8 +17,6 @@ limitations under the License. package unversioned import ( - "fmt" - "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/watch" ) @@ -81,20 +79,12 @@ func (c *nodes) Delete(name string) error { // Update updates an existing node. func (c *nodes) Update(node *api.Node) (*api.Node, error) { result := &api.Node{} - if len(node.ResourceVersion) == 0 { - err := fmt.Errorf("invalid update object, missing resource version: %v", node) - return nil, err - } err := c.r.Put().Resource(c.resourceName()).Name(node.Name).Body(node).Do().Into(result) return result, err } func (c *nodes) UpdateStatus(node *api.Node) (*api.Node, error) { result := &api.Node{} - if len(node.ResourceVersion) == 0 { - err := fmt.Errorf("invalid update object, missing resource version: %v", node) - return nil, err - } err := c.r.Put().Resource(c.resourceName()).Name(node.Name).SubResource("status").Body(node).Do().Into(result) return result, err } diff --git a/pkg/client/unversioned/persistentvolumeclaim.go b/pkg/client/unversioned/persistentvolumeclaim.go index 9798e7a29fa..bf5447d7589 100644 --- a/pkg/client/unversioned/persistentvolumeclaim.go +++ b/pkg/client/unversioned/persistentvolumeclaim.go @@ -17,8 +17,6 @@ limitations under the License. package unversioned import ( - "fmt" - "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/watch" ) @@ -77,10 +75,6 @@ func (c *persistentVolumeClaims) Create(claim *api.PersistentVolumeClaim) (resul func (c *persistentVolumeClaims) Update(claim *api.PersistentVolumeClaim) (result *api.PersistentVolumeClaim, err error) { result = &api.PersistentVolumeClaim{} - if len(claim.ResourceVersion) == 0 { - err = fmt.Errorf("invalid update object, missing resource version: %v", claim) - return - } err = c.client.Put().Namespace(c.namespace).Resource("persistentVolumeClaims").Name(claim.Name).Body(claim).Do().Into(result) return } diff --git a/pkg/client/unversioned/persistentvolumes.go b/pkg/client/unversioned/persistentvolumes.go index 252cc5cf660..2de17bb7183 100644 --- a/pkg/client/unversioned/persistentvolumes.go +++ b/pkg/client/unversioned/persistentvolumes.go @@ -17,8 +17,6 @@ limitations under the License. package unversioned import ( - "fmt" - "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/watch" ) @@ -72,10 +70,6 @@ func (c *persistentVolumes) Create(volume *api.PersistentVolume) (result *api.Pe func (c *persistentVolumes) Update(volume *api.PersistentVolume) (result *api.PersistentVolume, err error) { result = &api.PersistentVolume{} - if len(volume.ResourceVersion) == 0 { - err = fmt.Errorf("invalid update object, missing resource version: %v", volume) - return - } err = c.client.Put().Resource("persistentVolumes").Name(volume.Name).Body(volume).Do().Into(result) return } diff --git a/pkg/registry/generic/etcd/etcd.go b/pkg/registry/generic/etcd/etcd.go index 317ffea7972..a6532d9d42f 100644 --- a/pkg/registry/generic/etcd/etcd.go +++ b/pkg/registry/generic/etcd/etcd.go @@ -32,6 +32,7 @@ import ( "k8s.io/kubernetes/pkg/registry/generic" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/storage" + "k8s.io/kubernetes/pkg/util/validation/field" "k8s.io/kubernetes/pkg/watch" "github.com/golang/glog" @@ -285,6 +286,14 @@ func (e *Etcd) Update(ctx api.Context, obj runtime.Object) (runtime.Object, bool if err != nil { return nil, nil, err } + if newVersion == 0 { + // TODO: The Invalid error should has a field for Resource. + // After that field is added, we should fill the Resource and + // leave the Kind field empty. See the discussion in #18526. + qualifiedKind := unversioned.GroupKind{e.QualifiedResource.Group, e.QualifiedResource.Resource} + fieldErrList := field.ErrorList{field.Invalid(field.NewPath("metadata").Child("resourceVersion"), newVersion, "must be specified for an update")} + return nil, nil, kubeerr.NewInvalid(qualifiedKind, name, fieldErrList) + } if newVersion != version { return nil, nil, kubeerr.NewConflict(e.QualifiedResource, name, fmt.Errorf("the object has been modified; please apply your changes to the latest version and try again")) }