Make sudo switchable when Kubernetes mode is disabled

close #318

CONTRIBUTING.md

@@ -12,7 +12,11 @@ This project adheres to the [Spotify FOSS Code of Conduct][code-of-conduct]. By
 Before pushing a PR, please make sure you've added the appropriate unit tests (under `/tests/`) and integration tests (under `/tests/integration/`) for the changes you are making to the codebase.
 
 We use [flake8](http://flake8.pycqa.org/en/latest/) for code style checking.  
-We use [the standard Python framework for unit tests](https://docs.python.org/3.6/library/unittest.html). 
+
+We use [pytest for unit tests](http://pytest.readthedocs.io/en/latest/).
+Older commits make use of [the standard Python framework](https://docs.python.org/3.6/library/unittest.html) and [nose](https://nose.readthedocs.io/en/latest/) but they are no longer used.
+Feel free to migrate existing tests to pytest when adding new test cases.
+
 We use [Aloe](https://aloe.readthedocs.io/en/latest/) as framework for running integration tests. As [Cucumber](https://cucumber.io/), it is a [Gherkin-based](https://cucumber.io/docs/gherkin/reference/) framework for writing test scenarios using natural language.
 
 ## Running tests
@@ -32,10 +36,10 @@ You can run checks and unit tests individually with the following commands:
 
 ```
 # Code checks
-python3 -m "flake8" --ignore=W503
+python3 -m "flake8" --ignore=W503,E402
 
 # Unit tests
-python3 -m "nose"
+python3 -m pytest
 ```
 
 ### Integration tests

medusa/backup_cluster.py

@@ -176,9 +176,10 @@ def _build_backup_cmd(self):
         enable_md5_checks_option = '--enable-md5-checks' if self.enable_md5_checks else ''
 
         # Use %s placeholders in the below command to have them replaced by pssh using per host command substitution
-        command = 'mkdir -p {work}; cd {work} && medusa-wrapper sudo medusa {config} -vvv backup-node ' \
+        command = 'mkdir -p {work}; cd {work} && medusa-wrapper {sudo} medusa {config} -vvv backup-node ' \
                   '--backup-name {backup_name} {stagger} {enable_md5_checks} --mode {mode}' \
             .format(work=self.work_dir,
+                    sudo='sudo' if medusa.utils.evaluate_boolean(self.config.cassandra.use_sudo) else '',
                     config=f'--config-file {self.config.file_path}' if self.config.file_path else '',
                     backup_name=self.backup_name,
                     stagger=stagger_option,

medusa/backup_node.py

@@ -227,6 +227,7 @@ def main(config, backup_name_arg, stagger_time, enable_md5_checks_flag, mode):
             config
         )
 
+
 # Wait 2^i * 10 seconds between each retry, up to 2 minutes between attempts, which is right after the
 # attempt on which it waited for 60 seconds
 @retry(stop_max_attempt_number=7, wait_exponential_multiplier=10000, wait_exponential_max=120000)

medusa/config.py

@@ -22,6 +22,7 @@
 import sys
 
 import medusa.cassandra_utils
+import medusa.storage
 from medusa.utils import evaluate_boolean
 
 StorageConfig = collections.namedtuple(
@@ -37,7 +38,7 @@
     ['start_cmd', 'stop_cmd', 'config_file', 'cql_username', 'cql_password', 'check_running', 'is_ccm',
      'sstableloader_bin', 'nodetool_username', 'nodetool_password', 'nodetool_password_file_path', 'nodetool_host',
      'nodetool_port', 'certfile', 'usercert', 'userkey', 'sstableloader_ts', 'sstableloader_tspw',
-     'sstableloader_ks', 'sstableloader_kspw', 'nodetool_ssl', 'resolve_ip_addresses']
+     'sstableloader_ks', 'sstableloader_kspw', 'nodetool_ssl', 'resolve_ip_addresses', 'use_sudo']
 )
 
 SSHConfig = collections.namedtuple(
@@ -92,10 +93,12 @@
 DEFAULT_CONFIGURATION_PATH = pathlib.Path('/etc/medusa/medusa.ini')
 
 
-def parse_config(args, config_file):
-    config = configparser.ConfigParser(interpolation=None)
+def _build_default_config():
+    """Build a INI config parser with default values
 
-    # Set defaults
+    :return ConfigParser: default configuration
+    """
+    config = configparser.ConfigParser(interpolation=None)
 
     config['storage'] = {
         'host_file_separator': ',',
@@ -128,13 +131,14 @@ def parse_config(args, config_file):
         'check_running': 'nodetool version',
         'is_ccm': '0',
         'sstableloader_bin': 'sstableloader',
-        'resolve_ip_addresses': 'True'
+        'resolve_ip_addresses': 'True',
+        'use_sudo': 'True',
     }
 
     config['ssh'] = {
         'username': os.environ.get('USER') or '',
         'key_file': '',
-        'port': 22,
+        'port': '22',
         'cert_file': ''
     }
 
@@ -159,6 +163,17 @@ def parse_config(args, config_file):
         'cassandra_url': 'None',
         'use_mgmt_api': 'False'
     }
+    return config
+
+
+def parse_config(args, config_file):
+    """Parse a medusa.ini file and allow to override settings from command line
+
+    :param dict args: settings override. Higher priority than settings defined in medusa.ini
+    :param pathlib.Path config_file: path to medusa.ini file
+    :return: None
+    """
+    config = _build_default_config()
 
     if config_file is None and not DEFAULT_CONFIGURATION_PATH.exists():
         logging.error(
@@ -168,7 +183,8 @@ def parse_config(args, config_file):
 
     actual_config_file = DEFAULT_CONFIGURATION_PATH if config_file is None else config_file
     logging.debug('Loading configuration from {}'.format(actual_config_file))
-    config.read_file(actual_config_file.open())
+    with actual_config_file.open() as f:
+        config.read_file(f)
 
     # Override config file settings with command line options
     for config_section in config.keys():
@@ -182,6 +198,11 @@ def parse_config(args, config_file):
             if value is not None
         }})
 
+    if evaluate_boolean(config['kubernetes']['enabled']):
+        if evaluate_boolean(config['cassandra']['use_sudo']):
+            logging.warning('Forcing use_sudo to False because Kubernetes mode is enabled')
+        config['cassandra']['use_sudo'] = 'False'
+
     resolve_ip_addresses = evaluate_boolean(config['cassandra']['resolve_ip_addresses'])
     config.set('cassandra', 'resolve_ip_addresses', 'True' if resolve_ip_addresses else 'False')
     if config['storage']['fqdn'] == socket.getfqdn() and not resolve_ip_addresses:
@@ -199,9 +220,9 @@ def parse_config(args, config_file):
 def load_config(args, config_file):
     """Load configuration from a medusa.ini file
 
-    :param args: settings override. Higher priority than settings defined in medusa.ini
-    :param config_file: path to a medusa.ini file or None if default path should be used
-    :return: Medusa configuration
+    :param dict args: settings override. Higher priority than settings defined in medusa.ini
+    :param pathlib.Path config_file: path to a medusa.ini file or None if default path should be used
+    :return MedusaConfig: Medusa configuration
     """
     config = parse_config(args, config_file)
 

medusa/orchestration.py

@@ -14,7 +14,10 @@
 # limitations under the License.
 
 import logging
+
 from pssh.clients.ssh import ParallelSSHClient
+
+import medusa.utils
 from medusa.storage import divide_chunks
 
 
@@ -27,40 +30,43 @@ def display_output(host_outputs):
 
 
 class Orchestration(object):
-    def __init__(self, cassandra_config, pool_size=10):
+    def __init__(self, config, pool_size=10):
         self.pool_size = pool_size
-        self.cassandra_config = cassandra_config
+        self.config = config
 
-    def pssh_run(self, hosts, command, hosts_variables=None):
+    def pssh_run(self, hosts, command, hosts_variables=None, ssh_client=None):
         """
         Runs a command on hosts list using pssh under the hood
         Return: True (success) or False (error)
         """
+        if ssh_client is None:
+            ssh_client = ParallelSSHClient
         pssh_run_success = False
         success = []
         error = []
         i = 1
 
-        username = self.cassandra_config.ssh.username if self.cassandra_config.ssh.username != '' else None
-        port = int(self.cassandra_config.ssh.port)
-        pkey = self.cassandra_config.ssh.key_file if self.cassandra_config.ssh.key_file != '' else None
-        cert_file = self.cassandra_config.ssh.cert_file if self.cassandra_config.ssh.cert_file != '' else None
+        username = self.config.ssh.username if self.config.ssh.username != '' else None
+        port = int(self.config.ssh.port)
+        pkey = self.config.ssh.key_file if self.config.ssh.key_file != '' else None
+        cert_file = self.config.ssh.cert_file if self.config.ssh.cert_file != '' else None
 
         logging.info('Executing "{command}" on following nodes {hosts} with a parallelism/pool size of {pool_size}'
                      .format(command=command, hosts=hosts, pool_size=self.pool_size))
 
         for parallel_hosts in divide_chunks(hosts, self.pool_size):
 
-            client = ParallelSSHClient(parallel_hosts,
-                                       forward_ssh_agent=True,
-                                       pool_size=len(parallel_hosts),
-                                       user=username,
-                                       port=port,
-                                       pkey=pkey,
-                                       cert_file=cert_file)
+            client = ssh_client(parallel_hosts,
+                                forward_ssh_agent=True,
+                                pool_size=len(parallel_hosts),
+                                user=username,
+                                port=port,
+                                pkey=pkey,
+                                cert_file=cert_file)
             logging.debug('Batch #{i}: Running "{command}" on nodes {hosts} parallelism of {pool_size}'
                           .format(i=i, command=command, hosts=parallel_hosts, pool_size=len(parallel_hosts)))
-            output = client.run_command(command, host_args=hosts_variables, sudo=True)
+            output = client.run_command(command, host_args=hosts_variables,
+                                        sudo=medusa.utils.evaluate_boolean(self.config.cassandra.use_sudo))
             client.join(output)
 
             success = success + list(filter(lambda host_output: host_output.exit_code == 0, output))

medusa/restore_cluster.py

@@ -357,23 +357,23 @@ def _build_restore_cmd(self):
         verify_option = '--no-verify'
 
         # %s placeholders in the below command will get replaced by pssh using per host command substitution
-        # %s placeholders in the below command will get replaced by pssh using per host command substitution
-        command = 'mkdir -p {work}; cd {work} && medusa-wrapper sudo medusa {config} ' \
+        command = 'mkdir -p {work}; cd {work} && medusa-wrapper {sudo} medusa {config} ' \
                   '--fqdn=%s -vvv restore-node ' \
                   '{in_place} {keep_auth} %s {verify} --backup-name {backup} --temp-dir {temp_dir} ' \
                   '{use_sstableloader} {keyspaces} {tables}' \
             .format(work=self.work_dir,
+                    sudo='sudo' if medusa.utils.evaluate_boolean(self.config.cassandra.use_sudo) else '',
                     config=f'--config-file {self.config.file_path}' if self.config.file_path else '',
                     in_place=in_place_option,
                     keep_auth=keep_auth_option,
                     verify=verify_option,
                     backup=self.cluster_backup.name,
                     temp_dir=self.temp_dir,
-                    use_sstableloader='--use-sstableloader' if self.use_sstableloader is True else '',
+                    use_sstableloader='--use-sstableloader' if self.use_sstableloader else '',
                     keyspaces=keyspace_options,
                     tables=table_options)
 
-        logging.debug('Preparing to restore on all nodes with the following command {}'.format(command))
+        logging.debug('Preparing to restore on all nodes with the following command: {}'.format(command))
 
         return command
 

medusa/restore_node.py

@@ -95,7 +95,7 @@ def restore_node_locally(config, temp_dir, backup_name, in_place, keep_auth, see
 
     # Clean the commitlogs, the saved cache to prevent any kind of conflict
     # especially around system tables.
-    use_sudo = not medusa.utils.evaluate_boolean(config.kubernetes.enabled)
+    use_sudo = medusa.utils.evaluate_boolean(config.cassandra.use_sudo)
     clean_path(cassandra.commit_logs_path, use_sudo, keep_folder=True)
     clean_path(cassandra.saved_caches_path, use_sudo, keep_folder=True)
 
@@ -180,7 +180,8 @@ def restore_node_sstableloader(config, temp_dir, backup_name, in_place, keep_aut
 
     # Clean the restored data from local temporary folder
     if download_dir:
-        clean_path(download_dir, keep_folder=False)
+        use_sudo = medusa.utils.evaluate_boolean(config.cassandra.use_sudo)
+        clean_path(download_dir, use_sudo, keep_folder=False)
     return node_backup
 
 
@@ -253,7 +254,7 @@ def table_is_allowed_to_restore(keyspace, table, fqtns_to_restore):
     return True
 
 
-def clean_path(p, use_sudo=True, keep_folder=False):
+def clean_path(p, use_sudo, keep_folder=False):
     path = str(p)
     if p.exists() and os.path.isdir(path) and len(os.listdir(path)):
         logging.debug('Cleaning ({})'.format(path))

tests/config_test.py

@@ -16,6 +16,7 @@
 import os
 import pathlib
 import unittest
+import socket
 
 import medusa.config
 import medusa.utils
@@ -70,6 +71,55 @@ def test_args_settings_override(self):
         assert medusa.utils.evaluate_boolean(config.kubernetes.use_mgmt_api)
         assert config.ssh.username == 'Zeus'
 
+    def test_use_sudo_default(self):
+        """Ensure that, by default, use_sudo is enabled and kubernetes disabled"""
+        args = {}
+        config = medusa.config.load_config(args, self.medusa_config_file)
+        assert medusa.utils.evaluate_boolean(config.cassandra.use_sudo)
+        # Kubernetes must be disabled by default so use_sudo can be honored
+        assert not medusa.utils.evaluate_boolean(config.kubernetes.enabled)
+
+    def test_use_sudo_kubernetes_disabled(self):
+        """Ensure that use_sudo is honored when Kubernetes mode is disabled (default)"""
+        args = {'use_sudo': 'True'}
+        config = medusa.config.parse_config(args, self.medusa_config_file)
+        assert config['cassandra']['use_sudo'] == 'True', 'sudo should be used because Kubernetes mode is not enabled'
+
+        args = {'use_sudo': 'False'}
+        config = medusa.config.parse_config(args, self.medusa_config_file)
+        assert config['cassandra']['use_sudo'] == 'False', 'sudo should not be used as explicitly required'
+
+    def test_use_sudo_kubernetes_enabled(self):
+        """Ensure that use_sudo is disabled when Kubernetes mode is enabled"""
+        args = {'use_sudo': 'true'}
+        medusa_k8s_config = pathlib.Path(__file__).parent / "resources/config/medusa-kubernetes.ini"
+        config = medusa.config.parse_config(args, medusa_k8s_config)
+        assert config['cassandra']['use_sudo'] == 'False'
+
+    def test_overridden_fqdn(self):
+        """Ensure that a overridden fqdn in config is honored"""
+        args = {'fqdn': 'overridden-fqdn'}
+        config = medusa.config.parse_config(args, self.medusa_config_file)
+        assert config['storage']['fqdn'] == 'overridden-fqdn'
+
+    def test_fqdn_with_resolve_ip_addresses_enabled(self):
+        """Ensure that explicitly defined fqdn is untouched when DNS resolving is enabled"""
+        args = {
+            'fqdn': socket.getfqdn(),
+            'resolve_ip_addresses': 'True'
+        }
+        config = medusa.config.parse_config(args, self.medusa_config_file)
+        assert config['storage']['fqdn'] == socket.getfqdn()
+
+    def test_fqdn_with_resolve_ip_addresses_disabled(self):
+        """Ensure that fqdn is an IP address when DNS resolving is disabled"""
+        args = {
+            'fqdn': socket.getfqdn(),
+            'resolve_ip_addresses': 'False'
+        }
+        config = medusa.config.parse_config(args, self.medusa_config_file)
+        assert config['storage']['fqdn'] == socket.gethostbyname(socket.getfqdn())
+
 
 if __name__ == '__main__':
     unittest.main()

tests/integration/features/steps/integration_steps.py

@@ -432,7 +432,7 @@ def get_args(context, storage_provider, client_encryption, cassandra_url, use_mg
 
     storage_args = {"prefix": storage_prefix}
     cassandra_args = {
-        "is_ccm": 1,
+        "is_ccm": "1",
         "stop_cmd": CCM_STOP,
         "start_cmd": CCM_START,
         "cql_username": "cassandra",
@@ -452,7 +452,8 @@ def get_args(context, storage_provider, client_encryption, cassandra_url, use_mg
                 "sstableloader",
             )
         ),
-        "resolve_ip_addresses": False
+        "resolve_ip_addresses": "False",
+        "use_sudo": "True",
     }
 
     if client_encryption == 'with_client_encryption':