Re: [PATCH 2/2] rteval: services.py: Fix incorrect detection of container environment

[Punit Agrawal <punitagrawal@gmail.com>]

John Kacur <jkacur@redhat.com> writes:

> On Thu, 9 Sep 2021, Punit Agrawal wrote:
>
>> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> 
>> rteval mistakenly detects that it is running inside a container even
>> though it is running directly on the host. On further investigation
>> this was found to be due to change in behaviour around byte strings
>> and strings when going from python2 to python3.
>> 
>> In python3 byte strings are not equivalent to strings, i.e., b'' == ''
>> is False. The string comparison functions in services.py are still
>> relying on the old behaviour in python2 where they were equivalent.
>> 
>> Update the byte string processing by converting them to string.
>> 
>> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> ---
>>  rteval/sysinfo/services.py | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/rteval/sysinfo/services.py b/rteval/sysinfo/services.py
>> index 06ff5ae9cd0c..94857aea6be4 100644
>> --- a/rteval/sysinfo/services.py
>> +++ b/rteval/sysinfo/services.py
>> @@ -83,8 +83,8 @@ class SystemServices:
>>          self.__log(Log.DEBUG, "cmd: %s" % cmd)
>>          c = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
>>          for p in c.stdout:
>> -            # p are lines like "servicename.service status"
>> -            v = p.strip().split()
>> +            # p are lines like b'servicename.service status'
>> +            v = p.decode().strip().split()
>>              ret_services[v[0].split('.')[0]] = v[1]
>>          return ret_services
>>  
>> @@ -92,7 +92,7 @@ class SystemServices:
>>      def services_get(self):
>>          cmd = [getcmdpath('ps'), '-ocomm=', '1']
>>          c = subprocess.Popen(cmd, stdout=subprocess.PIPE)
>> -        self.__init = c.stdout.read().strip()
>> +        self.__init = c.stdout.read().decode().strip()
>>          if self.__init == 'systemd':
>>              self.__log(Log.DEBUG, "Using systemd to get services status")
>>              return self.__get_services_systemd()
>> -- 
>> 2.32.0
>> 
>> 
>
> Thanks, this looks good, I'm wondering however if the same result could be 
> achieved by appending text=True to the subprocess command in each of those
> methods? Would you like to test that and send me a new patch?

Thanks for the suggestion - I missed the "text=True" in the Popen() when
going through the documentation.

I will send a new version converting all Popen() sites in the file.

[...]