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

[18.09] backport fix substitution with non-empty env-var #1394

Merged
merged 1 commit into from
Sep 26, 2018

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Sep 26, 2018

backport of ec3daea (#1392) for 18.09

fixes #1391 for 18.09

git checkout -b 18.09_backport_ upstream/18.09
git cherry-pick -s -S -x  ec3daea0214b9600062e13413ce3062c9d3690cb
git push -u origin

cherry-pick was clean; no conflicts

Due to a typo, substitution would not work if the given
environment-variable was set.

Given the following docker compose file;

version: "3.7"

services:
  app:
    image: nginx:${version:-latest}

Deploying a stack with $version set would ignore the $version
environment variable, and use the default value instead;

version=alpine docker stack deploy -c docker-compose.yml foobar

Creating network foobar_default
Creating service foobar_app

docker service ls

ID                  NAME                MODE                REPLICAS            IMAGE               PORTS
rskkjxe6sm0w        foobar_app          replicated          1/1                 nginx:latest

This patch also fixes "soft default" not detecting empty environment variables,
only non-set environment variables.

Signed-off-by: Sebastiaan van Stijn github@gone.nl
(cherry picked from commit ec3daea)
Signed-off-by: Sebastiaan van Stijn github@gone.nl

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Due to a typo, substitution would not work if the given
environment-variable was set.

Given the following docker compose file;

```yaml
version: "3.7"

services:
  app:
    image: nginx:${version:-latest}
```

Deploying a stack with `$version` set would ignore the `$version`
environment variable, and use the default value instead;

```bash
version=alpine docker stack deploy -c docker-compose.yml foobar

Creating network foobar_default
Creating service foobar_app

docker service ls

ID                  NAME                MODE                REPLICAS            IMAGE               PORTS
rskkjxe6sm0w        foobar_app          replicated          1/1                 nginx:latest
```

This patch also fixes "soft default" not detecting empty environment variables,
only non-set environment variables.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit ec3daea)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯

@codecov-io
Copy link

Codecov Report

Merging #1394 into 18.09 will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##            18.09    #1394      +/-   ##
==========================================
+ Coverage   54.16%   54.18%   +0.01%     
==========================================
  Files         290      290              
  Lines       19275    19281       +6     
==========================================
+ Hits        10441    10447       +6     
  Misses       8168     8168              
  Partials      666      666

@thaJeztah
Copy link
Member Author

All green 👍 merging 😅

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.

5 participants