Revamp Dockerfile

[lmakarov]

I'm working on integrating blackfire with drude.io and came across the blackfire agent Dockerfile.
Please consider my take on improving it.

Summary of changes:

• use ADD instead of binaries in the repo
• install ca-certificates instead if curl (lighter)
• use BLACKFIRE_VERSION to define blackfire version
• set configuration variables via environment variables (this only give a good example on what can be overridden)
• remove "ENV PORT 8707" - not used anywhere
• replace CMD with ENTRYPOINT

[romainneutron]

Hello @lmakarov ,

Thanks for this contribution!
I'm sorry in advance for the late review, we can not work on it right now.

Don't worry, we'll review this very soon, and thanks again for contributing

[tucksaun]

Hello @lmakarov,

First, I'm really sorry about the late answer, I completely forgot to answer this.

Regarding your proposal:

• install ca-certificates instead if curl (lighter)

Actually, we want to allow blackfire curl to work and this image is also designed for this usage: using alpine and adding blackfire binary (vs. blackfire-agent) is done with this goal in mind.

• replace CMD with ENTRYPOINT

That would work pretty well in a single blackfire-agent context, however, that would currently prevent usages of the blackfire binary, or at least would make it more difficult by requiring end-users to overwrite the entrypoint.
Also, that would be a BC break for our users currently relying on this.

• use ADD instead of binaries in the repo

That's actually something we are not really keen to do because it would be more complicated, constraining and error prone for our releasing process:
Currently, we just need to commit to this repo and then Docker Hub is building the image, using remote URLs would require us to wait for all the other layers to be synchronized before being able to take care of Docker images.
Also, we have a staging releasing pipeline which is not reachable from external networks and that would just not work with remote URLs.

• set configuration variables via environment variables (this only give a good example on what can be overridden)
• remove "ENV PORT 8707" - not used anywhere

Those are good points, I will do the changes in the appropriate scripts, however, this will be applied by a bot, so you will loose ownership of the patch, is that okay for you?

Thanks again for contributing.

[lmakarov]

@tucksaun sure, go ahead and use this PR in full or partially.
Thanks for explaining the reasons for the way certain things are right now. It all makes sense now.

[tucksaun]

thanks @lmakarov it's going to be merged soon in our private repo so it will land here with next stable release.