-
Notifications
You must be signed in to change notification settings - Fork 441
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
More arches (ppc64le + arm64v8) and ubuntu #184
Conversation
10.0/Dockerfile
Outdated
@@ -24,6 +37,7 @@ RUN set -ex; \ | |||
export GNUPGHOME="$(mktemp -d)"; \ | |||
gpg --keyserver ha.pool.sks-keyservers.net --recv-keys B42F6819007F00F88E364FD4036A9C25BF357DD4; \ | |||
gpg --batch --verify /usr/local/bin/gosu.asc /usr/local/bin/gosu; \ | |||
{ command -v gpgconf > /dev/null && gpgconf --kill all || :; }; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{
and }
aren't needed here since we're already in a semi-colon-ized block (only need those in a series of &&
) 😇
10.0/Dockerfile
Outdated
echo 'Pin-Priority: 998'; \ | ||
} > /etc/apt/preferences.d/percona | ||
|
||
# bashbrew-architectures: ppc64le i386 amd64 arm64v8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ordering is a little odd -- is this just the order Bash spits them out? I wonder if it'll change from Bash version to version (hopefully not).
10.1/Dockerfile
Outdated
&& apt-get update \ | ||
&& apt-get install -y \ | ||
} | debconf-set-selections; \ | ||
XTRABACKUP='mariadb-backup-10.1'; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we call this variable just backupPackage
or something instead? (since it's now preferred to not use xtrabackup if we can avoid it?)
update.sh
Outdated
) | ||
declare -A dpkArchToBashbrew=( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dpkArchToBashbrew
should be dpkgArchToBashbrew
update.sh
Outdated
if ver="$(getRemoteVersion "$version" "$suite" "$arch")" && [ -n "$ver" ]; then | ||
arches="$arches ${dpkArchToBashbrew[$arch]}" | ||
fi | ||
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could use something like echo "$arches" | xargs -n1 | sort | xargs
to sort these more deterministically? ala arches="$(echo "$arches" | xargs -n1 | sort | xargs)"
?
097d434
to
6538ae6
Compare
Yay docker-library/php#666 🎉 🍰 |
…parsing Also adjust travis to use pgp-happy-eyeballs
6538ae6
to
195c843
Compare
Let's do this. |
&& apt-get update \ | ||
&& apt-get install -y \ | ||
} | debconf-set-selections; \ | ||
backupPackage='mariadb-backup-10.1'; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for breaking our existing Galera setup without warning... A heads-up next time will be nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm definitely sorry that your setup broke, but to say it was "without warning" is a little bit much.
We only have two ways of informing users of changes to images -- PRs/issues on this repository, and PRs with the library/mariadb
label on the official-images repository, of which this change had both, so there's unfortunately really not much more "heads-up" we could provide.
Additionally, I'd heartily recommend setting up some kind of testing environment where you can test image updates before deploying them to ensure that this sort of breakage doesn't affect you in the future.
All that being said, breaking folks wasn't an intention of this change, so it sounds like we need to do more to adapt this. Can you provide more detail about what your setup looks like and how this broke it? Ideally we'd love a couple simplified commands or a compose/stack YAML that can reproduce the failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm definitely sorry that your setup broke, but to say it was "without warning" is a little bit much.
I'm sorry it came out a bit harsh.
We only have two ways of informing users of changes to images -- PRs/issues on this repository, and PRs with the library/mariadb label on the official-images repository, of which this change had both, so there's unfortunately really not much more "heads-up" we could provide.
You are correct, but the change (xtrabackup -> mariabackup) should have been mentioned in the commit message (IMHO, its a breaking change).
Additionally, I'd heartily recommend setting up some kind of testing environment where you can test image updates before deploying them to ensure that this sort of breakage doesn't affect you in the future.
It didn't break the cluster, it only broke bootstrapping of new database nodes.
All that being said, breaking folks wasn't an intention of this change, so it sounds like we need to do more to adapt this. Can you provide more detail about what your setup looks like and how this broke it? Ideally we'd love a couple simplified commands or a compose/stack YAML that can reproduce the failure.
Galera use SST for the initial node bootstrapping and you can choose between mysqldump
, rsync
or xtrabackup
/mariabackup
. So when xtrabackup
was removed, you broke the xtrabackup
SST method. It was easy to fix by switching to the mariabackup
SST method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@klausenbusk do you think that we should add a symlink for xtrabackup
to mariabackup
like you had in #158 or should we just leave it as is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@klausenbusk do you think that we should add a symlink for xtrabackup to mariabackup like you had in #158 or should we just leave it as is?
Just leave it as it is, a symlink wouldn't solve it anyway.
Just to thread the needle, there were some minor issues with this on |
Closes #175
Closes #158
Closes #155
Closes #111