Skip to content

Comments

Fix Docker image#440

Merged
ob-stripe merged 1 commit intomasterfrom
ob-fix-docker
Apr 30, 2020
Merged

Fix Docker image#440
ob-stripe merged 1 commit intomasterfrom
ob-fix-docker

Conversation

@ob-stripe
Copy link
Contributor

Reviewers

r? @brandur-stripe @tomer-stripe
cc @stripe/dev-platform

Summary

Fix Docker image.

My change from yesterday to enable cgo (#439) unfortunately broke the Docker image for the CLI, because the Alpine image does not have the required libraries that the cgo-enabled stripe binary expects.

I've fixed this by disabling cgo for Linux, and keeping it enabled for macOS and Windows, and verified that the Docker image works again (by building locally). The main reason we wanted cgo was to fix the DNS issue on macOS anyway.

If it turns out that using cgo for Linux is desirable for some reason, there are a few ways we can enable it:

  • compile 2 different Linux binaries, one with cgo enabled and one with cgo disabled. Use the first one for distributing and the second one for the Docker image. (This will result in a much more complex `.goreleaser.yml file.)
  • compile the Linux binary with cgo enabled, and update the Docker image to support it. (But this will result in a larger Docker image.)

binary: stripe
env:
- CGO_ENABLED=1
- CGO_ENABLED=0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strictly speaking this was all that was needed to fix the issue. The rest is just opportunistic fixes/updates.

@brandur-stripe
Copy link

Seems fine! You might want to take at the failing build in the matrix — I don't think it's related to your change, but it seems like it's possible to get a data race while running the test suite.

@ob-stripe ob-stripe merged commit 73f3698 into master Apr 30, 2020
@ob-stripe ob-stripe deleted the ob-fix-docker branch April 30, 2020 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants