Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Private/stephenche/cp 47334 #5

Merged
merged 10 commits into from
Mar 19, 2024

Conversation

stephenchengCloud
Copy link
Owner

@stephenchengCloud stephenchengCloud commented Mar 6, 2024

  • Create new directory python3/libexec for nbd_client_manager.py
  • Adjust Makefile and CI check
  • Port nbd_client_manager.py to python3
  • Added unit test
  • Manually tested
    • connect an nbd server:
      image
    • disconnect the nbd server, removed the connect info:
      image
  • XenRT test
    • 3944595: Debian passed
    • 3944596: corosync4hostsHAP1 passed
    • 3944597: bvt-pool.seq passed

xensource.log from 3944597:

Mar 5 07:29:02 conapuk-25-01c xapi: [debug||6394 HTTP 127.0.0.1->:::80|pool.enable_ha R:ae4b797f50b7|helpers] about to call script without a timeout: /opt/xensource/bin/static-vdis attach b8d9978f-62a9-4919-ae6b-dbef1c5962e4
Mar 5 07:29:02 conapuk-25-01c xapi: [debug||6394 HTTP 127.0.0.1->:::80|pool.enable_ha R:ae4b797f50b7|helpers] /opt/xensource/bin/static-vdis attach b8d9978f-62a9-4919-ae6b-dbef1c5962e4 succeeded [ output = '/etc/xensource/static-vdis/1/disk\x0A' ]
...
Mar 5 07:37:20 conapuk-25-01c xapi: [ info||7003 /var/lib/xcp/xapi|host.ha_release_resources R:5ea616b86a5f|static_vdis] permanent_vdi_detach: vdi-uuid = 3b9846a7-63b2-48e4-b021-efc9528464f8
Mar 5 07:37:20 conapuk-25-01c xapi: [debug||7003 /var/lib/xcp/xapi|host.ha_release_resources R:5ea616b86a5f|helpers] about to call script without a timeout: /opt/xensource/bin/static-vdis detach 3b9846a7-63b2-48e4-b021-efc9528464f8
Mar 5 07:37:20 conapuk-25-01c xapi: [debug||7003 /var/lib/xcp/xapi|host.ha_release_resources R:5ea616b86a5f|helpers] /opt/xensource/bin/static-vdis detach 3b9846a7-63b2-48e4-b021-efc9528464f8 succeeded [ output = '' ]

call chain:
https://code.citrite.net/projects/XSU/repos/xen-api/browse/scripts/static-vdis#279
attach()->connect_nbd()->nbd_client_manager.connect_nbd()
https://code.citrite.net/projects/XSU/repos/xen-api/browse/scripts/static-vdis#328
detach()->disconnect_nbd_device()->nbd_client_manager.disconnect_nbd_device()

Copy link

github-actions bot commented Mar 6, 2024

pytype_reporter extracted 37 problem reports from pytype output

.

You can check the results of the job here

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

pylint

python3/unittest/test_nbd_client_manager.py|56 col 17| W0212: Access to a protected member _is_nbd_device_connected of a client class (protected-access)
python3/unittest/test_nbd_client_manager.py|61 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|66 col 12| W0212: Access to a protected member _is_nbd_device_connected of a client class (protected-access)
python3/unittest/test_nbd_client_manager.py|61 col 40| W0613: Unused argument 'mock_call' (unused-argument)
python3/unittest/test_nbd_client_manager.py|69| C0115: Missing class docstring (missing-class-docstring)
python3/unittest/test_nbd_client_manager.py|70 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|75 col 24| W0212: Access to a protected member _find_unused_nbd_device of a client class (protected-access)
python3/unittest/test_nbd_client_manager.py|80 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|86 col 12| W0212: Access to a protected member _find_unused_nbd_device of a client class (protected-access)
python3/unittest/test_nbd_client_manager.py|89| C0115: Missing class docstring (missing-class-docstring)
python3/unittest/test_nbd_client_manager.py|90 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|92 col 8| W0212: Access to a protected member _wait_for_nbd_device of a client class (protected-access)
python3/unittest/test_nbd_client_manager.py|95 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|97 col 8| W0212: Access to a protected member _wait_for_nbd_device of a client class (protected-access)
python3/unittest/test_nbd_client_manager.py|100| C0115: Missing class docstring (missing-class-docstring)
python3/unittest/test_nbd_client_manager.py|101 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|105 col 25| W0212: Access to a protected member _get_persistent_connect_info_filename of a client class (protected-access)
python3/unittest/test_nbd_client_manager.py|109| C0115: Missing class docstring (missing-class-docstring)
python3/unittest/test_nbd_client_manager.py|111 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|123 col 12| W0212: Access to a protected member _persist_connect_info of a client class (protected-access)
python3/unittest/test_nbd_client_manager.py|130 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|142 col 12| W0212: Access to a protected member _persist_connect_info of a client class (protected-access)
python3/unittest/test_nbd_client_manager.py|149| C0115: Missing class docstring (missing-class-docstring)
python3/unittest/test_nbd_client_manager.py|151 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|152 col 8| W0212: Access to a protected member _remove_persistent_connect_info of a client class (protected-access)
python3/unittest/test_nbd_client_manager.py|155| C0115: Missing class docstring (missing-class-docstring)
python3/unittest/test_nbd_client_manager.py|162 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|162 col 4| R0913: Too many arguments (6/5) (too-many-arguments)
python3/unittest/test_nbd_client_manager.py|162 col 31| W0621: Redefining name 'mock_open' from outer scope (line 9) (redefined-outer-name)
python3/unittest/test_nbd_client_manager.py|191| C0115: Missing class docstring (missing-class-docstring)
python3/unittest/test_nbd_client_manager.py|193 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|206 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|219 col 4| C0116: Missing function or method docstring (missing-function-docstring)

