Skip to content

Fix service update --env-add issue#25427

Merged
vdemeester merged 1 commit intomoby:masterfrom
yongtang:25404-service-update-env-add
Aug 10, 2016
Merged

Fix service update --env-add issue#25427
vdemeester merged 1 commit intomoby:masterfrom
yongtang:25404-service-update-env-add

Conversation

@yongtang
Copy link
Copy Markdown
Member

@yongtang yongtang commented Aug 5, 2016

- What I did

This fix tries to address the issue in #25404 where updating environmental variable in service update --env-add will not work.

The issue is because --env-add will only append the env, not update if the same env already exist.

- How I did it

This fix tracks the env variable with a map and update if the variable is the same.

- How to verify it

An integration test has been added.

- Description for the changelog

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

This fix fixes #25404.

Signed-off-by: Yong Tang yong.tang.github@outlook.com

@hvpareja
Copy link
Copy Markdown

hvpareja commented Aug 5, 2016

Seems like it's been a punctual error:

03:57:26 ---> Making bundle: validate-dco (in bundles/1.13.0-dev/validate-dco)
03:57:38 fatal: unable to access 'https://github.com/docker/docker.git/': Couldn't resolve host 'github.com'

Can you trigger the build again manually?

@icecrime
Copy link
Copy Markdown
Contributor

icecrime commented Aug 8, 2016

Ping @dnephin @vieux!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this needs to be an integration test.

We can fully test this change as a unit test. in api/client/service/update_test.go

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks @dnephin. The integration test has been removed and replaced with a unit test.

This fix tries to address the issue in 25404 where updating environmental
variable in `service update --env-add` will not work.

The issue is because `--env-add` will only append the env, not update if
the same env already exist.

This fix tracks the env variable with a map and update if the variable
is the same.

An integration test has been added.

This fix fixes 25404.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang force-pushed the 25404-service-update-env-add branch from 2b7b359 to c6de8ad Compare August 9, 2016 02:05
@dnephin
Copy link
Copy Markdown
Member

dnephin commented Aug 9, 2016

LGTM

@thaJeztah
Copy link
Copy Markdown
Member

retest this please

@thaJeztah
Copy link
Copy Markdown
Member

LGTM

@vdemeester vdemeester merged commit 59ca493 into moby:master Aug 10, 2016
@yongtang yongtang deleted the 25404-service-update-env-add branch August 10, 2016 15:43
@tiborvass tiborvass mentioned this pull request Aug 11, 2016
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.

--env-add flag doesn't update the variable as the help states

8 participants