Examples: Change Dockerfile ENTRYPOINT/CMD commands to exec format.

[baremaximum]

Commit Message:
Examples: Fixes issue where some examples were not working on systems with CRLF line endings.

Additional Description:
Several examples did not work on Windows due to the CRLF line endings
breaking the entrypoint declarations specified in the Dockerfiles.
Affected containers would immediately exit with error 127 file not found.
This PR both fixes that issue, while at the same time bringing the Dockerfiles more in line
with recommended Dockerfile best practices by changing the
entrypoint declarations to exec format for the affected examples.
Fixes issue #7712.

Risk Level:
Low

Testing:
Manual. Ran docker-compose up --build on affected examples to see if they worked now.

Docs Changes: None
Release Notes: N/A

Issues: #7712

[dio]

@baremaximum thanks. Could you fix DCO? https://github.com/envoyproxy/envoy/blob/master/CONTRIBUTING.md#fixing-dco

[dio]

Is this explicit "/bin/bash" required?

[baremaximum]

Unfortunately it is. It appears that the CRLF obscures the shebang at the top of the script somehow. Without this explicit instruction, the container immediately exits with error 1 exec user process caused "no such file or directory", as opposed to error 127 when the ENTRYPOINT is in non-exec format.

I know it's ugly, but it works, and at no meaningful cost.

As an alternative, I could also rewrite the Dockerfiles and move the contents of the shell scripts into them. The scripts are only a couple lines long, and I don't really understand why they're there. A Dockerfile is ultimately a shell script wrapped in a bunch of additional features. It makes more sense to me if they actually contain the shell instructions they run, instead of containing references to their instructions.

[phlax]

i think moving the scripts into the Dockerfiles would simplify things and make recipe more explicit. It would also remove the need to ADD and chmod these.

if you do go with execing i would consider using /bin/sh rather than bash as its more portable

[dio]

cc. @phlax

[phlax]

im thinking it would probably be best to land #12491 before refactoring the examples to ensure they dont break

[baremaximum]

Ok, I'll check in again once that testing has been merged in and refactor the Dockerfiles then. Should I make a new PR for that? @dio @phlax

[dio]

Should I make a new PR for that?

@baremaximum I don't think so. I think @lizan will review that PR and if that one is landed, then you can merge main.

[phlax]

@baremaximum #12491 landed

[baremaximum]

@phlax I made the changes to the CMD and ENDPOINTS. While I was at it I updated some docker-compose versions, just because why not?

However, I didn't move all the scripts into the Dockerfile. In the end it causes problems because unix signals get passed from the envoy process to the flask process, causing an interrupt of some kind (at least that is what I think happens, not totally sure about that). Either way though, you can't have two ongoing running processes in a Dockerfile. You have to use a script to start 2 or more processes.

This PR still fixes the CRLF issue though.

[phlax]

@baremaximum im trying to clarify the problem

i read this https://nickjanetakis.com/blog/fixing-exec-format-errors-with-docker-entrypoint-scripts-on-windows - and so i followed the suggested fix there - ie running dos2unix against each of the files - but this doesnt seem to have changed anything - so im wondering if the problem is with crlf or something else

