Encoding errors (#1519)

* Wherever check_output is used add encoding as even if we do not read stdout it might fail with a timeout or in exception handling when parsing stdout

* Using replacement everywhere when calling .run

Author: ArneTR

cron/client.py

@@ -124,9 +124,9 @@ def validate_temperature():
         validate_temperature.temperature_errors += 1
 
         # stress all cores with constant yes operation
-        subprocess.check_output('for i in $(seq $(nproc)); do yes > /dev/null & done', shell=True)
+        subprocess.check_output('for i in $(seq $(nproc)); do yes > /dev/null & done', shell=True, encoding='UTF-8', errors='replace')
         time.sleep(300)
-        subprocess.check_output(['killall', 'yes'])
+        subprocess.check_output(['killall', 'yes'], encoding='UTF-8', errors='replace')
 
         return False
 
@@ -287,9 +287,9 @@ def do_measurement_control():
             else:
                 set_status('job_no')
                 if config['cluster']['client']['shutdown_on_job_no']:
-                    subprocess.check_output(['sync'])
+                    subprocess.check_output(['sync'], encoding='UTF-8', errors='replace')
                     time.sleep(60) # sleep for 60 before going to suspend to allow logins to cluster when systems are fresh rebooted for maintenance
-                    subprocess.check_output(['sudo', 'systemctl', config['cluster']['client']['shutdown_on_job_no']])
+                    subprocess.check_output(['sudo', 'systemctl', config['cluster']['client']['shutdown_on_job_no']], encoding='UTF-8', errors='replace')
 
                 if not args.testing:
                     time.sleep(config['cluster']['client']['sleep_time_no_job'])

lib/scenario_runner.py

@@ -273,12 +273,12 @@ def _save_notes_runner(self):
         self.__notes_helper.save_to_db(self._run_id)
 
     def _clear_caches(self):
-        subprocess.check_output(['sync'])
+        subprocess.check_output(['sync'], encoding='UTF-8', errors='replace')
 
         if platform.system() == 'Darwin':
             return
         # 3 instructs kernel to drops page caches AND inode caches
-        subprocess.check_output(['sudo', '/usr/sbin/sysctl', '-w', 'vm.drop_caches=3'])
+        subprocess.check_output(['sudo', '/usr/sbin/sysctl', '-w', 'vm.drop_caches=3'], encoding='UTF-8', errors='replace')
 
     def _check_system(self, mode='start'):
         print(TerminalColors.HEADER, '\nChecking system', TerminalColors.ENDC)
@@ -315,6 +315,7 @@ def _checkout_repository(self):
                 check=True,
                 capture_output=True,
                 encoding='UTF-8',
+                errors='replace'
             )
 
             if problematic_symlink := utils.find_outside_symlinks(self._repo_folder):
@@ -376,6 +377,7 @@ def _checkout_relations(self):
                 check=True,
                 capture_output=True,
                 encoding='UTF-8',
+                errors='replace'
             )
 
             if 'commit_hash' in relation:
@@ -384,6 +386,7 @@ def _checkout_relations(self):
                     check=True,
                     capture_output=True,
                     encoding='UTF-8',
+                    errors='replace',
                     cwd=relation_path,
                 )
 
@@ -616,13 +619,15 @@ def _check_container_is_running(self, container_name: str, step_description: str
                 ['docker', 'logs', container_name],
                 check=False,
                 capture_output=True,
-                encoding='UTF-8'
+                encoding='UTF-8',
+                errors='replace',
             )
             inspect_ps = subprocess.run(
                 ['docker', 'inspect', '--format={{.State.ExitCode}}', container_name],
                 check=False,
                 capture_output=True,
-                encoding='UTF-8'
+                encoding='UTF-8',
+                errors='replace',
             )
             exit_code = inspect_ps.stdout.strip() if inspect_ps.returncode == 0 else "unknown"
 
