Support for etcd migration #30500

wojtek-t · 2016-08-12T11:33:13Z

This change is

wojtek-t · 2016-08-12T11:37:48Z

cluster/saltbase/salt/etcd/etcd.manifest

@@ -27,7 +27,7 @@
 "containers":[
    {
    "name": "etcd-container",
-    "image": "gcr.io/google_containers/etcd:3.0.3",
+    "image": "gcr.io/groovy-sentry-504/etcd:3.0.4",


@lavalamp - this is the result of calling "make push" on etcd images directory after changes from this PR.

I guess that the way to proceed with this PR would be:

first it needs to be LGTMed

the from this PR I will build a new image and push to gcr

then we will merge this PR

Or am I missing something?

Yeah I think that's the process.

wojtek-t · 2016-08-12T14:10:29Z

@lavalamp - the third commit is not yet fully tested.

wojtek-t · 2016-08-12T14:25:55Z

@lavalamp - the third commit is not yet fully tested.

To be clear - it's not yet tested on container VM.

Also, there are still some issues with the "migrate-if-needed.sh" script, but this will result in some more local changes, to the overall idea shouldn't change.

wojtek-t · 2016-08-12T14:38:24Z

OK - I checked that ConvtainerVM also works (modulo some problems with the migrate-if-needed script that I will fix on Monday).

wojtek-t · 2016-08-12T14:50:36Z

@lavalamp - actually, I think I fixed the problem with "migrate-if-needed" script too.

So this is basically ready for review.

lavalamp · 2016-08-12T17:28:41Z

cluster/gce/config-test.sh

@@ -36,7 +36,7 @@ KUBE_APISERVER_REQUEST_TIMEOUT=300
 PREEMPTIBLE_NODE=${PREEMPTIBLE_NODE:-false}
 PREEMPTIBLE_MASTER=${PREEMPTIBLE_MASTER:-false}

-MASTER_OS_DISTRIBUTION=${KUBE_MASTER_OS_DISTRIBUTION:-${KUBE_OS_DISTRIBUTION:-gci}}
+MASTER_OS_DISTRIBUTION=${KUBE_MASTER_OS_DISTRIBUTION:-${KUBE_OS_DISTRIBUTION:-debian}}


wojtek-t · 2016-08-16T14:28:02Z

@lavalamp - PTAL

GCE and GKE failures are because lack of image and they are expected (it works for me locally)
integration tests are failing due to "out of memory" which is unrelated problem and seems like infrastructural one

lavalamp · 2016-08-16T20:57:52Z

lgtm, just nits.

Review status: 0 of 9 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

cluster/gce/gci/configure-helper.sh, line 546 [r4] (raw file):

  sed -i -e "s@{{ *hostname *}}@$host_name@g" "${temp_file}"
  sed -i -e "s@{{ *etcd_cluster *}}@$etcd_cluster@g" "${temp_file}"
  sed -i -e "s@{{ *storage_backend *}}@${STORAGE_BACKEND:-}@g" "${temp_file}"

To what file will this change apply? I can't find the thing that it will be running against in this PR.

cluster/images/etcd/Makefile, line 22 [r4] (raw file):

TAG?=3.0.4
ARCH?=amd64
REGISTRY?=gcr.io/groovy-sentry-504

revert?

cluster/saltbase/salt/etcd/etcd.manifest, line 30 [r1] (raw file):

Previously, lavalamp (Daniel Smith) wrote…

Yeah I think that's the process.

revert this line :)

cluster/saltbase/salt/etcd/etcd.manifest, line 40 [r3] (raw file):

Previously, wojtek-t (Wojciech Tyczynski) wrote…

Moved parameters to env vars

I meant package the entire command up as a script. But this is at least better.

hack/test-update-storage-objects.sh, line 162 [r4] (raw file):

TARGET_STORAGE="etcd3" \
  DATA_DIRECTORY="${ETCD_DIR}" \
  ETCDCTL=$(which etcdctl) \

nit: tab/space

Comments from Reviewable

wojtek-t · 2016-08-17T07:27:02Z

Review status: 0 of 9 files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

cluster/gce/gci/configure-helper.sh, line 546 [r4] (raw file):

Previously, lavalamp (Daniel Smith) wrote…

To what file will this change apply? I can't find the thing that it will be running against in this PR.

All those sed`s here are modifying etcd.manifest file from salt (GCI doesn't work with pillars and is inventing its own templating language with sed`s - this is riduclous I know, but this is how it works).

cluster/images/etcd/Makefile, line 22 [r4] (raw file):

Previously, lavalamp (Daniel Smith) wrote…

revert?

Done

cluster/saltbase/salt/etcd/etcd.manifest, line 30 [r1] (raw file):

Previously, lavalamp (Daniel Smith) wrote…

revert this line :)

Done

cluster/saltbase/salt/etcd/etcd.manifest, line 40 [r3] (raw file):

Previously, lavalamp (Daniel Smith) wrote…

I meant package the entire command up as a script. But this is at least better.

Ahh I see. But that's way more invasive change so I would prefer not do to it now.

hack/test-update-storage-objects.sh, line 162 [r4] (raw file):

Previously, lavalamp (Daniel Smith) wrote…

nit: tab/space

Done

Comments from Reviewable

wojtek-t · 2016-08-17T07:28:18Z

@lavalamp - comments applied

There is one more thing we need to solve, which is "under what tag push the new image". It probably shouldn't be 3.0.4, but probably also not 3.0.5. Maybe 3.0.4.1? Or sth like that?

