storage.Interface KV impl. of etcd v3 #23387

hongchaodeng · 2016-03-23T17:33:45Z

This is the initial implementation of #22448.

The PR consists of two parts:

add godep of "clientv3" and "integration" (for testing)
create new package "etcd3" under "pkg/storage/"
implement KV methods of storage.Interface using etcd v3 APIs
- Create, Set, Get, Delete, GetToList, List, GuaranteedUpdate

k8s-github-robot · 2016-03-23T17:41:26Z

Labelling this PR as size/XL

k8s-bot · 2016-03-23T18:14:51Z

GCE e2e build/test passed for commit 2265c02658ef11d704970a8c8a9947d15cdd44e5.

k8s-bot · 2016-03-23T18:21:57Z

GCE e2e build/test passed for commit a719726a717cba6d3bfa3a72861cf527f1302676.

k8s-github-robot · 2016-03-23T18:22:34Z

The author of this PR is not in the whitelist for merge, can one of the admins add the 'ok-to-merge' label?

wojtek-t · 2016-03-23T20:59:34Z

@timothysc @lavalamp - FYI

lavalamp · 2016-03-23T21:09:02Z

pkg/storage/etcd/v3_helper_test.go

+limitations under the License.
+*/
+
+package etcd


I suggest making a new package for etcd3 and keeping the same function names rather than adding new functions to the same package.

(If there's common functionality you want to reuse, that's fine, move it to a separate package :) )

Otherwise, there's no way for users to avoid importing the etcd definition they don't want to use. Among other reasons.

Sure. Will do it.

k8s-bot · 2016-03-24T00:17:09Z

GCE e2e build/test passed for commit 9dff3d90315d425ceb8f3c982d195d416a4faf36.

k8s-bot · 2016-03-24T00:23:14Z

GCE e2e build/test passed for commit c7ca2588aed468694a3b2b88acc82a85afb2f56b.

k8s-bot · 2016-03-24T00:27:06Z

GCE e2e build/test passed for commit 108c467447d0807d0e432d9918cce803631cd530.

k8s-bot · 2016-03-24T00:36:51Z

GCE e2e build/test passed for commit 77c3fabac7786bdc5ec58aaf9d180a78c2e872d5.

timothysc · 2016-03-24T02:56:00Z

I'll look over in the a.m. , still catching up from PTO.

k8s-bot · 2016-03-24T04:30:25Z

GCE e2e build/test passed for commit 7a603376d8b8d074c305e83db2385154fbebce99.

wojtek-t · 2016-03-24T08:10:25Z

I will take a look later today.

wojtek-t · 2016-03-24T11:36:10Z

pkg/storage/etcd3/v3_helper.go