@@ -1119,9 +1124,9 @@ def _setup_networks(self):
                 subprocess.run(['docker', 'network', 'rm', network], stderr=subprocess.DEVNULL, check=False)
 
                 if self.__usage_scenario['networks'][network] and self.__usage_scenario['networks'][network].get('internal', False):
-                    subprocess.check_output(['docker', 'network', 'create', '--internal', network])
+                    subprocess.check_output(['docker', 'network', 'create', '--internal', network], encoding='UTF-8', errors='replace')
                 else:
-                    subprocess.check_output(['docker', 'network', 'create', network])
+                    subprocess.check_output(['docker', 'network', 'create', network], encoding='UTF-8', errors='replace')
 
                 self.__networks.append(network)
         else:

lib/utils.py

@@ -204,8 +204,7 @@ def get_metric_providers_names(config):
     return [(m.split('.')[-1]) for m in metric_providers_keys]
 
 def get_architecture():
-    ps = subprocess.run(['uname', '-s'],
-            check=True, stderr=subprocess.PIPE, stdout=subprocess.PIPE, encoding='UTF-8', errors='replace')
+    ps = subprocess.run(['uname', '-s'], check=True, stderr=subprocess.PIPE, stdout=subprocess.PIPE, encoding='UTF-8', errors='replace')
     output = ps.stdout.strip().lower()
 
     if output == 'darwin':

metric_providers/base.py

