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

Added the naming of container veth's #16729

Closed
wants to merge 2 commits into from
Closed

Added the naming of container veth's #16729

wants to merge 2 commits into from

Conversation

mattmarich
Copy link

If a container name is supplied it names the veth after this, if one is not applied it calls the veth after the auto generated container name. Make's tracking container eths a lot easier if you need to monitor container traffic via devops tools.

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "master" git@github.com:mattmarich/docker.git somewhere
$ cd somewhere
$ git rebase -i HEAD~2
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Ammending updates the existing PR. You DO NOT need to open a new one.

@GordonTheTurtle GordonTheTurtle added status/0-triage dco/no Automatically set by a bot when one of the commits lacks proper signature labels Oct 2, 2015
@jessfraz
Copy link
Contributor

jessfraz commented Oct 2, 2015

im pretty sure @mavenugo has some opinions on veth naming

@mattmarich
Copy link
Author

Sorry about not signing, first pull request on github. If there's opinions on veth naming feel free to make changes. This worked for my specific application as we needed to add containers to our system graphs and bandwidth limit specific containers to varying speeds.

@phemmer
Copy link
Contributor

phemmer commented Oct 3, 2015

I think it would be more useful to have the interface name in the docker inspect output. We provide the PID of the main process, why not the interface name?
The reason why I might suggest this over naming the interface after the container name is in case one uses container IDs as the primary source of identification. docker inspect --format '{{.State.InterfaceName}}' $foo would work if using both IDs & names.

But, there's no reason we can't have both.

@LK4D4
Copy link
Contributor

LK4D4 commented Oct 23, 2015

ping @mrjana @mavenugo for design desicion

@cpuguy83
Copy link
Member

There's no DCO here, so shouldn't be reviewing yet.

@shishir-a412ed
Copy link
Contributor

@mattmarich any updates on this one ?

@icecrime
Copy link
Contributor

icecrime commented Nov 5, 2015

It's been 1 month, the commit is not signed: closing this.

@icecrime icecrime closed this Nov 5, 2015
@mattmarich
Copy link
Author

This was signed soon after there was a complaint. If you see I merged the commits under my github user once I realized I had accidentally pushed as my gitlab user.

On Nov 5, 2015, at 10:09 AM, Arnaud Porterie notifications@github.com wrote:

It's been 1 month, the commit is not signed: closing this.


Reply to this email directly or view it on GitHub.

@thaJeztah
Copy link
Member

@mattmarich feel free to open a new PR, we can continue the design discussion there. Any thoughts on the comments so far? I.e., would #16729 (comment) work for you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/networking dco/no Automatically set by a bot when one of the commits lacks proper signature status/1-design-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants