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

Search for etcd in PATH, don't install if installed #5427

Merged
merged 5 commits into from
Nov 15, 2019

Conversation

morgo
Copy link
Contributor

@morgo morgo commented Nov 12, 2019

This is a revision of #5365

I have changed from strictly "make it a prerequisite" to "only install it if not present".

I resumed this PR because I noticed that we also distribute etcd in the Vitess packages. This PR is required to transition out of that. I think it is important to not ship 3rd party software (unless it is compiled in such as a library) because then Vitess is on the hook for security updates.

All the install and contributor guides already add $VTROOT/bin to $PATH, so switching to require it in path should work fine vs. ETCD_BINDIR.

Signed-off-by: Morgan Tocker <tocker@gmail.com>
Signed-off-by: Morgan Tocker <tocker@gmail.com>
Signed-off-by: Morgan Tocker <tocker@gmail.com>
Signed-off-by: Morgan Tocker <tocker@gmail.com>
Signed-off-by: Morgan Tocker <tocker@gmail.com>
@morgo
Copy link
Contributor Author

morgo commented Nov 12, 2019

I checked this with:

make docker_bootstrap && go run test.go -pull=false

The prepared statement test currently fails (expected), and I had to re-run the mysql57.backup_mysqlctld test.. but it otherwise passes :-)

@morgo morgo requested a review from deepthi November 12, 2019 22:27
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

lgtm

@morgo morgo merged commit bd7cb8a into vitessio:master Nov 15, 2019
@morgo
Copy link
Contributor Author

morgo commented Nov 15, 2019

A bug was discovered in this, where the scripts are not compatible with etcd 3.4.

Since this changes to not install etcd if it's installed, it will break if the system has etcd 3.4 installed. I will followup and figure out what we need to fix.

@morgo morgo deleted the morgo-etcd-as-prereq branch November 15, 2019 21:58
This was referenced Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants