)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"3e1174a985828367f123345787e03b1192890b30","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"95296ea2_3a4a411b","updated":"2026-02-19 15:06:58.000000000","message":"I was doubting whether we should skip or fail in the cases of missing volume type or pool. I ended up at +2 for two reasons:\n1. If the type or pool is missing when running the audit the actions should not be generated anyway.\n\n2. If the type or pool disappear between the action plan is generated and executed I think it would still make sense to run the rest of the actions","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"7d2043d6cd59cf60882ed2535982a19e2b7dcc27","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e99b049b_aaa299c1","updated":"2026-02-11 17:43:42.000000000","message":"recheck","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ae5c20a877d424700307abbe7487685aa3f9087b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7abaf828_f6fd023b","in_reply_to":"66899e35_1423706c","updated":"2026-02-23 09:46:44.000000000","message":"Consistency with migration is valid point. From that point of view, we should fail volume_migrations when pool or type do not exist. Now, i\u0027m inclined to set them as FAILED in those cases, any other opinion?\n\nWRT all actions skipped in a plan, it\u0027s good discussion although out of the scope of this change. Current behavior in that case is AP goes to SUCCEDED with an status_messate: \"one or more actions were skipped\". We may treat differently when all actions fail and set it to FAILED or even SKIPPED (although currently SKIPPED is not a status for AP). I guess that should be discussed in a separate blueprint or spec.","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d41b99c191cdaef350ce5d19d6f20fe96a77f805","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"66899e35_1423706c","in_reply_to":"6ca1ede8_0e36a735","updated":"2026-02-20 17:12:43.000000000","message":"ack we may what to defien that a action plan with only skipted acction shoudl be treaded as failed?\n\ni.e. for a acntion plan to succeed it must have at lest one succeded action?","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"bab2405921c76cfe35383f19e9347d7249687503","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"bb07332e_ebb1ac59","in_reply_to":"7abaf828_f6fd023b","updated":"2026-02-23 11:49:41.000000000","message":"+1 to failing in cases of missing dest. type or pool for consistency with the migration action. I think both options would be ok, but it\u0027s good UX to be consistent in ambiguous situations. About the point of the result of the action plan, I think the current behaviour is fine as long as the user can clearly see what happens (and the current status message is ok imo). Regardless I agree with Alfredo that the action plan result is out of scope for this patch","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"38b7935fc28941b30fc04927e8b6dc703a221e33","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e49356df_f0184f57","in_reply_to":"95296ea2_3a4a411b","updated":"2026-02-19 18:37:55.000000000","message":"+1 that aligns with my thining on thsi as well","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"afd565d7ccd0310fc506cf8f8fddcbb517615206","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9c0f771f_c08bc1b0","in_reply_to":"bb07332e_ebb1ac59","updated":"2026-02-23 13:39:35.000000000","message":"yeah, the result of a all Skipped action plan is a consequence of these series of patches, but we can address that in a different patch/blueprint too, to improve user experience.\nThe major concern was really the skip for invalid destinations, which would end to be different from instance migration and also may not work for all use cases.","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"1742efd041bdf842898c60b576e41a751540dc5c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6ca1ede8_0e36a735","in_reply_to":"e49356df_f0184f57","updated":"2026-02-19 20:41:45.000000000","message":"Note that for 2. we are changing current behavior, which may not fit for all use cases. Running an Action Plan that ends with all migrations SKIPPED (when dest pool does not exists), gives the impression that some optimization was made, but the AP did nothing (and volumes still exists).\nAnother thing, we have a different behavior in migrate instance, which fails when destination node does not exist or is disabled[1]. Shouldn\u0027t be the same behavior in both?\n\nI will place my -1 so we can discuss about these 2 action\u0027s pre_condition\n\n[1] https://github.com/openstack/watcher/blob/9d5a584c9b74d04a75ea7d24f293d0b4600aeb39/watcher/applier/actions/migration.py#L204-L209","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"1742efd041bdf842898c60b576e41a751540dc5c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6750a63e_0f111daa","updated":"2026-02-19 20:41:45.000000000","message":"Thanks for proposing Alfredo!","commit_id":"a092d8932620d08c50b3783a81bca70eab220047"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"38b7935fc28941b30fc04927e8b6dc703a221e33","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f4a16f12_d79debb8","updated":"2026-02-19 18:37:55.000000000","message":"im ok with this but lets revisit on monday once folks are back","commit_id":"a092d8932620d08c50b3783a81bca70eab220047"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"8f556dec17931807441ccf996efcfc78253fdbe9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e0152188_2d060aff","updated":"2026-02-23 19:44:50.000000000","message":"This looks good, for the failures we are just antecipating the issue that would happen in execute() today. The skip ones are a good improvement to the action.","commit_id":"b7e8d459fcfdf1bcc7d7340b248043de3f968b41"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"d23f647deff23f4b3e251836f6e7bb2429d4712e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"fa804f84_0a816620","updated":"2026-02-24 15:46:31.000000000","message":"lgtm","commit_id":"b7e8d459fcfdf1bcc7d7340b248043de3f968b41"}],"doc/source/actions/volume_migration.rst":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"500a9e0217609a53d4b85c5311e7f310c0644fab","unresolved":true,"context_lines":[{"line_number":48,"context_line":"phase in the following cases:"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"- The volume does not exist"},{"line_number":51,"context_line":"- The destination_type does not exist (if specified)"},{"line_number":52,"context_line":"- The destination_node (pool) does not exist (if specified)"},{"line_number":53,"context_line":"- The migration_type is \u0027retype\u0027 and the destination_type is the same as the"},{"line_number":54,"context_line":"  current volume type"},{"line_number":55,"context_line":"- The migration_type is \u0027migrate\u0027 and the destination_node is the same as the"}],"source_content_type":"text/x-rst","patch_set":2,"id":"49be4d18_cd99c282","line":52,"range":{"start_line":51,"start_character":0,"end_line":52,"end_character":59},"updated":"2026-02-19 14:57:06.000000000","message":"I am not convinced that we should skip instead of failing, for the destination_* does not exit cases. If the volume exists but can\u0027t be migrated, it is a failure from Action Migrate point of view.","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"38b7935fc28941b30fc04927e8b6dc703a221e33","unresolved":true,"context_lines":[{"line_number":48,"context_line":"phase in the following cases:"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"- The volume does not exist"},{"line_number":51,"context_line":"- The destination_type does not exist (if specified)"},{"line_number":52,"context_line":"- The destination_node (pool) does not exist (if specified)"},{"line_number":53,"context_line":"- The migration_type is \u0027retype\u0027 and the destination_type is the same as the"},{"line_number":54,"context_line":"  current volume type"},{"line_number":55,"context_line":"- The migration_type is \u0027migrate\u0027 and the destination_node is the same as the"}],"source_content_type":"text/x-rst","patch_set":2,"id":"89eefba6_2e52f040","line":52,"range":{"start_line":51,"start_character":0,"end_line":52,"end_character":59},"in_reply_to":"49be4d18_cd99c282","updated":"2026-02-19 18:37:55.000000000","message":"if it does not exit at the api when we defiend the audit i woudl agree\n\nas autis can be defiend an drun for a long period fo time\nand since action plan can sit in a pendign state for a protacted period of time i fell liek it ok to skip if the planner has product and action and the destionation type or pool noloner exits when teh appler comes to evaulate it.\n\nwe shoudl avlidate this when creating an audit in the future however if we are not doing that already adn return a 400 in the api or makr the audit as failed if we only validate it at the descion engine stage\n\nthis is very late to catch this issue if its not timing related.\n\ni woudl consdier this part of input validation","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"}],"watcher/applier/actions/volume_migration.py":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ae392dda46655383bf9ab3f4e75b008025a2d280","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        # if the volume is in-use. If the volume has no attachments"},{"line_number":139,"context_line":"        # allow cinder to decided if it can be migrated."},{"line_number":140,"context_line":"        if not volume.attachments:"},{"line_number":141,"context_line":"            LOG.debug(f\"volume: {volume.id} has no attachments\")"},{"line_number":142,"context_line":"            return True"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # since it has attachments we need to validate nova\u0027s constraints"}],"source_content_type":"text/x-python","patch_set":2,"id":"d386415c_c4add039","line":141,"in_reply_to":"0ee34bd0_dfef019a","updated":"2026-02-11 15:39:19.000000000","message":"out of scope","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ae392dda46655383bf9ab3f4e75b008025a2d280","unresolved":false,"context_lines":[{"line_number":145,"context_line":"        instance_id \u003d volume.attachments[0][\u0027server_id\u0027]"},{"line_number":146,"context_line":"        instance_status \u003d self.nova_util.find_instance(instance_id).status"},{"line_number":147,"context_line":"        LOG.debug("},{"line_number":148,"context_line":"            f\"volume: {volume.id} is attached to instance: {instance_id} \""},{"line_number":149,"context_line":"            f\"in instance status: {instance_status}\")"},{"line_number":150,"context_line":"        # NOTE(sean-k-mooney): This used to allow RESIZED which"},{"line_number":151,"context_line":"        # is the resize_verify task state, that is not an acceptable time"}],"source_content_type":"text/x-python","patch_set":2,"id":"dbe25125_c4baffdb","line":148,"in_reply_to":"406c15bd_5d5499d8","updated":"2026-02-11 15:39:19.000000000","message":"out of scope","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ae5c20a877d424700307abbe7487685aa3f9087b","unresolved":true,"context_lines":[{"line_number":190,"context_line":"    def abort(self):"},{"line_number":191,"context_line":"        pass"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def pre_condition(self):"},{"line_number":194,"context_line":"        \"\"\"Check volumemigration preconditions"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"        Skipping conditions:"}],"source_content_type":"text/x-python","patch_set":2,"id":"f65e0564_02c383c4","line":193,"in_reply_to":"742bcfa0_69f1b789","updated":"2026-02-23 09:46:44.000000000","message":"Classes calling the pre_condition method are already logging when pre_condition is started, when a ActionSkipped exception is raised and also when any other exception is happening. I think that\u0027s enough.","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"38b7935fc28941b30fc04927e8b6dc703a221e33","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        Skipping conditions:"},{"line_number":197,"context_line":"        - Volume does not exist"},{"line_number":198,"context_line":"        - Volume type is already the destination type in retype migrtion"},{"line_number":199,"context_line":"        - Volume is already on the destination node in migrate migration"},{"line_number":200,"context_line":"        - Destination node (if specified) does not exist"},{"line_number":201,"context_line":"        - Destination type (if specified) does not exist"},{"line_number":202,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"42abe238_f9f4df32","line":199,"in_reply_to":"41e25c71_74d9aebb","updated":"2026-02-19 18:37:55.000000000","message":"Done","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ae392dda46655383bf9ab3f4e75b008025a2d280","unresolved":true,"context_lines":[{"line_number":196,"context_line":"        Skipping conditions:"},{"line_number":197,"context_line":"        - Volume does not exist"},{"line_number":198,"context_line":"        - Volume type is already the destination type in retype migrtion"},{"line_number":199,"context_line":"        - Volume is already on the destination node in migrate migration"},{"line_number":200,"context_line":"        - Destination node (if specified) does not exist"},{"line_number":201,"context_line":"        - Destination type (if specified) does not exist"},{"line_number":202,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"6c4040b3_91fbe4e7","line":199,"in_reply_to":"51330489_4f3c4a68","updated":"2026-02-11 15:39:19.000000000","message":"Classes calling the pre_condition method are already logging when pre_condition is started, when a ActionSkipped exception is raised and also when any other exception is happening. I think that\u0027s enough.","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"3e1174a985828367f123345787e03b1192890b30","unresolved":true,"context_lines":[{"line_number":196,"context_line":"        Skipping conditions:"},{"line_number":197,"context_line":"        - Volume does not exist"},{"line_number":198,"context_line":"        - Volume type is already the destination type in retype migrtion"},{"line_number":199,"context_line":"        - Volume is already on the destination node in migrate migration"},{"line_number":200,"context_line":"        - Destination node (if specified) does not exist"},{"line_number":201,"context_line":"        - Destination type (if specified) does not exist"},{"line_number":202,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"8bc1d44e_1d322e5d","line":199,"in_reply_to":"6c4040b3_91fbe4e7","updated":"2026-02-19 15:06:58.000000000","message":"I think you might have replied to the wrong comment? This one points out a typo in line 198","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"5ec36fa57e5f2d04f6dce604c2cb1078b9032b52","unresolved":true,"context_lines":[{"line_number":196,"context_line":"        Skipping conditions:"},{"line_number":197,"context_line":"        - Volume does not exist"},{"line_number":198,"context_line":"        - Volume type is already the destination type in retype migrtion"},{"line_number":199,"context_line":"        - Volume is already on the destination node in migrate migration"},{"line_number":200,"context_line":"        - Destination node (if specified) does not exist"},{"line_number":201,"context_line":"        - Destination type (if specified) does not exist"},{"line_number":202,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"41e25c71_74d9aebb","line":199,"in_reply_to":"8bc1d44e_1d322e5d","updated":"2026-02-19 16:35:45.000000000","message":"yep","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"38b7935fc28941b30fc04927e8b6dc703a221e33","unresolved":true,"context_lines":[{"line_number":190,"context_line":"    def abort(self):"},{"line_number":191,"context_line":"        pass"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def pre_condition(self):"},{"line_number":194,"context_line":"        \"\"\"Check volumemigration preconditions"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"        Skipping conditions:"}],"source_content_type":"text/x-python","patch_set":3,"id":"18cc2244_a1db382a","line":193,"in_reply_to":"8e823599_f7a929c4","updated":"2026-02-19 18:37:55.000000000","message":"this is nice to have yes but we could fix this when we start adopting typeing  more widly\n```\n:raise: exception.ActionSkipped\n:returns: None\n```","commit_id":"a092d8932620d08c50b3783a81bca70eab220047"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"38b7935fc28941b30fc04927e8b6dc703a221e33","unresolved":true,"context_lines":[{"line_number":191,"context_line":"        pass"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def pre_condition(self):"},{"line_number":194,"context_line":"        \"\"\"Check volumemigration preconditions"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"        Skipping conditions:"},{"line_number":197,"context_line":"        - Volume does not exist"}],"source_content_type":"text/x-python","patch_set":3,"id":"05b0513e_bc193287","line":194,"in_reply_to":"0ca417a0_c12301ee","updated":"2026-02-19 18:37:55.000000000","message":"this is a nit althoyhg i agree with it\n\nif you need to rebase i would update this to \u0027volume migration\u0027","commit_id":"a092d8932620d08c50b3783a81bca70eab220047"}],"watcher/tests/unit/applier/actions/test_volume_migration.py":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ae392dda46655383bf9ab3f4e75b008025a2d280","unresolved":false,"context_lines":[{"line_number":12,"context_line":"# See the License for the specific language governing permissions and"},{"line_number":13,"context_line":"# limitations under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"from unittest import mock"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from cinderclient import exceptions as cinder_exception"},{"line_number":18,"context_line":"import jsonschema"}],"source_content_type":"text/x-python","patch_set":2,"id":"b3076aeb_68acbea8","line":15,"in_reply_to":"32f772a3_1caf862e","updated":"2026-02-11 15:39:19.000000000","message":"unittest is stdlib while cinderclient is third-party so different blocks","commit_id":"8d68ca1b3df05286ae78db5c9ec32a1fdf257238"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"38b7935fc28941b30fc04927e8b6dc703a221e33","unresolved":true,"context_lines":[{"line_number":319,"context_line":"            \"Volume type is already storage1-typename\","},{"line_number":320,"context_line":"            self.action_retype.pre_condition)"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    def test_pre_condition_migrate_same_node(self):"},{"line_number":323,"context_line":"        # Create volume on the same node as destination"},{"line_number":324,"context_line":"        volume \u003d self.fake_volume(host\u003d\"storage1-poolname\")"},{"line_number":325,"context_line":"        self.m_c_helper.get_volume.return_value \u003d volume"}],"source_content_type":"text/x-python","patch_set":3,"id":"819b0a12_9fd3e544","line":322,"in_reply_to":"e07fe248_bb80e253","updated":"2026-02-19 18:37:55.000000000","message":"swap is now just an alise for migrate and is deprectated","commit_id":"a092d8932620d08c50b3783a81bca70eab220047"}]}