@@ -56,7 +56,7 @@ def check_system(self, check_command="default", check_error_message=None, check_
                     call_string = f"{self._current_dir}/{call_string}"
                 check_command = [f"{call_string}", '-c']
 
-            ps = subprocess.run(check_command, capture_output=True, encoding='UTF-8', check=False)
+            ps = subprocess.run(check_command, capture_output=True, encoding='UTF-8', errors='replace', check=False)
             if ps.returncode != 0:
                 if check_error_message is None:
                     check_error_message = ps.stderr

metric_providers/lmsensors/abstract_provider.py

@@ -43,7 +43,7 @@ def check_system(self, check_command="default", check_error_message=None, check_
         super().check_system(check_command=None)
 
         # Run 'sensors' command and capture the output
-        ps = subprocess.run(['sensors'], capture_output=True, text=True, check=False)
+        ps = subprocess.run(['sensors'], capture_output=True, encoding='UTF-8', errors='replace', check=False)
         if ps.returncode != 0:
             raise MetricProviderConfigurationError(f"{self._metric_name} provider could not be started.\nCannot run the 'sensors' command. Did you install lm-sensors?.\n\nAre you running in a VM / cloud / shared hosting?\nIf so please disable the {self._metric_name} provider in the config.yml")
 

metric_providers/network/connections/proxy/container/provider.py

@@ -50,8 +50,8 @@ def get_docker_params(self, no_proxy_list=''):
         elif platform.system() == 'Linux':
             # Under Linux there is no way to directly link to the host
             cmd =  "ip addr show dev $(ip route | grep default | awk '{print $5}') | grep 'inet '| awk '{print $2}'| cut -f1 -d'/'"
-            ps = subprocess.run(cmd, shell=True, check=True, text=True, capture_output=True)
-            proxy_addr = ps.stdout.strip()
+            output = subprocess.check_output(cmd, shell=True, encoding='UTF-8', errors='replace')
+            proxy_addr = output.strip()
         else:
             proxy_addr = 'host.docker.internal'
 

metric_providers/powermetrics/provider.py

@@ -74,7 +74,7 @@ def stop_profiling(self):
             # There is really no better way of doing this as of now. Keeping the process id for instance in a hash and
             # killing only that would also fail du to root permissions missing. If we add an /etc/sudoers entry with a
             # wildcard for a PID we open up a security hole. Happy to take suggestions on this one!
-            subprocess.check_output('sudo /usr/bin/killall powermetrics', shell=True)
+            subprocess.check_output('sudo /usr/bin/killall powermetrics', shell=True, encoding='UTF-8', errors='replace')
             print('Killed powermetrics process with killall!')
             if self._pm_process_count > 0:
                 print('-----------------------------------------------------------------------------------------------------------------')
@@ -89,7 +89,7 @@ def stop_profiling(self):
             time.sleep(1)
             count += 1
             if count >= 60:
-                subprocess.check_output('sudo /usr/bin/killall -9 powermetrics', shell=True)
+                subprocess.check_output('sudo /usr/bin/killall -9 powermetrics', shell=True, encoding='UTF-8', errors='replace')
                 raise RuntimeError('powermetrics had to be killed with kill -9. Values can not be trusted!')
 
         self._ps = None

tests/metric_providers/test_metric_providers.py

@@ -72,9 +72,9 @@ def test_splitting_by_group():
 
 @pytest.mark.skipif(utils.get_architecture() == 'macos', reason="macOS does not support disk used capturing atm")
 def test_disk_statvfs_providers():
-    disk_id_docker = subprocess.check_output("stat -c '%d' $(docker info --format '{{.DockerRootDir}}')", shell=True, encoding='UTF-8')
+    disk_id_docker = subprocess.check_output("stat -c '%d' $(docker info --format '{{.DockerRootDir}}')", shell=True, encoding='UTF-8', errors='replace')
 
-    disk_id_root = subprocess.check_output(['stat', '-c', '%d', '/'], encoding='UTF-8')
+    disk_id_root = subprocess.check_output(['stat', '-c', '%d', '/'], encoding='UTF-8', errors='replace')
 
     if disk_id_docker != disk_id_root:
         pytest.skip('Docker data root is not on same disk and thus cannot determine disk use through standard provider')

tools/cluster/cleanup_original.py

@@ -46,13 +46,13 @@ def check_timers(data):
 ## Update time
 # may throw exception, but we need to check if time sync calls work, as we do not know what the actual time is
 # Typically in cluster installations port 123 is blocked and a local time server is available. Thus the guard function here
-subprocess.check_output(['sudo', 'timedatectl', 'set-ntp', 'true']) # this will trigger immediate update
+subprocess.check_output(['sudo', 'timedatectl', 'set-ntp', 'true'], encoding='UTF-8', errors='replace') # this will trigger immediate update
 time.sleep(5)
 ntp_status = subprocess.check_output(['timedatectl', '-a'], encoding='UTF-8', errors='replace')
 if 'System clock synchronized: yes' not in ntp_status or 'NTP service: active' not in ntp_status:
     raise RuntimeError('System clock could not be synchronized', ntp_status)
 
-subprocess.check_output(['sudo', 'timedatectl', 'set-ntp', 'false']) # we want NTP always off in clusters
+subprocess.check_output(['sudo', 'timedatectl', 'set-ntp', 'false'], encoding='UTF-8', errors='replace') # we want NTP always off in clusters
 time.sleep(2)
 ntp_status = subprocess.check_output(['timedatectl', '-a'], encoding='UTF-8', errors='replace')
 if 'System clock synchronized: yes' not in ntp_status:
@@ -93,6 +93,7 @@ def check_timers(data):
         stdout=subprocess.PIPE,
         stderr=subprocess.STDOUT, # put both in one stream
         encoding='UTF-8',
+        errors='replace'
     )
     if ps.returncode != 0:
         raise RuntimeError(f"sudo apt update failed: {ps.stdout}")
@@ -116,6 +117,7 @@ def check_timers(data):
         stdout=subprocess.PIPE,
         stderr=subprocess.STDOUT, # put both in one stream
         encoding='UTF-8',
+        errors='replace'
     )
     if ps.returncode != 0:
         raise RuntimeError(f"sudo apt full-upgrade -y failed: {ps.stdout}")

tools/update_commit_data.py

@@ -43,6 +43,7 @@
             check=True,
             capture_output=True,
             encoding='UTF-8',
+            errors='replace',
             cwd=args.folder
         )
         commit_timestamp = commit_timestamp.stdout.strip("\n")