From d999f29085e89c6cba3ef0459eda6848214ba9b5 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Thu, 4 Jun 2015 15:49:43 -0600 Subject: [PATCH 1/6] Proof of concept. --- flocker/node/script.py | 2 +- flocker/provision/_install.py | 26 +++++++++++++++----------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/flocker/node/script.py b/flocker/node/script.py index 9c19be53a9..2ccbe9db76 100644 --- a/flocker/node/script.py +++ b/flocker/node/script.py @@ -281,7 +281,7 @@ def get_service(self, reactor, options): configuration = get_configuration(options) host = configuration['control-service']['hostname'] port = configuration['control-service']['port'] - ip = _get_external_ip(host, port) + ip = configuration.get('public-ip', _get_external_ip(host, port)) tls_info = _context_factory_and_credential( options["agent-config"].parent(), host, port) diff --git a/flocker/provision/_install.py b/flocker/provision/_install.py index 5fde935657..8d0b59ac12 100644 --- a/flocker/provision/_install.py +++ b/flocker/provision/_install.py @@ -520,7 +520,9 @@ def task_enable_flocker_agent(distribution, control_node, dataset_backend=DatasetBackend.zfs, dataset_backend_configuration=dict( pool=u'flocker' - )): + ), + public_ip=None, + ): """ Configure and enable the flocker agents. @@ -535,18 +537,19 @@ def task_enable_flocker_agent(distribution, control_node, 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['public-ip'] = 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), ) if distribution in ('centos-7', 'fedora-20'): return sequence([ @@ -992,6 +995,7 @@ def configure_cluster(cluster, dataset_backend_configuration): dataset_backend_configuration=( dataset_backend_configuration ), + public_ip=node._node.private_ips[0] if node._node.driver.name == 'Amazon EC2' else None, )]), ), ]) for certnkey, node From 301b3fd25de56cee0b5f27dc89a5b599b6e5098d Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Thu, 4 Jun 2015 15:56:08 -0600 Subject: [PATCH 2/6] Actually provide the **public** address. --- flocker/provision/_install.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flocker/provision/_install.py b/flocker/provision/_install.py index 8d0b59ac12..47d713a20c 100644 --- a/flocker/provision/_install.py +++ b/flocker/provision/_install.py @@ -995,7 +995,7 @@ def configure_cluster(cluster, dataset_backend_configuration): dataset_backend_configuration=( dataset_backend_configuration ), - public_ip=node._node.private_ips[0] if node._node.driver.name == 'Amazon EC2' else None, + public_ip=node.address if node._node.driver.name == 'Amazon EC2' else None, )]), ), ]) for certnkey, node From 5248e27ddf1cb1ba38e097bd0e816fc94818e239 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Fri, 5 Jun 2015 00:51:59 -0600 Subject: [PATCH 3/6] Dataset agent and tests. --- flocker/node/script.py | 28 ++++++---- flocker/node/test/test_script.py | 88 +++++++++++++++++++++++++++++++- 2 files changed, 105 insertions(+), 11 deletions(-) diff --git a/flocker/node/script.py b/flocker/node/script.py index 2ccbe9db76..c082eba430 100644 --- a/flocker/node/script.py +++ b/flocker/node/script.py @@ -281,7 +281,7 @@ def get_service(self, reactor, options): configuration = get_configuration(options) host = configuration['control-service']['hostname'] port = configuration['control-service']['port'] - ip = configuration.get('public-ip', _get_external_ip(host, port)) + ip = configuration.get('hostname', _get_external_ip(host, port)) tls_info = _context_factory_and_credential( options["agent-config"].parent(), host, port) @@ -467,6 +467,9 @@ class AgentService(PRecord): control_service_host = field(type=bytes, mandatory=True) control_service_port = field(type=int, mandatory=True) + node_hostname = field(type=(bytes, 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) @@ -488,8 +491,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'] @@ -498,8 +503,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, @@ -572,12 +579,15 @@ 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=hostname, node_uuid=node_uuid, ) def get_loop_service(self, deployer): diff --git a/flocker/node/test/test_script.py b/flocker/node/test/test_script.py index 65b6ce5515..a48fab0987 100644 --- a/flocker/node/test/test_script.py +++ b/flocker/node/test/test_script.py @@ -196,6 +196,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, @@ -219,6 +221,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' + + agent_service = AgentService.from_configuration(config) + + self.assertEqual(agent_service.node_hostname, "hostname.example") + class AgentServiceGetAPITests(SynchronousTestCase): """ @@ -410,6 +427,46 @@ def get_external_ip(host, port): 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(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=hostname, + node_uuid=self.ca_set.node.uuid, + ), + deployer, + ) + class AgentServiceLoopTests(SynchronousTestCase): """ @@ -542,8 +599,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 = [] @@ -558,6 +616,32 @@ 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.assertIn(spied[0], "hostname.local") + def test_missing_configuration_file(self): """ ``AgentServiceFactory.get_service`` raises an ``IOError`` if the given From c042b7be316aaed974568577aa6f601d41592836 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Fri, 5 Jun 2015 01:37:01 -0600 Subject: [PATCH 4/6] Use updated config name. --- flocker/provision/_install.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flocker/provision/_install.py b/flocker/provision/_install.py index 47d713a20c..77fc37befc 100644 --- a/flocker/provision/_install.py +++ b/flocker/provision/_install.py @@ -546,7 +546,7 @@ def task_enable_flocker_agent(distribution, control_node, "dataset": dataset_backend_configuration, } if public_ip is not None: - agent_config['public-ip'] = public_ip + agent_config['hostname'] = public_ip put_config_file = put( path='/etc/flocker/agent.yml', content=yaml.safe_dump(agent_config), From f13fbd161b7db95337a7824fc451e8963d57f419 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Fri, 5 Jun 2015 02:07:47 -0600 Subject: [PATCH 5/6] Add a hint of docs. --- docs/using/installing/index.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/using/installing/index.rst b/docs/using/installing/index.rst index b6835b58b2..37ae06e5fd 100644 --- a/docs/using/installing/index.rst +++ b/docs/using/installing/index.rst @@ -498,6 +498,9 @@ 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), +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. From 611376fce4d4f5b14273f580a3da0ee8cd7999d5 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Fri, 5 Jun 2015 02:51:56 -0600 Subject: [PATCH 6/6] Generate unicode. --- flocker/node/script.py | 13 ++++++++----- flocker/node/test/test_script.py | 14 +++++++++----- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/flocker/node/script.py b/flocker/node/script.py index c082eba430..137101b303 100644 --- a/flocker/node/script.py +++ b/flocker/node/script.py @@ -86,13 +86,13 @@ 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. """ sock = socket() try: sock.setblocking(False) sock.connect_ex((host, port)) - return unicode(sock.getsockname()[0], "ascii") + return sock.getsockname()[0] finally: sock.close() @@ -289,7 +289,8 @@ def get_service(self, reactor, options): 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, @@ -467,7 +468,7 @@ class AgentService(PRecord): control_service_host = field(type=bytes, mandatory=True) control_service_port = field(type=int, mandatory=True) - node_hostname = field(type=(bytes, type(None)), + node_hostname = field(type=(str, type(None)), initial=None, mandatory=True) # Cannot use type=NodeCredential because one of the tests really wants to @@ -587,7 +588,9 @@ def get_deployer(self, api): hostname = self.node_hostname node_uuid = self.node_credential.uuid return deployer_factory( - api=api, hostname=hostname, node_uuid=node_uuid, + api=api, + hostname=unicode(hostname, "ascii"), + node_uuid=node_uuid, ) def get_loop_service(self, deployer): diff --git a/flocker/node/test/test_script.py b/flocker/node/test/test_script.py index a48fab0987..7bb5670336 100644 --- a/flocker/node/test/test_script.py +++ b/flocker/node/test/test_script.py @@ -84,6 +84,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 @@ -390,7 +392,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): @@ -421,7 +423,7 @@ 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, @@ -436,7 +438,7 @@ def test_hostname(self): class Deployer(PRecord): api = field(mandatory=True) - hostname = field(mandatory=True) + hostname = field(unicode, mandatory=True) node_uuid = field(mandatory=True) agent_service = self.agent_service.set( @@ -461,7 +463,7 @@ class Deployer(PRecord): self.assertEqual( Deployer( api=api, - hostname=hostname, + hostname=unicode(hostname, "ascii"), node_uuid=self.ca_set.node.uuid, ), deployer, @@ -640,7 +642,9 @@ def deployer_factory(node_uuid, hostname, cluster_uuid): agent = AgentServiceFactory(deployer_factory=deployer_factory) agent.get_service(reactor, options) - self.assertIn(spied[0], "hostname.local") + self.assertEqual( + (spied[0], type(spied[0])), + ("hostname.local", unicode)) def test_missing_configuration_file(self): """