WIP: Uses docker to build Go and packages #750

zmoazeni · 2019-05-31T18:36:18Z

This is related to the conversation at #740

Note: this is a draft PR to request feedback

I still need to convert script/cibuild-gh-ost-replica-tests and localtests/test.sh.

Overall this works really well. I have made a few changes to the output directories, which I imagine you'll have opinions on. ./build.sh still builds the binaries and deb/rpm/tarball packages.

I added a new option for ./script/cibuild to optionally skip building everything. Though I do wonder if build.sh should be pushed into ./script/build and ./script/cibuild either be deleted (and folded into the build script). I'm not sure, something feels off organizationally with those behaviors.

This allows us to quickly upgrade (or test) go versions. It also simplifies the current process that downloads the go binaries and messing with the GOPATH. We build in the docker/ folder to avoid sending the entire directory (vendor/ included) to the docker context. That can add some unneccesary time to the build since we're not copying anything.

zmoazeni · 2019-05-31T18:52:58Z

@shlomi-noach here's a first look at the updates to the build process. This is still WIP. My next steps are to convert script/cibuild-gh-ost-replica-tests and localtests/test.sh but it's at a spot to get some initial feedback.

zmoazeni · 2019-05-31T18:37:27Z

.gitignore

 /libexec/
 /.vendor/
+/.cache
+/build


I changed the output of the build artifacts. Now everything ends up in ./build:

% find ./build ./build ./build/dist ./build/dist/gh-ost-binary-osx-20190531141809.tar.gz ./build/dist/gh-ost_1.0.48_amd64.deb ./build/dist/gh-ost-1.0.48-1.x86_64.rpm ./build/dist/gh-ost-binary-linux-20190531141809.tar.gz ./build/bin ./build/bin/gh-ost-linux-amd64 ./build/bin/gh-ost-darwin-amd64

Totally get it if you don't like this. I thought I'd offer it up first though. That way we're not splitting the ./bin and /tmp/gh-ost-release directories.

build/ makes perfect sense! Let's add /build/ to .gitignore

Will change!

For my own education, how does git treat /build and /build/ differently?

zmoazeni · 2019-05-31T18:40:17Z

build.sh

-    exit 1
+  if [ "$goos" == "darwin" ]; then
+    # specify cross compiler
+    envs="$envs -e CC=x86_64h-apple-darwin14-cc"


This is the magic of dockercore/golang-cross. By specifying the CC that's already bundled, we can cross compile for darwin.

I'm 90% sure this is fine for the release. I copied this over to a mac machine I have and verified that I can run at least --version. (I might eat my hat once the localtests, but I'm pretty confident this will be okay)

zmoazeni · 2019-05-31T18:41:49Z

build.sh

+      && fpm -v "${RELEASE_VERSION}" --epoch 1 -f -s dir -n gh-ost -m "shlomi-noach <[email protected]>" --description "GitHub'"'"'s Online Schema Migrations for MySQL " --url "https://${PACKAGE_PATH}" --vendor "GitHub" --license "Apache 2.0" -C /gh-ost --prefix=/ -t rpm . \
+      && cp *.rpm ${OUTPUT_PATH}'
+
+  $fpm_cmd bash -c 'mkdir -p /gh-ost/usr/bin \


You can test out the output of any of these in docker too:

% docker run --rm -it -v $(pwd):/ghost ubuntu dpkg -c /ghost/build/dist/gh-ost_1.0.48_amd64.deb drwxrwxrwx 0/0 0 2019-05-31 18:18 ./ drwxr-xr-x 0/0 0 2019-05-31 18:18 ./usr/ drwxr-xr-x 0/0 0 2019-05-31 18:18 ./usr/share/ drwxr-xr-x 0/0 0 2019-05-31 18:18 ./usr/share/doc/ drwxr-xr-x 0/0 0 2019-05-31 18:18 ./usr/share/doc/gh-ost/ -rw-r--r-- 0/0 155 2019-05-31 18:18 ./usr/share/doc/gh-ost/changelog.gz drwxr-xr-x 0/0 0 2019-05-31 18:18 ./usr/bin/ -rwxr-xr-x 0/0 10029736 2019-05-31 18:18 ./usr/bin/gh-ost % docker run --rm -it -v $(pwd):/ghost fedora rpm -qlp /ghost/build/dist/gh-ost-1.0.48-1.x86_64.rpm /usr/bin/gh-ost

zmoazeni · 2019-05-31T18:43:19Z

build.sh

-  ldflags="-X main.AppVersion=${RELEASE_VERSION}"
+function distro_packages() {
+  echo "Creating Distro full packages"
+  docker build -t gh-ost/fpm -f docker/Dockerfile.fpm docker


It's true that this will build the fpm image every time, but the docker layer cache will make this super fast on repeated runs.

At some point I will want to get rid of fpm.

zmoazeni · 2019-05-31T18:44:39Z

script/cibuild

-script/build
-
-cd .gopath/src/github.com/github/gh-ost
+if [ "$1" != "fast" ]; then


If you just want to run gofmt and test, you can run this as ./script/cibuild fast.

However, see my top-level comment. Something feels off that this is separate from build.sh. I'm thinking that this code should just be combined into one script at this point.

zmoazeni · 2019-05-31T18:46:15Z

script/common

+  RELEASE_VERSION=$(cat RELEASE_VERSION)
+fi
+
+DGO="docker run -u $(id -u):$(id -g) --rm \


-u $(id -u):$(id -g) I do this so that the user/group permissions of the files generated stay the host user's. Otherwise it becomes root:root and a permissions pain.

zmoazeni · 2019-05-31T18:48:34Z

script/go

-
-cd .gopath/src/github.com/github/gh-ost
-go "$@"
+$DGO go "$@"


This still felt somewhat useful. Shouldn't be necessary if you have go installed locally, but 🤷‍♂️

zmoazeni · 2019-05-31T18:50:20Z

test.sh

@@ -1,13 +0,0 @@
-#!/bin/bash


I'm not sure where this script runs. This is equivalent to ./script/go test ./go/... (or ./script/go test -count 1 ./go/... if you want to disable test caching)

zmoazeni · 2019-05-31T18:51:12Z

build.sh

+  envs="-e GOOS=$goos \
+    -e GOARCH=amd64 \
+    -e CGO_ENABLED=1 \
+    -e GOCACHE=/go/src/${PACKAGE_PATH}/.cache \


This is necessary since we don't run these commands as root inside the container (so we can't write to /).

