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

Use recommended EXEC form of commands #2172

Merged
merged 5 commits into from
Jul 17, 2020
Merged

Use recommended EXEC form of commands #2172

merged 5 commits into from
Jul 17, 2020

Conversation

bwateratmsft
Copy link
Contributor

@bwateratmsft bwateratmsft commented Jul 17, 2020

Fixes #2090

NOTE: the "Other" scaffolding still uses the other form (CMD /usr/games/fortune -a | cowsay). I don't think there's value in changing that for two reasons--

  1. The "Other" Dockerfile is little more than a demonstration of how Dockerfiles look.
  2. The intended behavior of the image does not work when using the EXEC form + sh, -c. It is supposed to output a random bit of text (the output from /usr/games/fortune) and put it into a text box being spoken by the Docker whale (the pipe into cowsay). If the EXEC form is used, nothing is output to the terminal doing the docker run; instead the output is just into the container itself.

@bwateratmsft bwateratmsft requested a review from a team as a code owner July 17, 2020 13:00
@karolz-ms
Copy link
Contributor

karolz-ms commented Jul 17, 2020

Regarding the "Other" Dockerfile, the EXEC form works for me:

FROM docker/whalesay:latest
LABEL Name=scaffold Version=0.0.1
RUN apt-get -y update && apt-get install -y fortunes
CMD ["sh", "-c", "/usr/games/fortune -a | cowsay"]
$ docker run --rm -it  scaffold:latest
 ______________________________________ 
/ "Intelligence without character is a \
| dangerous thing."                    |
|                                      |
\ -- G. Steinem                        /
 -------------------------------------- 
    \
     \
      \     
                    ##        .            
              ## ## ##       ==            
           ## ## ## ##      ===            
       /""""""""""""""""___/ ===        
  ~~~ {~~ ~~~~ ~~~ ~~~~ ~~ ~ /  ===- ~~~   
       \______ o          __/            
        \    \        __/             
          \____\______/   

(tried on a Mac)

Copy link
Contributor

@karolz-ms karolz-ms left a comment

Choose a reason for hiding this comment

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

LGTM. Consider changing the "Other" case too for consistency; it seems to be working as expected.

@bwateratmsft
Copy link
Contributor Author

Regarding the "Other" Dockerfile, the EXEC form works for me:

Weird, I did exactly what you're doing and it did not work for me. I'll try again.

@bwateratmsft
Copy link
Contributor Author

Regarding the "Other" Dockerfile, the EXEC form works for me:

Weird, I did exactly what you're doing and it did not work for me. I'll try again.

Yup, this time it's working. shrug

@bwateratmsft bwateratmsft merged commit c8cf547 into master Jul 17, 2020
@bwateratmsft bwateratmsft deleted the bmw/execform branch July 17, 2020 20:08
Dmarch28 pushed a commit to Dmarch28/vscode-docker that referenced this pull request Mar 4, 2021
@microsoft microsoft locked and limited conversation to collaborators Oct 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use exec form of the CMD Dockerfile command in files we scaffold
2 participants