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

cluster/gce and test/: update etcd:3.2.24-0 image to etcd:3.2.24-1 #68896

Merged
merged 1 commit into from
Sep 21, 2018

Conversation

ixdy
Copy link
Member

@ixdy ixdy commented Sep 20, 2018

What this PR does / why we need it: follow-up to #59664 to use the new etcd image revision in various GCE locations.

The only differences between etcd:3.2.24-0 and etcd:3.2.24-1 is that the latter is a manifest list, rather than an amd64 image. Of course, on GCE this should basically make no difference.

kubeadm is currently using a tag without a revision (no -0 or -1), but we'll likely fix that in a follow-up PR.

Release note:

Update to use manifest list for etcd image

/assign @tpepper @jpbetz @neolit123 @mkumatag @dims

@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Sep 20, 2018
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 20, 2018
@k8s-ci-robot k8s-ci-robot added sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 20, 2018
@ixdy
Copy link
Member Author

ixdy commented Sep 20, 2018

The 3.2.24-1 tag hasn't been promoted yet, so this should fail tests.

@neolit123
Copy link
Member

/kind cleanup
/lgtm

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Sep 20, 2018
@ixdy
Copy link
Member Author

ixdy commented Sep 20, 2018

Comparing 3.2.24-0 and 3.2.24-1 using https://github.com/GoogleContainerTools/container-diff:

$ container-diff diff k8s.gcr.io/etcd:3.2.24-0 staging-k8s.gcr.io/etcd:3.2.24-1 --type=file

-----File-----

These entries have been added to k8s.gcr.io/etcd:3.2.24-0:
FILE                                 SIZE
/usr/local/bin/etcd-3.2.18           17M
/usr/local/bin/etcdctl-3.2.18        14.5M

These entries have been deleted from k8s.gcr.io/etcd:3.2.24-0: None

These entries have been changed between k8s.gcr.io/etcd:3.2.24-0 and staging-k8s.gcr.io/etcd:3.2.24-1:
FILE                                       SIZE1        SIZE2
/usr/local/bin/migrate-if-needed.sh        3.8K         3.8K

@ixdy
Copy link
Member Author

ixdy commented Sep 20, 2018

