Make ResourceVersion a string internally instead of uint64

Allows us to define different watch versioning regimes in the future as well as to encode information with the resource version. This changes /watch/resources?resourceVersion=3 to start the watch at 4 instead of 3, which means clients can read a resource version and then send it back to the server. Clients should no longer do math on resource versions.
duda · Oct 7, 2014 · 82bcdd3 · 82bcdd3
1 parent 31e02b8
commit 82bcdd3
Show file tree

Hide file tree

Showing 54 changed files with 518 additions and 240 deletions.
diff --git a/cmd/kubecfg/kubecfg.go b/cmd/kubecfg/kubecfg.go
@@ -311,7 +311,7 @@ func executeAPIRequest(ctx api.Context, method string, c *client.Client) bool {
 	validStorage := checkStorage(storage)
 	verb := ""
 	setBody := false
-	var version uint64
+	var version string
 
 	printer := getPrinter()
 
@@ -369,7 +369,7 @@ func executeAPIRequest(ctx api.Context, method string, c *client.Client) bool {
 		r.ParseSelectorParam("labels", *selector)
 	}
 	if setBody {
-		if version != 0 {
+		if len(version) > 0 {
 			data := readConfig(storage, c.RESTClient.Codec)
 			obj, err := latest.Codec.Decode(data)
 			if err != nil {

diff --git a/pkg/api/errors/etcd/etcd.go b/pkg/api/errors/etcd/etcd.go
@@ -64,3 +64,10 @@ func InterpretDeleteError(err error, kind, name string) error {
 		return err
 	}
 }
+
+// InterpretResourceVersionError returns the appropriate api error
+// for a failure to convert the resource version of an object sent
+// to the API to an etcd uint64 index.
+func InterpretResourceVersionError(err error, kind, value string) error {
+	return errors.NewInvalid(kind, "", errors.ErrorList{errors.NewFieldInvalid("resourceVersion", value)})
+}
diff --git a/pkg/api/latest/latest_test.go b/pkg/api/latest/latest_test.go
@@ -19,6 +19,7 @@ package latest
 import (
 	"encoding/json"
 	"reflect"
+	"strconv"
 	"testing"
 
 	internal "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -41,14 +42,27 @@ var apiObjectFuzzer = fuzz.New().NilChance(.5).NumElements(1, 1).Funcs(
 		// TODO: Fix JSON/YAML packages and/or write custom encoding
 		// for uint64's. Somehow the LS *byte* of this is lost, but
 		// only when all 8 bytes are set.
-		j.ResourceVersion = c.RandUint64() >> 8
+		j.ResourceVersion = strconv.FormatUint(c.RandUint64()>>8, 10)
 		j.SelfLink = c.RandString()
 
 		var sec, nsec int64
 		c.Fuzz(&sec)
 		c.Fuzz(&nsec)
 		j.CreationTimestamp = util.Unix(sec, nsec).Rfc3339Copy()
 	},
+	func(j *internal.ObjectReference, c fuzz.Continue) {
+		// We have to customize the randomization of TypeMetas because their
+		// APIVersion and Kind must remain blank in memory.
+		j.APIVersion = c.RandString()
+		j.Kind = c.RandString()
+		j.Namespace = c.RandString()
+		j.Name = c.RandString()
+		// TODO: Fix JSON/YAML packages and/or write custom encoding
+		// for uint64's. Somehow the LS *byte* of this is lost, but
+		// only when all 8 bytes are set.
+		j.ResourceVersion = strconv.FormatUint(c.RandUint64()>>8, 10)
+		j.FieldPath = c.RandString()
+	},
 	func(intstr *util.IntOrString, c fuzz.Continue) {
 		// util.IntOrString will panic if its kind is set wrong.
 		if c.RandBool() {
@@ -120,12 +134,12 @@ func TestInternalRoundTrip(t *testing.T) {
 }
 
 func TestResourceVersioner(t *testing.T) {
-	pod := internal.Pod{TypeMeta: internal.TypeMeta{ResourceVersion: 10}}
+	pod := internal.Pod{TypeMeta: internal.TypeMeta{ResourceVersion: "10"}}
 	version, err := ResourceVersioner.ResourceVersion(&pod)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if version != 10 {
+	if version != "10" {
 		t.Errorf("unexpected version %d", version)
 	}
 }

diff --git a/pkg/api/ref_test.go b/pkg/api/ref_test.go
@@ -37,7 +37,7 @@ func TestGetReference(t *testing.T) {
 			obj: &Pod{
 				TypeMeta: TypeMeta{
 					ID:              "foo",
-					ResourceVersion: 42,
+					ResourceVersion: "42",
 					SelfLink:        "/api/v1beta1/pods/foo",
 				},
 			},
@@ -46,14 +46,14 @@ func TestGetReference(t *testing.T) {
 				APIVersion:      "v1beta1",
 				Name:            "foo",
 				UID:             "foo",
-				ResourceVersion: 42,
+				ResourceVersion: "42",
 			},
 		},
 		"serviceList": {
 			obj: &ServiceList{
 				TypeMeta: TypeMeta{
 					ID:              "foo",
-					ResourceVersion: 42,
+					ResourceVersion: "42",
 					SelfLink:        "/api/v1beta2/services",
 				},
 			},
@@ -62,14 +62,14 @@ func TestGetReference(t *testing.T) {
 				APIVersion:      "v1beta2",
 				Name:            "foo",
 				UID:             "foo",
-				ResourceVersion: 42,
+				ResourceVersion: "42",
 			},
 		},
 		"badSelfLink": {
 			obj: &ServiceList{
 				TypeMeta: TypeMeta{
 					ID:              "foo",
-					ResourceVersion: 42,
+					ResourceVersion: "42",
 					SelfLink:        "v1beta2/services",
 				},
 			},

diff --git a/pkg/api/serialization_test.go b/pkg/api/serialization_test.go
@@ -21,6 +21,7 @@ import (
 	"flag"
 	"math/rand"
 	"reflect"
+	"strconv"
 	"testing"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -49,7 +50,7 @@ var apiObjectFuzzer = fuzz.New().NilChance(.5).NumElements(1, 1).Funcs(
 		// TODO: Fix JSON/YAML packages and/or write custom encoding
 		// for uint64's. Somehow the LS *byte* of this is lost, but
 		// only when all 8 bytes are set.
-		j.ResourceVersion = c.RandUint64() >> 8
+		j.ResourceVersion = strconv.FormatUint(c.RandUint64()>>8, 10)
 		j.SelfLink = c.RandString()
 
 		var sec, nsec int64
@@ -66,14 +67,27 @@ var apiObjectFuzzer = fuzz.New().NilChance(.5).NumElements(1, 1).Funcs(
 		// TODO: Fix JSON/YAML packages and/or write custom encoding
 		// for uint64's. Somehow the LS *byte* of this is lost, but
 		// only when all 8 bytes are set.
-		j.ResourceVersion = c.RandUint64() >> 8
+		j.ResourceVersion = strconv.FormatUint(c.RandUint64()>>8, 10)
 		j.SelfLink = c.RandString()
 
 		var sec, nsec int64
 		c.Fuzz(&sec)
 		c.Fuzz(&nsec)
 		j.CreationTimestamp = util.Unix(sec, nsec).Rfc3339Copy()
 	},
+	func(j *api.ObjectReference, c fuzz.Continue) {
+		// We have to customize the randomization of TypeMetas because their
+		// APIVersion and Kind must remain blank in memory.
+		j.APIVersion = c.RandString()
+		j.Kind = c.RandString()
+		j.Namespace = c.RandString()
+		j.Name = c.RandString()
+		// TODO: Fix JSON/YAML packages and/or write custom encoding
+		// for uint64's. Somehow the LS *byte* of this is lost, but
+		// only when all 8 bytes are set.
+		j.ResourceVersion = strconv.FormatUint(c.RandUint64()>>8, 10)
+		j.FieldPath = c.RandString()
+	},
 	func(intstr *util.IntOrString, c fuzz.Continue) {
 		// util.IntOrString will panic if its kind is set wrong.
 		if c.RandBool() {

diff --git a/pkg/api/types.go b/pkg/api/types.go
@@ -250,7 +250,7 @@ type TypeMeta struct {
 	ID                string    `json:"id,omitempty" yaml:"id,omitempty"`
 	CreationTimestamp util.Time `json:"creationTimestamp,omitempty" yaml:"creationTimestamp,omitempty"`
 	SelfLink          string    `json:"selfLink,omitempty" yaml:"selfLink,omitempty"`
-	ResourceVersion   uint64    `json:"resourceVersion,omitempty" yaml:"resourceVersion,omitempty"`
+	ResourceVersion   string    `json:"resourceVersion,omitempty" yaml:"resourceVersion,omitempty"`
 	APIVersion        string    `json:"apiVersion,omitempty" yaml:"apiVersion,omitempty"`
 	Namespace         string    `json:"namespace,omitempty" yaml:"namespace,omitempty"`
 	UID               string    `json:"uid,omitempty" yaml:"uid,omitempty"`
@@ -675,7 +675,7 @@ type ObjectReference struct {
 	Name            string `json:"name,omitempty" yaml:"name,omitempty"`
 	UID             string `json:"uid,omitempty" yaml:"uid,omitempty"`
 	APIVersion      string `json:"apiVersion,omitempty" yaml:"apiVersion,omitempty"`
-	ResourceVersion uint64 `json:"resourceVersion,omitempty" yaml:"resourceVersion,omitempty"`
+	ResourceVersion string `json:"resourceVersion,omitempty" yaml:"resourceVersion,omitempty"`
 
 	// Optional. If referring to a piece of an object instead of an entire object, this string
 	// should contain a valid field access statement. For example,

diff --git a/pkg/api/v1beta1/conversion.go b/pkg/api/v1beta1/conversion.go
@@ -17,12 +17,78 @@ limitations under the License.
 package v1beta1
 
 import (
+	"strconv"
+
 	newer "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/conversion"
 )
 
 func init() {
 	newer.Scheme.AddConversionFuncs(
+		// TypeMeta has changed type of ResourceVersion internally
+		func(in *newer.TypeMeta, out *TypeMeta, s conversion.Scope) error {
+			out.APIVersion = in.APIVersion
+			out.Kind = in.Kind
+			out.Namespace = in.Namespace
+			out.ID = in.ID
+			out.CreationTimestamp = in.CreationTimestamp
+			out.SelfLink = in.SelfLink
+			out.Annotations = in.Annotations
+
+			if len(in.ResourceVersion) > 0 {
+				v, err := strconv.ParseUint(in.ResourceVersion, 10, 64)
+				if err != nil {
+					return err
+				}
+				out.ResourceVersion = v
+			}
+			return nil
+		},
+		func(in *TypeMeta, out *newer.TypeMeta, s conversion.Scope) error {
+			out.APIVersion = in.APIVersion
+			out.Kind = in.Kind
+			out.Namespace = in.Namespace
+			out.ID = in.ID
+			out.CreationTimestamp = in.CreationTimestamp
+			out.SelfLink = in.SelfLink
+			out.Annotations = in.Annotations
+
+			if in.ResourceVersion != 0 {
+				out.ResourceVersion = strconv.FormatUint(in.ResourceVersion, 10)
+			}
+			return nil
+		},
+
+		// ObjectReference has changed type of ResourceVersion internally
+		func(in *newer.ObjectReference, out *ObjectReference, s conversion.Scope) error {
+			out.APIVersion = in.APIVersion
+			out.Kind = in.Kind
+			out.Namespace = in.Namespace
+			out.Name = in.Name
+			out.FieldPath = in.FieldPath
+
+			if len(in.ResourceVersion) > 0 {
+				v, err := strconv.ParseUint(in.ResourceVersion, 10, 64)
+				if err != nil {
+					return err
+				}
+				out.ResourceVersion = v
+			}
+			return nil
+		},
+		func(in *ObjectReference, out *newer.ObjectReference, s conversion.Scope) error {
+			out.APIVersion = in.APIVersion
+			out.Kind = in.Kind
+			out.Namespace = in.Namespace
+			out.Name = in.Name
+			out.FieldPath = in.FieldPath
+
+			if in.ResourceVersion != 0 {
+				out.ResourceVersion = strconv.FormatUint(in.ResourceVersion, 10)
+			}
+			return nil
+		},
+
 		// EnvVar's Key is deprecated in favor of Name.
 		func(in *newer.EnvVar, out *EnvVar, s conversion.Scope) error {
 			out.Value = in.Value

diff --git a/pkg/api/v1beta2/conversion.go b/pkg/api/v1beta2/conversion.go
@@ -17,12 +17,78 @@ limitations under the License.
 package v1beta2
 
 import (
+	"strconv"
+
 	newer "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/conversion"
 )
 
 func init() {
 	newer.Scheme.AddConversionFuncs(
+		// TypeMeta has changed type of ResourceVersion internally
+		func(in *newer.TypeMeta, out *TypeMeta, s conversion.Scope) error {
+			out.APIVersion = in.APIVersion
+			out.Kind = in.Kind
+			out.Namespace = in.Namespace
+			out.ID = in.ID
+			out.CreationTimestamp = in.CreationTimestamp
+			out.SelfLink = in.SelfLink
+			out.Annotations = in.Annotations
+
+			if len(in.ResourceVersion) > 0 {
+				v, err := strconv.ParseUint(in.ResourceVersion, 10, 64)
+				if err != nil {
+					return err
+				}
+				out.ResourceVersion = v
+			}
+			return nil
+		},
+		func(in *TypeMeta, out *newer.TypeMeta, s conversion.Scope) error {
+			out.APIVersion = in.APIVersion
+			out.Kind = in.Kind
+			out.Namespace = in.Namespace
+			out.ID = in.ID
+			out.CreationTimestamp = in.CreationTimestamp
+			out.SelfLink = in.SelfLink
+			out.Annotations = in.Annotations
+
+			if in.ResourceVersion != 0 {
+				out.ResourceVersion = strconv.FormatUint(in.ResourceVersion, 10)
+			}
+			return nil
+		},
+
+		// ObjectReference has changed type of ResourceVersion internally
+		func(in *newer.ObjectReference, out *ObjectReference, s conversion.Scope) error {
+			out.APIVersion = in.APIVersion
+			out.Kind = in.Kind
+			out.Namespace = in.Namespace
+			out.Name = in.Name
+			out.FieldPath = in.FieldPath
+
+			if len(in.ResourceVersion) > 0 {
+				v, err := strconv.ParseUint(in.ResourceVersion, 10, 64)
+				if err != nil {
+					return err
+				}
+				out.ResourceVersion = v
+			}
+			return nil
+		},
+		func(in *ObjectReference, out *newer.ObjectReference, s conversion.Scope) error {
+			out.APIVersion = in.APIVersion
+			out.Kind = in.Kind
+			out.Namespace = in.Namespace
+			out.Name = in.Name
+			out.FieldPath = in.FieldPath
+
+			if in.ResourceVersion != 0 {
+				out.ResourceVersion = strconv.FormatUint(in.ResourceVersion, 10)
+			}
+			return nil
+		},
+
 		// EnvVar's Key is deprecated in favor of Name.
 		func(in *newer.EnvVar, out *EnvVar, s conversion.Scope) error {
 			out.Value = in.Value

diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go
@@ -78,7 +78,7 @@ type SimpleRESTStorage struct {
 	fakeWatch                *watch.FakeWatcher
 	requestedLabelSelector   labels.Selector
 	requestedFieldSelector   labels.Selector
-	requestedResourceVersion uint64
+	requestedResourceVersion string
 
 	// The id requested, and location to return for ResourceLocation
 	requestedResourceLocationID string
@@ -144,7 +144,7 @@ func (storage *SimpleRESTStorage) Update(ctx api.Context, obj runtime.Object) (<
 }
 
 // Implement ResourceWatcher.
-func (storage *SimpleRESTStorage) Watch(ctx api.Context, label, field labels.Selector, resourceVersion uint64) (watch.Interface, error) {
+func (storage *SimpleRESTStorage) Watch(ctx api.Context, label, field labels.Selector, resourceVersion string) (watch.Interface, error) {
 	storage.requestedLabelSelector = label
 	storage.requestedFieldSelector = field
 	storage.requestedResourceVersion = resourceVersion

diff --git a/pkg/apiserver/interfaces.go b/pkg/apiserver/interfaces.go
@@ -54,7 +54,7 @@ type ResourceWatcher interface {
 	// are supported; an error should be returned if 'field' tries to select on a field that
 	// isn't supported. 'resourceVersion' allows for continuing/starting a watch at a
 	// particular version.
-	Watch(ctx api.Context, label, field labels.Selector, resourceVersion uint64) (watch.Interface, error)
+	Watch(ctx api.Context, label, field labels.Selector, resourceVersion string) (watch.Interface, error)
 }
 
 // Redirector know how to return a remote resource's location.