fd.write("none")
# Set the NBD queue size to the same as the qcow2 cluster size
with open("/sys/block/" + nbd + "/queue/max_sectors_kb", "w") as fd:
with open("/sys/block/" + nbd + "/queue/max_sectors_kb", "w", encoding="utf-8") as fd:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
C0301: Line too long (102/100) (line-too-long)

def __init__(self, nbd_device):
super(NbdDeviceNotFound, self).__init__(
"NBD device '{}' does not exist".format(nbd_device))
"NBD device '{}' does not exist".format(nbd_device)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
C0209: Formatting a regular string which could be an f-string (consider-using-f-string)

def __init__(self, path):
self._path = path
self._lock_file = None

def _lock(self):
"""Acquire the lock"""
flags = fcntl.LOCK_EX
self._lock_file = open(self._path, 'w+')
self._lock_file = open(self._path, "w+")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
W1514: Using open without explicitly specifying an encoding (unspecified-encoding)

def __init__(self, path):
self._path = path
self._lock_file = None

def _lock(self):
"""Acquire the lock"""
flags = fcntl.LOCK_EX
self._lock_file = open(self._path, 'w+')
self._lock_file = open(self._path, "w+")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
R1732: Consider using 'with' for resource-allocating operations (consider-using-with)

fd.write("8")

return nbd_device
except NbdDeviceNotFound as exn:
LOGGER.warn('Failed to find free nbd device: %s', exn)
LOGGER.warn("Failed to find free nbd device: %s", exn)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
W4902: Using deprecated method warn() (deprecated-method)

mock_process.returncode = 1

with self.assertRaises(subprocess.CalledProcessError) as cm:
nbd_client_manager._call(["invalid_cmd"])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
W0212: Access to a protected member _call of a client class (protected-access)


@patch('nbd_client_manager.os.path.exists')
@patch('nbd_client_manager._call')
class TestIsNbdDeviceConnected(unittest.TestCase):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
C0115: Missing class docstring (missing-class-docstring)

@patch('nbd_client_manager._call')
class TestIsNbdDeviceConnected(unittest.TestCase):

def test_nbd_device_connected(self, mock_call, mock_exists):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
C0116: Missing function or method docstring (missing-function-docstring)

mock_exists.return_value = True
mock_call.return_value = 0

result = nbd_client_manager._is_nbd_device_connected('/dev/nbd0')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
W0212: Access to a protected member _is_nbd_device_connected of a client class (protected-access)

self.assertTrue(result)
mock_call.assert_called_once_with(["nbd-client", "-check", "/dev/nbd0"], error=False)

def test_nbd_device_not_connected(self, mock_call, mock_exists):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
C0116: Missing function or method docstring (missing-function-docstring)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

pylint

python3/unittest/test_nbd_client_manager.py|191| C0115: Missing class docstring (missing-class-docstring)
python3/unittest/test_nbd_client_manager.py|193 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|206 col 4| C0116: Missing function or method docstring (missing-function-docstring)
python3/unittest/test_nbd_client_manager.py|219 col 4| C0116: Missing function or method docstring (missing-function-docstring)

