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

Add stack config command (carry 2740) #3544

Merged
merged 3 commits into from
Apr 29, 2022

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Apr 8, 2022

Add stack config command.

Fixes #2365

Usage:

# output interpolated and merged config
docker stack config -c docker-compose.prod.yml -c docker-compose.dev.yml
# also possible to input from stdin, like docker stack deploy
docker stack config -c -
# output merged config without interpolation
docker stack config -c docker-compose.prod.yml -c docker-compose.dev.yml --skip-interpolation
# possible to pipe it back to docker stack deploy, like this
# interpolating twice is not a good idea, so one should skip interpolation
docker stack config -c docker-compose.prod.yml -c docker-compose.dev.yml --skip-interpolation | docker stack deploy -c -

- What I did
Added stack config command that outputs on stdout the merged and/or interpolated config files for docker stack deploy.
Now it is no longer required to have docker-compose in order to see how configs are interpolated and merged before deploying your stack.

Currently, in our team, we currently use something like this to check our merge/compilation output:

docker-compose -f config1 -f config2 config | docker stack deploy -c -

The interpolation and merge that docker-compose config provides is untrue in a swarm deploy context since docker-compose and docker/cli are two separate projects and use different methods for interpolation and merge. With the latest version of docker-compose we encountered some problems with the merged/interpolated format not being accepted by docker stack deploy. I decided to bring a config command to docker/cli as well so we won't encounter this problem in the future.

Related issues with latest docker-compose version:
docker/compose#7773
docker/compose#7778

I also added relevant unit tests for the two new merges.

- How I did it
I used the existing dependencies (the yaml lib) and functionality present in docker cli (the stack loader and the compose loader) in order to load the config files, merge/interpolate them and then output the yaml to stdout.

Testing on our own docker-compose files, I also noticed that when merging with docker stack deploy some properties did not merge in an expected way and produced unexpected consequences.

  • the volume property in each service, would not take into account the target of the volume when merging.
    I believe that in many cases, you would have a prod compose file with named or anonymous volumes and a dev compose file with a bind mounted volume to the same target. The merge of docker stack deploy would just concatenate the volumes array and throw an error since you try to mount to the same target. See the example in (How to verify it section of this PR), on how this PR merges them.

  • the command property and entrypoint in each service, would just be concatenated, whereas with docker-compose config it would've been replaced. Meaning, for example, that if in configFile1 you had a command like cat file1.txt and in configFile2 you have a command like cat file2.txt the merge of docker stack deploy -c configFile1 -c configFile2 would result in the command: cat file1.txt cat file2.txt which is definitely not what most people would expect and want. Modified this as well so that the command in configFile2 if exists will override the command in configFile1. See the example in (How to verify it section of this PR), on how this PR merges the command/entrypoint.

- How to verify it

All you need is two very simple config files:

# docker-compose.test1.yml
version: '3.7'

services:
  bash:
    image: bash
    command: cat randomFile.txt
    volumes:
      - app:/app
    working_dir: /app

volumes:
  app:
# docker-compose.test2.yml
version: '3.7'

services:
  bash:
    image: bash
    command: echo $ENV_STRING
    volumes:
      - ./src/app:/app
    working_dir: /app

Now, if you run $ENV_STRING=helloworld docker-local stack config -c docker-compose.test1.yml -c docker-compose.test2.yml (this is with interpolation), you will get something like this:

version: "3.7"
services:
  bash:
    command:
    - echo
    - helloworld
    image: bash
    environment:
      ENV_STRING: helloworld
    volumes:
    - type: bind
      source: /Users/flow/Projects/docker-test/src/app
      target: /app
    working_dir: /app
volumes:
  app: {}

If you run docker-local stack config -c docker-compose.test1.yml -c docker-compose.test2.yml --skip-interpolation (this is without interpolation), you will get something like this:

version: "3.7"
services:
  bash:
    command:
    - echo
    - $ENV_STRING
    image: bash
    environment:
      ENV_STRING: helloworld
    volumes:
    - type: bind
      source: /Users/flow/Projects/docker-test/src/app
      target: /app
    working_dir: /app
volumes:
  app: {}

- Description for the changelog
Added docker stack config command that outputs the merged and interpolated config files as utilised by stack deploy.

- A picture of a cute animal (not mandatory but encouraged)
Here is neko ikiru san to apologize for the long read
image

P.S. I am new to Go, please let me know if I did anything in a not so Go way

@codecov-commenter
Copy link

Codecov Report

Merging #3544 (86a097a) into master (429d716) will increase coverage by 0.04%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master    #3544      +/-   ##
==========================================
+ Coverage   59.05%   59.09%   +0.04%     
==========================================
  Files         282      285       +3     
  Lines       23796    23860      +64     
==========================================
+ Hits        14053    14101      +48     
- Misses       8883     8896      +13     
- Partials      860      863       +3     

@thaJeztah
Copy link
Member Author

Ah, one import to update

cli/command/stack/config_test.go:4:2: `io/ioutil` is in the blacklist (depguard)
	"io/ioutil"
	^

Make use of existing modules and functions in order to output the merged configs.
Added skip interpolation flag of variables, so that you can pipe the output back to stack deploy without much hassle.

Signed-off-by: Stoica-Marcu Floris-Andrei <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Stoica-Marcu Floris-Andrei <[email protected]>
Signed-off-by: Stoica-Marcu Floris-Andrei <[email protected]>
@thaJeztah thaJeztah force-pushed the carry_2740_add_config_command branch from 86a097a to cff702d Compare April 8, 2022 12:56
@thaJeztah thaJeztah marked this pull request as ready for review April 29, 2022 09:57
@ndeloof
Copy link
Contributor

ndeloof commented Apr 29, 2022

On reason users have to run compose config with stack command is that the latter does not support .env files.
(see https://docs.docker.com/compose/environment-variables/)

Note when using docker stack deploy
The .env file feature only works when you use the docker-compose up command and does not work with docker stack deploy.

we could add support for --env-file in stack commands as a workaround.

see moby/moby#29133

@thaJeztah
Copy link
Member Author

we could add support for --env-file in stack commands as a workaround.

Yeah, so that one is tricky, because --env-file !=== --env-file (e.g. for docker run --env-file or docker service create --env-file). We really need to rethink how to handle all of that 😞

Copy link
Member Author

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM (I only rebased the PR; changes look good)

@thaJeztah thaJeztah merged commit 1497633 into docker:master Apr 29, 2022
@thaJeztah thaJeztah deleted the carry_2740_add_config_command branch April 29, 2022 12:03
albers added a commit to albers/docker-cli that referenced this pull request Apr 29, 2022
albers added a commit to albers/docker-cli that referenced this pull request Apr 29, 2022
This adds bash completion for docker#3544.

Signed-off-by: Harald Albers <[email protected]>
matt9ucci added a commit to matt9ucci/DockerCompletion that referenced this pull request Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Add command to print config
4 participants