Maintaining these files in the host directory does have a benefit of repeated builds being faster if you keep around the ./.cache folder.

zmoazeni · 2019-05-31T18:51:51Z

build.sh

+  osshort=$goos

+  envs="-e GOOS=$goos \
+    -e GOARCH=amd64 \


Technically, nothing is stopping this from extending to also offering 386 or windows at this point. But I didn't go that far.

There's an open issue+PR for supporting Raspberry Pi. Truly.

Hah. I haven't tried compiling for ARM but it doesn't look that difficult https://www.thepolyglotdeveloper.com/2017/04/cross-compiling-golang-applications-raspberry-pi/

I imagine ARM is going to get more popular with the Intel NUC crowd and the rumors MacBook Pro will ditch Intel and move to it.

Only build distributions when -d is passed

zmoazeni · 2019-06-06T04:05:14Z

@shlomi-noach Ok this is getting very close to a full PR. However in this process I started running into the intermittent tests that I mentioned in #738.

So I noodled on it a bit. At first I started with a similar tactic you already had of starting a single server for the version and running all tests against it.

Then after seeing the intermittent failures, I decided to start/kill a docker container for each test. It's slower but safer because we don't leave any remnants of a previous test on a fresh container boot. But even after that I'm still seeing errors crop up.

I thought perhaps this was a v1.12.x only issue, but even changing the GO_VERSION file back to 1.9.2. But I still saw errors.

I wonder if perhaps there's a default variable with the dbdeployer mysql servers that isn't lenient enough that we're tripping over. Or maybe something with the default values we're passing gh-ost. I'm not sure.

I haven't dug deeper yet, but here are two snippets from intermittent failures: https://gist.github.com/zmoazeni/4ea85d89fa07280f6fc5c41c0e2020c0

Note: they are not the same tests that regularly fail. Seems to happen all over place.

shlomi-noach · 2019-06-06T06:50:55Z

@zmoazeni thank you for your work thus far!

I'm not sure either about these intermittent replicaiton-not-running failures. I've seen these from time to time and I'm just not sure why they keep coming up.

OK, some news: at a strange timing coincidence, we (GitHub) are to now move all our internal builds to run on Docker. All our *-build-deploy-tarball builds mentioned earlier will need to be dockerized.

This makes me both grateful for the work you've done, as well as uncomfortable that if you continue doing it, you'll be contributing your volunteer work on something we now need to do internally on our pay job. This is all new, so I don't have much insight into the work yet, but please consider this as heads up -- and for your consideration ❤️

zmoazeni · 2019-06-06T13:56:50Z

Ah! Thanks for letting me know. I really appreciate that. What I'll do then is back off this initiative until you get more information and let you push it forward. I'll leave this PR up so feel free to pick-n-choose what makes sense for that. And if you have any questions about why I did something a certain way, definitely feel free to ask.

I was also wondering if we should do this on orchestrator too, but GH's internal build changes will require that too.

I'll probably poke at these intermittent failures with this code, maybe I can find a reliable way to reproduce it on a single test. I feel like it's pretty close. My first intuition is that this isn't gh-ost code itself running into an issue.

With the "ERROR replication not running; Slave_IO_Running=No, Slave_SQL_Running=No", I might try turning on the general log and inspecting the MySQL container when the test fails to see what it ran up to that point to break replication.

There are some legitimate retries that can occur during testing. Namely `logic.ExpectProcess()` (in `applier.go`). We'll look for a process that does exist, but timing-wise doesn't have the `state` or `info` columns populated. Without this, the test will fail abruptly.

zmoazeni · 2020-09-22T13:20:44Z

Closing unmerged. I have since moved my focus elsewhere.

zmoazeni added 3 commits May 31, 2019 14:26

Drops unnecessary test.sh

a011854

Drop note about downloading go

5c85dfb

zmoazeni commented May 31, 2019

View reviewed changes

zmoazeni added 5 commits May 31, 2019 19:12

Restructure the docker files

db1ef37

WIP dbdeployer in docker preping for tests

2753ce1

Working localtests/test-docker.sh

37f60a7

We don't need this anymore

acdf7bb

Run localtests against docker containers

05e6c31

Only build distributions when -d is passed

zmoazeni mentioned this pull request Jun 7, 2019

Intermittent failures in localtests #753

Closed

zmoazeni added 3 commits June 10, 2019 09:28

Fail if we can't build or docker doesn't start

48a5bf0

Add suffix to ignore

f051fa3

zmoazeni closed this Sep 22, 2020

WIP: Uses docker to build Go and packages #750

WIP: Uses docker to build Go and packages #750

Uh oh!

Conversation

zmoazeni commented May 31, 2019

Uh oh!

zmoazeni commented May 31, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zmoazeni commented Jun 6, 2019

Uh oh!

shlomi-noach commented Jun 6, 2019

Uh oh!

zmoazeni commented Jun 6, 2019

Uh oh!

zmoazeni commented Sep 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants