starlingx/distcloud
Code Issues Proposed changes

Update patch set 14

Browse Source 
Patch Set 14:

(21 comments)

Patch-set: 14
Attention: {"person_ident":"Gerrit User 35230 \u003c35230@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_35230\u003e replied on the change"}
This commit is contained in:
Gerrit User 35230 2024-05-06 20:02:38 +00:00 committed by Gerrit Code Review
parent b1582115fa
commit 3fcf9ef4b2
3 changed files with 461 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
0a3c96c766297b7312d6a6fdff9acd8a0b319e87 Normal file
View File

@ -0,0 +1,27 @@
{
"comments": [
{
"unresolved": true,
"key": {
"uuid": "4bd371fe_ce3d801b",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 14
},
"lineNbr": 2382,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 0,
"message": "We should keep the copy(), otherwise the original dictionaries inside subcloud_manager are going to be modified in line 2387, which modifies the \u0027undefined state\u0027 test meaning.",
"range": {
"startLine": 2380,
"startChar": 0,
"endLine": 2382,
"endChar": 82
},
"revId": "0a3c96c766297b7312d6a6fdff9acd8a0b319e87",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
}
]
}

321
362bfbc4f4b1b8493fe1c7f27134b7514d408dc4
View File

@ -176,6 +176,327 @@
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "2ac69ce5_f4fdfbc2",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 637,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "test_get_peer_system_list_with_unavailable_peer",
"range": {
"startLine": 637,
"startChar": 8,
"endLine": 637,
"endChar": 33
},
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "35f34ff9_65131f38",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 662,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "This test needs to verify that the run_deploy_phases function properly executes each phase with the **correct order** successfully (returning True). My suggestion is to mock the subcloud_deploy_\u003c\u003e functions and pass a deploy_phases_to_run list containing all states except one and in a random order, then assert that the subcloud_deploy functions where called in the correct order.\n\nThen have a separate test where one of the inner functions returns false to indicate a failure and then assert that the run_deploy_phases also returns false.",
"range": {
"startLine": 654,
"startChar": 0,
"endLine": 662,
"endChar": 9
},
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "51726334_040d3a57",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 666,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "Please add another test to verify the success case, where the subcloud backup status is set to \u0027validate-failed\u0027.",
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "cb26d4da_c3da6a4d",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 690,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "DEPLOY_STATE_RECONFIGURING_NETWORK",
"range": {
"startLine": 690,
"startChar": 48,
"endLine": 690,
"endChar": 65
},
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "cfa8e139_aa83311e",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 719,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "Suggestion: loop over the payload to do the assertions, you should be able to access the updated_subcloud as if it was a dictionary (so you can use the payload key).",
"range": {
"startLine": 708,
"startChar": 0,
"endLine": 719,
"endChar": 60
},
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "1c1c145f_5156ffbc",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 720,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "Add another assertion to verify that the deploy_status was updated to DEPLOY_STATE_DONE",
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "d44bfe05_2460209b",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 725,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "DEPLOY_STATE_RECONFIGURING_NETWORK",
"range": {
"startLine": 725,
"startChar": 48,
"endLine": 725,
"endChar": 72
},
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "1e59f2b7_e3216858",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 730,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "Also assert that the subcloud deploy_status was updated to DEPLOY_STATE_RECONFIGURING_NETWORK_FAILED",
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "5bffa324_09ef772b",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 741,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "configure_system_controller_network",
"range": {
"startLine": 741,
"startChar": 13,
"endLine": 741,
"endChar": 27
},
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "b20c76f0_b0a3d729",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 744,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "DEPLOY_STATE_RECONFIGURING_NETWORK",
"range": {
"startLine": 744,
"startChar": 48,
"endLine": 744,
"endChar": 65
},
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "1af78281_467b0406",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 761,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "Use the assert_not_called() method.",
"range": {
"startLine": 761,
"startChar": 8,
"endLine": 761,
"endChar": 50
},
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "53241f78_c271cb3f",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 773,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "DEPLOY_STATE_RECONFIGURING_NETWORK",
"range": {
"startLine": 773,
"startChar": 48,
"endLine": 773,
"endChar": 65
},
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "72bf7583_eb9b9b0d",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 784,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "Use the assert_not_called() method.",
"range": {
"startLine": 784,
"startChar": 8,
"endLine": 784,
"endChar": 50
},
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "04743e57_56f827ec",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 787,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "Please add a new assertion to verify that the subcloud deploy_status was updated to DEPLOY_STATE_RECONFIGURING_NETWORK_FAILED",
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "904477d4_b32fcebb",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 11
},
"lineNbr": 794,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "DEPLOY_STATE_RECONFIGURING_NETWORK",
"range": {
"startLine": 794,
"startChar": 48,
"endLine": 794,
"endChar": 65
},
"revId": "362bfbc4f4b1b8493fe1c7f27134b7514d408dc4",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {

113
da6df0e7527e75e5976a82f63d788195d9cc7d88 Normal file
View File

@ -0,0 +1,113 @@
{
"comments": [
{
"unresolved": true,
"key": {
"uuid": "93954c2d_650220b5",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 14
},
"lineNbr": 1341,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "mock_kubeoperator().delete_cert_manager_certificate.assert_called_once()",
"range": {
"startLine": 1341,
"startChar": 8,
"endLine": 1341,
"endChar": 56
},
"revId": "da6df0e7527e75e5976a82f63d788195d9cc7d88",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "264fed38_71e05c57",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 14
},
"lineNbr": 3516,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "I believe this can be safely removed, the self.saved_payload is not passed to the migrate_subcloud and the name and admin_password is not used in any assertions.",
"range": {
"startLine": 3513,
"startChar": 8,
"endLine": 3516,
"endChar": 10
},
"revId": "da6df0e7527e75e5976a82f63d788195d9cc7d88",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "303e977e_994fdf09",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 14
},
"lineNbr": 3660,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "I think it\u0027s also woth asserting that on the following _unmanage_system_peer_subcloud tests that the dc_client.update_subcloud() function is called/not called.",
"revId": "da6df0e7527e75e5976a82f63d788195d9cc7d88",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "afd40210_20abaf81",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 14
},
"lineNbr": 3964,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "This assertion is currently not useful. I think you\u0027d want to assert that the os.rename() was called with the new name.",
"range": {
"startLine": 3964,
"startChar": 8,
"endLine": 3964,
"endChar": 76
},
"revId": "da6df0e7527e75e5976a82f63d788195d9cc7d88",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "7b66a2c2_c025c164",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_subcloud_manager.py",
"patchSetId": 14
},
"lineNbr": 3974,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T20:02:38Z",
"side": 1,
"message": "Suggestion: since new_subcloud_name and the alarm setup is required on both tests, you could move them to the setUp function.",
"range": {
"startLine": 3973,
"startChar": 8,
"endLine": 3974,
"endChar": 67
},
"revId": "da6df0e7527e75e5976a82f63d788195d9cc7d88",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
}
]
}