+
+// extractRevision extracts revision from obj.
+// If obj has resource version >0, it will be reset to 0.
+func extractRevision(versioner storage.Versioner, obj runtime.Object) (int64, error) {


We should probably rename it to reflect the fact that resource version is set to 0 if different.
Maybe:
extractAndResetRevision
?

hongchaodeng · 2016-03-30T17:02:59Z

Fixed comments

timothysc · 2016-03-30T17:06:24Z

pkg/storage/etcd3/store.go

+		}
+		if !txnResp.Succeeded {
+			getResp = (*clientv3.GetResponse)(txnResp.Responses[0].GetResponseRange())
+			glog.Infof("deletion of %s failed because of a conflict, going to retry", key)


Is there a reason for Info level vs V4? Just wondering...

My mistake. I set V4 for guaranteedUpdate though :(

timothysc · 2016-03-30T17:15:38Z

Minor comments, then lgtm for part 1.

k8s-bot · 2016-03-30T17:19:35Z

GCE e2e build/test passed for commit da6cda1157b6410275db1d6eda413e210b99bf9c.

This implements Get(), Create(), Delete(), GetToList(), List(), GuaranteedUpdate().

hongchaodeng · 2016-03-30T17:22:05Z

Addressed comments and squashed commits.
Thanks for the review :)

k8s-bot · 2016-03-30T17:35:25Z

GCE e2e build/test passed for commit 75eba2a39678fd0ada54c60c06077e4fa1ca819f.

k8s-bot · 2016-03-30T18:00:54Z

GCE e2e build/test passed for commit 00ddf06.

k8s-bot · 2016-03-30T18:01:15Z

GCE e2e build/test passed for commit c374a3218a544cb88d2ba89219c81958860bfe1c.

timothysc · 2016-03-30T18:23:57Z

thx @hongchaodeng for seeing this through. part 1 lgtm.

wojtek-t · 2016-03-30T22:10:44Z

LGTM too

k8s-github-robot · 2016-03-31T10:20:09Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-03-31T10:48:01Z

GCE e2e build/test failed for commit 00ddf06.

Please reference the list of currently known flakes when examining this failure. If you request a re-test, you must reference the issue describing the flake.

k8s-github-robot · 2016-03-31T15:13:16Z

@timothysc
You must link to the test flake issue which caused you to request this manual re-test.
Re-test requests should be in the form of: k8s-bot test this issue: #<number>
Here is the list of open test flakes.

hongchaodeng · 2016-03-31T16:14:07Z

Flake issue: #22671

gmarek · 2016-03-31T17:18:18Z

@k8s-bot test this issue #22671

gmarek · 2016-03-31T17:18:55Z

Readding lgtm label.

k8s-bot · 2016-03-31T17:55:51Z

GCE e2e build/test passed for commit 00ddf06.

k8s-github-robot · 2016-03-31T18:05:23Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-03-31T18:37:06Z

GCE e2e build/test passed for commit 00ddf06.

k8s-github-robot · 2016-03-31T18:46:42Z

Automatic merge from submit-queue

googlebot added the cla: yes label Mar 23, 2016

hongchaodeng mentioned this pull request Mar 23, 2016

etcd v3 initial change #22604

Closed

hongchaodeng force-pushed the e branch from 2265c02 to a719726 Compare March 23, 2016 17:41

k8s-github-robot assigned thockin Mar 23, 2016

k8s-github-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 23, 2016

k8s-github-robot added the needs-ok-to-merge label Mar 23, 2016

wojtek-t assigned wojtek-t and unassigned thockin Mar 23, 2016

lavalamp reviewed Mar 23, 2016
View reviewed changes

hongchaodeng force-pushed the e branch 2 times, most recently from 108c467 to 77c3fab Compare March 24, 2016 00:03

wojtek-t reviewed Mar 24, 2016
View reviewed changes

timothysc reviewed Mar 30, 2016
View reviewed changes

etcd (v3) store: implements KV methods of storage.Interface

00ddf06

This implements Get(), Create(), Delete(), GetToList(), List(), GuaranteedUpdate().

hongchaodeng force-pushed the e branch from c374a32 to 00ddf06 Compare March 30, 2016 17:21

timothysc added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 30, 2016

timothysc added ok-to-merge and removed needs-ok-to-merge labels Mar 30, 2016

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 31, 2016

gmarek added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 31, 2016

k8s-github-robot merged commit d807690 into kubernetes:master Mar 31, 2016

hongchaodeng deleted the e branch March 31, 2016 18:49

hongchaodeng mentioned this pull request Jul 22, 2016

etcd v3 as storage backend for APIServer kubernetes/enhancements#44

Closed

30 tasks

storage.Interface KV impl. of etcd v3 #23387

storage.Interface KV impl. of etcd v3 #23387

Conversation

hongchaodeng commented Mar 23, 2016

k8s-github-robot commented Mar 23, 2016

k8s-bot commented Mar 23, 2016

k8s-bot commented Mar 23, 2016

k8s-github-robot commented Mar 23, 2016

wojtek-t commented Mar 23, 2016

lavalamp Mar 23, 2016

Choose a reason for hiding this comment

lavalamp Mar 23, 2016

Choose a reason for hiding this comment

lavalamp Mar 23, 2016

Choose a reason for hiding this comment

hongchaodeng Mar 23, 2016

Choose a reason for hiding this comment

k8s-bot commented Mar 24, 2016

k8s-bot commented Mar 24, 2016

k8s-bot commented Mar 24, 2016

k8s-bot commented Mar 24, 2016

timothysc commented Mar 24, 2016

k8s-bot commented Mar 24, 2016

wojtek-t commented Mar 24, 2016

wojtek-t Mar 24, 2016

Choose a reason for hiding this comment

hongchaodeng commented Mar 30, 2016

timothysc Mar 30, 2016

Choose a reason for hiding this comment

hongchaodeng Mar 30, 2016

Choose a reason for hiding this comment

timothysc commented Mar 30, 2016

k8s-bot commented Mar 30, 2016

hongchaodeng commented Mar 30, 2016

k8s-bot commented Mar 30, 2016

k8s-bot commented Mar 30, 2016

k8s-bot commented Mar 30, 2016

timothysc commented Mar 30, 2016

wojtek-t commented Mar 30, 2016

k8s-github-robot commented Mar 31, 2016

k8s-bot commented Mar 31, 2016

k8s-github-robot commented Mar 31, 2016

hongchaodeng commented Mar 31, 2016

gmarek commented Mar 31, 2016

gmarek commented Mar 31, 2016

k8s-bot commented Mar 31, 2016

k8s-github-robot commented Mar 31, 2016

k8s-bot commented Mar 31, 2016

k8s-github-robot commented Mar 31, 2016