)]}'
{"doc/source/drivers/xenserver.rst":[{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"6120a03ed8ac446d3f5df94ba001684a9590f968","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"This is particularly useful for deploying overclouds that use"},{"line_number":20,"context_line":"XenServer for VM hosting as the Compute node must be run as a virtual"},{"line_number":21,"context_line":"machine on the XenServer host it wil be controlling."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Usage"},{"line_number":24,"context_line":"\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":7,"id":"7a2fa921_b3ef6b86","line":21,"updated":"2015-10-07 16:03:37.000000000","message":"nit - s/wil/will","commit_id":"55a103d7b132ceaa18d235cbb602564f27ec50bf"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"ef530de1a16e0d12f1a48c1ccc65be767ad0a941","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"This is particularly useful for deploying overclouds that use"},{"line_number":20,"context_line":"XenServer for VM hosting as the Compute node must be run as a virtual"},{"line_number":21,"context_line":"machine on the XenServer host it wil be controlling."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Usage"},{"line_number":24,"context_line":"\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":7,"id":"7a2fa921_d77c33b7","line":21,"in_reply_to":"7a2fa921_b3ef6b86","updated":"2015-10-08 08:57:53.000000000","message":"Done","commit_id":"55a103d7b132ceaa18d235cbb602564f27ec50bf"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"6120a03ed8ac446d3f5df94ba001684a9590f968","unresolved":false,"context_lines":[{"line_number":23,"context_line":"Usage"},{"line_number":24,"context_line":"\u003d\u003d\u003d\u003d\u003d"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"* Install one VM per hypervisor using XenServer 6.5 and the \"Other Install Media\" template, which will ensure that they are HVM guests"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"* Set the HVM guests to boot from network first"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"7a2fa921_f3c96379","line":26,"updated":"2015-10-07 16:03:37.000000000","message":"IIUC this explains overcloud usecase, maybe move \u0027Install one VM per hypervisor\u0027 to the overview, and here just mention something like \u0027Create bunch of vms using xenserver...\u0027?","commit_id":"55a103d7b132ceaa18d235cbb602564f27ec50bf"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"ef530de1a16e0d12f1a48c1ccc65be767ad0a941","unresolved":false,"context_lines":[{"line_number":23,"context_line":"Usage"},{"line_number":24,"context_line":"\u003d\u003d\u003d\u003d\u003d"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"* Install one VM per hypervisor using XenServer 6.5 and the \"Other Install Media\" template, which will ensure that they are HVM guests"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"* Set the HVM guests to boot from network first"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"7a2fa921_774c3f9e","line":26,"in_reply_to":"7a2fa921_f3c96379","updated":"2015-10-08 08:57:53.000000000","message":"Done","commit_id":"55a103d7b132ceaa18d235cbb602564f27ec50bf"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"b3ee9b5b23f220db1c9f0399afeeffa7f2363370","unresolved":false,"context_lines":[{"line_number":21,"context_line":"machine on the XenServer host it will be controlling."},{"line_number":22,"context_line":"In this case, one VM per hypervisor needs to be installed."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This support has been tested with XenServer 6.5"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Usage"},{"line_number":27,"context_line":"\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":8,"id":"5a2ca52d_3fc0192a","line":24,"updated":"2015-10-14 19:42:54.000000000","message":"nit: s/6.5/6.5./","commit_id":"135d7f4accf7af6460fa29c6ad492ebe5288384e"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"b3ee9b5b23f220db1c9f0399afeeffa7f2363370","unresolved":false,"context_lines":[{"line_number":30,"context_line":""},{"line_number":31,"context_line":"* Set the HVM guests to boot from network first"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"* If your generated initramfs does not have the fix for `bug 1498576`_, disable the Xen PV drivers as a work around"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"::"},{"line_number":36,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"5a2ca52d_9f0ccd95","line":33,"updated":"2015-10-14 19:42:54.000000000","message":"please make sure these lines don\u0027t exceed 79 chars.","commit_id":"135d7f4accf7af6460fa29c6ad492ebe5288384e"}],"ironic/drivers/modules/ssh.py":[{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"67b41db8a376095d4a6bdfd1f768dd06f0731093","unresolved":false,"context_lines":[{"line_number":61,"context_line":"    \u0027ssh_address\u0027: _(\"IP address or hostname of the node to ssh into. \""},{"line_number":62,"context_line":"                     \"Required.\"),"},{"line_number":63,"context_line":"    \u0027ssh_username\u0027: _(\"username to authenticate as. Required.\"),"},{"line_number":64,"context_line":"    \u0027ssh_virt_type\u0027: _(\"virtualization software to use; one of vbox, virsh, \""},{"line_number":65,"context_line":"                       \"vmware, parallels. Required.\")"},{"line_number":66,"context_line":"}"},{"line_number":67,"context_line":"OTHER_PROPERTIES \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_a12f59a0","line":64,"updated":"2015-10-06 10:41:57.000000000","message":"Needs updating too.","commit_id":"f60e0b09cce6d01a20bb1bb06018e9e48e97a41d"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"4ca94ccf669e4cd94e40a8144a7b9e6c14f39b8d","unresolved":false,"context_lines":[{"line_number":61,"context_line":"    \u0027ssh_address\u0027: _(\"IP address or hostname of the node to ssh into. \""},{"line_number":62,"context_line":"                     \"Required.\"),"},{"line_number":63,"context_line":"    \u0027ssh_username\u0027: _(\"username to authenticate as. Required.\"),"},{"line_number":64,"context_line":"    \u0027ssh_virt_type\u0027: _(\"virtualization software to use; one of vbox, virsh, \""},{"line_number":65,"context_line":"                       \"vmware, parallels. Required.\")"},{"line_number":66,"context_line":"}"},{"line_number":67,"context_line":"OTHER_PROPERTIES \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_8826f966","line":64,"in_reply_to":"7a2fa921_a12f59a0","updated":"2015-10-06 13:24:32.000000000","message":"Done","commit_id":"f60e0b09cce6d01a20bb1bb06018e9e48e97a41d"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"67b41db8a376095d4a6bdfd1f768dd06f0731093","unresolved":false,"context_lines":[{"line_number":228,"context_line":"    elif virt_type \u003d\u003d \u0027xenserver\u0027:"},{"line_number":229,"context_line":"        return {"},{"line_number":230,"context_line":"            \u0027base_cmd\u0027: \u0027LC_ALL\u003dC /opt/xensource/bin/xe\u0027,"},{"line_number":231,"context_line":"            \u0027start_cmd\u0027: \u0027vm-start uuid\u003d{_NodeName_} \u0026\u0026 sleep 10s\u0027,"},{"line_number":232,"context_line":"            \u0027stop_cmd\u0027: \u0027vm-shutdown uuid\u003d{_NodeName_} force\u003dtrue\u0027,"},{"line_number":233,"context_line":"            \u0027reboot_cmd\u0027: \u0027vm-reboot uuid\u003d{_NodeName_}\u0027,"},{"line_number":234,"context_line":"            \u0027list_all\u0027: \"vm-list --minimal | tr \u0027,\u0027 \u0027\\n\u0027\","}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_8136dd01","line":231,"updated":"2015-10-06 10:41:57.000000000","message":"Question - why is sleep added here?","commit_id":"f60e0b09cce6d01a20bb1bb06018e9e48e97a41d"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"4ca94ccf669e4cd94e40a8144a7b9e6c14f39b8d","unresolved":false,"context_lines":[{"line_number":228,"context_line":"    elif virt_type \u003d\u003d \u0027xenserver\u0027:"},{"line_number":229,"context_line":"        return {"},{"line_number":230,"context_line":"            \u0027base_cmd\u0027: \u0027LC_ALL\u003dC /opt/xensource/bin/xe\u0027,"},{"line_number":231,"context_line":"            \u0027start_cmd\u0027: \u0027vm-start uuid\u003d{_NodeName_} \u0026\u0026 sleep 10s\u0027,"},{"line_number":232,"context_line":"            \u0027stop_cmd\u0027: \u0027vm-shutdown uuid\u003d{_NodeName_} force\u003dtrue\u0027,"},{"line_number":233,"context_line":"            \u0027reboot_cmd\u0027: \u0027vm-reboot uuid\u003d{_NodeName_}\u0027,"},{"line_number":234,"context_line":"            \u0027list_all\u0027: \"vm-list --minimal | tr \u0027,\u0027 \u0027\\n\u0027\","}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_888279ff","line":231,"in_reply_to":"7a2fa921_8136dd01","updated":"2015-10-06 13:24:32.000000000","message":"The sleep is due to what appeared to be a race (due to a bug in XenServer) - after start_cmd the power-state of the VM had not been updated, so Ironic thought the VM was not yet running and attempted to start a second time.\n\nIt seems therefore that start_cmd must not return until power_state is updated.","commit_id":"f60e0b09cce6d01a20bb1bb06018e9e48e97a41d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"9072378abe48ff0581b766c990f31f4c4bd7ebb0","unresolved":false,"context_lines":[{"line_number":228,"context_line":"    elif virt_type \u003d\u003d \u0027xenserver\u0027:"},{"line_number":229,"context_line":"        return {"},{"line_number":230,"context_line":"            \u0027base_cmd\u0027: \u0027LC_ALL\u003dC /opt/xensource/bin/xe\u0027,"},{"line_number":231,"context_line":"            \u0027start_cmd\u0027: \u0027vm-start uuid\u003d{_NodeName_} \u0026\u0026 sleep 10s\u0027,"},{"line_number":232,"context_line":"            \u0027stop_cmd\u0027: \u0027vm-shutdown uuid\u003d{_NodeName_} force\u003dtrue\u0027,"},{"line_number":233,"context_line":"            \u0027reboot_cmd\u0027: \u0027vm-reboot uuid\u003d{_NodeName_}\u0027,"},{"line_number":234,"context_line":"            \u0027list_all\u0027: \"vm-list --minimal | tr \u0027,\u0027 \u0027\\n\u0027\","}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_ab816f4c","line":231,"updated":"2015-10-06 13:35:59.000000000","message":"The sleep 10 here is a bit odd, I saw your comment on a previous patch set about it. Mind adding a comment inline explaning why we have to sleep here? Others may raise the same question when reading the code","commit_id":"326814469f11c6307032f9c63e9ccfae7b273cbc"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"6120a03ed8ac446d3f5df94ba001684a9590f968","unresolved":false,"context_lines":[{"line_number":119,"context_line":"def _get_command_sets(virt_type):"},{"line_number":120,"context_line":"    \"\"\"Retrieves the virt_type-specific commands to control power"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    Required commands are as base:"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    follows_cmd: Used by most sub-commands as the primary executable"},{"line_number":125,"context_line":"    list_all: Lists all VMs (by virt_type identifier) that can be managed."}],"source_content_type":"text/x-python","patch_set":7,"id":"7a2fa921_98392e44","line":122,"updated":"2015-10-07 16:03:37.000000000","message":"This should not be changed I guess :)","commit_id":"55a103d7b132ceaa18d235cbb602564f27ec50bf"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"ef530de1a16e0d12f1a48c1ccc65be767ad0a941","unresolved":false,"context_lines":[{"line_number":119,"context_line":"def _get_command_sets(virt_type):"},{"line_number":120,"context_line":"    \"\"\"Retrieves the virt_type-specific commands to control power"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    Required commands are as base:"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    follows_cmd: Used by most sub-commands as the primary executable"},{"line_number":125,"context_line":"    list_all: Lists all VMs (by virt_type identifier) that can be managed."}],"source_content_type":"text/x-python","patch_set":7,"id":"7a2fa921_573ee3da","line":122,"in_reply_to":"7a2fa921_98392e44","updated":"2015-10-08 08:57:53.000000000","message":"I have no idea how this change got in there! That\u0027s a pretty obvious change... Can\u0027t even think of any sed magic I might have done to do it automatically.\n\nThanks for pointing it out.","commit_id":"55a103d7b132ceaa18d235cbb602564f27ec50bf"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"b3ee9b5b23f220db1c9f0399afeeffa7f2363370","unresolved":false,"context_lines":[{"line_number":248,"context_line":"            \u0027get_boot_device\u0027: ("},{"line_number":249,"context_line":"                \"{_BaseCmd_} vm-param-get uuid\u003d{_NodeName_}\""},{"line_number":250,"context_line":"                \" --param-name\u003dHVM-boot-params param-key\u003dorder | cut -b 1\"),"},{"line_number":251,"context_line":"            \u0027set_boot_device\u0027: ("},{"line_number":252,"context_line":"                \"{_BaseCmd_} vm-param-set uuid\u003d{_NodeName_}\""},{"line_number":253,"context_line":"                \" HVM-boot-params:order\u003d\u0027{_BootDevice_}\u0027\"),"},{"line_number":254,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a2ca52d_9f44cd74","line":251,"updated":"2015-10-14 19:42:54.000000000","message":"nit: since all the others have \u0027set_boot_device\u0027 listed before \u0027get_boot_device\u0027, it\u0027d be nice if this was in the same order.\n\n(Not sure why that order but might as well be consistent.)","commit_id":"135d7f4accf7af6460fa29c6ad492ebe5288384e"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"b974ec2efb224cf02489078563da3159ba07a519","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                \"{_BaseCmd_} list -i {_NodeName_} | \""},{"line_number":236,"context_line":"                \"awk \u0027/^Boot order:/ {print $3}\u0027\"),"},{"line_number":237,"context_line":"        }"},{"line_number":238,"context_line":"    elif virt_type \u003d\u003d \u0027xenserver\u0027:"},{"line_number":239,"context_line":"        return {"},{"line_number":240,"context_line":"            \u0027base_cmd\u0027: \u0027LC_ALL\u003dC /opt/xensource/bin/xe\u0027,"},{"line_number":241,"context_line":"            # Note(bobba): XenServer appears to have a condition where"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a29b11f_3633c2f7","line":238,"updated":"2015-10-23 08:26:05.000000000","message":"Lots of substitutions and other things going on in strings below, but only a few are tested - get_boot_device() and set_boot_device() as far as i can see. Can we please have test cases for others as well.","commit_id":"3846373fa55eb45cbada8dc9eb8db505af0b350c"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"55f5a7846bc06dea7fe12ea98f2f43776eb6c5e8","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                \"{_BaseCmd_} list -i {_NodeName_} | \""},{"line_number":236,"context_line":"                \"awk \u0027/^Boot order:/ {print $3}\u0027\"),"},{"line_number":237,"context_line":"        }"},{"line_number":238,"context_line":"    elif virt_type \u003d\u003d \u0027xenserver\u0027:"},{"line_number":239,"context_line":"        return {"},{"line_number":240,"context_line":"            \u0027base_cmd\u0027: \u0027LC_ALL\u003dC /opt/xensource/bin/xe\u0027,"},{"line_number":241,"context_line":"            # Note(bobba): XenServer appears to have a condition where"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a29b11f_77de14de","line":238,"in_reply_to":"3a29b11f_173d608e","updated":"2015-10-23 10:08:04.000000000","message":"Thanks a lot.\n\n  \u003e\u003e they are the easiest to break tests, and the easiest to \n  \u003e\u003e \"fix to match the expected result\" without any confirmation \n  \u003e\u003e that they will work in the real world.\n\nIt is true.  Well, that\u0027s the way most of stuffs are unit tested in Ironic today.  Ironic runs lots of external commands - partitioning, mounting, writing image, iscsi* commands, blkid, etc. The only way to reliably \"unit test\" is to check if the string command generated is the one we expect it to be.  Yes, the unit test breaks more often, but hey it is a good thing in my opinion.  The unit test has done what it was supposed to do ;-)","commit_id":"3846373fa55eb45cbada8dc9eb8db505af0b350c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"180effa644d15da81ef4f435d72002d2a1e14602","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                \"{_BaseCmd_} list -i {_NodeName_} | \""},{"line_number":236,"context_line":"                \"awk \u0027/^Boot order:/ {print $3}\u0027\"),"},{"line_number":237,"context_line":"        }"},{"line_number":238,"context_line":"    elif virt_type \u003d\u003d \u0027xenserver\u0027:"},{"line_number":239,"context_line":"        return {"},{"line_number":240,"context_line":"            \u0027base_cmd\u0027: \u0027LC_ALL\u003dC /opt/xensource/bin/xe\u0027,"},{"line_number":241,"context_line":"            # Note(bobba): XenServer appears to have a condition where"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a29b11f_767ffa14","line":238,"in_reply_to":"3a29b11f_3633c2f7","updated":"2015-10-23 08:42:30.000000000","message":"I\u0027m not convinced there is value in duplicating the commands returned from __get_command_sets, particularly because that is such a simple function. It would be a different matter if we could have a unit test that simulated the output from base_cmd and piped that to any remaining commands - but I don\u0027t like the idea of unit tests spawning shells to run \u0027tr\u0027 or \u0027cut\u0027, etc.\n\nThe tests I\u0027ve added so far are just to add the equivalents of the existing tests.\n\nWhat do you see as the value of adding further tests to check the output of __get_command_sets?","commit_id":"3846373fa55eb45cbada8dc9eb8db505af0b350c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"70015ffe521e637aa7dce493cb5ff186bd0ab812","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                \"{_BaseCmd_} list -i {_NodeName_} | \""},{"line_number":236,"context_line":"                \"awk \u0027/^Boot order:/ {print $3}\u0027\"),"},{"line_number":237,"context_line":"        }"},{"line_number":238,"context_line":"    elif virt_type \u003d\u003d \u0027xenserver\u0027:"},{"line_number":239,"context_line":"        return {"},{"line_number":240,"context_line":"            \u0027base_cmd\u0027: \u0027LC_ALL\u003dC /opt/xensource/bin/xe\u0027,"},{"line_number":241,"context_line":"            # Note(bobba): XenServer appears to have a condition where"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a29b11f_173d608e","line":238,"in_reply_to":"3a29b11f_5c48dfb2","updated":"2015-10-23 09:50:10.000000000","message":"I made my case that I don\u0027t think they are useful, but I do understand why you think they are.  As you have much more investment in the project than I do, I will happily implement the tests - I\u0027ll try and get a new patch today.\n\nI just cringe at unit tests that test for exact complicated strings rather than expected functionality - they are the easiest to break tests, and the easiest to \"fix to match the expected result\" without any confirmation that they will work in the real world.","commit_id":"3846373fa55eb45cbada8dc9eb8db505af0b350c"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"07dfce6073b7a2f18f7150188506e3a25bedc0ed","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                \"{_BaseCmd_} list -i {_NodeName_} | \""},{"line_number":236,"context_line":"                \"awk \u0027/^Boot order:/ {print $3}\u0027\"),"},{"line_number":237,"context_line":"        }"},{"line_number":238,"context_line":"    elif virt_type \u003d\u003d \u0027xenserver\u0027:"},{"line_number":239,"context_line":"        return {"},{"line_number":240,"context_line":"            \u0027base_cmd\u0027: \u0027LC_ALL\u003dC /opt/xensource/bin/xe\u0027,"},{"line_number":241,"context_line":"            # Note(bobba): XenServer appears to have a condition where"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a29b11f_f6146aa0","line":238,"in_reply_to":"3a29b11f_767ffa14","updated":"2015-10-23 08:55:07.000000000","message":"The value I am seeing in extending the unit tests for every command is the same value that I am seeing in doing it for the commands get_boot_device and set_boot_device.  What if we made a typo in {_NodeName_} and mentioned it like {NodeName}.  Writing unit tests for that, and seeing that  node uuid gets substituted for {_NodeName_} gives me confirmation that this is working fine.  \n\nExercise start_cmd, make sure that it called \n\n  LC_ALL\u003dC /opt/xensource/bin/xe vm-start uuid\u003d\u003cnode-uuid\u003e \u0026\u0026 sleep 10s\n  \n\nAfter all, we write unit tests to prove that our code works.\n\nI agree the fact that \"hey we don\u0027t do it for others, why should we do it for this one?\", but yeah it\u0027s good to do and makes it only better in my opinion. \n\nThoughts ?","commit_id":"3846373fa55eb45cbada8dc9eb8db505af0b350c"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"7282adc720fa17c03e06091c1bbbf57bc812c1f1","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                \"{_BaseCmd_} list -i {_NodeName_} | \""},{"line_number":236,"context_line":"                \"awk \u0027/^Boot order:/ {print $3}\u0027\"),"},{"line_number":237,"context_line":"        }"},{"line_number":238,"context_line":"    elif virt_type \u003d\u003d \u0027xenserver\u0027:"},{"line_number":239,"context_line":"        return {"},{"line_number":240,"context_line":"            \u0027base_cmd\u0027: \u0027LC_ALL\u003dC /opt/xensource/bin/xe\u0027,"},{"line_number":241,"context_line":"            # Note(bobba): XenServer appears to have a condition where"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a29b11f_5c48dfb2","line":238,"in_reply_to":"3a29b11f_7c991bc3","updated":"2015-10-23 09:40:02.000000000","message":"\u003e Would a useful test be to check the output from __get_command_sets \n\u003e (for each virt driver) and assert that {_NodeName_} is present in the \n\u003e commands we expect it to be present in?\n\nIf we had code for each virt driver differently, then wouldn\u0027t it be useful\nto assert every different path of code available is unit tested ?\n\n\u003e The issue I have is that none of these tests check that this code works \n\u003e because the real code will all run external commands and the tests are \n\u003e simply checking that the string is the same as the string returned from \n\u003e __get_command_sets, but with substitution for {_NodeName_}. As such, the \n\u003e actual code that is being tested in each of these virt driver tests is \n\u003e identical - they are testing the framework around __get_command_sets.\n\nIt does more than that in my opinion. As you said, it tests {_NodeName_} is\nsubstituted or not. That\u0027s first thing.  If asserting only {_NodeName_} is \nsubstituted was our motive, then we needn\u0027t have separate unit tests for each \nof the different virt drivers.  I mean we needn\u0027t have three of below \n(excluding newly proposed one) of the below just to assert if {_NodeName_} is \nsubstituted.  \n\n  $ grep test_management_interface_get_boot_device ironic/tests/unit/drivers/modules/test_ssh.py\n      def test_management_interface_get_boot_device_vbox(self, mock_exc,\n      def test_management_interface_get_boot_device_parallels(self, mock_exc,\n      def test_management_interface_get_boot_device_virsh(self, mock_exc,\n      def test_management_interface_get_boot_device_xenserver(self, mock_exc,\n  $\n  \nBut we have it today and I feel the reason it should be is because we wanted a\nconfirmation that each command on the virt driver is just going to run fine\nas we expect it.\n  \n\u003e I think for useful unit tests, what we need is to abstract the base command \n\u003e run from some python code that modifies the output from the virt command \n\u003e into what the ssh driver is expecting; although that\u0027s clearly beyond the \n\u003e scope of this addition?\n\nYes we can have that and that\u0027s clearly out of scope.  What I was just asking\nwas if we can add unit tests for other commands (especially for the ones that\ndoes substitutions) to be unit tested in the similar way that is already done\nfor two of the commands.\n\nFeel free to disagree ;-)\n\nSince we both disagree, I wonder what other feel about it.","commit_id":"3846373fa55eb45cbada8dc9eb8db505af0b350c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"6a0df608add4ce67a1b25ed55fc9b1940d71ba96","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                \"{_BaseCmd_} list -i {_NodeName_} | \""},{"line_number":236,"context_line":"                \"awk \u0027/^Boot order:/ {print $3}\u0027\"),"},{"line_number":237,"context_line":"        }"},{"line_number":238,"context_line":"    elif virt_type \u003d\u003d \u0027xenserver\u0027:"},{"line_number":239,"context_line":"        return {"},{"line_number":240,"context_line":"            \u0027base_cmd\u0027: \u0027LC_ALL\u003dC /opt/xensource/bin/xe\u0027,"},{"line_number":241,"context_line":"            # Note(bobba): XenServer appears to have a condition where"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a29b11f_7c991bc3","line":238,"in_reply_to":"3a29b11f_f6146aa0","updated":"2015-10-23 09:25:22.000000000","message":"Would a useful test be to check the output from __get_command_sets (for each virt driver) and assert that {_NodeName_} is present in the commands we expect it to be present in?\n\nThe issue I have is that none of these tests check that this code works because the real code will all run external commands and the tests are simply checking that the string is the same as the string returned from __get_command_sets, but with substitution for {_NodeName_}\n\nAs such, the actual code that is being tested in each of these virt driver tests is identical - they are testing the framework around __get_command_sets.\n\nI think for useful unit tests, what we need is to abstract the base command run from some python code that modifies the output from the virt command into what the ssh driver is expecting; although that\u0027s clearly beyond the scope of this addition?","commit_id":"3846373fa55eb45cbada8dc9eb8db505af0b350c"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"14e2bc9619c23706f2192f6159e47f150ff5c508","unresolved":false,"context_lines":[{"line_number":237,"context_line":"        }"},{"line_number":238,"context_line":"    elif virt_type \u003d\u003d \u0027xenserver\u0027:"},{"line_number":239,"context_line":"        return {"},{"line_number":240,"context_line":"            \u0027base_cmd\u0027: \u0027LC_ALL\u003dC /opt/xensource/bin/xe\u0027,"},{"line_number":241,"context_line":"            # Note(bobba): XenServer appears to have a condition where"},{"line_number":242,"context_line":"            #              vm-start can return before the power-state"},{"line_number":243,"context_line":"            #              has been updated to \u0027running\u0027.  Ironic"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a29b11f_e899067a","line":240,"updated":"2015-10-22 14:54:46.000000000","message":"Is xen stuff placed into /opt by default?","commit_id":"3846373fa55eb45cbada8dc9eb8db505af0b350c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"180effa644d15da81ef4f435d72002d2a1e14602","unresolved":false,"context_lines":[{"line_number":237,"context_line":"        }"},{"line_number":238,"context_line":"    elif virt_type \u003d\u003d \u0027xenserver\u0027:"},{"line_number":239,"context_line":"        return {"},{"line_number":240,"context_line":"            \u0027base_cmd\u0027: \u0027LC_ALL\u003dC /opt/xensource/bin/xe\u0027,"},{"line_number":241,"context_line":"            # Note(bobba): XenServer appears to have a condition where"},{"line_number":242,"context_line":"            #              vm-start can return before the power-state"},{"line_number":243,"context_line":"            #              has been updated to \u0027running\u0027.  Ironic"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a29b11f_f6b8eac9","line":240,"in_reply_to":"3a29b11f_e899067a","updated":"2015-10-23 08:42:30.000000000","message":"This is where XenServer\u0027s commands are stored, yes.  It\u0027s not true of libvirt+xen but this power driver is specifically for XenServer.","commit_id":"3846373fa55eb45cbada8dc9eb8db505af0b350c"}],"ironic/tests/unit/drivers/modules/test_ssh.py":[{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"6120a03ed8ac446d3f5df94ba001684a9590f968","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        self.assertEqual(expected_base_cmd, info[\u0027cmd_set\u0027][\u0027base_cmd\u0027])"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    def test__get_boot_device_map_parallels(self):"},{"line_number":181,"context_line":"        boot_map \u003d ssh._get_boot_device_map(\u0027parallels\u0027)"},{"line_number":182,"context_line":"        self.assertEqual(\u0027net0\u0027, boot_map[boot_devices.PXE])"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def test__get_boot_device_map_vbox(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a2fa921_93802f48","line":181,"updated":"2015-10-07 16:03:37.000000000","message":"There are also these tests, maybe worth adding one for xenserver too.","commit_id":"55a103d7b132ceaa18d235cbb602564f27ec50bf"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"ef530de1a16e0d12f1a48c1ccc65be767ad0a941","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        self.assertEqual(expected_base_cmd, info[\u0027cmd_set\u0027][\u0027base_cmd\u0027])"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    def test__get_boot_device_map_parallels(self):"},{"line_number":181,"context_line":"        boot_map \u003d ssh._get_boot_device_map(\u0027parallels\u0027)"},{"line_number":182,"context_line":"        self.assertEqual(\u0027net0\u0027, boot_map[boot_devices.PXE])"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def test__get_boot_device_map_vbox(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a2fa921_123ab9cc","line":181,"in_reply_to":"7a2fa921_93802f48","updated":"2015-10-08 08:57:53.000000000","message":"Done","commit_id":"55a103d7b132ceaa18d235cbb602564f27ec50bf"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"b3ee9b5b23f220db1c9f0399afeeffa7f2363370","unresolved":false,"context_lines":[{"line_number":837,"context_line":"    @mock.patch.object(ssh, \u0027_get_connection\u0027, autospec\u003dTrue)"},{"line_number":838,"context_line":"    @mock.patch.object(ssh, \u0027_get_hosts_name_for_node\u0027, autospec\u003dTrue)"},{"line_number":839,"context_line":"    @mock.patch.object(ssh, \u0027_ssh_execute\u0027, autospec\u003dTrue)"},{"line_number":840,"context_line":"    def test_management_interface_set_boot_device_virsh_ok(self, mock_exc,"},{"line_number":841,"context_line":"                                                           mock_h,"},{"line_number":842,"context_line":"                                                           mock_get_conn):"},{"line_number":843,"context_line":"        fake_name \u003d \u0027fake-name\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a2ca52d_9a205b9d","line":840,"updated":"2015-10-14 19:42:54.000000000","message":"Just to be consistent, maybe add a similar test for xenserver.","commit_id":"135d7f4accf7af6460fa29c6ad492ebe5288384e"}]}