Restore Logs Manager - Logs Readers functions #2568

[FroggyFlox]

Fixes #2568
@phillxnet, @Hooverdan96, ready for review

We currently have 2 different logs readers:

• static: use cat, and tail without the -f flag
• live: uses tail -f

Both of these use subprocess.Popen() to read the log files.
In line with most of the other fixes that we have had to make for our move to Python3 (#2567), we need to specify we want it to return a str rather than the new bytes default.

We also were failing in the static reader due to the use of the now deprecated xrange() function. Based on my research, Python3's range() behaves similarly to Python2's xrange(). Switching to range() here fixes our issue.

Functional testing

Under this branch, all static and live readers work in the webUI.

Unit testing

rockdev:/opt/rockstor # cd src/rockstor/ ; poetry run django-admin test ; cd -
 Creating test database for alias 'default'...
Creating test database for alias 'smart_manager'...
System check identified no issues (0 silenced).
........................................F..FE................................................................................................................................................................................................................
======================================================================
ERROR: test_validate_taskdef_meta (rockstor.storageadmin.tests.test_config_backup.ConfigBackupTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/storageadmin/tests/test_config_backup.py", line 1926, in test_validate_taskdef_meta
    ret = validate_taskdef_meta(self.sa_ml, m, t)
  File "/opt/rockstor/src/rockstor/storageadmin/views/config_backup.py", line 216, in validate_taskdef_meta
    taskdef_meta["share"] = unicode(target_share_id)
NameError: name 'unicode' is not defined

======================================================================
FAIL: test_valid_requests (rockstor.storageadmin.tests.test_config_backup.ConfigBackupTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib64/python3.6/unittest/mock.py", line 1183, in patched
    return func(*args, **keywargs)
  File "/opt/rockstor/src/rockstor/storageadmin/tests/test_config_backup.py", line 1258, in test_valid_requests
    self.assertEqual(response.status_code, status.HTTP_200_OK, msg=response.data)
AssertionError: 500 != 200 : ["'utf-8' codec can't decode byte 0x8b in position 1: invalid start byte", 'Traceback (most recent call last):\n  File "/opt/rockstor/src/rockstor/rest_framework_custom/generic_view.py", line 41, in _handle_exception\n    yield\n  File "/opt/rockstor/src/rockstor/storageadmin/views/config_backup.py", line 662, in post\n    cbo = backup_config()\n  File "/opt/rockstor/src/rockstor/system/config_backup.py", line 76, in backup_config\n    cbo = ConfigBackup(filename=gz_name, md5sum=md5sum(fp), size=size)\n  File "/opt/rockstor/src/rockstor/system/osi.py", line 1444, in md5sum\n    for l in tfo.readlines():\n  File "/usr/lib64/python3.6/codecs.py", line 321, in decode\n    (result, consumed) = self._buffer_decode(data, self.errors, final)\nUnicodeDecodeError: \'utf-8\' codec can\'t decode byte 0x8b in position 1: invalid start byte\n']

======================================================================
FAIL: test_validate_task_definitions (rockstor.storageadmin.tests.test_config_backup.ConfigBackupTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/storageadmin/tests/test_config_backup.py", line 1986, in test_validate_task_definitions
    "expected = {}.".format(ret, out),
AssertionError: Lists differ: [] != [{'task_type': 'snapshot', 'name': 'snap_d[538 chars]4'}}]

Second list contains 3 additional elements.
First extra element 0:
{'task_type': 'snapshot', 'name': 'snap_daily_ts01', 'crontabwindow': '*-*-*-*-*-*', 'enabled': False, 'crontab': '42 3 * * *', 'meta': {'writable': True, 'visible': True, 'prefix': 'snap_daily_ts01', 'share': '3', 'max_count': '4'}}

Diff is 711 characters long. Set self.maxDiff to None to see it. : Unexpected validate_task_definitions() result:
 returned = [].
 expected = [{'task_type': 'snapshot', 'name': 'snap_daily_ts01', 'crontabwindow': '*-*-*-*-*-*', 'enabled': False, 'crontab': '42 3 * * *', 'meta': {'writable': True, 'visible': True, 'prefix': 'snap_daily_ts01', 'share': '3', 'max_count': '4'}}, {'task_type': 'scrub', 'name': 'rockpool_scrub', 'crontabwindow': '*-*-*-*-*-*', 'enabled': False, 'crontab': '42 3 * * 5', 'meta': {'pool_name': 'rock-pool', 'pool': '2'}}, {'task_type': 'scrub', 'name': 'rockpool2_scrub', 'crontabwindow': '*-*-*-*-*-*', 'enabled': False, 'crontab': '42 3 * * 5', 'meta': {'pool_name': 'rock-pool2', 'pool': '4'}}].

----------------------------------------------------------------------
Ran 253 tests in 12.294s

FAILED (failures=2, errors=1)
Destroying test database for alias 'default'...
Destroying test database for alias 'smart_manager'...

[phillxnet]

@FroggyFlox Thanks for seeing to this fix: much appreciated.
I see that in this file we import our subprocess Popen & PIPE from gevent:

from gevent.subprocess import Popen, PIPE

We may have to watch compatibility there - as I'm assuming gevent is a wrapper in this case. I'd not noticed that previously.

But regarding this pull request, all looks dandy and substituting the patched file here resulted in the expected Log Reader fix:

With the downloadable log selector/zipper working as well, as before.

Thanks also for the detailed write-up.

[FroggyFlox]

I see that in this file we import our subprocess Popen & PIPE from gevent:

Oh i didn't notice that... Interesting. I'll be curious to try importing that from subprocess directly and see...