From e38ae543b88f7db220e234159b717271251ef248 Mon Sep 17 00:00:00 2001 From: John Dewey Date: Wed, 16 May 2018 22:47:48 -0700 Subject: [PATCH] Delegated driver acts as managed (#1301) The delegated driver defaults to `managed`, just like every other driver in Molecule. This driver now adheres to an instance-config API by default. Only, when `managed` is `False` does the driver force the developer to configure connectivity. Fixes: #1292 --- CHANGELOG.rst | 2 +- molecule/command/login.py | 3 +- molecule/driver/delegated.py | 64 ++++++++++++- test/unit/command/conftest.py | 10 ++ test/unit/command/test_login.py | 4 +- test/unit/conftest.py | 4 + test/unit/driver/test_delegated.py | 149 ++++++++++++++++++++++++++++- 7 files changed, 224 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fa661f015..08e322d01 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -890,7 +890,7 @@ Breaking Changes * Added support for libvirt provider. * Added ``molecule check`` to check playbook syntax. * Testinfra parameters can now be set as vars in molecule.yml. -* Running testinfra tests in parallel is no longer the default behavior. +* Running testinfra tests in parallel is no longer the default behaviour. 1.5.1 ===== diff --git a/molecule/command/login.py b/molecule/command/login.py index c4197605f..186a849f9 100644 --- a/molecule/command/login.py +++ b/molecule/command/login.py @@ -71,8 +71,7 @@ def execute(self): :return: None """ c = self._config - if (not c.state.created - and (c.driver.delegated and not c.driver.managed)): + if ((not c.state.created) and c.driver.managed): msg = 'Instances not created. Please create instances first.' util.sysexit_with_message(msg) diff --git a/molecule/driver/delegated.py b/molecule/driver/delegated.py index f08407d17..0c8b94fa6 100644 --- a/molecule/driver/delegated.py +++ b/molecule/driver/delegated.py @@ -19,6 +19,7 @@ # DEALINGS IN THE SOFTWARE. from molecule import logger +from molecule import util from molecule.driver import base LOG = logger.get_logger(__name__) @@ -30,13 +31,26 @@ class Delegated(base.Base): the default driver used in Molecule. Under this driver, it is the developers responsibility to implement the - create and destroy actions. + create and destroy playbooks. `Managed` is the default behaviour of all + drivers. .. code-block:: yaml driver: name: delegated + However, the developer must adhere to the instance-config API. The + developer's create playbook must provide the following instance-config + data, and the developer's destroy playbook must reset the instance-config. + + .. code-block:: yaml + + - address: ssh_endpoint + identity_file: ssh_identity_file + instance: instance_name + port: ssh_port_as_string + user: ssh_user + Molecule can also skip the provisioning/deprovisioning steps. It is the developers responsibility to manage the instances, and properly configure Molecule to connect to said instances. @@ -80,7 +94,7 @@ class Delegated(base.Base): platforms: - name: instance-vagrant - Provide the files Molecule will preserve upon each subcommand execution. + Provide the files Molecule will preserve post `destroy` action. .. code-block:: yaml @@ -104,6 +118,14 @@ def name(self, value): @property def login_cmd_template(self): + if self.managed: + connection_options = ' '.join(self.ssh_connection_options) + + return ('ssh {{address}} ' + '-l {{user}} ' + '-p {{port}} ' + '-i {{identity_file}} ' + '{}').format(connection_options) return self.options['login_cmd_template'] @property @@ -112,15 +134,53 @@ def default_safe_files(self): @property def default_ssh_connection_options(self): + if self.managed: + return self._get_ssh_connection_options() return [] def login_options(self, instance_name): + if self.managed: + d = {'instance': instance_name} + + return util.merge_dicts(d, + self._get_instance_config(instance_name)) return {'instance': instance_name} def ansible_connection_options(self, instance_name): + if self.managed: + try: + d = self._get_instance_config(instance_name) + + return { + 'ansible_user': + d['user'], + 'ansible_host': + d['address'], + 'ansible_port': + d['port'], + 'ansible_private_key_file': + d['identity_file'], + 'connection': + 'ssh', + 'ansible_ssh_common_args': + ' '.join(self.ssh_connection_options), + } + except StopIteration: + return {} + except IOError: + # Instance has yet to be provisioned , therefore the + # instance_config is not on disk. + return {} return self.options['ansible_connection_options'] def _created(self): if self.managed: return super(Delegated, self)._created() return 'unknown' + + def _get_instance_config(self, instance_name): + instance_config_dict = util.safe_load_file( + self._config.driver.instance_config) + + return next(item for item in instance_config_dict + if item['instance'] == instance_name) diff --git a/test/unit/command/conftest.py b/test/unit/command/conftest.py index a951571ad..77a7ce8a0 100644 --- a/test/unit/command/conftest.py +++ b/test/unit/command/conftest.py @@ -40,3 +40,13 @@ def command_driver_delegated_section_data(): } } } + + +@pytest.fixture +def command_driver_delegated_managed_section_data(): + return { + 'driver': { + 'name': 'delegated', + 'managed': True, + } + } diff --git a/test/unit/command/test_login.py b/test/unit/command/test_login.py index 990489276..173f7ff71 100644 --- a/test/unit/command/test_login.py +++ b/test/unit/command/test_login.py @@ -39,9 +39,9 @@ def test_execute(mocker, _instance): @pytest.mark.parametrize( - 'config_instance', ['command_driver_delegated_section_data'], + 'config_instance', ['command_driver_delegated_managed_section_data'], indirect=True) -def test_execute_raises_when_not_converged(patched_logger_critical, _instance): +def test_execute_raises_when_not_created(patched_logger_critical, _instance): _instance._config.state.change_state('created', False) with pytest.raises(SystemExit) as e: diff --git a/test/unit/conftest.py b/test/unit/conftest.py index b1cd47a1f..3f6b6b007 100644 --- a/test/unit/conftest.py +++ b/test/unit/conftest.py @@ -60,9 +60,13 @@ def _molecule_dependency_galaxy_section_data(): @pytest.fixture def _molecule_driver_section_data(): + print 'I HERE' return { 'driver': { 'name': 'docker', + 'options': { + 'managed': True, + }, }, } diff --git a/test/unit/driver/test_delegated.py b/test/unit/driver/test_delegated.py index 504d88dfe..7d23a043f 100644 --- a/test/unit/driver/test_delegated.py +++ b/test/unit/driver/test_delegated.py @@ -27,7 +27,16 @@ @pytest.fixture -def _driver_section_data(): +def _driver_managed_section_data(): + return { + 'driver': { + 'name': 'delegated', + } + } + + +@pytest.fixture +def _driver_unmanaged_section_data(): return { 'driver': { 'name': 'delegated', @@ -35,7 +44,8 @@ def _driver_section_data(): 'login_cmd_template': 'docker exec -ti {instance} bash', 'ansible_connection_options': { 'ansible_connection': 'docker' - } + }, + 'managed': False, } } } @@ -62,13 +72,23 @@ def test_name_property(_instance): @pytest.mark.parametrize( - 'config_instance', ['_driver_section_data'], indirect=True) + 'config_instance', ['_driver_unmanaged_section_data'], indirect=True) def test_options_property(_instance): x = { 'ansible_connection_options': { 'ansible_connection': 'docker' }, 'login_cmd_template': 'docker exec -ti {instance} bash', + 'managed': False, + } + + assert x == _instance.options + + +@pytest.mark.parametrize( + 'config_instance', ['_driver_managed_section_data'], indirect=True) +def test_options_property_when_managed(_instance): + x = { 'managed': True, } @@ -76,13 +96,26 @@ def test_options_property(_instance): @pytest.mark.parametrize( - 'config_instance', ['_driver_section_data'], indirect=True) + 'config_instance', ['_driver_unmanaged_section_data'], indirect=True) def test_login_cmd_template_property(_instance): x = 'docker exec -ti {instance} bash' assert x == _instance.login_cmd_template +@pytest.mark.parametrize( + 'config_instance', ['_driver_managed_section_data'], indirect=True) +def test_login_cmd_template_property_when_managed(_instance): + x = ('ssh {address} -l {user} -p {port} -i {identity_file} ' + '-o UserKnownHostsFile=/dev/null ' + '-o ControlMaster=auto ' + '-o ControlPersist=60s ' + '-o IdentitiesOnly=yes ' + '-o StrictHostKeyChecking=no') + + assert x == _instance.login_cmd_template + + def test_safe_files_property(_instance): assert [] == _instance.safe_files @@ -99,22 +132,112 @@ def test_managed_property(_instance): assert _instance.managed +@pytest.mark.parametrize( + 'config_instance', ['_driver_unmanaged_section_data'], indirect=True) def test_default_ssh_connection_options_property(_instance): assert [] == _instance.default_ssh_connection_options +@pytest.mark.parametrize( + 'config_instance', ['_driver_managed_section_data'], indirect=True) +def test_default_ssh_connection_options_property_when_managed(_instance): + x = [ + '-o UserKnownHostsFile=/dev/null', + '-o ControlMaster=auto', + '-o ControlPersist=60s', + '-o IdentitiesOnly=yes', + '-o StrictHostKeyChecking=no', + ] + + assert x == _instance.default_ssh_connection_options + + +@pytest.mark.parametrize( + 'config_instance', ['_driver_unmanaged_section_data'], indirect=True) def test_login_options(_instance): assert {'instance': 'foo'} == _instance.login_options('foo') @pytest.mark.parametrize( - 'config_instance', ['_driver_section_data'], indirect=True) + 'config_instance', ['_driver_managed_section_data'], indirect=True) +def test_login_options_when_managed(mocker, _instance): + m = mocker.patch( + 'molecule.driver.delegated.Delegated._get_instance_config') + m.return_value = { + 'instance': 'foo', + 'address': '172.16.0.2', + 'user': 'cloud-user', + 'port': 22, + 'identity_file': '/foo/bar', + } + + x = { + 'instance': 'foo', + 'address': '172.16.0.2', + 'user': 'cloud-user', + 'port': 22, + 'identity_file': '/foo/bar', + } + assert x == _instance.login_options('foo') + + +@pytest.mark.parametrize( + 'config_instance', ['_driver_unmanaged_section_data'], indirect=True) def test_ansible_connection_options(_instance): x = {'ansible_connection': 'docker'} assert x == _instance.ansible_connection_options('foo') +@pytest.mark.parametrize( + 'config_instance', ['_driver_managed_section_data'], indirect=True) +def test_ansible_connection_options_when_managed(mocker, _instance): + m = mocker.patch( + 'molecule.driver.delegated.Delegated._get_instance_config') + m.return_value = { + 'instance': 'foo', + 'address': '172.16.0.2', + 'user': 'cloud-user', + 'port': 22, + 'identity_file': '/foo/bar', + } + + x = { + 'ansible_host': + '172.16.0.2', + 'ansible_port': + 22, + 'ansible_user': + 'cloud-user', + 'ansible_private_key_file': + '/foo/bar', + 'connection': + 'ssh', + 'ansible_ssh_common_args': ('-o UserKnownHostsFile=/dev/null ' + '-o ControlMaster=auto ' + '-o ControlPersist=60s ' + '-o IdentitiesOnly=yes ' + '-o StrictHostKeyChecking=no'), + } + assert x == _instance.ansible_connection_options('foo') + + +def test_ansible_connection_options_handles_missing_instance_config_managed( + mocker, _instance): + m = mocker.patch('molecule.util.safe_load_file') + m.side_effect = IOError + + assert {} == _instance.ansible_connection_options('foo') + + +def test_ansible_connection_options_handles_missing_results_key_when_managed( + mocker, _instance): + m = mocker.patch('molecule.util.safe_load_file') + m.side_effect = StopIteration + + assert {} == _instance.ansible_connection_options('foo') + + def test_instance_config_property(_instance): x = os.path.join(_instance._config.scenario.ephemeral_directory, 'instance_config.yml') @@ -122,6 +245,8 @@ def test_instance_config_property(_instance): assert x == _instance.instance_config +@pytest.mark.parametrize( + 'config_instance', ['_driver_unmanaged_section_data'], indirect=True) def test_ssh_connection_options_property(_instance): assert [] == _instance.ssh_connection_options @@ -170,3 +295,17 @@ def test_created_unknown_when_managed_false( def test_property(_instance): assert 'false' == _instance._converged() + + +def test_get_instance_config(mocker, _instance): + m = mocker.patch('molecule.util.safe_load_file') + m.return_value = [{ + 'instance': 'foo', + }, { + 'instance': 'bar', + }] + + x = { + 'instance': 'foo', + } + assert x == _instance._get_instance_config('foo')