)]}'
{"ironic/drivers/modules/drac/boot.py":[{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"b8021319940dc1f3f2f1092b791be9a87e6232ea","unresolved":true,"context_lines":[{"line_number":70,"context_line":"            boot_devices.CDROM: sushy.VIRTUAL_MEDIA_CD"},{"line_number":71,"context_line":"        }"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def _validate_vendor(self, task):"},{"line_number":74,"context_line":"        vendor \u003d task.node.properties.get(\u0027vendor\u0027)"},{"line_number":75,"context_line":"        if not vendor:"},{"line_number":76,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":5,"id":"24eb2bfd_c8a61ff7","line":73,"updated":"2021-02-04 12:00:26.000000000","message":"is this necessary to get it working? Isn\u0027t there already validation when using unsupported interface (as idrac-virtual-media-boot) with selected hardware type (as redfish) that would catch such misconfiguration?\n\nOf course, if somebody configures idrac hardware type with non-Dell server, then it will cause issues. But in this and in more general case, shouldn\u0027t there be validation for all interfaces and hardware types across Ironic? For example, to also validate that somebody does not try e.g., iLO with Dell servers?","commit_id":"6ac7ba6b4b37ffe13442b4d3d676373c75a8fbfa"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"7ff972c51a2bd4f0ffedfc1f06a577b69dceed23","unresolved":true,"context_lines":[{"line_number":70,"context_line":"            boot_devices.CDROM: sushy.VIRTUAL_MEDIA_CD"},{"line_number":71,"context_line":"        }"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def _validate_vendor(self, task):"},{"line_number":74,"context_line":"        vendor \u003d task.node.properties.get(\u0027vendor\u0027)"},{"line_number":75,"context_line":"        if not vendor:"},{"line_number":76,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":5,"id":"b12c1b5e_e557e60c","line":73,"in_reply_to":"24eb2bfd_c8a61ff7","updated":"2021-02-04 12:11:20.000000000","message":"\u003e Of course, if somebody configures idrac hardware type with non-Dell server, then it will cause issues. \n\nExactly.\n\n\u003e But in this and in more general case, shouldn\u0027t there be validation for all interfaces and hardware types across Ironic?\n\nI expect most of other drivers to fall quickly and loudly if used with incorrect hardware (e.g. irmc with Dell), but this boot interface is a flavor of Redfish, so the failure is going to be quite obscure (and late).","commit_id":"6ac7ba6b4b37ffe13442b4d3d676373c75a8fbfa"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"e071f0922a4fd67259c361508d244052ddf2056d","unresolved":false,"context_lines":[{"line_number":70,"context_line":"            boot_devices.CDROM: sushy.VIRTUAL_MEDIA_CD"},{"line_number":71,"context_line":"        }"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def _validate_vendor(self, task):"},{"line_number":74,"context_line":"        vendor \u003d task.node.properties.get(\u0027vendor\u0027)"},{"line_number":75,"context_line":"        if not vendor:"},{"line_number":76,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":5,"id":"81825c32_88181d05","line":73,"in_reply_to":"52d6a0ca_9dfad7c9","updated":"2021-02-05 09:34:42.000000000","message":"it\u0027s getting complicated :)\n- Vendor detection relies on management interface implemented and working, if it is implemented and not working vendor cannot be known, however that also means that baremetal does not support needed vendor.\n- Vendor detection works among Redfish and IPMI baremetals, in other cases (WS-Man, iLO etc.) using incorrect configuration should result in empty vendor\n- For Redfish and IPMI could assume that by default if vendor can be detected, then it is supported with exception of Redfish virtual media boot for iDRAC\n- For idrac hw type all non-Dell vendors should fail for all interfaces, even if somehow user could live with idrac-redfish implementation for some time\n- for other vendor specific hw types the same - should fail if vendor cannot be detected, but that\u0027s just to replace current error message when it fails to connect (I haven\u0027t tried so not sure how self-explanatory it is)\n- Something that I have missed or overlooked (further tinkering needed)\n\nOn the other hand I don\u0027t believe users misconfigure their ironic so that they pick incorrect vendor specific hw type that is meant for different vendor. Effort might not be worth it, but then it\u0027s just this inconsistency across interface validation.","commit_id":"6ac7ba6b4b37ffe13442b4d3d676373c75a8fbfa"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c9ee95321a0ec4a2110f86ee5a3079f266babbb5","unresolved":false,"context_lines":[{"line_number":70,"context_line":"            boot_devices.CDROM: sushy.VIRTUAL_MEDIA_CD"},{"line_number":71,"context_line":"        }"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def _validate_vendor(self, task):"},{"line_number":74,"context_line":"        vendor \u003d task.node.properties.get(\u0027vendor\u0027)"},{"line_number":75,"context_line":"        if not vendor:"},{"line_number":76,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":5,"id":"52d6a0ca_9dfad7c9","line":73,"in_reply_to":"7a9b7219_652757b8","updated":"2021-02-04 16:24:02.000000000","message":"I don\u0027t object to that, if you have any specific ideas - please propose them.","commit_id":"6ac7ba6b4b37ffe13442b4d3d676373c75a8fbfa"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"b48eda0965d2f65fbf7a79710215cf19a642326d","unresolved":false,"context_lines":[{"line_number":70,"context_line":"            boot_devices.CDROM: sushy.VIRTUAL_MEDIA_CD"},{"line_number":71,"context_line":"        }"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def _validate_vendor(self, task):"},{"line_number":74,"context_line":"        vendor \u003d task.node.properties.get(\u0027vendor\u0027)"},{"line_number":75,"context_line":"        if not vendor:"},{"line_number":76,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":5,"id":"b5880bf7_dd2f53b5","line":73,"in_reply_to":"81825c32_88181d05","updated":"2021-02-05 11:05:19.000000000","message":"\u003e On the other hand I don\u0027t believe users misconfigure their ironic so that they pick incorrect vendor specific hw type that is meant for different vendor.\n\nOkay, I\u0027ll delete the part here.","commit_id":"6ac7ba6b4b37ffe13442b4d3d676373c75a8fbfa"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"aac264a3058b8568d76e39119ea3790b48b89f18","unresolved":false,"context_lines":[{"line_number":70,"context_line":"            boot_devices.CDROM: sushy.VIRTUAL_MEDIA_CD"},{"line_number":71,"context_line":"        }"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def _validate_vendor(self, task):"},{"line_number":74,"context_line":"        vendor \u003d task.node.properties.get(\u0027vendor\u0027)"},{"line_number":75,"context_line":"        if not vendor:"},{"line_number":76,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a9b7219_652757b8","line":73,"in_reply_to":"b12c1b5e_e557e60c","updated":"2021-02-04 15:22:42.000000000","message":"was just thinking that pattern could be applied globally to have consistent error message when misconfiguring. But, ok, for other cases connection to BMC will not be established - fail early.","commit_id":"6ac7ba6b4b37ffe13442b4d3d676373c75a8fbfa"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"e071f0922a4fd67259c361508d244052ddf2056d","unresolved":true,"context_lines":[{"line_number":75,"context_line":"        if not vendor:"},{"line_number":76,"context_line":"            return"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        if \u0027Dell\u0027 not in vendor.split():"},{"line_number":79,"context_line":"            raise exception.InvalidParameterValue("},{"line_number":80,"context_line":"                _(\"The %(iface)s boot interface is only suitable for nodes \""},{"line_number":81,"context_line":"                  \"produced by Dell EMC, but %(node)s has vendor %(vendor)s\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"22dcf4dc_a1a6f305","line":78,"range":{"start_line":78,"start_character":8,"end_line":78,"end_character":40},"updated":"2021-02-05 09:34:42.000000000","message":"This needs implementation of vendor detection in idrac-wsman management interface. It is possible to mix together WS-Man and Redfish interfaces, e.g., use WS-Man for every supported interface, but pick idrac-redfish-virtual-media for boot.\n\nAlternatively, could remove this validation altogether with belief that users will not pick idrac hw type for non-Dell hw (also this way validation will be consistently missing for all idrac-* interfaces leaving only special case for generic redfish-virtual-media).","commit_id":"6ac7ba6b4b37ffe13442b4d3d676373c75a8fbfa"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"b48eda0965d2f65fbf7a79710215cf19a642326d","unresolved":true,"context_lines":[{"line_number":75,"context_line":"        if not vendor:"},{"line_number":76,"context_line":"            return"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        if \u0027Dell\u0027 not in vendor.split():"},{"line_number":79,"context_line":"            raise exception.InvalidParameterValue("},{"line_number":80,"context_line":"                _(\"The %(iface)s boot interface is only suitable for nodes \""},{"line_number":81,"context_line":"                  \"produced by Dell EMC, but %(node)s has vendor %(vendor)s\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"652f911f_b4189484","line":78,"range":{"start_line":78,"start_character":8,"end_line":78,"end_character":40},"in_reply_to":"22dcf4dc_a1a6f305","updated":"2021-02-05 11:05:19.000000000","message":"On line 75 we will skip validation if there is no vendor detected. I think this is an improvement overall, even if not perfect.\n\nYou\u0027re welcome to provide a wsman implementation, but I\u0027m not a wsman/dracclient expert and cannot do it for you.","commit_id":"6ac7ba6b4b37ffe13442b4d3d676373c75a8fbfa"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"17ae346a383e2074fe3970c5324be9c9a7ebdc28","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        if not vendor:"},{"line_number":76,"context_line":"            return"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        if \u0027Dell\u0027 not in vendor.split():"},{"line_number":79,"context_line":"            raise exception.InvalidParameterValue("},{"line_number":80,"context_line":"                _(\"The %(iface)s boot interface is only suitable for nodes \""},{"line_number":81,"context_line":"                  \"produced by Dell EMC, but %(node)s has vendor %(vendor)s\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"306b6c53_3fd20c78","line":78,"range":{"start_line":78,"start_character":8,"end_line":78,"end_character":40},"in_reply_to":"652f911f_b4189484","updated":"2021-02-05 11:16:33.000000000","message":"Ack","commit_id":"6ac7ba6b4b37ffe13442b4d3d676373c75a8fbfa"}],"ironic/drivers/modules/redfish/boot.py":[{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"c131c051780869ad45be6987af11ac5f9b9e6489","unresolved":true,"context_lines":[{"line_number":382,"context_line":"        if \u0027Dell\u0027 in vendor.split():"},{"line_number":383,"context_line":"            raise exception.InvalidParameterValue("},{"line_number":384,"context_line":"                _(\"The %(iface)s boot interface is not suitable for node \""},{"line_number":385,"context_line":"                  \"%(node)s with vendor %(vendor)s, use \""},{"line_number":386,"context_line":"                  \"idrac-redfish-virtual-media instead\")"},{"line_number":387,"context_line":"                % {\u0027iface\u0027: task.node.boot_interface,"},{"line_number":388,"context_line":"                   \u0027node\u0027: task.node.uuid, \u0027vendor\u0027: vendor})"},{"line_number":389,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"4845109b_b85cb56f","line":386,"range":{"start_line":385,"start_character":52,"end_line":386,"end_character":54},"updated":"2021-02-08 05:55:22.000000000","message":"Better to also mention to change node hardware type to \u0027idrac\u0027.","commit_id":"cf22604c5863920b56e0b16dbf30894cbb544130"}],"ironic/drivers/modules/redfish/management.py":[{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"f11623af29b5b67f36af2111b1a74b9e80d08da3","unresolved":true,"context_lines":[{"line_number":677,"context_line":"                                \u0027component\u0027: component,"},{"line_number":678,"context_line":"                                \u0027uuid\u0027: task.node.uuid})"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"    def detect_vendor(self, task):"},{"line_number":681,"context_line":"        \"\"\"Detects, stores, and returns the hardware vendor."},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"        Uses the System\u0027s Manufacturer field."}],"source_content_type":"text/x-python","patch_set":2,"id":"6a5c5e4c_47247c7e","line":680,"range":{"start_line":680,"start_character":4,"end_line":680,"end_character":34},"updated":"2021-01-25 13:36:25.000000000","message":"Would it make sense to move this to the refactoring patch (https://review.opendev.org/c/openstack/ironic/+/771595) as it\u0027s fairly generic and not tied to virtual media?","commit_id":"ddb065cb36cc111117ff076ecd54cf0f700b5285"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6d530fd366c23afe1d848fc380ee47e93a7295c8","unresolved":true,"context_lines":[{"line_number":677,"context_line":"                                \u0027component\u0027: component,"},{"line_number":678,"context_line":"                                \u0027uuid\u0027: task.node.uuid})"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"    def detect_vendor(self, task):"},{"line_number":681,"context_line":"        \"\"\"Detects, stores, and returns the hardware vendor."},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"        Uses the System\u0027s Manufacturer field."}],"source_content_type":"text/x-python","patch_set":2,"id":"98a9f91c_4e586598","line":680,"range":{"start_line":680,"start_character":4,"end_line":680,"end_character":34},"in_reply_to":"6a5c5e4c_47247c7e","updated":"2021-01-25 14:59:08.000000000","message":"This change is potentially backportable, adding detect_vendor alone won\u0027t. I\u0027ll move it out in case I have to abandon this patch.","commit_id":"ddb065cb36cc111117ff076ecd54cf0f700b5285"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"e428df6d31390bbe0f8169a636262e2932172e64","unresolved":true,"context_lines":[{"line_number":678,"context_line":"                                \u0027uuid\u0027: task.node.uuid})"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"    def detect_vendor(self, task):"},{"line_number":681,"context_line":"        \"\"\"Detects, stores, and returns the hardware vendor."},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"        Uses the System\u0027s Manufacturer field."},{"line_number":684,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"6908e3d2_6c6f8eb7","line":681,"updated":"2021-01-27 23:15:06.000000000","message":"Whatever change this method ends up in, can the docstring be updated? See https://review.opendev.org/c/openstack/ironic/+/771595/1/ironic/drivers/modules/ipmitool.py#1228","commit_id":"ddb065cb36cc111117ff076ecd54cf0f700b5285"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"e428df6d31390bbe0f8169a636262e2932172e64","unresolved":true,"context_lines":[{"line_number":685,"context_line":"        :param task: A task from TaskManager."},{"line_number":686,"context_line":"        :raises: InvalidParameterValue if an invalid component, indicator"},{"line_number":687,"context_line":"            or state is specified."},{"line_number":688,"context_line":"        :raises: MissingParameterValue if a required parameter is missing"},{"line_number":689,"context_line":"        :returns: String representing the BMC reported Vendor or"},{"line_number":690,"context_line":"                  Manufacturer, otherwise returns None."},{"line_number":691,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"6593fcb2_f6821e79","line":688,"updated":"2021-01-27 23:15:06.000000000","message":"get_system raises RedfishConnectionError or RedfishError. InvalidParameterValue and MissingParameterValue are from ipmitool._parse_driver_info so I don\u0027t think these exceptions need to be part of the contract for every detect_vendor implementation.","commit_id":"ddb065cb36cc111117ff076ecd54cf0f700b5285"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"0bb0190f8219da893d62ee90e0f5bbbf272b228b","unresolved":true,"context_lines":[{"line_number":685,"context_line":"        :param task: A task from TaskManager."},{"line_number":686,"context_line":"        :raises: InvalidParameterValue if an invalid component, indicator"},{"line_number":687,"context_line":"            or state is specified."},{"line_number":688,"context_line":"        :raises: MissingParameterValue if a required parameter is missing"},{"line_number":689,"context_line":"        :returns: String representing the BMC reported Vendor or"},{"line_number":690,"context_line":"                  Manufacturer, otherwise returns None."},{"line_number":691,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"78684bde_b67d486a","line":688,"in_reply_to":"6593fcb2_f6821e79","updated":"2021-01-28 13:09:50.000000000","message":"Invalid/MissingParameterValue can be raised by virtually any driver since all of them rely on driver_info values.","commit_id":"ddb065cb36cc111117ff076ecd54cf0f700b5285"}]}