Skip to content
This repository has been archived by the owner on Nov 23, 2019. It is now read-only.

Adding global alias config fields in endpointsettings #22

Merged
merged 1 commit into from
Jan 10, 2016

Conversation

mavenugo
Copy link
Contributor

@mavenugo mavenugo commented Jan 8, 2016

@calavera @tiborvass as discussed, this brings in the fields required to support global alias.
The e2e functionality including integration with docker and libnetwork is currently here :
https://github.com/mavenugo/docker/commits/alias_3

Once this PR is merged, I will push the subsequent PR in docker/docker for any discussion.

cc @dnephin

Signed-off-by: Madhu Venugopal madhu@docker.com

@@ -216,6 +216,7 @@ type HostConfig struct {
GroupAdd []string // List of additional groups that the container process will run as
IpcMode IpcMode // IPC namespace to use for the container
Links []string // List of links (in the name:alias form)
Aliases []string // List of aliases
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should make this name more specific to the type of aliases?

HostnameAliases maybe ?

Or if the correct term is Endpoint in this context maybe EndpointAliases ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually. I reviewed what has been done in moby/moby#19001 and I think that make sense.
If we go with the NetworkingConfig approach, then we can remove this field from HostConfig.
Since the Aliases is carried in EndpointSettings already.

@calavera
Copy link
Contributor

calavera commented Jan 8, 2016

LGTM

@mavenugo mavenugo changed the title Adding global alias config fields in endpointsettings and hostConfig Adding global alias config fields in endpointsettings Jan 8, 2016
@vdemeester
Copy link
Contributor

@mavenugo I think you can push a PR on docker/docker with temporary vendoring your change, just so we have time to review it, wdyt ? 👼

@mavenugo
Copy link
Contributor Author

mavenugo commented Jan 9, 2016

@vdemeester @calavera am waiting on moby/moby#19198 to be merged before pushing the docker side changes for the alias functionality as discussed.
I hope we can get this in soon so that we can avoid temporary vendor-in changes.

@icecrime
Copy link

icecrime commented Jan 9, 2016

LGTM

@calavera
Copy link
Contributor

calavera commented Jan 9, 2016

needs rebase, sorry

Signed-off-by: Madhu Venugopal <madhu@docker.com>
@mavenugo
Copy link
Contributor Author

mavenugo commented Jan 9, 2016

@calavera thanks. rebased now.

@thaJeztah
Copy link
Member

ping @calavera tests are green 👍

icecrime pushed a commit that referenced this pull request Jan 10, 2016
Adding global alias config fields in endpointsettings
@icecrime icecrime merged commit 7f8feaf into docker:master Jan 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants