)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"098bce05878286f1312f2d9815db806bb71ac371","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b5911b21_c82f757f","updated":"2025-07-08 09:31:05.000000000","message":"recheck","commit_id":"8da3c786028db3999ec0e7629ff30f0ff40c22fa"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"887ddc099b33541fe5a17ae3d529ec8699c6d07f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"595e0bb6_b5315387","updated":"2025-08-06 21:21:10.000000000","message":"++ tests looks good, and increase the coverage for all scenarios in the new feature.\n-1: i think that we should double check the action and see if is really a cold migration instead of a live one, otherwise the test isn\u0027t really validating the code, wdyt?\nOther comments are more nits, thanks for proposing this patch!","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0f93c19dd5213442b3f9387fc7f7316f7bca2eee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"bd101076_da0698f4","updated":"2025-07-21 18:46:59.000000000","message":"+1 mainly because i have not had time to review this in detail.\n\nthe added test cases all sound valid.\nill try and review this with the actual feature over the next week.","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"52aaf36d4284ed545eee36fc0498d265d9f33857","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"58d654f8_a52f63cd","updated":"2025-08-21 13:55:39.000000000","message":"-1: since there is no api microversion change, we need to make sure that theses don\u0027t run on stable branches. This plugin repo is branchless, once it merges, it will run the tests in stable branches and break the CI.\nWe might need to add a configuration option to enable the run of them and set the default to FAlse (never run) and move to True only in master branch.","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"df88309230fc632ef4685b56d5865a01d898b320","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b023996a_a7492a0e","updated":"2025-07-10 00:40:00.000000000","message":"Thanks for the feedback! I\u0027ve made a small correction and add the test.","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"6ea772a7d3adcec85a48c2607d1d1513496be0f2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7235cd54_36dab46e","updated":"2025-07-14 10:33:03.000000000","message":"from my limited tempest knowledge, this lgtm, the tests are passing in https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_85c/openstack/85c7a7bcc52a4ababba6e1f27aecf5e4/testr_results.html:\n```\nwatcher_tempest_plugin.tests.scenario.test_execute_host_maintenance.TestExecuteHostMaintenanceStrategy \t7 \t7 \t0 \t0 \t0 \tDetail \t\ntest_exec_host_maintenance_disable_cold_inactive_instances[host_maintenance,id-31b8523e-2f5b-4002-a3cf-678fbc215f0b,strategy] pass\ntest_exec_host_maintenance_disable_migration_inactive_instances[host_maintenance,id-f5e6d7c8-1234-5678-9012-ab34cd56ef78,strategy] pass\ntest_execute_host_maintenance_disable_both_migrations[host_maintenance,id-863b16b5-3c74-413c-98a8-d9ce5deb539c,strategy]\tpass\ntest_execute_host_maintenance_disable_cold_migration[host_maintenance,id-053025b9-acdf-4bf5-b3c3-a132396a2de4,strategy]\tpass\ntest_execute_host_maintenance_disable_live_migration[host_maintenance,id-a1b2c3d4-e5f6-7890-ab12-cd34ef567890,strategy]   pass\ntest_execute_host_maintenance_strategy[host_maintenance,id-17afd352-1929-46dd-a10a-63c90bb9255d,strategy]\tpass\ntest_execute_host_maintenance_strategy_backup_node[host_maintenance,id-cc5a0f1b-e8d2-4813-b012-874982d15d06,strategy]\tpass\n```","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"21904351321f517d65353b4a8e26a0094b6df4f8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6558986c_0135768f","updated":"2025-09-03 16:08:27.000000000","message":"hi, do you plan to update this change?","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"d421edb41d85f0c55b8dd804ecfcfab7f18c15ba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"004229f9_034fa748","updated":"2025-07-10 04:39:24.000000000","message":"recheck","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"84509463447718d67d878620a67f178cd7eeec19","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b02c07f4_2362039f","updated":"2025-07-13 13:19:40.000000000","message":"recheck","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"5b93a410da1db01e1e7b9ea5df0d9c9bb406432f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d40eb1f4_537b5b82","updated":"2025-08-20 16:45:37.000000000","message":"recheck\n\nmain patch was udpated","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"5f6e3e0491c4d73be83aac3bf4cab7f649c21f54","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ff0e8f27_a9d6c42f","updated":"2025-07-18 06:13:20.000000000","message":"recheck after skipping tempest tests https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/955302","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"0c791456cd6d626acc988481984fe40d948f9fb7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a603ad8b_7219ffcb","in_reply_to":"58d654f8_a52f63cd","updated":"2025-09-04 12:44:46.000000000","message":"Thanks for this reminder! I didn\u0027t notice this before. \nSo here, I added a new configuration option, and used that to skip the new tests in the host maintenance strategy. Currently I don\u0027t see a similar pattern to skip specific tests in a test class, so I\u0027m not quite sure whether my implementation here is a proper way to do it or not. Wish to learn more about this @@","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"0c791456cd6d626acc988481984fe40d948f9fb7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b910584d_4b2800eb","updated":"2025-09-04 12:44:46.000000000","message":"Thank you for the reviews, and so sorry for this late comeback :((\n\nI made several changes, but probably the CI will not run on the added tests, because I also added a configuration option to block them from running by default (if I do it correctly and it works). I\u0027m unsure how to properly setup the CI job configuration to enable it, so I just submit this patch as-is, and probably need some guidance to set up a job for the CI to run the new tests.\n\nBut I hope the current implementation can be reviewed still. Thanks!","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"735ad228c377fece3fc3c83821424f61cbb910a8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9de3e4f5_08dc4bb1","updated":"2025-09-05 01:35:47.000000000","message":"recheck","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"09913bc2d4b807cd0074abb8a3ea18b99b507502","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e3f746e0_4d5c9958","in_reply_to":"b910584d_4b2800eb","updated":"2025-09-05 18:45:44.000000000","message":"Thanks for updating the patch! I would wait a bit more to get more reviews in the current patch set. And get more ideas. I may refactor the `execute_strategy` method too, since I also want to reuse part of it, but today is not possible.\nIf you want to test this change, you will need to update .zuul.yaml file in watcher repo, like this: https://review.opendev.org/c/openstack/watcher/+/958678/3/.zuul.yaml#84\nto enable your new config option.\nAfter that, you can update this tempest change, adding the Depends-On on watcher change. So every new update here, will be tested.\nWe will be able to merge afterwards in the order: watcher .zuul.yaml change -\u003e watcher-tempest-plugin change.\nIn case we merge after the creation of 2025.2 branch, then we backport the .zuul.yaml change too, np.","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"5a4bdb7629aca207e050dd0f664a0cebec8ecf91","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"463c3e86_b4dc0c34","in_reply_to":"e3f746e0_4d5c9958","updated":"2025-09-09 05:44:24.000000000","message":"Thanks for your detailed instruction!","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"dba52bbe7d917e80402e71ac8b3aab76febf0751","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"8bb3a721_13787ced","updated":"2025-09-12 01:20:04.000000000","message":"I have updated the new test config and reformat the host maintenance tests accordingly. Now the execute_strategy will return the audit uuid. \n \nThere is a patch to Watcher repo to enable the new tests for this patch: https://review.opendev.org/c/openstack/watcher/+/960698\n\nThanks!","commit_id":"2fe2a90da460b1b489de42d798a4a035fc22b716"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"2a6e9a88230e7635e9fe5d0ff54ea0cfe2fe00a7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e0ac739c_0d83420b","updated":"2025-09-12 10:41:44.000000000","message":"Some of the new tests failed (`test_exec_host_maintenance_disable_cold_inactive_instances` and `test_exec_host_maintenance_disable_migration_inactive_instances`). \n\nLooking back to the logic, my best guess is there are existing active instance on the host we are testing to, so there is an unexpected `migrate` action (for `test_exec_host_maintenance_disable_cold_inactive_instances`) and unexpected `stop` action (for `test_exec_host_maintenance_disable_migration_inactive_instances`). If that\u0027s the case then the test needs to either make sure the host it created the instance to has that only instance, or it needs to go through and stop/pause all of its instances first.\n\nWill very appreciate if I can have any piece of advice on regarding this.","commit_id":"2fe2a90da460b1b489de42d798a4a035fc22b716"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"f499a301dffae7fb48fadab3dd90cf2981891773","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"4cfd6a43_7cd21607","updated":"2025-09-12 08:20:33.000000000","message":"recheck","commit_id":"2fe2a90da460b1b489de42d798a4a035fc22b716"}],"watcher_tempest_plugin/config.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"09913bc2d4b807cd0074abb8a3ea18b99b507502","unresolved":true,"context_lines":[{"line_number":128,"context_line":"             \"The format is \u0027X.Y\u0027, where \u0027X\u0027 and \u0027Y\u0027 are int values.\""},{"line_number":129,"context_line":"    ),"},{"line_number":130,"context_line":"    cfg.BoolOpt("},{"line_number":131,"context_line":"        \"disable_migration_supported\","},{"line_number":132,"context_line":"        default\u003dFalse,"},{"line_number":133,"context_line":"        help\u003d\"Whether the Watcher service supports disable_live_migration \""},{"line_number":134,"context_line":"             \"and disable_cold_migration parameters in host_maintenance \""}],"source_content_type":"text/x-python","patch_set":3,"id":"8a784bbf_934c3a4a","line":131,"range":{"start_line":131,"start_character":9,"end_line":131,"end_character":36},"updated":"2025-09-05 18:45:44.000000000","message":"I would use:\n`run_host_maintenance_disable_migration_tests`\nand defauls to False.\nI actually proposing this pattern (run_{feature}_tests), which I am also using here:\nhttps://review.opendev.org/c/openstack/watcher-tempest-plugin/+/956116/6/watcher_tempest_plugin/config.py\nwhich doesn\u0027t mean that we already agree on something :)","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"5a4bdb7629aca207e050dd0f664a0cebec8ecf91","unresolved":false,"context_lines":[{"line_number":128,"context_line":"             \"The format is \u0027X.Y\u0027, where \u0027X\u0027 and \u0027Y\u0027 are int values.\""},{"line_number":129,"context_line":"    ),"},{"line_number":130,"context_line":"    cfg.BoolOpt("},{"line_number":131,"context_line":"        \"disable_migration_supported\","},{"line_number":132,"context_line":"        default\u003dFalse,"},{"line_number":133,"context_line":"        help\u003d\"Whether the Watcher service supports disable_live_migration \""},{"line_number":134,"context_line":"             \"and disable_cold_migration parameters in host_maintenance \""}],"source_content_type":"text/x-python","patch_set":3,"id":"169226ef_2c952dd7","line":131,"range":{"start_line":131,"start_character":9,"end_line":131,"end_character":36},"in_reply_to":"8a784bbf_934c3a4a","updated":"2025-09-09 05:44:24.000000000","message":"Acknowledged","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"09913bc2d4b807cd0074abb8a3ea18b99b507502","unresolved":true,"context_lines":[{"line_number":132,"context_line":"        default\u003dFalse,"},{"line_number":133,"context_line":"        help\u003d\"Whether the Watcher service supports disable_live_migration \""},{"line_number":134,"context_line":"             \"and disable_cold_migration parameters in host_maintenance \""},{"line_number":135,"context_line":"             \"strategy. Should be set to True only when testing against \""},{"line_number":136,"context_line":"             \"a Watcher deployment that includes these parameters. \""},{"line_number":137,"context_line":"             \"Defaults to False for backward compatibility with stable \""},{"line_number":138,"context_line":"             \"branches.\""}],"source_content_type":"text/x-python","patch_set":3,"id":"1bc69534_83292d35","line":135,"range":{"start_line":135,"start_character":22,"end_line":135,"end_character":23},"updated":"2025-09-05 18:45:44.000000000","message":"I think that we can mention (2025.2 release) here. Since it merged already.","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"5a4bdb7629aca207e050dd0f664a0cebec8ecf91","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        default\u003dFalse,"},{"line_number":133,"context_line":"        help\u003d\"Whether the Watcher service supports disable_live_migration \""},{"line_number":134,"context_line":"             \"and disable_cold_migration parameters in host_maintenance \""},{"line_number":135,"context_line":"             \"strategy. Should be set to True only when testing against \""},{"line_number":136,"context_line":"             \"a Watcher deployment that includes these parameters. \""},{"line_number":137,"context_line":"             \"Defaults to False for backward compatibility with stable \""},{"line_number":138,"context_line":"             \"branches.\""}],"source_content_type":"text/x-python","patch_set":3,"id":"b23495ee_974b3ed3","line":135,"range":{"start_line":135,"start_character":22,"end_line":135,"end_character":23},"in_reply_to":"1bc69534_83292d35","updated":"2025-09-09 05:44:24.000000000","message":"Acknowledged","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"09913bc2d4b807cd0074abb8a3ea18b99b507502","unresolved":true,"context_lines":[{"line_number":134,"context_line":"             \"and disable_cold_migration parameters in host_maintenance \""},{"line_number":135,"context_line":"             \"strategy. Should be set to True only when testing against \""},{"line_number":136,"context_line":"             \"a Watcher deployment that includes these parameters. \""},{"line_number":137,"context_line":"             \"Defaults to False for backward compatibility with stable \""},{"line_number":138,"context_line":"             \"branches.\""},{"line_number":139,"context_line":"    ),"},{"line_number":140,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":3,"id":"dfef6481_c2d2fe8e","line":138,"range":{"start_line":137,"start_character":63,"end_line":138,"end_character":21},"updated":"2025-09-05 18:45:44.000000000","message":"previous releases.","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"5a4bdb7629aca207e050dd0f664a0cebec8ecf91","unresolved":false,"context_lines":[{"line_number":134,"context_line":"             \"and disable_cold_migration parameters in host_maintenance \""},{"line_number":135,"context_line":"             \"strategy. Should be set to True only when testing against \""},{"line_number":136,"context_line":"             \"a Watcher deployment that includes these parameters. \""},{"line_number":137,"context_line":"             \"Defaults to False for backward compatibility with stable \""},{"line_number":138,"context_line":"             \"branches.\""},{"line_number":139,"context_line":"    ),"},{"line_number":140,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":3,"id":"96577a45_56fdc8c1","line":138,"range":{"start_line":137,"start_character":63,"end_line":138,"end_character":21},"in_reply_to":"dfef6481_c2d2fe8e","updated":"2025-09-09 05:44:24.000000000","message":"Acknowledged","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"}],"watcher_tempest_plugin/tests/scenario/test_execute_host_maintenance.py":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"89eb856960bf97c23723d9328d40fa1a02690546","unresolved":true,"context_lines":[{"line_number":149,"context_line":"                self.assertNotEqual("},{"line_number":150,"context_line":"                    initial_hosts[instance[\u0027id\u0027]], new_host)"},{"line_number":151,"context_line":"                # Verify instance is active after cold migration"},{"line_number":152,"context_line":"                server \u003d self.mgr.servers_client.show_server("},{"line_number":153,"context_line":"                    instance[\u0027id\u0027])[\u0027server\u0027]"},{"line_number":154,"context_line":"                self.assertEqual(\u0027ACTIVE\u0027, server[\u0027status\u0027])"},{"line_number":155,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"360bfdd1_36d66efd","line":152,"updated":"2025-07-09 10:02:31.000000000","message":"this is great, ideally we would also check the `migrate` action generated is a cold migration, but that is not exposed to the test currently by the `execute_strategy` method. That is a known issue, so I wouldn\u0027t block this patch becaus of it","commit_id":"8da3c786028db3999ec0e7629ff30f0ff40c22fa"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"df88309230fc632ef4685b56d5865a01d898b320","unresolved":false,"context_lines":[{"line_number":149,"context_line":"                self.assertNotEqual("},{"line_number":150,"context_line":"                    initial_hosts[instance[\u0027id\u0027]], new_host)"},{"line_number":151,"context_line":"                # Verify instance is active after cold migration"},{"line_number":152,"context_line":"                server \u003d self.mgr.servers_client.show_server("},{"line_number":153,"context_line":"                    instance[\u0027id\u0027])[\u0027server\u0027]"},{"line_number":154,"context_line":"                self.assertEqual(\u0027ACTIVE\u0027, server[\u0027status\u0027])"},{"line_number":155,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"e7987448_02bef6cd","line":152,"in_reply_to":"360bfdd1_36d66efd","updated":"2025-07-10 00:40:00.000000000","message":"Acknowledged","commit_id":"8da3c786028db3999ec0e7629ff30f0ff40c22fa"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"89eb856960bf97c23723d9328d40fa1a02690546","unresolved":true,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    @decorators.idempotent_id(\u0027053025b9-acdf-4bf5-b3c3-a132396a2de4\u0027)"},{"line_number":157,"context_line":"    @decorators.attr(type\u003d[\u0027strategy\u0027, \u0027host_maintenance\u0027])"},{"line_number":158,"context_line":"    def test_execute_host_maintenance_disable_cold_migration(self):"},{"line_number":159,"context_line":"        # This test verifies that when cold migration is disabled,"},{"line_number":160,"context_line":"        # active instances are live migrated (cold migration disabled"},{"line_number":161,"context_line":"        # doesn\u0027t affect active instances)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ab284dfb_9f55b88a","line":158,"updated":"2025-07-09 10:02:31.000000000","message":"I think we should also add a test case to cover the case when cold migration is disabled but there are inactive instances","commit_id":"8da3c786028db3999ec0e7629ff30f0ff40c22fa"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"df88309230fc632ef4685b56d5865a01d898b320","unresolved":false,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    @decorators.idempotent_id(\u0027053025b9-acdf-4bf5-b3c3-a132396a2de4\u0027)"},{"line_number":157,"context_line":"    @decorators.attr(type\u003d[\u0027strategy\u0027, \u0027host_maintenance\u0027])"},{"line_number":158,"context_line":"    def test_execute_host_maintenance_disable_cold_migration(self):"},{"line_number":159,"context_line":"        # This test verifies that when cold migration is disabled,"},{"line_number":160,"context_line":"        # active instances are live migrated (cold migration disabled"},{"line_number":161,"context_line":"        # doesn\u0027t affect active instances)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3735feb2_cc03bf59","line":158,"in_reply_to":"ab284dfb_9f55b88a","updated":"2025-07-10 00:40:00.000000000","message":"Done","commit_id":"8da3c786028db3999ec0e7629ff30f0ff40c22fa"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"887ddc099b33541fe5a17ae3d529ec8699c6d07f","unresolved":true,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        self.addCleanup(self.rollback_compute_nodes_status)"},{"line_number":120,"context_line":"        self.addCleanup(self.wait_delete_instances_from_model)"},{"line_number":121,"context_line":"        instances \u003d self._create_one_instance_per_host()"},{"line_number":122,"context_line":"        # wait for compute model updates"},{"line_number":123,"context_line":"        self.wait_for_instances_in_model(instances)"},{"line_number":124,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7850cf82_d07a3485","line":121,"range":{"start_line":121,"start_character":25,"end_line":121,"end_character":54},"updated":"2025-08-06 21:21:10.000000000","message":"nit: in the end, you could just create one instance in a host, and work with that host as the maintenance_node.","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"0c791456cd6d626acc988481984fe40d948f9fb7","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        self.addCleanup(self.rollback_compute_nodes_status)"},{"line_number":120,"context_line":"        self.addCleanup(self.wait_delete_instances_from_model)"},{"line_number":121,"context_line":"        instances \u003d self._create_one_instance_per_host()"},{"line_number":122,"context_line":"        # wait for compute model updates"},{"line_number":123,"context_line":"        self.wait_for_instances_in_model(instances)"},{"line_number":124,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"00a436d1_f9fcdb24","line":121,"range":{"start_line":121,"start_character":25,"end_line":121,"end_character":54},"in_reply_to":"7850cf82_d07a3485","updated":"2025-09-04 12:44:46.000000000","message":"Done","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"887ddc099b33541fe5a17ae3d529ec8699c6d07f","unresolved":true,"context_lines":[{"line_number":142,"context_line":"                                                \u0027migrate\u0027],"},{"line_number":143,"context_line":"                              **audit_kwargs)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # Verify instances are (cold) migrated to different nodes"},{"line_number":146,"context_line":"        for instance in instances:"},{"line_number":147,"context_line":"            if initial_hosts[instance[\u0027id\u0027]] \u003d\u003d src_node:"},{"line_number":148,"context_line":"                new_host \u003d self.get_host_for_server(instance[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"06562b04_d4ebc75b","line":145,"range":{"start_line":145,"start_character":2,"end_line":145,"end_character":65},"updated":"2025-08-06 21:21:10.000000000","message":"you can\u0027t tell that was a cold migrate instead of a live migrate, just checking that it migrated. Is it possible to check that info in the action details?","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"a14ff1100daa10c3b598e464e1665ba43dd04d70","unresolved":true,"context_lines":[{"line_number":142,"context_line":"                                                \u0027migrate\u0027],"},{"line_number":143,"context_line":"                              **audit_kwargs)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # Verify instances are (cold) migrated to different nodes"},{"line_number":146,"context_line":"        for instance in instances:"},{"line_number":147,"context_line":"            if initial_hosts[instance[\u0027id\u0027]] \u003d\u003d src_node:"},{"line_number":148,"context_line":"                new_host \u003d self.get_host_for_server(instance[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"7bcff7bd_6d819478","line":145,"range":{"start_line":145,"start_character":2,"end_line":145,"end_character":65},"in_reply_to":"05de20c5_1fa57049","updated":"2025-09-09 15:10:58.000000000","message":"That\u0027s also a good thing, then I can start prepare the next patchset that way. Thanks!","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"0c791456cd6d626acc988481984fe40d948f9fb7","unresolved":true,"context_lines":[{"line_number":142,"context_line":"                                                \u0027migrate\u0027],"},{"line_number":143,"context_line":"                              **audit_kwargs)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # Verify instances are (cold) migrated to different nodes"},{"line_number":146,"context_line":"        for instance in instances:"},{"line_number":147,"context_line":"            if initial_hosts[instance[\u0027id\u0027]] \u003d\u003d src_node:"},{"line_number":148,"context_line":"                new_host \u003d self.get_host_for_server(instance[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"b1a26983_7c9ce8f4","line":145,"range":{"start_line":145,"start_character":2,"end_line":145,"end_character":65},"in_reply_to":"06562b04_d4ebc75b","updated":"2025-09-04 12:44:46.000000000","message":"Thanks for the concerns. Currently I don\u0027t see a possible way to do this, because the `execute_strategy` method has no return. \nTherefore in patch 3, I\u0027m trying to do it by checking the action from the audit list (just simply take the last audit from the list and check the action). Tbh, I\u0027m not sure it will work correctly, and this is kinda like an uncertain way to do it...","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"83bccc3073d7904ef2c5bfcdf4e36d08920485f0","unresolved":true,"context_lines":[{"line_number":142,"context_line":"                                                \u0027migrate\u0027],"},{"line_number":143,"context_line":"                              **audit_kwargs)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # Verify instances are (cold) migrated to different nodes"},{"line_number":146,"context_line":"        for instance in instances:"},{"line_number":147,"context_line":"            if initial_hosts[instance[\u0027id\u0027]] \u003d\u003d src_node:"},{"line_number":148,"context_line":"                new_host \u003d self.get_host_for_server(instance[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"05de20c5_1fa57049","line":145,"range":{"start_line":145,"start_character":2,"end_line":145,"end_character":65},"in_reply_to":"71635a21_9c8d89c6","updated":"2025-09-09 08:35:16.000000000","message":"imo I think we should include a return in `execute_strategy` in this patch so we can add more checks in the test, and later refactor the method in another patch","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"79af874e70d7c034a7d137285af58004b2890202","unresolved":true,"context_lines":[{"line_number":142,"context_line":"                                                \u0027migrate\u0027],"},{"line_number":143,"context_line":"                              **audit_kwargs)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # Verify instances are (cold) migrated to different nodes"},{"line_number":146,"context_line":"        for instance in instances:"},{"line_number":147,"context_line":"            if initial_hosts[instance[\u0027id\u0027]] \u003d\u003d src_node:"},{"line_number":148,"context_line":"                new_host \u003d self.get_host_for_server(instance[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"dcfb732f_9a07f635","line":145,"range":{"start_line":145,"start_character":2,"end_line":145,"end_character":65},"in_reply_to":"b1a26983_7c9ce8f4","updated":"2025-09-05 09:57:52.000000000","message":"I think PS3 will work, but I don\u0027t like too much relying on the fact that the audit from the test will be the last executed. I think it would be better to return the action plan and audit from the `execute_strategy` method and then use that for checks. It might be considered outside the scope of this patch to change that method, but it would not break anything and it will be useful for other tests in the future. Overall I feel like we should decouple the login for executing the audit and the checks like https://github.com/openstack/watcher-tempest-plugin/blob/505bbdbc86690f89ed421288ef4bf5762004a305/watcher_tempest_plugin/tests/scenario/base.py#L995-L999 in that method, but that should definitely be done in a different patch.","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"09913bc2d4b807cd0074abb8a3ea18b99b507502","unresolved":true,"context_lines":[{"line_number":142,"context_line":"                                                \u0027migrate\u0027],"},{"line_number":143,"context_line":"                              **audit_kwargs)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # Verify instances are (cold) migrated to different nodes"},{"line_number":146,"context_line":"        for instance in instances:"},{"line_number":147,"context_line":"            if initial_hosts[instance[\u0027id\u0027]] \u003d\u003d src_node:"},{"line_number":148,"context_line":"                new_host \u003d self.get_host_for_server(instance[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"eda332f6_e31c7ae8","line":145,"range":{"start_line":145,"start_character":2,"end_line":145,"end_character":65},"in_reply_to":"dcfb732f_9a07f635","updated":"2025-09-05 18:45:44.000000000","message":"Agree that we need to refactor `execute_strategy`. It does too many things there, and it doesn\u0027t return any useful info. By breaking it into smaller function we can have a better reuse of it. But yeah, checking the most recent audit is not reliable imo as long term solution.","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"5a4bdb7629aca207e050dd0f664a0cebec8ecf91","unresolved":true,"context_lines":[{"line_number":142,"context_line":"                                                \u0027migrate\u0027],"},{"line_number":143,"context_line":"                              **audit_kwargs)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # Verify instances are (cold) migrated to different nodes"},{"line_number":146,"context_line":"        for instance in instances:"},{"line_number":147,"context_line":"            if initial_hosts[instance[\u0027id\u0027]] \u003d\u003d src_node:"},{"line_number":148,"context_line":"                new_host \u003d self.get_host_for_server(instance[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"71635a21_9c8d89c6","line":145,"range":{"start_line":145,"start_character":2,"end_line":145,"end_character":65},"in_reply_to":"eda332f6_e31c7ae8","updated":"2025-09-09 05:44:24.000000000","message":"So here, I guess we all agree the better way is to having a refactored `execute_strategy` so we can have at least a useful return (audit, actionplan, etc.)\n\nImo I don\u0027t think I should include that in this patch, as the scope seems not very relevant. Probably there should be another separated patch to refactor the `execute_strategy`, and I can try doing that along side this one, so that we can try to merge that first, then come to this one. \n\nOr if you think the change to `execute_strategy` shouldn\u0027t be too much, then I can try to include that here. The most simple, straightforward way may be just to add a return statement to the existing one, but it might raise concern for good practice as that function is also doing many assertions. Breaking that down into smaller functions sound better, but the current one is quite big and does many things, we also need to be more careful about its existing usage across the codebase.  \n\nI would love to hear your opinion before moving forward 😊","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"887ddc099b33541fe5a17ae3d529ec8699c6d07f","unresolved":true,"context_lines":[{"line_number":176,"context_line":"                \"disable_cold_migration\": True"},{"line_number":177,"context_line":"            }"},{"line_number":178,"context_line":"        }"},{"line_number":179,"context_line":"        self.execute_strategy(goal_name, strategy_name,"},{"line_number":180,"context_line":"                              expected_actions\u003d[\u0027change_nova_service_state\u0027,"},{"line_number":181,"context_line":"                                                \u0027migrate\u0027],"},{"line_number":182,"context_line":"                              **audit_kwargs)"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    @decorators.idempotent_id(\u002731b8523e-2f5b-4002-a3cf-678fbc215f0b\u0027)"},{"line_number":185,"context_line":"    @decorators.attr(type\u003d[\u0027strategy\u0027, \u0027host_maintenance\u0027])"},{"line_number":186,"context_line":"    def test_exec_host_maintenance_disable_cold_inactive_instances(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"439d59e0_1fd32e34","line":183,"range":{"start_line":179,"start_character":0,"end_line":183,"end_character":0},"updated":"2025-08-06 21:21:10.000000000","message":"Same thing as previous test.","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"887ddc099b33541fe5a17ae3d529ec8699c6d07f","unresolved":true,"context_lines":[{"line_number":219,"context_line":"            }"},{"line_number":220,"context_line":"        }"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        # No migrations should occur, only change_nova_service_state"},{"line_number":223,"context_line":"        self.execute_strategy(goal_name, strategy_name,"},{"line_number":224,"context_line":"                              expected_actions\u003d[\u0027change_nova_service_state\u0027],"},{"line_number":225,"context_line":"                              **audit_kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ee4c8683_2fb7d850","line":222,"range":{"start_line":222,"start_character":0,"end_line":222,"end_character":68},"updated":"2025-08-06 21:21:10.000000000","message":"this looks fine :)","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"0c791456cd6d626acc988481984fe40d948f9fb7","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            }"},{"line_number":220,"context_line":"        }"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        # No migrations should occur, only change_nova_service_state"},{"line_number":223,"context_line":"        self.execute_strategy(goal_name, strategy_name,"},{"line_number":224,"context_line":"                              expected_actions\u003d[\u0027change_nova_service_state\u0027],"},{"line_number":225,"context_line":"                              **audit_kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"93fb912f_6bb764fb","line":222,"range":{"start_line":222,"start_character":0,"end_line":222,"end_character":68},"in_reply_to":"ee4c8683_2fb7d850","updated":"2025-09-04 12:44:46.000000000","message":"Acknowledged","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"887ddc099b33541fe5a17ae3d529ec8699c6d07f","unresolved":true,"context_lines":[{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    @decorators.idempotent_id(\u0027f5e6d7c8-1234-5678-9012-ab34cd56ef78\u0027)"},{"line_number":277,"context_line":"    @decorators.attr(type\u003d[\u0027strategy\u0027, \u0027host_maintenance\u0027])"},{"line_number":278,"context_line":"    def test_exec_host_maintenance_disable_migration_inactive_instances(self):"},{"line_number":279,"context_line":"        # This test creates inactive instances to verify that when both"},{"line_number":280,"context_line":"        # migrations are disabled, only change_nova_service_state occurs"},{"line_number":281,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9877722d_ef12f427","line":278,"range":{"start_line":278,"start_character":8,"end_line":278,"end_character":71},"updated":"2025-08-06 21:21:10.000000000","message":"it is really nice that you are covering all scenarios, I was just thinking if we can reduce the number of tests here, but not the scenarios, since each test will take some time to finish, increasing the total time to complete the CI job.\ne.g, if instead of creating one instance per host, you could create 2 instance in a single host, which will be the source host, and only one instance you could shutoff to check that if continues in source host.","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"0c791456cd6d626acc988481984fe40d948f9fb7","unresolved":false,"context_lines":[{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    @decorators.idempotent_id(\u0027f5e6d7c8-1234-5678-9012-ab34cd56ef78\u0027)"},{"line_number":277,"context_line":"    @decorators.attr(type\u003d[\u0027strategy\u0027, \u0027host_maintenance\u0027])"},{"line_number":278,"context_line":"    def test_exec_host_maintenance_disable_migration_inactive_instances(self):"},{"line_number":279,"context_line":"        # This test creates inactive instances to verify that when both"},{"line_number":280,"context_line":"        # migrations are disabled, only change_nova_service_state occurs"},{"line_number":281,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1abde534_00580d9f","line":278,"range":{"start_line":278,"start_character":8,"end_line":278,"end_character":71},"in_reply_to":"9877722d_ef12f427","updated":"2025-09-04 12:44:46.000000000","message":"Done","commit_id":"36074cb31682b1f2033d19db3b4668506f1011e5"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"09913bc2d4b807cd0074abb8a3ea18b99b507502","unresolved":true,"context_lines":[{"line_number":57,"context_line":"                           \"No migrate actions found to verify\")"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        for action in migrate_actions:"},{"line_number":60,"context_line":"            input_params \u003d action.get(\u0027input_parameters\u0027, {})"},{"line_number":61,"context_line":"            actual_migration_type \u003d input_params.get(\u0027migration_type\u0027)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"            self.assertEqual(expected_migration_type, actual_migration_type,"}],"source_content_type":"text/x-python","patch_set":3,"id":"dc684ccd_247e06f6","line":60,"range":{"start_line":60,"start_character":0,"end_line":60,"end_character":61},"updated":"2025-09-05 18:45:44.000000000","message":"this is not available from GET action_plans. So you will need to retrieve each action individually, so it will return the input_parameters for you:\ne.g.:\n```\naction \u003d self.client.show_action(action_uuid)\naction_input_params \u003d action[\u0027input_parameters\u0027]\n```","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"5a4bdb7629aca207e050dd0f664a0cebec8ecf91","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                           \"No migrate actions found to verify\")"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        for action in migrate_actions:"},{"line_number":60,"context_line":"            input_params \u003d action.get(\u0027input_parameters\u0027, {})"},{"line_number":61,"context_line":"            actual_migration_type \u003d input_params.get(\u0027migration_type\u0027)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"            self.assertEqual(expected_migration_type, actual_migration_type,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ad414ba0_fd5e7417","line":60,"range":{"start_line":60,"start_character":0,"end_line":60,"end_character":61},"in_reply_to":"dc684ccd_247e06f6","updated":"2025-09-09 05:44:24.000000000","message":"Acknowledged","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"09913bc2d4b807cd0074abb8a3ea18b99b507502","unresolved":true,"context_lines":[{"line_number":65,"context_line":"                             f\"but got {actual_migration_type} for action \""},{"line_number":66,"context_line":"                             f\"{action.get(\u0027uuid\u0027, \u0027unknown\u0027)}\")"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def _verify_latest_migration_type(self, expected_migration_type):"},{"line_number":69,"context_line":"        \"\"\"Verify migration type of the most recent audit\u0027s migrate actions.\"\"\""},{"line_number":70,"context_line":"        # Try to get the most recent audit"},{"line_number":71,"context_line":"        _, audits \u003d self.client.list_audits()"}],"source_content_type":"text/x-python","patch_set":3,"id":"ce1050dc_a0364290","line":68,"range":{"start_line":68,"start_character":8,"end_line":68,"end_character":37},"updated":"2025-09-05 18:45:44.000000000","message":"This would break at some point if we start to run some tests in parallel.","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"5a4bdb7629aca207e050dd0f664a0cebec8ecf91","unresolved":false,"context_lines":[{"line_number":65,"context_line":"                             f\"but got {actual_migration_type} for action \""},{"line_number":66,"context_line":"                             f\"{action.get(\u0027uuid\u0027, \u0027unknown\u0027)}\")"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def _verify_latest_migration_type(self, expected_migration_type):"},{"line_number":69,"context_line":"        \"\"\"Verify migration type of the most recent audit\u0027s migrate actions.\"\"\""},{"line_number":70,"context_line":"        # Try to get the most recent audit"},{"line_number":71,"context_line":"        _, audits \u003d self.client.list_audits()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7c4dc7a4_d01ad208","line":68,"range":{"start_line":68,"start_character":8,"end_line":68,"end_character":37},"in_reply_to":"ce1050dc_a0364290","updated":"2025-09-09 05:44:24.000000000","message":"Acknowledged","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"09913bc2d4b807cd0074abb8a3ea18b99b507502","unresolved":true,"context_lines":[{"line_number":168,"context_line":"    def test_execute_host_maintenance_disable_live_migration(self):"},{"line_number":169,"context_line":"        # This test verifies that when live migration is disabled,"},{"line_number":170,"context_line":"        # active instances are cold migrated to other nodes"},{"line_number":171,"context_line":"        self._check_disable_migration_supported()"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        self.addCleanup(self.rollback_compute_nodes_status)"},{"line_number":174,"context_line":"        self.addCleanup(self.wait_delete_instances_from_model)"}],"source_content_type":"text/x-python","patch_set":3,"id":"4e93514b_09f7c36b","line":171,"range":{"start_line":171,"start_character":13,"end_line":171,"end_character":47},"updated":"2025-09-05 18:45:44.000000000","message":"nit: this is not wrong, since you would skip the test if the option is not enabled.\nbut I would break this file in 3 classe. A base class with resource_setup part. A class with the existing tests. A new class that has all new tests that you are adding. Then you can skip the entire class using `skip_checks` class method.","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":38075,"name":"Ho Minh Quang Ngo","display_name":"Quang Ngo","email":"quang.ngo@canonical.com","username":"Quang-Ngo"},"change_message_id":"5a4bdb7629aca207e050dd0f664a0cebec8ecf91","unresolved":false,"context_lines":[{"line_number":168,"context_line":"    def test_execute_host_maintenance_disable_live_migration(self):"},{"line_number":169,"context_line":"        # This test verifies that when live migration is disabled,"},{"line_number":170,"context_line":"        # active instances are cold migrated to other nodes"},{"line_number":171,"context_line":"        self._check_disable_migration_supported()"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        self.addCleanup(self.rollback_compute_nodes_status)"},{"line_number":174,"context_line":"        self.addCleanup(self.wait_delete_instances_from_model)"}],"source_content_type":"text/x-python","patch_set":3,"id":"6b6ee655_8bc4d8e6","line":171,"range":{"start_line":171,"start_character":13,"end_line":171,"end_character":47},"in_reply_to":"4e93514b_09f7c36b","updated":"2025-09-09 05:44:24.000000000","message":"Acknowledged","commit_id":"ec40bb530bf0262a08b0f4b23f92d4c765a951d0"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"ce8620412bf773b861ddf20e187762b6192eb670","unresolved":true,"context_lines":[{"line_number":229,"context_line":"            **audit_kwargs)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"        # Verify that live migration is used"},{"line_number":232,"context_line":"        self.verify_migration_type_from_audit(audit_uuid, \u0027live\u0027)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    @decorators.idempotent_id(\u002731b8523e-2f5b-4002-a3cf-678fbc215f0b\u0027)"},{"line_number":235,"context_line":"    @decorators.attr(type\u003d[\u0027strategy\u0027, \u0027host_maintenance\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"a57f0bbe_5f9c00f3","line":232,"updated":"2025-09-19 15:14:16.000000000","message":"this test is the only missing the check on the instance current host, I think we should add it as well here","commit_id":"2fe2a90da460b1b489de42d798a4a035fc22b716"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"ce8620412bf773b861ddf20e187762b6192eb670","unresolved":true,"context_lines":[{"line_number":253,"context_line":"        )"},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"        # Wait for compute model updates"},{"line_number":256,"context_line":"        self.wait_for_instances_in_model([instance])"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"        goal_name \u003d \"cluster_maintaining\""},{"line_number":259,"context_line":"        strategy_name \u003d \"host_maintenance\""}],"source_content_type":"text/x-python","patch_set":4,"id":"6f8600d2_e5df946a","line":256,"updated":"2025-09-19 15:14:16.000000000","message":"@quang.ngo@canonical.com looking at the logs from the failed `test_exec_host_maintenance_disable_cold_inactive_instances` test, I think the problem is that this line is not actually waiting much, since the instance is already in the model, so my guess is that this function does not check its state, so it does not track the change. \n\nThe decision engine gets the notification for the instance update after the audit has ran:\n```\nSep 12 09:04:03.599746 np3fc2c5e3a4c94 watcher-decision-engine[99611]: INFO watcher.decision_engine.model.notification.nova [None req-4188e76e-327f-472f-bcb8-390503438c84 None None] Event \u0027instance.update\u0027 received from nova-compute:np3fc2c5e3a4c94 with metadata {\u0027message_id\u0027: \u0027653c70d5-9d5f-4935-b99c-c3b45601bc4e\u0027, \u0027timestamp\u0027: \u00272025-09-12 09:04:03.594917\u0027}\nSep 12 09:04:03.600104 np3fc2c5e3a4c94 watcher-decision-engine[99611]: DEBUG watcher.decision_engine.model.notification.nova [None req-4188e76e-327f-472f-bcb8-390503438c84 None None] {\u0027nova_object.name\u0027: \u0027InstanceUpdatePayload\u0027, \u0027nova_object.namespace\u0027: \u0027nova\u0027, \u0027nova_object.version\u0027: \u00272.1\u0027, \u0027nova_object.data\u0027: {\u0027state_update\u0027: {\u0027nova_object.name\u0027: \u0027InstanceStateUpdatePayload\u0027, \u0027nova_object.namespace\u0027: \u0027nova\u0027, \u0027nova_object.version\u0027: \u00271.0\u0027, \u0027nova_object.data\u0027: {\u0027old_state\u0027: \u0027paused\u0027, \u0027state\u0027: \u0027paused\u0027, \u0027old_task_state\u0027: \u0027deleting\u0027, \u0027new_task_state\u0027: \u0027deleting\u0027}}, \u0027audit_period\u0027: {\u0027nova_object.name\u0027: \u0027AuditPeriodPayload\u0027, \u0027nova_object.namespace\u0027: \u0027nova\u0027, \u0027nova_object.version\u0027: \u00271.0\u0027, \u0027nova_object.data\u0027: {\u0027audit_period_beginning\u0027: \u00272025-09-12T09:00:00Z\u0027, \u0027audit_period_ending\u0027: \u00272025-09-12T09:04:03Z\u0027}}, \u0027old_display_name\u0027: None, \u0027tags\u0027: [], \u0027uuid\u0027: \u0027817c1508-2f39-4c72-b4b4-113c35db3e9b\u0027, \u0027user_id\u0027: \u00273ce4a0ee70dc43778f52241e9fac7508\u0027, \u0027tenant_id\u0027: \u002704dd48ed47214b83931aae32e6c6be97\u0027, \u0027reservation_id\u0027: \u0027r-ltiet8sh\u0027, \u0027display_name\u0027: \u0027tempest-TestExecuteHostMaintenanceDisableMigration-server-544071965\u0027, \u0027display_description\u0027: None, \u0027host_name\u0027: \u0027tempest-testexecutehostmaintenancedisablemigration-server-54407\u0027, \u0027host\u0027: \u0027np1b40b4e8087c4\u0027, \u0027node\u0027: \u0027np1b40b4e8087c4\u0027, \u0027os_type\u0027: None, \u0027architecture\u0027: None, \u0027availability_zone\u0027: \u0027nova\u0027, \u0027flavor\u0027: {\u0027nova_object.name\u0027: \u0027FlavorPayload\u0027, \u0027nova_object.namespace\u0027: \u0027nova\u0027, \u0027nova_object.version\u0027: \u00271.4\u0027, \u0027nova_object.data\u0027: {\u0027flavorid\u0027: \u002742\u0027, \u0027memory_mb\u0027: 192, \u0027vcpus\u0027: 1, \u0027root_gb\u0027: 1, \u0027ephemeral_gb\u0027: 0, \u0027name\u0027: \u0027m1.nano\u0027, \u0027swap\u0027: 0, \u0027rxtx_factor\u0027: 1.0, \u0027vcpu_weight\u0027: 0, \u0027disabled\u0027: False, \u0027is_public\u0027: True, \u0027extra_specs\u0027: {\u0027hw_rng:allowed\u0027: \u0027True\u0027}, \u0027projects\u0027: None, \u0027description\u0027: None}}, \u0027image_uuid\u0027: \u0027e9c61514-f150-48b2-8ab6-b643826f3515\u0027, \u0027key_name\u0027: None, \u0027kernel_id\u0027: \u0027\u0027, \u0027ramdisk_id\u0027: \u0027\u0027, \u0027created_at\u0027: \u00272025-09-12T09:03:35Z\u0027, \u0027launched_at\u0027: \u00272025-09-12T09:03:51Z\u0027, \u0027terminated_at\u0027: None, \u0027deleted_at\u0027: None, \u0027updated_at\u0027: \u00272025-09-12T09:03:58Z\u0027, \u0027state\u0027: \u0027paused\u0027, \u0027power_state\u0027: \u0027paused\u0027, \u0027task_state\u0027: \u0027deleting\u0027, \u0027progress\u0027: 0, \u0027ip_addresses\u0027: [{\u0027nova_object.name\u0027: \u0027IpPayload\u0027, \u0027nova_object.namespace\u0027: \u0027nova\u0027, \u0027nova_object.version\u0027: \u00271.0\u0027, \u0027nova_object.data\u0027: {\u0027label\u0027: \u0027tempest-TestExecuteHostMaintenanceDisableMigration-1678721948-network\u0027, \u0027mac\u0027: \u0027fa:16:3e:62:d5:d0\u0027, \u0027meta\u0027: {}, \u0027port_uuid\u0027: \u002770bc2ea9-fe78-49d5-b30e-2fd0b18392c0\u0027, \u0027version\u0027: 4, \u0027address\u0027: \u002710.1.0.10\u0027, \u0027device_name\u0027: \u0027tap70bc2ea9-fe\u0027}}], \u0027block_devices\u0027: None, \u0027metadata\u0027: {}, \u0027locked\u0027: False, \u0027auto_disk_config\u0027: \u0027MANUAL\u0027, \u0027request_id\u0027: \u0027req-4188e76e-327f-472f-bcb8-390503438c84\u0027, \u0027action_initiator_user\u0027: \u00273ce4a0ee70dc43778f52241e9fac7508\u0027, \u0027action_initiator_project\u0027: \u002704dd48ed47214b83931aae32e6c6be97\u0027, \u0027locked_reason\u0027: None, \u0027shares\u0027: None}} {{(pid\u003d99611) info /opt/stack/watcher/watcher/decision_engine/model/notification/nova.py:369}}\n```\nfrom https://b9f1be74c1b760538085-7acdaeb08b6e8b73cd064b3790f4bf61.ssl.cf5.rackcdn.com/openstack/ce15ad3b6b87427fbc8c76e84e909006/controller/logs/screen-watcher-decision-engine.txt\n\nThe strategy finishes a bit before that \n```\nSep 12 09:04:00.161557 np3fc2c5e3a4c94 watcher-decision-engine[99611]: DEBUG watcher.notifications.base [None req-4e001dd5-5304-4b2f-b079-79ba54bd5bd0 None None] Emitting notification `audit.strategy.end` {{(pid\u003d99611) _emit /opt/stack/watcher/watcher/notifications/base.py:177}}\n```\n\nBecause the data model is not updated, the strategy sees the instance as active and tries to migrate it. We need some way to wait before starting the audit until the data model is up to date. A quick and dirty option would be to sleep for some time, but a better solution I think is to enhance https://github.com/openstack/watcher-tempest-plugin/blob/1e78f4999f99989f5032b121cca25fb6985a767a/watcher_tempest_plugin/tests/scenario/base.py#L1037 or implement a new method that waits until the instance is in a given status. @viroel@gmail.com you have worked on this before, wdyt?","commit_id":"2fe2a90da460b1b489de42d798a4a035fc22b716"}]}