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

Allow using custom ARM-based Docker images #219

Merged
merged 4 commits into from
Jun 9, 2023
Merged

Allow using custom ARM-based Docker images #219

merged 4 commits into from
Jun 9, 2023

Conversation

orkhanahmadov
Copy link
Contributor

@orkhanahmadov orkhanahmadov commented Apr 18, 2023

This PR allows using custom ARM-based Docker images.

We always pre-build vapor image with extras (like Imagick extension), push it to AWS ECR then use that ECR image for deployments in production.Dockerfile. Disallowing custom ARM images prevents doing this.

@orkhanahmadov orkhanahmadov changed the title Revert change that prevents custom Docker images Revert change that prevents using custom Docker images Apr 18, 2023
@orkhanahmadov orkhanahmadov marked this pull request as ready for review April 18, 2023 09:14
@orkhanahmadov orkhanahmadov changed the title Revert change that prevents using custom Docker images Revert change that prevents using custom Docker ARM images Apr 18, 2023
@nunomaduro nunomaduro marked this pull request as draft April 18, 2023 11:31
@nunomaduro
Copy link
Member

Bugfix release 1.55.2 introduced a breaking change for everyone that uses custom Docker ARM images for deployments.

What do you mean by "introduced a breaking change", if we shipped ARM for docker yesterday? We still allow custom images if customers wish to build their own amd64 image. However, for arm64, that we shipped yesterday, we are developing internally some things that we wish to ship before we allow our customers to build custom ARM builds.

@orkhanahmadov
Copy link
Contributor Author

orkhanahmadov commented Apr 18, 2023

@nunomaduro sorry the for confusion. When we started having issues on our deployments and when started investigating the issue, also looking into new ARM-based images, the latest available versions of the package were 1.55.1 and 1.55.2 they both disallowed using any custom images, which was a breaking change. Now I noticed 1.55.3 was released and it also allows x86 Docker images again.

But I think you can still consider this PR since it also allows using ARM-based custom Docker images too.

In our use case, we won't be using laravelphp/vapor:php82-arm as a deployment image, because we add more steps to it, which unnecessarily increases build time on every deployment. We always use our own AWS ECR pre-build image which uses laravelphp/vapor:php82-arm as a base.

@orkhanahmadov orkhanahmadov changed the title Revert change that prevents using custom Docker ARM images Allow using custom ARM-based Docker images Apr 18, 2023
@orkhanahmadov
Copy link
Contributor Author

Just updated the PR title and description to better reflect the purpose of this PR.

@joedixon
Copy link
Contributor

Is this relevant or should I open a support ticket?

Hey @aubreychiduku can you open a support ticket for this please as it's an unrelated issue.

@orkhanahmadov
Copy link
Contributor Author

orkhanahmadov commented Apr 18, 2023

More context:

To build the base image for ARM I also noticed that the image requires a __VAPOR_RUNTIME argument. We pass it manually:

docker build -t vapor-base . --build-arg __VAPOR_RUNTIME=docker-arm

I mentioned this yesterday to Taylor's tweet with the ARM support announcement.

@joedixon
Copy link
Contributor

Hey @orkhanahmadov we do plan to add support for custom images using the docker-arm runtime. However, we want to put some safeguards in place to prevent the accidental deployment of an Arm image to a Lambda running x86.

We would advise against pre-building your image until we have this update in place which should be in the coming days.

Closing this PR for now.

@joedixon joedixon closed this Apr 18, 2023
@pyr0hu
Copy link

pyr0hu commented May 28, 2023

@joedixon any updates on this one?

@imrodrigoalves
Copy link

@joedixon When should we expect this feature to be available?

@joedixon joedixon reopened this Jun 6, 2023
@joedixon
Copy link
Contributor

joedixon commented Jun 6, 2023

I'm going to give this another test, but assuming it all works as expected, I think we can get this merged now.

@joedixon joedixon marked this pull request as ready for review June 9, 2023 10:18
@joedixon joedixon merged commit b7e8fe8 into laravel:master Jun 9, 2023
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.

5 participants