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

Docker support (Docker Compose and Dockerfile) #128

Merged
merged 1 commit into from
Jun 26, 2019

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Jul 12, 2017

/!\ This piece of software is experimental and may break your app. Don't use it without a proper versioning strategy.

This is a part of my global plan to add a first-class Docker support in Symfony. It requires dunglas/symfony-docker#5.

This PR allows recipes to updates docker-compose.yaml (to add services and volumes for instance) and the Dockerfile (to add PHP extensions for instance).

I'll blog about this when I'll be back from vacation.

@francoispluchino
Copy link

With the proposal #173, this PR would become useless, but you could add this feature to Flex by creating a Composer plugin requiring Flex in her dependencies.

@stof
Copy link
Member

stof commented Oct 4, 2017

Well, creating a plugin as described in #173 would not help much AFAIU, as it would also require extending the flex server to support more fields in the recipes (your proposal does not only involve changing Flex, but also the recipes)

@francoispluchino
Copy link

It is not necessary to add new server-side fields, if the plugin is able to retrieve information elsewhere, for example: in the composer extra section, or in local file.

@dunglas
Copy link
Member Author

dunglas commented Oct 4, 2017

For this PR for instance, it must retrieve fields from the recipe, so from the server (because the server parses and modifies the recipe).

@dunglas
Copy link
Member Author

dunglas commented Oct 4, 2017

But regarding this PR, it can be made more generic as Dockerfiles are just text files, and docker-compose.yaml YAML files. I will probably rename the configurators, but I would like to test it as is before (it requires some tiny server-side modifications).

@francoispluchino
Copy link

@dunglas Regarding the required new fields, see my comment.

@dunglas
Copy link
Member Author

dunglas commented May 28, 2018

@fabpot has done the required changes on the Flex server side. I need to test if it works now. I'll do it as soon as possible.

@covex-nn
Copy link
Contributor

covex-nn commented May 28, 2018

You use json array to describe services for docker-compose.yaml. Each item of that array represents a line of a yaml file, and it is a string with some spaces at the beginning. Indent must be validated somehow, because all recipes must use a same indent in docker-compose.yaml. If one recipe will use 2 spaces, and the other - will use 4 (or even 3), then docker-compose will not be able to start services. But these services can be launched independently. Try it for yourself:

version: '3.4'

services:
    mongo: # first recipe, 4 spaces
        image: mongo:3.6
  mysql: # second recipe, 2 spaces
    image: mysql:5.7

Also, what if a service name (or a volume name) for two different recipes will be equal?

@covex-nn
Copy link
Contributor

I can skip DockerfileConfigurator by removing ###> recipes ### from my Dockerfile. But it won't be possible to skip DockerComposerConfigurator - it will add not validated $config to my docker-compose.yaml anyway =(

@dunglas dunglas mentioned this pull request May 28, 2018
@maxhelias
Copy link
Contributor

maxhelias commented May 28, 2018

Would not it be relevant to disable docker support for people who do not use it so that there is no loss of performance in case they already have a docker-composer.yaml or Dockerfile file at the root of their project ?

I see this with an options in the composer.json but why not in another way if someone has a better idea

@maxhelias
Copy link
Contributor

maxhelias commented Jan 23, 2019

@covex-nn we have add some new features.
Now you can enable / disable support in composer.json with symfony.docker extra.
The indentation of the file is taken into consideration and each new service will be added at the end of the list.
The format in the recipe looks like this symfony/recipes#413
I'm waiting your feedback 😃

Note : a rebase is necessary for fix .gitignore and AbstractConfigurator do not take into consideration their modication

Copy link
Contributor

@maxhelias maxhelias left a comment

Choose a reason for hiding this comment

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

Some small correction and it will be perfect 😃
It is to better position the services

src/Configurator/DockerComposeConfigurator.php Outdated Show resolved Hide resolved
src/Configurator/DockerComposeConfigurator.php Outdated Show resolved Hide resolved
@dunglas
Copy link
Member Author

dunglas commented May 11, 2019

This one is ready to be merged.

@maxhelias
Copy link
Contributor

maxhelias commented May 13, 2019

If anyone wants to try, just following these instruction

Clone a docker skeleton :

git clone https://github.com/dunglas/symfony-docker

Run the docker :

docker-compose up -d

Switch branch :

Define the fork repo on repositories section like this :

"repositories": [
 {
   "type": "vcs",
   "url": "https://github.com/dunglas/flex"
 }
],

Define the branch dev-docker for symfony/flex dependencies on composer.json

Test a recipe :

SYMFONY_ENDPOINT=https://symfony.sh/r/github.com/symfony/recipes/{PR} composer req {recipe}

Recipe available :

Mercure : symfony/recipes#583
Messenger : symfony/recipes#413
Doctrine : symfony/recipes#113

@teohhanhui
Copy link

Would love to review this before it gets merged. Hope we're not in a hurry, after 2 years.

@dunglas
Copy link
Member Author

dunglas commented Jun 13, 2019

Hi there, can we merge this one?

@dunglas dunglas force-pushed the docker branch 2 times, most recently from 7a4d907 to d889abf Compare June 19, 2019 18:58
@fabpot
Copy link
Member

fabpot commented Jun 26, 2019

@dunglas Can you cleanup the commit history? That's the last thing to do before merging!

@dunglas dunglas force-pushed the docker branch 3 times, most recently from fd7593c to 37b0e3f Compare June 26, 2019 16:57
@dunglas
Copy link
Member Author

dunglas commented Jun 26, 2019

@fabpot last comments fixed, and history cleaned.

@teohhanhui

git (or something else) might do EOL conversion... So that assumption is dangerous.

Makes sense, changed to use a regex then.

Maybe use the same markers as Docker official images?

Flex uses the same markers everywhere. I'm not sure if it's a good idea to change just for Docker.

@teohhanhui
Copy link

Flex uses the same markers everywhere. I'm not sure if it's a good idea to change just for Docker.

Indeed it's not important, as long as it's appropriate for the target file's syntax. 😄

@fabpot
Copy link
Member

fabpot commented Jun 26, 2019

Thank you @dunglas.

@fabpot fabpot merged commit 78c2bec into symfony:master Jun 26, 2019
fabpot added a commit that referenced this pull request Jun 26, 2019
This PR was merged into the 1.3-dev branch.

Discussion
----------

Docker support (Docker Compose and Dockerfile)

/!\ This piece of software is experimental and may break your app. Don't use it without a proper versioning strategy.

This is a part of my global plan to add a first-class Docker support in Symfony. It requires dunglas/symfony-docker#5.

This PR allows recipes to updates `docker-compose.yaml` (to add services and volumes for instance) and the `Dockerfile` (to add PHP extensions for instance).

I'll blog about this when I'll be back from vacation.

Commits
-------

78c2bec Docker support (Docker Compose and Dockerfile)
@dunglas dunglas deleted the docker branch July 1, 2019 17:34
@ro0NL ro0NL mentioned this pull request Jul 10, 2019
@Plopix
Copy link

Plopix commented Sep 21, 2020

question here regarding Docker Integration: symfony/symfony#38259

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.

None yet

10 participants