lavalamp · 2016-08-17T17:00:27Z

I can see arguments for any of:

3.1.0-beta.1
3.0.5-beta.1
3.0.4-migration.1

wojtek-t · 2016-08-18T06:38:50Z

OK - so I pushed the image "3.0.4-migration.1" to gcr. But in head we are still leaving the main image. This is just to enable more tests.

wojtek-t · 2016-08-18T06:38:59Z

Self-applying lgtm based on above comments.

k8s-bot · 2016-08-18T07:10:30Z

GCE e2e build/test passed for commit f3c3267.

k8s-github-robot · 2016-08-19T06:49:22Z

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

k8s-bot · 2016-08-19T07:33:57Z

GCE e2e build/test passed for commit f3c3267.

k8s-github-robot · 2016-08-19T07:34:06Z

Automatic merge from submit-queue

wojtek-t added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. release-note-none Denotes a PR that doesn't merit a release note. labels Aug 12, 2016

wojtek-t assigned lavalamp Aug 12, 2016

googlebot added the cla: yes label Aug 12, 2016

k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 12, 2016

wojtek-t reviewed Aug 12, 2016
View reviewed changes

wojtek-t mentioned this pull request Aug 12, 2016

POC: etcd2 -> etcd3 migration #30447

Closed

wojtek-t force-pushed the etcd_migration branch from 481976e to 102c9d0 Compare August 12, 2016 14:03

wojtek-t force-pushed the etcd_migration branch from 2257add to d27c118 Compare August 12, 2016 14:49

wojtek-t changed the title ~~[WIP] Support for etcd migration~~ Support for etcd migration Aug 12, 2016

lavalamp reviewed Aug 12, 2016
View reviewed changes

wojtek-t force-pushed the etcd_migration branch from d27c118 to d9e2039 Compare August 12, 2016 18:40

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 13, 2016

wojtek-t force-pushed the etcd_migration branch from d9e2039 to 93acb75 Compare August 16, 2016 08:39

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 16, 2016

k8s-github-robot mentioned this pull request Aug 16, 2016

[k8s.io] Pods should be submitted and removed [Conformance] {E2eNode Suite} #30566

Closed

wojtek-t force-pushed the etcd_migration branch 2 times, most recently from e30691c to f32be7d Compare August 16, 2016 13:21

wojtek-t force-pushed the etcd_migration branch from f32be7d to d18c8e6 Compare August 17, 2016 07:26

wojtek-t force-pushed the etcd_migration branch 2 times, most recently from c2a9753 to 7a029dc Compare August 17, 2016 11:03

wojtek-t added 2 commits August 18, 2016 08:31

Support for etcd migration

b42023e

Add migration test

f3c3267

wojtek-t force-pushed the etcd_migration branch from 7a029dc to f3c3267 Compare August 18, 2016 06:31

wojtek-t added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Aug 18, 2016

wojtek-t added this to the v1.4 milestone Aug 18, 2016

wojtek-t mentioned this pull request Aug 18, 2016

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

Closed

30 tasks

wojtek-t added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Aug 19, 2016

k8s-github-robot merged commit 1e09eb7 into kubernetes:master Aug 19, 2016

wojtek-t deleted the etcd_migration branch September 6, 2016 11:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for etcd migration #30500

Support for etcd migration #30500

wojtek-t commented Aug 12, 2016 •

edited

wojtek-t Aug 12, 2016

wojtek-t Aug 12, 2016

lavalamp Aug 12, 2016

wojtek-t commented Aug 12, 2016

wojtek-t commented Aug 12, 2016

wojtek-t commented Aug 12, 2016

wojtek-t commented Aug 12, 2016

lavalamp Aug 12, 2016

wojtek-t Aug 12, 2016

wojtek-t commented Aug 16, 2016

lavalamp commented Aug 16, 2016

wojtek-t commented Aug 17, 2016

wojtek-t commented Aug 17, 2016

lavalamp commented Aug 17, 2016

wojtek-t commented Aug 18, 2016

wojtek-t commented Aug 18, 2016

k8s-bot commented Aug 18, 2016

k8s-github-robot commented Aug 19, 2016

k8s-bot commented Aug 19, 2016

k8s-github-robot commented Aug 19, 2016

Support for etcd migration #30500

Support for etcd migration #30500

Conversation

wojtek-t commented Aug 12, 2016 • edited

wojtek-t Aug 12, 2016

Choose a reason for hiding this comment

wojtek-t Aug 12, 2016

Choose a reason for hiding this comment

lavalamp Aug 12, 2016

Choose a reason for hiding this comment

wojtek-t commented Aug 12, 2016

wojtek-t commented Aug 12, 2016

wojtek-t commented Aug 12, 2016

wojtek-t commented Aug 12, 2016

lavalamp Aug 12, 2016

Choose a reason for hiding this comment

wojtek-t Aug 12, 2016

Choose a reason for hiding this comment

wojtek-t commented Aug 16, 2016

lavalamp commented Aug 16, 2016

wojtek-t commented Aug 17, 2016

wojtek-t commented Aug 17, 2016

lavalamp commented Aug 17, 2016

wojtek-t commented Aug 18, 2016

wojtek-t commented Aug 18, 2016

k8s-bot commented Aug 18, 2016

k8s-github-robot commented Aug 19, 2016

k8s-bot commented Aug 19, 2016

k8s-github-robot commented Aug 19, 2016

wojtek-t commented Aug 12, 2016 •

edited