Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

Propagate etcd version from cluster.yaml to etcdadm #722

Merged
merged 1 commit into from
Jul 5, 2017

Conversation

ytsarev
Copy link
Contributor

@ytsarev ytsarev commented Jul 2, 2017

Without this additional propagation etcdadm falls back to default 3.1.3 version as in
https://github.com/iflix-letsplay/kube-aws/blob/master/etcdadm/etcdadm#L88

In case when any other version is specified in cluster.yaml it will create the deviation between running etcd version and the one that is used for periodic snapshotting, e.g. running 3.1.8 in the main container and still using default 3.1.3 for etcdadm based operations.

This PR fixes the deviation.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 2, 2017
@ytsarev ytsarev mentioned this pull request Jul 2, 2017
@codecov-io
Copy link

codecov-io commented Jul 2, 2017

Codecov Report

Merging #722 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #722   +/-   ##
=======================================
  Coverage   34.33%   34.33%           
=======================================
  Files          55       55           
  Lines        3073     3073           
=======================================
  Hits         1055     1055           
  Misses       1859     1859           
  Partials      159      159

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b542fb1...f932d51. Read the comment docs.

@@ -618,6 +618,7 @@ write_files:
ETCD_ADVERTISE_CLIENT_URLS=https://$advertised_hostname:2379
ETCD_LISTEN_PEER_URLS=https://$private_ip:2380
ETCD_INITIAL_ADVERTISE_PEER_URLS=https://$advertised_hostname:2380" >> /var/run/coreos/etcd-environment
ETCD_VERSION={{.Etcd.Version}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is an easy to miss redirect happening in this file, line you add should be inside that redirect to have an effect

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure i get you properly, can you clarify with code example ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this is confusing but given what we have so far:

echo "KUBE_AWS_ASSUMED_HOSTNAME=$advertised_hostname
*snip*
ETCD_INITIAL_ADVERTISE_PEER_URLS=https://$advertised_hostname:2380" >> /var/run/coreos/etcd-environment
ETCD_VERSION={{.Etcd.Version}}

It should be the something like the below:

echo "KUBE_AWS_ASSUMED_HOSTNAME=$advertised_hostname
*snip*
ETCD_INITIAL_ADVERTISE_PEER_URLS=https://$advertised_hostname:2380
ETCD_VERSION={{.Etcd.Version}}" >> /var/run/coreos/etcd-environment

or ETCD_VERSION would be just thrown away.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting it guys. Actually first commit was both wrong and redundant, i removed it.
Cloud formation based propagation is enough to fix the problem

@ytsarev ytsarev closed this Jul 4, 2017
@ytsarev ytsarev reopened this Jul 4, 2017
@mumoshu
Copy link
Contributor

mumoshu commented Jul 5, 2017

@ytsarev Ah, that made sense! Thanks for your contribution 👍

@mumoshu mumoshu merged commit 1af89af into kubernetes-retired:master Jul 5, 2017
@mumoshu mumoshu modified the milestone: v0.9.7 Jul 5, 2017
@mousavian mousavian deleted the upstream/etcdversion branch February 25, 2019 04:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants