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

Fix the auto-setup image and test images in CI #175

Merged
merged 8 commits into from
Dec 12, 2023
Merged

Conversation

tdeebswihart
Copy link
Collaborator

What was changed

I combined the auto-setup and server dockerfiles to work around an issue with (as far as I can tell) docker buildx bake and ENTRYPOINTs in multi-stage builds.

I also added a simple test to CI that ensures our images work

Why?

docker buildx bake doesn't seem to let you override the entrypoint of the current build context, so when we changed to using a bakefile we broke the auto-setup image. We can work around this by combining the two into a single multi-stage dockerfile and picking the target we want to build separately.

I added two new stages to the server dockerfile to handle this:

  1. server - the server itself without the auto-setup stuff. This is equivalent to what the server.Dockerfile would build prior to this change
  2. auto-setup - this is the server-with-auto-setup image

`docker buildx bake` doesn't seem to let you override the entrypoint of
the current build context, so when we changed to using a bakefile we
broke the auto-setup image. We can work around this by combining the two
into a single multi-stage dockerfile and picking the target we want to
build separately.

I added two new stages to the server dockerfile to handle this:

1. `server` - the server itself without the auto-setup stuff. This is
equivalent to what the server.Dockerfile would build prior to this
change
2. `auto-setup` - this is the server-with-auto-setup image
@tdeebswihart tdeebswihart requested a review from a team as a code owner December 12, 2023 15:33
Comment on lines +1 to +5
COMPOSE_PROJECT_NAME=temporal
CASSANDRA_VERSION=3.11
ELASTICSEARCH_VERSION=8.0.0
MYSQL_VERSION=8
POSTGRESQL_VERSION=13
Copy link
Contributor

Choose a reason for hiding this comment

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

I might have missed it; what step uses this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are default values used by docker-compose

@tdeebswihart tdeebswihart merged commit e2b4748 into main Dec 12, 2023
5 checks passed
@tdeebswihart tdeebswihart deleted the tds/fix-auto-setup branch December 12, 2023 21:46
@@ -0,0 +1,68 @@
version: "3.5"
Copy link
Member

Choose a reason for hiding this comment

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

I guess docker-compose repo should be included as submodule here, to make it nice.

@@ -1,26 +0,0 @@
ARG GOPROXY
Copy link
Member

Choose a reason for hiding this comment

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

Long time ago we had one Dockerfile. Then I split it "per image". There was a good reason for it, but I couldn't recall. Why do you merging them back?

Copy link
Collaborator Author

@tdeebswihart tdeebswihart Jan 18, 2024

Choose a reason for hiding this comment

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

I combined the auto-setup and server dockerfiles to work around an issue with (as far as I can tell) docker buildx bake and ENTRYPOINTs in multi-stage builds.

Buildx won't let you override the entrypoint the way we needed to in a multi-stage build. I don't recall why (it's been a month), but it doesn't let you do it.

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.

3 participants