Skip to content

Add the datapath-addr in libnetwork#1719

Merged
aboch merged 1 commit intomoby:masterfrom
fcrisciani:data_path
Apr 24, 2017
Merged

Add the datapath-addr in libnetwork#1719
aboch merged 1 commit intomoby:masterfrom
fcrisciani:data_path

Conversation

@fcrisciani
Copy link
Copy Markdown

During configuration in SWARM mode is now possible to pass an additional
parameter --datapath-addr <ip|interface>.
The information is going to be used to configure which is the interface
that is going to be used for the data path for global scope drivers.
Up to now the only driver really using this extra parameter is the
overlay driver.

Signed-off-by: Flavio Crisciani flavio.crisciani@docker.com

Comment thread agent.go Outdated
if err := c.agentInit(listenAddr, bindAddr, advAddr); err != nil {
logrus.Infof("Initializing Libnetwork Agent Listen-Addr=%s Local-addr=%s Adv-addr=%s Data-addr=%s Remote-addr =%s",
listenAddr, bindAddr, advAddr, dataAddr, remoteAddr)
if advAddr != "" && dataAddr != "" && agent == nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

dataAddr != "" should not be a requisite for starting the networking agent

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In the deamon side, it is always set to something, if the flag is passed will contain the value, else will be fallback to the adv address

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should not assume how the caller will pass these values.
From libnetwork point of view, all what is needed to start the agent is the advertise address being set.

Comment thread agent.go Outdated
d.DiscoverNew(discoverapi.NodeDiscovery, discoverapi.NodeDiscoveryData{
Address: agent.advertiseAddr,
if err := d.DiscoverNew(discoverapi.NodeDiscovery, discoverapi.NodeDiscoveryData{
Address: agent.dataPathAddr,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On the same line, the decision of setting Address: agent.dataPathAddr or Address: agent.advertiseAddr IMO should be taken by libnetwork code based on the fact if agent.dataPathAddr != ""

Comment thread agent.go Outdated
ch, cancel := nDB.Watch(libnetworkEPTable, "", "")
nodeCh, cancel := nDB.Watch(networkdb.NodeTable, "", "")

dpAddr := dataPathAddr
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What about preserving the original agent parameters and add a method instead:

func (a *agent) datapathAddress() string {
	a.Lock()
	defer a.Unlock()
	if a.dataPathAddr != "" {
		return a.dataPathAddr
	}
	return a.advertiseAddr
}

@sanimej
Copy link
Copy Markdown

sanimej commented Apr 13, 2017

With the datapath-addr option if the user doesn't set it consistently on all nodes it can lead to connectivity issues. The same applies during the upgrade as well. If there is an option for the user to quickly identify such misconfig it will help. May be we can show it in the node inspect ?

Though its beyond the scope of this PR making a comment here so that we can consider it in the design.

@fcrisciani
Copy link
Copy Markdown
Author

@sanimej agree, having the info in the inspect sounds a very good idea to me, will look into it

Copy link
Copy Markdown
Contributor

@aboch aboch left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of the comments.

LGTM

During configuration in SWARM mode is now possible to pass an additional
parameter --data-path-addr <ip|interface>.
The information is going to be used to configure which is the interface
that is going to be used for the data path for global scope drivers.
Up to now the only driver really using this extra parameter is the
overlay driver.

Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
@aboch aboch merged commit 5f62c01 into moby:master Apr 24, 2017
@fcrisciani fcrisciani deleted the data_path branch April 26, 2017 21:36
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.

3 participants