Skip to content

Conversation

@chmouel
Copy link

@chmouel chmouel commented Dec 17, 2014

Allow overriding a user on the command line from the one specified in
the fig.yaml

Tests are a bit tricky since regenerating a docker image with multiple
users just for this is consuming so let's catch the error instead and
make sure it works.

Signed-off-by: Chmouel Boudjnah chmouel@chmouel.com

@aanand
Copy link

aanand commented Dec 17, 2014

Nice, thanks.

  • docker run supports both -u and --user - we should too.
  • The test seems to be failing: https://app.wercker.com/#buildstep/5491bae16b3ba8733d8f148d
  • I don't think the test needs to check the output of the command - just inspect the created container and check it's got the right value for "User".

Allow overriding a user on the command line from the one specified in
the fig.yaml

We are testing on the behavior of how the busybox image is made which is
kind of tricky but hopefully that shouln't change.

Signed-off-by: Chmouel Boudjnah <chmouel@chmouel.com>
@chmouel
Copy link
Author

chmouel commented Dec 17, 2014

Thanks @aanand, great suggestions

@IanVS
Copy link

IanVS commented Feb 14, 2015

@chmouel, this would be an awesome feature. Do you think you'll have a chance to finish it up? If not, I could take a crack at it. It looks like it's pretty darn close.

@chmouel
Copy link
Author

chmouel commented Feb 14, 2015

@IanVS feel free to take it over :)

@IanVS
Copy link

IanVS commented Feb 14, 2015

OK I think I have something that will work, but I'm not sure how to structure the assert of the test. To get the "User" as @aanand suggests, what should I do? container.user doesn't exist, and it seems neither does container.dictionary['Config']['User']

What's the right way to do this? Thanks!

@IanVS
Copy link

IanVS commented Feb 15, 2015

@chmouel, I couldn't figure out how to use your commit directly, so I created a new pull request,#971 . Hope that's alright. I'm new to contributing to OSS, still learning the ropes.

@bfirsh
Copy link

bfirsh commented Mar 11, 2015

Replaced by #971. Thanks!

@bfirsh bfirsh closed this Mar 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants