Merge pull request #19342 from caesarxuchao/remove-resourceversion-check

Auto commit by PR queue bot

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) {

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").

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).

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
 }

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{

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
 }

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
 }

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
 }

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
 }

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"))
 			}