return PERSISTENT_INFO_DIR + '/' + number
matched = re.search("/dev/nbd([0-9]+)", device)
if not matched:
raise Exception(f"Can not get the nbd number for device: {device}")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
W0719: Raising too general exception: Exception (broad-exception-raised)

mock_exists.return_value = True
mock_call.return_value = 1

result = nbd_client_manager._is_nbd_device_connected('/dev/nbd1')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
W0212: Access to a protected member _is_nbd_device_connected of a client class (protected-access)

self.assertFalse(result)
mock_call.assert_called_once_with(["nbd-client", "-check", "/dev/nbd1"], error=False)

def test_nbd_device_not_found(self, mock_call, mock_exists):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
C0116: Missing function or method docstring (missing-function-docstring)


# Testing the function with a non-existent device
with self.assertRaises(nbd_client_manager.NbdDeviceNotFound):
nbd_client_manager._is_nbd_device_connected('/dev/nbd2')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
W0212: Access to a protected member _is_nbd_device_connected of a client class (protected-access)

self.assertFalse(result)
mock_call.assert_called_once_with(["nbd-client", "-check", "/dev/nbd1"], error=False)

def test_nbd_device_not_found(self, mock_call, mock_exists):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
W0613: Unused argument 'mock_call' (unused-argument)

class TestRemovePersistentConnectInfo(unittest.TestCase):
@patch('nbd_client_manager.os.remove')
def test_remove_persistent_connect_info(self, mock_os_remove):
nbd_client_manager._remove_persistent_connect_info('/dev/nbd0')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
W0212: Access to a protected member _remove_persistent_connect_info of a client class (protected-access)

nbd_client_manager._remove_persistent_connect_info('/dev/nbd0')
mock_os_remove.assert_called_once_with('/var/run/nonpersistent/nbd/0')

class TestConnectNbd(unittest.TestCase):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
C0115: Missing class docstring (missing-class-docstring)

@patch('nbd_client_manager._persist_connect_info')
@patch('nbd_client_manager.open')
@patch('nbd_client_manager.FILE_LOCK', MagicMock()) # Mocking FILE_LOCK
def test_connect_nbd(self, mock_open, mock_persist_info, mock_wait_for_nbd, mock_find_unused, mock_call):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
C0116: Missing function or method docstring (missing-function-docstring)

@patch('nbd_client_manager._persist_connect_info')
@patch('nbd_client_manager.open')
@patch('nbd_client_manager.FILE_LOCK', MagicMock()) # Mocking FILE_LOCK
def test_connect_nbd(self, mock_open, mock_persist_info, mock_wait_for_nbd, mock_find_unused, mock_call):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
R0913: Too many arguments (6/5) (too-many-arguments)

@patch('nbd_client_manager._persist_connect_info')
@patch('nbd_client_manager.open')
@patch('nbd_client_manager.FILE_LOCK', MagicMock()) # Mocking FILE_LOCK
def test_connect_nbd(self, mock_open, mock_persist_info, mock_wait_for_nbd, mock_find_unused, mock_call):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
W0621: Redefining name 'mock_open' from outer scope (line 9) (redefined-outer-name)

@stephenchengCloud stephenchengCloud force-pushed the private/stephenche/CP-47334 branch 8 times, most recently from 138fcf1 to aee9e60 Compare March 8, 2024 01:23
@patch('nbd_client_manager._remove_persistent_connect_info')
@patch('nbd_client_manager._call')
@patch('nbd_client_manager._wait_for_nbd_device')
class TestDisconnectNbdDevice(unittest.TestCase):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
C0115: Missing class docstring (missing-class-docstring)

@stephenchengCloud stephenchengCloud force-pushed the private/stephenche/CP-47334 branch from 8cfb6ab to 032d67e Compare March 12, 2024 06:32
@stephenchengCloud stephenchengCloud force-pushed the private/stephenche/CP-47334 branch from 032d67e to f1a7292 Compare March 14, 2024 09:51
During building, our build system uses both python2 and python3 to
compile against .py files and f-string won't be accepted
by py2 compiling.

Signed-off-by: Stephen Cheng <[email protected]>
@stephenchengCloud stephenchengCloud force-pushed the private/stephenche/CP-47334 branch from f1a7292 to 7719ef8 Compare March 15, 2024 02:28
@stephenchengCloud stephenchengCloud merged commit 1a2730b into feature/py3 Mar 19, 2024
21 checks passed
@stephenchengCloud stephenchengCloud deleted the private/stephenche/CP-47334 branch July 31, 2024 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant