Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decouple etcd node.expiration logic from DeleitonTimestamp #23923

Merged
merged 1 commit into from
Apr 17, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/registry/generic/etcd/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func (e *Etcd) Update(ctx api.Context, obj runtime.Object) (runtime.Object, bool
// function, we are resetting resourceVersion to the initial value here.
//
// TODO: In fact, we should probably return a DeepCopy of obj in all places.
err := e.Storage.Versioner().UpdateObject(obj, nil, resourceVersion)
err := e.Storage.Versioner().UpdateObject(obj, resourceVersion)
if err != nil {
return nil, nil, err
}
Expand All @@ -301,7 +301,7 @@ func (e *Etcd) Update(ctx api.Context, obj runtime.Object) (runtime.Object, bool
creating = false
if doUnconditionalUpdate {
// Update the object's resource version to match the latest etcd object's resource version.
err = e.Storage.Versioner().UpdateObject(obj, res.Expiration, res.ResourceVersion)
err = e.Storage.Versioner().UpdateObject(obj, res.ResourceVersion)
if err != nil {
return nil, nil, err
}
Expand Down
7 changes: 1 addition & 6 deletions pkg/storage/etcd/api_object_versioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,9 @@ package etcd

import (
"strconv"
"time"

"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/meta"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/runtime"
"k8s.io/kubernetes/pkg/storage"
)
Expand All @@ -32,14 +30,11 @@ import (
type APIObjectVersioner struct{}

// UpdateObject implements Versioner
func (a APIObjectVersioner) UpdateObject(obj runtime.Object, expiration *time.Time, resourceVersion uint64) error {
func (a APIObjectVersioner) UpdateObject(obj runtime.Object, resourceVersion uint64) error {
accessor, err := meta.Accessor(obj)
if err != nil {
return err
}
if expiration != nil {
accessor.SetDeletionTimestamp(&unversioned.Time{Time: *expiration})
}
versionString := ""
if resourceVersion != 0 {
versionString = strconv.FormatUint(resourceVersion, 10)
Expand Down
12 changes: 1 addition & 11 deletions pkg/storage/etcd/api_object_versioner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@ package etcd

import (
"testing"
"time"

"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/unversioned"
storagetesting "k8s.io/kubernetes/pkg/storage/testing"
)

Expand All @@ -34,18 +32,10 @@ func TestObjectVersioner(t *testing.T) {
t.Errorf("unexpected version: %d %v", ver, err)
}
obj := &storagetesting.TestResource{ObjectMeta: api.ObjectMeta{ResourceVersion: "a"}}
if err := v.UpdateObject(obj, nil, 5); err != nil {
if err := v.UpdateObject(obj, 5); err != nil {
t.Fatalf("unexpected error: %v", err)
}
if obj.ResourceVersion != "5" || obj.DeletionTimestamp != nil {
t.Errorf("unexpected resource version: %#v", obj)
}
now := unversioned.Time{Time: time.Now()}
obj = &storagetesting.TestResource{ObjectMeta: api.ObjectMeta{ResourceVersion: "a"}}
if err := v.UpdateObject(obj, &now.Time, 5); err != nil {
t.Fatalf("unexpected error: %v", err)
}
if obj.ResourceVersion != "5" || *obj.DeletionTimestamp != now {
t.Errorf("unexpected resource version: %#v", obj)
}
}
9 changes: 3 additions & 6 deletions pkg/storage/etcd/etcd_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ func (h *etcdHelper) extractObj(response *etcd.Response, inErr error, objPtr run
return body, nil, fmt.Errorf("unable to decode object %s into %v", gvk.String(), reflect.TypeOf(objPtr))
}
// being unable to set the version does not prevent the object from being extracted
_ = h.versioner.UpdateObject(objPtr, node.Expiration, node.ModifiedIndex)
_ = h.versioner.UpdateObject(objPtr, node.ModifiedIndex)
return body, node, err
}

Expand Down Expand Up @@ -465,7 +465,7 @@ func (h *etcdHelper) decodeNodeList(nodes []*etcd.Node, filter storage.FilterFun
return err
}
// being unable to set the version does not prevent the object from being extracted
_ = h.versioner.UpdateObject(obj, node.Expiration, node.ModifiedIndex)
_ = h.versioner.UpdateObject(obj, node.ModifiedIndex)
if filter(obj) {
v.Set(reflect.Append(v, reflect.ValueOf(obj).Elem()))
}
Expand Down Expand Up @@ -556,9 +556,6 @@ func (h *etcdHelper) GuaranteedUpdate(ctx context.Context, key string, ptrToType
meta := storage.ResponseMeta{}
if node != nil {
meta.TTL = node.TTL
if node.Expiration != nil {
meta.Expiration = node.Expiration
}
meta.ResourceVersion = node.ModifiedIndex
}
// Get the object to be written by calling tryUpdate.
Expand Down Expand Up @@ -590,7 +587,7 @@ func (h *etcdHelper) GuaranteedUpdate(ctx context.Context, key string, ptrToType
}

// Since update object may have a resourceVersion set, we need to clear it here.
if err := h.versioner.UpdateObject(ret, meta.Expiration, 0); err != nil {
if err := h.versioner.UpdateObject(ret, 0); err != nil {
return errors.New("resourceVersion cannot be set on objects store in etcd")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/etcd/etcd_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func (w *etcdWatcher) decodeObject(node *etcd.Node) (runtime.Object, error) {
}

// ensure resource version is set on the object we load from etcd
if err := w.versioner.UpdateObject(obj, node.Expiration, node.ModifiedIndex); err != nil {
if err := w.versioner.UpdateObject(obj, node.ModifiedIndex); err != nil {
utilruntime.HandleError(fmt.Errorf("failure to version api object (%d) %#v: %v", node.ModifiedIndex, obj, err))
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/storage/etcd3/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ func (s *store) updateState(st *objState, userUpdate storage.UpdateFunc) (runtim
}
if version != 0 {
// We cannot store object with resourceVersion in etcd. We need to reset it.
if err := s.versioner.UpdateObject(ret, nil, 0); err != nil {
if err := s.versioner.UpdateObject(ret, 0); err != nil {
return nil, fmt.Errorf("UpdateObject failed: %v", err)
}
}
Expand All @@ -379,7 +379,7 @@ func decode(codec runtime.Codec, versioner storage.Versioner, value []byte, objP
return err
}
// being unable to set the version does not prevent the object from being extracted
versioner.UpdateObject(objPtr, nil, uint64(rev))
versioner.UpdateObject(objPtr, uint64(rev))
return nil
}

Expand All @@ -396,7 +396,7 @@ func decodeList(elems []*elemForDecode, filter storage.FilterFunc, ListPtr inter
return err
}
// being unable to set the version does not prevent the object from being extracted
versioner.UpdateObject(obj, nil, elem.rev)
versioner.UpdateObject(obj, elem.rev)
if filter(obj) {
v.Set(reflect.Append(v, reflect.ValueOf(obj).Elem()))
}
Expand Down
7 changes: 1 addition & 6 deletions pkg/storage/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ limitations under the License.
package storage

import (
"time"

"golang.org/x/net/context"
"k8s.io/kubernetes/pkg/runtime"
"k8s.io/kubernetes/pkg/types"
Expand All @@ -31,7 +29,7 @@ type Versioner interface {
// UpdateObject sets storage metadata into an API object. Returns an error if the object
// cannot be updated correctly. May return nil if the requested object does not need metadata
// from database.
UpdateObject(obj runtime.Object, expiration *time.Time, resourceVersion uint64) error
UpdateObject(obj runtime.Object, resourceVersion uint64) error
// UpdateList sets the resource version into an API list object. Returns an error if the object
// cannot be updated correctly. May return nil if the requested object does not need metadata
// from database.
Expand All @@ -49,9 +47,6 @@ type ResponseMeta struct {
// zero or negative in some cases (objects may be expired after the requested
// expiration time due to server lag).
TTL int64
// Expiration is the time at which the node that contained the returned object will expire and be deleted.
// This can be nil if there is no expiration time set for the node.
Expiration *time.Time
// The resource version of the node that contained the returned object.
ResourceVersion uint64
}
Expand Down