Add stack config command

[SMFloris]

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

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

[codecov-commenter]

Codecov Report

Merging #2740 into master will increase coverage by 0.03%.
The diff coverage is 69.56%.

@@            Coverage Diff             @@
##           master    #2740      +/-   ##
==========================================
+ Coverage   57.14%   57.18%   +0.03%     
==========================================
  Files         297      298       +1     
  Lines       18634    18679      +45     
==========================================
+ Hits        10649    10681      +32     
- Misses       7126     7135       +9     
- Partials      859      863       +4     

[thaJeztah]

Thank you! I haven't had time yet to look closely at these changes, but it came up in our slack channel.

I like the general idea; still thinking somewhat about naming of that command. It's good that it matches the docker-compose equivalent, but I think that name was a bit poorly chosen.

For docker stack, using config as name for the command could also easily lead to confusing it to be related to docker config, so need to do some thinking what would be a better alternative.

[olljanat]

For docker stack, using config as name for the command could also easily lead to confusing it to be related to docker config, so need to do some thinking what would be a better alternative.

Maybe docker stack preview , docker stack validate or docker stack print ?

[SMFloris]

For docker stack, using config as name for the command could also easily lead to confusing it to be related to docker config, so need to do some thinking what would be a better alternative.

Maybe docker stack preview , docker stack validate or docker stack print ?

I would add, borrowing a bit from Terraform: docker stack plan

docker stack config is a term borrowed from docker-compose, but I agree it might lead to confusion with the existing docker config command. Still, since its already used in docker-compose it might be easier for people to use it and adopt it. Alternative to the alternative (ATTA): docker stack deploy --config

docker stack preview is a good candidate, since you actually preview what you would deploy in term of config files. The only downside is that when I think of the preview word, it also makes me think in docker stack terms that I would get to see what will be updated/created/left untouched. The same arguments can be said about the docker stack plan alternative. ATTA: docker stack deploy --preview or docker stack deploy --dry-run

docker stack validate is a great alternative. It's pretty clear that it is about the config files and their validation, but it also makes me think that it will either exit successfully if the config is valid, or error out when the config might not be valid. Maybe adding an --output-config argument or something would make it more verbose. ATTA: docker stack deploy --validate

docker stack print is for me not clear what it does from the get-go, but in the context of config files is a good alternative since it would print the merged config files.

@thaJeztah , what are your thoughts on these alternatives?

[olljanat]

I would add, borrowing a bit from Terraform: docker stack plan

Good point, even when Terraform architecture it selves is quite awful (IMO) that term "plan" make sense on both of thus use cases.

[albers]

Removing myself as a reviewer because completion is not touched in this PR.

[silvin-lubecki]

question: @SMFloris why not using LoadComposefile function instead, which actually calls getConfigDetails ?

[SMFloris]

Great question!

If I recall correctly, I didn't use it because I wanted to have control on how the compose files are actually getting loaded.
The --skip-interpolation flag is being sent down to the composeLoader.load() call, like so:

optsFunc := func(options *composeLoader.Options) {
	options.SkipInterpolation = skipInterpolation
}
config, err := composeLoader.Load(configFiles, optsFunc)

This is kinda important to the use-case of this new command, seeing the output with and without interpolation to figure out where things went wrong and if you're deploying the right thing.

Another reason is that LoadComposefile also does some other things besides loading compose files, it also does some validation (forbidden, deprecated and unsupported properties). I thought those validations were just for deploy since the validation of the composeLoader.load is also used. 🤔 Also, since in my mind I'm piping the output back to docker stack deploy -c - I saw no reason in doing deploy validations when not doing a deploy.

The final reason, since I wasn't that familiar with the code-base and because I added a new command I didn't want to mess too much with the existing signatures of the methods 😅

Now that I'm more familiar with the codebase. I can change the LoadComposefile function to pass down some options to composeLoader.load() call if you want me too (perhaps even adding an option to skip those other validations).

[silvin-lubecki]

@thaJeztah any thoughts on that? I'm a bit on the fence, as if the output is pipped to the docker stack deploy command, then the config should print the warnings I guess... but it can also be pipped to the docker-compose command...
@SMFloris propose to call LoadComposeFile and add the --skip-validation option so one can disable the validation... WDYT?

[SMFloris]

@thaJeztah , could we get your input on the above, please? 😄 We are doing some ugly hacks with downgrading docker-compose in order to get valid docker stack deploy files

[thaJeztah]

it also does some validation (forbidden, deprecated and unsupported properties). I thought those validations were just for deploy since the validation of the composeLoader.load is also used

So if we're trying to replicate docker-compose config, that command is described as "Validate and view the Compose file"; from that perspective, I think it would make sense to have it validate the compose-file, so that only a valid file would be generated.

but it can also be pipped to the docker-compose command...

Not sure if we should consider if for that (as docker-compose has a config command itself 🤔

propose to call LoadComposeFile and add the --skip-validation option so one can disable the validation... WDYT?

Do we know why the --skip-validation option was added in compose? Wondering what the exact use-case is if that would allow it to produce invalid output (or is it skipping specific validation steps?)

[SMFloris]

@thaJeztah , I managed to track the inclusion of these options down to #1128

Maybe reading through that a bit can help jog some memories 😄

[codecov-io]

Codecov Report

Merging #2740 (1b66bb0) into master (6916b42) will increase coverage by 0.02%.
The diff coverage is 68.18%.

@@            Coverage Diff             @@
##           master    #2740      +/-   ##
==========================================
+ Coverage   57.14%   57.17%   +0.02%     
==========================================
  Files         297      298       +1     
  Lines       18634    18677      +43     
==========================================
+ Hits        10649    10679      +30     
- Misses       7126     7135       +9     
- Partials      859      863       +4     

[SMFloris]

@silvin-lubecki @thaJeztah , Any updates or new thoughts on this PR? I'm gonna keep updating this PR and keep it from going stale unless you guys have other thoughts 😄

[SMFloris]

@silvin-lubecki @thaJeztah Any new developments on this?

[thaJeztah]

Arf, sorry for the very long delay; I had some review comment, but looks like I never posted them 😞

[thaJeztah]

it also does some validation (forbidden, deprecated and unsupported properties). I thought those validations were just for deploy since the validation of the composeLoader.load is also used

So if we're trying to replicate docker-compose config, that command is described as "Validate and view the Compose file"; from that perspective, I think it would make sense to have it validate the compose-file, so that only a valid file would be generated.

but it can also be pipped to the docker-compose command...

Not sure if we should consider if for that (as docker-compose has a config command itself 🤔

propose to call LoadComposeFile and add the --skip-validation option so one can disable the validation... WDYT?

Do we know why the --skip-validation option was added in compose? Wondering what the exact use-case is if that would allow it to produce invalid output (or is it skipping specific validation steps?)

[thaJeztah]

Wondering if we should reverse the default (skip interpolation by default, but allow users to opt-in, with --interpolate)

[SMFloris]

I was just about to push this change, but actually saw that the compose loader (loader.go) we use also has a --skip-interpolation flag. It might cause confusion passing !interpolate to it.

@thaJeztah , Should I keep it as it is to avoid confusion or remove --skip-interpolation in favor of --interpolate?

EDIT: clarification

[SMFloris]

@thaJeztah , have you thought on how to proceed?

[SMFloris]

@thaJeztah , thanks for your review. I'll take a better look over it tonight and come back 😄