(so interestingly 3.2.24-0 didn't bundle etcd 3.2.18?)

@ixdy
Copy link
Member Author

ixdy commented Sep 20, 2018

And the diffs in migrate-if-needed.sh:

@@ -18,7 +18,7 @@
 # This script performs etcd upgrade based on the following environmental
 # variables:
 # TARGET_STORAGE - API of etcd to be used (supported: 'etcd2', 'etcd3')
-# TARGET_VERSION - etcd release to be used (supported: '2.2.1', '2.3.7', '3.0.17', '3.1.12', '3.2.24')
+# TARGET_VERSION - etcd release to be used (supported: '2.2.1', '2.3.7', '3.0.17', '3.1.12', '3.2.18', '3.2.24')
 # DATA_DIRECTORY - directory with etcd data
 #
 # The current etcd version and storage format is detected based on the
@@ -29,7 +29,8 @@
 # - 2.2.1/etcd2 -> 2.3.7/etcd2
 # - 2.3.7/etcd2 -> 3.0.17/etcd2
 # - 3.0.17/etcd3 -> 3.1.12/etcd3
-# - 3.1.12/etcd3 -> 3.2.24/etcd3
+# - 3.1.12/etcd3 -> 3.2.18/etcd3
+# - 3.2.18/etcd3 -> 3.2.24/etcd3
 #
 # NOTE: The releases supported in this script has to match release binaries
 # present in the etcd image (to make this script work correctly).
@@ -42,7 +43,7 @@

 # NOTE: BUNDLED_VERSION has to match release binaries present in the
 # etcd image (to make this script work correctly).
-BUNDLED_VERSIONS="2.2.1, 2.3.7, 3.0.17, 3.1.12, 3.2.24"
+BUNDLED_VERSIONS="2.2.1, 2.3.7, 3.0.17, 3.1.12, 3.2.18 3.2.24"

 ETCD_NAME="${ETCD_NAME:-etcd-$(hostname)}"
 if [ -z "${DATA_DIRECTORY:-}" ]; then

@jpbetz
Copy link
Contributor

jpbetz commented Sep 20, 2018

@ixdy We should replace 3.2.18 with 3.2.24. We only want one patch version of each minor version. This is almost certainly my fault, I must not have sent a PR out to update migrate-if-needed when first publishing 3.2.24.

@ixdy
Copy link
Member Author

ixdy commented Sep 20, 2018

ack, there's also a comma missing in BUNDLED_VERSIONS in migrate-status.sh:

BUNDLED_VERSIONS="2.2.1, 2.3.7, 3.0.17, 3.1.12, 3.2.18 3.2.24"

/hold
We need to fix this.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 20, 2018
@ixdy
Copy link
Member Author

ixdy commented Sep 20, 2018

@eparis @jbeda @mikedanese @roberthbailey @zmerlynn can one of you please approve? thanks!

@ixdy
Copy link
Member Author

ixdy commented Sep 20, 2018

/retest

(this might be a little eager, we'll see...)

@ixdy
Copy link
Member Author

ixdy commented Sep 20, 2018

/hold cancel
k8s.gcr.io/etcd:3.2.24-1 has been fixed.

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 20, 2018
@ixdy
Copy link
Member Author

ixdy commented Sep 20, 2018

once more, now that images are published:
/retest

@neolit123
Copy link
Member

/retest

1 similar comment
@ixdy
Copy link
Member Author

ixdy commented Sep 21, 2018

/retest

@ixdy
Copy link
Member Author

ixdy commented Sep 21, 2018

@tpepper @timothysc what are the thoughts on this PR? Without it kubeadm and GCE cluster scripts are currently pointing at different (but roughly equivalent) etcd images.

@neolit123
Copy link
Member

neolit123 commented Sep 21, 2018

@ixdy
we had a chat about it today on a short call.

@timothysc mentioned that a new etcd version is coming soon, so if that new version only has the -x revisions (no tag without revisions) then we are going to make kubeadm use the -x revisions.

unless there are plans to remove etcd:3.2.24 (without revision) eventually, this patch is redundant.
is there an estimate if that might happen?

@ixdy
Copy link
Member Author

ixdy commented Sep 21, 2018

I don't think we plan to remove etcd:3.2.24, no.

The issue this PR resolves is that 3.2.24 points to the same image as 3.2.24-1, so kubeadm is effectively already using 3.2.24-1, whereas the GCE scripts are still using 3.2.24-0.

@neolit123
Copy link
Member

The issue this PR resolves is that 3.2.24 points to the same image as 3.2.24-1, so kubeadm is effectively already using 3.2.24-1, whereas the GCE scripts are still using 3.2.24-0.

oh, sorry wrong PR. i though i was writing in my kubeadm PR that did the 3.2.24-1 bump. o_O.

this should be merged ASAP, IMHO.

@ixdy
Copy link
Member Author

ixdy commented Sep 21, 2018

yeah, the PR title is not helping here. let me fix that.

@ixdy ixdy changed the title Update to use etcd:3.2.24-1 image cluster/gce and test/: update etcd:3.2.24-0 image to etcd:3.2.24-1 Sep 21, 2018
@ixdy
Copy link
Member Author

ixdy commented Sep 21, 2018

@bowei @gmarek @jszczepkowski @vishh @mwielgus @MaciekPytel @jingax10 can one of you please approve?

@tpepper
Copy link
Member

tpepper commented Sep 21, 2018

My preference is to have things all in alignment, so "Without it kubeadm and GCE cluster scripts are currently pointing at different (but roughly equivalent) etcd images" == bad.

@ixdy
Copy link
Member Author

ixdy commented Sep 21, 2018

Also, the PR e2e tests are passing, and container-diff shows -0 and -1 to be equivalent, so I believe this is low-risk.

@tpepper
Copy link
Member

tpepper commented Sep 21, 2018

/milestone v1.12

@k8s-ci-robot k8s-ci-robot added this to the v1.12 milestone Sep 21, 2018
@tpepper
Copy link
Member

tpepper commented Sep 21, 2018

/priority critical-urgent

@k8s-ci-robot k8s-ci-robot added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Sep 21, 2018
@ixdy
Copy link
Member Author

ixdy commented Sep 21, 2018

/assign @roberthbailey

You approved #68898 yesterday, can you also approve this? :)

@roberthbailey
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ixdy, neolit123, roberthbailey

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 21, 2018
@tpepper
Copy link
Member

tpepper commented Sep 21, 2018

/retest

@k8s-ci-robot k8s-ci-robot merged commit 95ab206 into kubernetes:master Sep 21, 2018
@gabrielfsousa
Copy link

so... do i need to upgrade the etcd ?
on the change log
"Default etcd server version is unchanged from v1.11: v3.2.18"

@dims
Copy link
Member

dims commented Sep 28, 2018

@gabrielfsousa i logged a bug #69216

@ixdy
Copy link
Member Author

ixdy commented Sep 28, 2018

#68318 is what actually updated etcd and had a release note, but yes, the dependencies section needs to be updated.

@wenjiaswe
Copy link
Contributor

cc @yuwenma This is the PR to update etcd:3.2.24-0 to etcd:3.2.24-1, which you may use as ref for etcd image tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants