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.

Sign up for GitHub

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

[FLOC-2137] Allow the public IP address of the node to be statically configured #2059

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

[FLOC-2137] Allow the public IP address of the node to be statically configured #2059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d999f29
Proof of concept.
Jun 4, 2015
301b3fd
Actually provide the **public** address.
Jun 4, 2015
5248e27
Dataset agent and tests.
Jun 5, 2015
c042b7b
Use updated config name.
Jun 5, 2015
f13fbd1
Add a hint of docs.
Jun 5, 2015
611376f
Generate unicode.
Jun 5, 2015
583b9db
Merge remote-tracking branch 'origin/master' into public-ips-FLOC-2137
wallrj Oct 22, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions docs/config/configuring-nodes-storage.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ When configuring node agents, consider whether the control service location you
You should never choose ``127.0.0.1`` or ``localhost`` as the hostname, even if the control service is on same machine as the node agent, as this will keep the control service from correctly identifying the agent's IP address.

.. warning::
It is important to note that the flocker nodes will refuse to communicate with the flocker agent if there is a misconfiguration in the hostname.
Please ensure that your hostname is configured correctly before proceeding, because any errors can result in failures.
It is important to note that the flocker nodes will refuse to communicate with the flocker agent if there is a misconfiguration in the hostname.
Please ensure that your hostname is configured correctly before proceeding, because any errors can result in failures.

Please note that the interface you choose will be the one that linked traffic will be routed over.
If you're in environment where some interfaces have bandwidth costs and some are free (for example, AWS), ensure that you choose the private interface where bandwidth costs don't apply.
Expand All @@ -31,12 +31,15 @@ The optional ``port`` variable is the port on the control node to connect to.
This value must agree with the configuration for the control service telling it on what port to listen.
Omit the ``port`` from both configurations and the services will automatically agree.

If node has a public address that is different than its local address (for example, on an EC2 instance),
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't strictly necessary though, right? We after all manage without right now. And it adds an extra burden of configuration on users that they don't necessarily need. Maybe indicate it's useful but not required? Or the caveats of not doing it?

Copy link
Contributor

Choose a reason for hiding this comment

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

One thing we need to keep in mind: bandwidth costs. Data sent within AWS private network is free, data sent over external IPs costs money. I would hope that DNS lookups for AWS hosts within AWS get turned into the private IP, in which case documenting we want explicit hostname (as opposed to "address" which implies IP) is important. If hostnames always resolve to external IPs then it's possible that following this advice may cost our users a bunch of money on AWS, depending how AWS does bandwidth accounting...

then the file should include a ``hostname`` item with a public address of the node.

The file must also include a ``dataset`` item.
This selects and configures a dataset backend.
All nodes must be configured to use the same dataset backend.

.. note::
You can only choose a single backend at a time, and changing backends is not currently supported.
You can only choose a single backend at a time, and changing backends is not currently supported.

List of Supported Backends
==========================
Expand All @@ -45,7 +48,7 @@ The following pages describe how to configure the backends currently supported b

.. toctree::
:maxdepth: 1

openstack-configuration
aws-configuration
emc-configuration
Expand All @@ -56,7 +59,7 @@ The following pages describe how to configure the backends currently supported b
saratogaspeed-configuration
zfs-configuration
loopback-configuration
Flocker supports pluggable storage backends.

Flocker supports pluggable storage backends.
Any storage system that is able to present itself as a network-based block device can serve as the underlying storage for a Docker data volume managed by Flocker.
If the storage backend you are looking for is not currently supported by Flocker, you can consider :ref:`contributing it <build-flocker-driver>`.
35 changes: 24 additions & 11 deletions flocker/node/script.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def _get_external_ip(host, port):
:param host: A host to connect to.
:param port: The port to connect to.

:return unicode: IP address of external interface on this node.
:return bytes: IP address of external interface on this node.
"""
while True:
try:
Expand Down Expand Up @@ -312,15 +312,16 @@ def get_service(self, reactor, options):
configuration = get_configuration(options)
host = configuration['control-service']['hostname']
port = configuration['control-service']['port']
ip = self.get_external_ip(host, port)
ip = configuration.get('hostname', self.get_external_ip(host, port))
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest only running get_external_ip if configuration doesn't have an entry, given it adds noise to control service logs.


tls_info = _context_factory_and_credential(
options["agent-config"].parent(), host, port)

return AgentLoopService(
reactor=reactor,
deployer=self.deployer_factory(
node_uuid=tls_info.node_credential.uuid, hostname=ip,
node_uuid=tls_info.node_credential.uuid,
hostname=unicode(ip, "ascii"),
cluster_uuid=tls_info.node_credential.cluster_uuid),
host=host, port=port,
context_factory=tls_info.context_factory,
Expand Down Expand Up @@ -501,6 +502,9 @@ class AgentService(PRecord):
control_service_host = field(type=bytes, mandatory=True)
control_service_port = field(type=int, mandatory=True)

node_hostname = field(type=(str, type(None)),
initial=None, mandatory=True)

# Cannot use type=NodeCredential because one of the tests really wants to
# set this to None.
node_credential = field(mandatory=True)
Expand All @@ -522,8 +526,10 @@ def from_configuration(cls, configuration):
:return: A new instance of ``cls`` with values loaded from the
configuration.
"""
host = configuration['control-service']['hostname']
port = configuration['control-service']['port']
control_service_host = configuration['control-service']['hostname']
control_service_port = configuration['control-service']['port']

node_hostname = configuration.get('hostname')

node_credential = configuration['node-credential']
ca_certificate = configuration['ca-certificate']
Expand All @@ -532,8 +538,10 @@ def from_configuration(cls, configuration):
backend_name = api_args.pop('backend')

return cls(
control_service_host=host,
control_service_port=port,
control_service_host=control_service_host,
control_service_port=control_service_port,

node_hostname=node_hostname,

node_credential=node_credential,
ca_certificate=ca_certificate,
Expand Down Expand Up @@ -610,12 +618,17 @@ def get_deployer(self, api):
backend = self.get_backend()
deployer_factory = self.deployers[backend.deployer_type]

address = self.get_external_ip(
self.control_service_host, self.control_service_port,
)
if self.node_hostname is None:
hostname = self.get_external_ip(
self.control_service_host, self.control_service_port,
)
else:
hostname = self.node_hostname
node_uuid = self.node_credential.uuid
return deployer_factory(
api=api, hostname=address, node_uuid=node_uuid,
api=api,
hostname=unicode(hostname, "ascii"),
node_uuid=node_uuid,
)

def get_loop_service(self, deployer):
Expand Down
96 changes: 92 additions & 4 deletions flocker/node/test/test_script.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ def setup_config(test, control_address=u"10.0.0.1", control_port=1234,
def deployer_factory_stub(**kw):
if set(kw.keys()) != {"node_uuid", "cluster_uuid", "hostname"}:
raise TypeError("wrong arguments")
if not isinstance(kw["hostname"], unicode):
raise TypeError("hostname not unicode")
return deployer


Expand Down Expand Up @@ -203,6 +205,8 @@ def test_initialized(self):
control_service_host=host,
control_service_port=port,

node_hostname=None,

# Compare this separately :/
node_credential=None,
ca_certificate=self.ca_set.root.credential.certificate,
Expand All @@ -226,6 +230,21 @@ def test_initialized(self):
),
)

def test_initialized_hostname(self):
host = b"192.0.2.13"
port = 2314
name = u"from_config-test"

setup_config(self, control_address=host, control_port=port, name=name)
options = DatasetAgentOptions()
options.parseOptions([b"--agent-config", self.config.path])
config = get_configuration(options)
config['hostname'] = 'hostname.example'
Copy link
Contributor

Choose a reason for hiding this comment

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

So.... everywhere else we use IPs. This changes means we're OK with DNS hostnames appearing in e.g. REST API output? That might violate our schema.


agent_service = AgentService.from_configuration(config)

self.assertEqual(agent_service.node_hostname, "hostname.example")


class AgentServiceGetAPITests(SynchronousTestCase):
"""
Expand Down Expand Up @@ -423,7 +442,7 @@ def test_backend_selection(self):

class Deployer(PRecord):
api = field(mandatory=True)
hostname = field(mandatory=True)
hostname = field(type=unicode, mandatory=True)
node_uuid = field(mandatory=True)

class WrongDeployer(PRecord):
Expand Down Expand Up @@ -454,7 +473,47 @@ def get_external_ip(host, port):
self.assertEqual(
Deployer(
api=api,
hostname=ip,
hostname=unicode(ip, "ascii"),
node_uuid=self.ca_set.node.uuid,
),
deployer,
)

def test_hostname(self):
"""
``AgentService.get_deployer`` creates a new deployer supplied with the
hostname from the configuration file, if one is provided.
"""
hostname = "hostname.example"

class Deployer(PRecord):
api = field(mandatory=True)
hostname = field(unicode, mandatory=True)
node_uuid = field(mandatory=True)

agent_service = self.agent_service.set(
"node_hostname", hostname,
).set(
"backends", [
BackendDescription(
name=self.agent_service.backend_name,
needs_reactor=False, needs_cluster_id=False,
api_factory=None, deployer_type=DeployerType.p2p,
),
],
).set(
"deployers", {
DeployerType.p2p: Deployer,
},
)

api = object()
deployer = agent_service.get_deployer(api)

self.assertEqual(
Deployer(
api=api,
hostname=unicode(hostname, "ascii"),
node_uuid=self.ca_set.node.uuid,
),
deployer,
Expand Down Expand Up @@ -604,8 +663,9 @@ def test_config_validated(self):

def test_deployer_factory_called_with_ip(self):
"""
``AgentServiceFactory.main`` calls its ``deployer_factory`` with one
of the node's IPs.
If the configuration doesn't specify a ``hostname``,
``AgentServiceFactory.main`` calls its ``deployer_factory`` with one of
the node's IPs.
"""
spied = []

Expand All @@ -620,6 +680,34 @@ def deployer_factory(node_uuid, hostname, cluster_uuid):
agent.get_service(reactor, options)
self.assertIn(spied[0], get_all_ips())

def test_deployer_factory_called_with_hostname(self):
"""
If the configuration does specify a ``hostname``,
``AgentServiceFactory.main`` calls its ``deployer_factory`` with one of
the hostname.
"""
spied = []

def deployer_factory(node_uuid, hostname, cluster_uuid):
spied.append(hostname)
return object()

reactor = MemoryCoreReactor()
options = DatasetAgentOptions()

config = yaml.safe_load(self.config.getContent())
config["hostname"] = "hostname.local"
self.config.setContent(yaml.safe_dump(config))

options.parseOptions([b"--agent-config", self.config.path])

agent = AgentServiceFactory(deployer_factory=deployer_factory)
agent.get_service(reactor, options)

self.assertEqual(
(spied[0], type(spied[0])),
("hostname.local", unicode))

def test_missing_configuration_file(self):
"""
``AgentServiceFactory.get_service`` raises an ``IOError`` if the given
Expand Down
26 changes: 15 additions & 11 deletions flocker/provision/_install.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,8 @@ def _remove_dataset_fields(content):


def task_configure_flocker_agent(control_node, dataset_backend,
dataset_backend_configuration):
dataset_backend_configuration,
public_ip=None):
"""
Configure the flocker agents by writing out the configuration file.

Expand All @@ -968,19 +969,21 @@ def task_configure_flocker_agent(control_node, dataset_backend,
u"backend": dataset_backend.name,
})

agent_config = {
"version": 1,
"control-service": {
"hostname": control_node,
"port": 4524,
},
"dataset": dataset_backend_configuration,
}
if public_ip is not None:
agent_config['hostname'] = public_ip
put_config_file = put(
path='/etc/flocker/agent.yml',
content=yaml.safe_dump(
{
"version": 1,
"control-service": {
"hostname": control_node,
"port": 4524,
},
"dataset": dataset_backend_configuration,
},
),
content=yaml.safe_dump(agent_config),
log_content_filter=_remove_dataset_fields

)
return sequence([put_config_file])

Expand Down Expand Up @@ -1435,6 +1438,7 @@ def configure_cluster(cluster, dataset_backend_configuration):
dataset_backend_configuration=(
dataset_backend_configuration
),
public_ip=node.address if node._node.driver.name == 'Amazon EC2' else None,
),
task_enable_docker_plugin(node.distribution),
task_enable_flocker_agent(
Expand Down