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

Add xtrabackup to the image, update the apt sources #5

Merged
merged 1 commit into from Sep 27, 2018

Conversation

Projects
None yet
2 participants
@tebriel
Contributor

tebriel commented Sep 12, 2018

SURPRISE. Bet you didn't think you'd see updates to this container. JK I'm sure you did.

  • The galeracluster apt repos for wsrep and galera have moved.
  • The latest version of mysql 5.6 puts a log-error line into /etc/mysql/mysql.conf.d/mysqld.cnf which prevents logging to the console (as our runtime.cnf is evaluated before mysqld.cnf).
  • To set the percona debian repos, you install a deb and that deb sets your apt sources 🤷‍♂️
  • Set some more sane defaults for the runtime.cnf.
  • Install percona xtrabackup, as it's very different and not compatible with the current backup scripts, I just left those along and you can call out to xtrabackup. But apparently it needs access to the unix socket for backing up :(

I won't be upset if you don't approve or want to merge this. But I thought I'd contribute upstream if you wanted it.

@brimstone

Thanks for the patch! I left a question inline. I'd like tests to pass before merging this.

@@ -96,7 +97,7 @@ if [ "${1:-}" = "garbd" ]; then
fi
if [ -e /etc/mysql/conf.d/runtime.cnf ]; then
# This is our first run.
# This is not our first run.

This comment has been minimized.

@brimstone
&& sed -i 's|^log-error.*$||' /etc/mysql/mysql.conf.d/mysqld.cnf
# last line removes the log-error line from the mysqld.conf which prevented logging to console
ADD https://repo.percona.com/apt/percona-release_0.1-6.jessie_all.deb percona-release_0.1-6.jessie_all.deb

This comment has been minimized.

@brimstone

brimstone Sep 13, 2018

Owner

Is curl or wget in this image already? Could this and the next layer be reworked to perform the download and install in one layer?

This comment has been minimized.

@tebriel

tebriel Sep 13, 2018

Contributor

i dunno, lemme check!

This comment has been minimized.

@tebriel

tebriel Sep 13, 2018

Contributor

nope, neither exist. I could run a package wget then remove it after all in the same layer if you prefer.

This comment has been minimized.

@brimstone

brimstone Sep 15, 2018

Owner

It's 7.6K, I think we can afford to leave it as is.

This comment has been minimized.

@tebriel

tebriel Sep 15, 2018

Contributor

Okay, sounds good.

@brimstone

This comment has been minimized.

Owner

brimstone commented Sep 15, 2018

Any thoughts on the failing node0 test? I'm just waiting on the tests to pass to merge this.

@tebriel

This comment has been minimized.

Contributor

tebriel commented Sep 15, 2018

They fail consistently on my laptop, I think this is a bug that I've seen before in the container where bootstrapping with a mysql root password causes it to exit and never start, but I haven't gotten deep enough to figure it out. When I docker ps i see multiple created containers that aren't started. I'll try to get those sorted out soon.

@brimstone brimstone merged commit cc5c4c9 into brimstone:master Sep 27, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment