Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

weave env --restore #1327

Merged
merged 3 commits into from
Aug 19, 2015
Merged

weave env --restore #1327

merged 3 commits into from
Aug 19, 2015

Conversation

rade
Copy link
Member

@rade rade commented Aug 18, 2015

weave env stashes the existing DOCKER_HOST in ORIG_DOCKER_HOST, and weave env --restore sets DOCKER_HOST to $ORIG_DOCKER_HOST.

Note that weave env --restore always executes locally, which means it will work when the env has a 'broken' DOCKER_HOST.

Closes #1288.

@rade rade self-assigned this Aug 18, 2015
@rade
Copy link
Member Author

rade commented Aug 18, 2015

@bboreham would appreciate some feedback here on what this does and how it does it.

Possible point of contention...

  • weave env --restore does not unset ORIG_DOCKER_HOST, so the env is not the same as before weave env was invoked. However, that means weave env --restore is idempotent, which imo is desirable

exec_remote "$@"
exit $?
fi

shift 1
if [ "$1" = "env" -a "$2" = "--restore" ] ; then

This comment was marked as abuse.

`weave env` stashes the existing DOCKER_HOST in ORIG_DOCKER_HOST, and
`weave env --restore` sets DOCKER_HOST to $ORIG_DOCKER_HOST.

Closes #1288.
@rade
Copy link
Member Author

rade commented Aug 18, 2015

When weave env --restore is invoked without a DOCKER_HOST_ORIG, weave prints a warning on stderr and exits with 1. Alternatives I've considered: a) silent exit 0, b) silent exit 1, c) warning and exit 0. Anybody want to argue for a different choice than the one I made?

@rade
Copy link
Member Author

rade commented Aug 18, 2015

@bboreham I think this is ready modulo docs and squashing. Would appreciate some feedback first.

@rade rade changed the title WIP weave env --restore weave env --restore Aug 18, 2015
@bboreham
Copy link
Contributor

The implementation lgtm.

I am unclear how prevalent the set-up is where setting DOCKER_HOST in your regular environment just works. It doesn't do anything for me because the sockets are protected so I have to sudo every command.

@rade
Copy link
Member Author

rade commented Aug 19, 2015

The new command also works in situations where DOCKER_HOST starts off unset/null. i.e. weave env --restore sets it to back to blank. (NB: minor wart: it is not unsetting DOCKER_HOST, only setting it to null).

bboreham added a commit that referenced this pull request Aug 19, 2015
@bboreham bboreham merged commit db51b77 into master Aug 19, 2015
@rade
Copy link
Member Author

rade commented Aug 19, 2015

Umm. This wasn't ready to merge. Still needs docs!

rade added a commit that referenced this pull request Aug 19, 2015
@rade rade deleted the 1288_restore_env branch August 19, 2015 20:32
rade added a commit that referenced this pull request Aug 28, 2015
@rade rade modified the milestone: 1.1.0 Aug 29, 2015
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.

2 participants