switching from /bin/command to ["/bin/command"] format means the script is execed as opposed to run anyway - and as the entrypoint is (i think) run by /bin/sh anyway the shebang shouldnt really be necessary. Which, in summary, is really confusing me. Why should it be necessary to prefix the executable with /bin/sh (ref: https://docs.docker.com/engine/reference/builder/#entrypoint)

i guess my concern, apart from a little confusion, is if that this is breaking on windows, we want to ensure that it stays fixed - or that we can detect what would break. I guess it would be helpful to have the verify-examples run on windows too - i have no way to test this locally.

Several examples did not work on Windows due to the CRLF line endings breaking the entrypoint declarations

can you tell me which examples break specificallly ?

[phlax]

i guess im also trying to figure if its a problem only locally - reading here https://willi.am/blog/2016/08/11/docker-for-windows-dealing-with-windows-line-endings/ - it suggests a cli flag to git when you checkout (see: "Cloning Git Projects With Unix Line Endings")

[phlax]

this should have a newline at the end

[baremaximum]

The affected examples are:

• cors (frontend service and backend service)
• csrf (crosssite service and samesite service)
• ext_authz
• front-proxy (service1 and service2)
• jaeger-native-tracing (service1 and service2)
• jaeger-tracing (service1 and service2)
• load-reporting-service (http_service_1)
• zipkin-tracing (service1 and service2)

The shebang thing is a bit confusing to me as well. It's some issue arising from how docker deals with these commands. But I don't understand that process well enough to guess at what the issue might be.

When I googled the error message I ended up on this stack overflow question which led me to the answer:

https://stackoverflow.com/questions/51508150/standard-init-linux-go190-exec-user-process-caused-no-such-file-or-directory

If I go in to my editor and change the line endings of the .sh files manually to LF, the issue gets resolved. So it's, definitely the line endings causing the problem.

While there is a flag to preserve line endings when cloning, it's easy to forget. I thought I had it set to always preserve line endings. I would argue that it's best to make the change to spare anybody else that happens to forget the headache of figuring out what might be going wrong. It's not totally immediately obvious.

[phlax]

The affected examples are: ...

hmm - not easy to spot a pattern there

also im noticing that the zipkin example fails, and your PR fixes it by switching CMD rather than the entrypoint - in this case i don't think there are any shebangs that could cause the identified problem

after reading the stackoverflow article that you posted, i think i ~understand why the explicit /bin/sh in the entrypoint works as a workaround. Im still not sure that this workaround is the best solution, or whether the problem should be solved by checking out the git repo correctly.

I would argue that it's best to make the change to spare anybody else that happens to forget the headache of figuring out what might be going wrong.

perhaps - im not a regular windows user, so im not sure whether this problem is a "misconfiguration" in setup, or whether most windows users would hit this problem following the docs etc.

i would have no objections to adding the explicit shebang - this PR seems to do a bit more - ie switch cmds to exec format also - which is also not a bad thing, but is confusing the problem/fix

[phlax]

@envoyproxy/windows-dev

im wondering if the fix proposed here is necessary in the general case of windows users getting started with the examples

[baremaximum]

The zipkin example is failing because it uses the services from the front-proxy example that were failing. The same thing is true for the jaeger examples.

I made the changes in those examples mostly because it's recommended best practice to write CMD statements in this form. From the Dockerfile best practices:

CMD

The CMD instruction should be used to run the software contained in your image, along with any arguments. CMD should almost always be used in the form of CMD ["executable", "param1", "param2"…]. Thus, if the image is for a service, such as Apache and Rails, you would run something like CMD ["apache2","-DFOREGROUND"]. Indeed, this form of the instruction is recommended for any service-based image.

In most other cases, CMD should be given an interactive shell, such as bash, python and perl. For example, CMD ["perl", "-de0"], CMD ["python"], or CMD ["php", "-a"]. Using this form means that when you execute something like docker run -it python, you’ll get dropped into a usable shell, ready to go. CMD should rarely be used in the manner of CMD ["param", "param"] in conjunction with ENTRYPOINT, unless you and your expected users are already quite familiar with how ENTRYPOINT works.

https://docs.docker.com/develop/develop-images/dockerfile_best-practices/

However, in the case of this repo, these changes don't have any impact.

[phlax]

can i suggest making this change in a separate PR

[baremaximum]

Sure!

[phlax]

@baremaximum - i think this is a good change - but we probs want to check other files etc - please do PR if you get chance

[baremaximum]

@phlax Done! :)

[phlax]

also this - if its not necessary then i would separate out the change

[baremaximum]

Oh that was an accident. That change shouldn't be in there. I'll take it out.

[phlax]

@baremaximum thanks for explanation - i see now

can i suggest changing the title/description to reflect that it updates CMD as well as ENTRYPOINT

it would be good to get another opinion on adding the /bin/sh - but otherwise i think the changes are good - apart from the commented nits

[sunjayBhatia]

@envoyproxy/windows-dev

im wondering if the fix proposed here is necessary in the general case of windows users getting started with the examples

These examples are being run in Linux containers on Windows, which is not something the windows-dev sub-team is focused on (rather we're focused on native Windows support)

Regardless, it seems valuable to support cloning Envoy source code with different line endings regardless of development platform if the change fixes that

If it is a priority for the project to support Linux development on Windows/Windows Subsystem for Linux to widen the potential developer pool, that sounds great, though we should be clear what we say when we mean Windows support etc. I don't know if we want to run the Linux examples on Windows in CI, I was hoping we end up building the equivalent Windows-native examples/containers etc. and run validation on those in CI

[phlax]

Regardless, it seems valuable to support cloning Envoy source code with different line endings

lets keep them then

in CI, I was hoping we end up building the equivalent Windows-native examples/containers etc.

thanks for clarifying that

[phlax]

unnecessary change/whitespace

[phlax]

thanks - looks good - one last newline needs to be added/back

[phlax]

theres still a missing newline here

[dio]

Awesome. Great job. Thanks!