Alon Bar-Lev has posted comments on this change. Change subject: services, setup: Serial console proxy [WIP] ......................................................................
Patch Set 2: (9 comments) ok, I was confused between the "services" content and that they actually not a service... the sshd service is not available yet. I am unsure that the service set up logger should be used by these. http://gerrit.ovirt.org/#/c/35906/2/packaging/services/ovirt-console-proxy/vmproxy File packaging/services/ovirt-console-proxy/vmproxy: Line 20: Line 21: return user_guid, vm_name Line 22: Line 23: def run_virsh_console(vm, host): Line 24: os.execlp('virsh', 'virsh', '-c', 'qemu+tls://' + host + '/system', 'console', vm) what is the authentication between the virsh and the destination? Line 25: Line 26: def main(): Line 27: # FIXME: Logger Line 28: Line 43: while True: Line 44: vm_list = consoles.keys() Line 45: Line 46: for index, vm in enumerate(vm_list): Line 47: print "%d. %s" % (index+1, vm) please make sure you are python3 ready. Line 48: Line 49: try: Line 50: vm_index = int(raw_input('> ')) Line 51: except ValueError: Line 46: for index, vm in enumerate(vm_list): Line 47: print "%d. %s" % (index+1, vm) Line 48: Line 49: try: Line 50: vm_index = int(raw_input('> ')) python3 alert here as well. Line 51: except ValueError: Line 52: pass Line 53: else: Line 54: if 1 <= vm_index <= len(consoles): Line 47: print "%d. %s" % (index+1, vm) Line 48: Line 49: try: Line 50: vm_index = int(raw_input('> ')) Line 51: except ValueError: please do not swallow exceptions. Line 52: pass Line 53: else: Line 54: if 1 <= vm_index <= len(consoles): Line 55: vm_name = vm_list[vm_index-1] Line 49: try: Line 50: vm_index = int(raw_input('> ')) Line 51: except ValueError: Line 52: pass Line 53: else: why pass and else? pattern is: try: do something process result on success path except ...: handle errors Line 54: if 1 <= vm_index <= len(consoles): Line 55: vm_name = vm_list[vm_index-1] Line 56: run_virsh_console(vm_name, consoles[vm_name]) Line 57: else: http://gerrit.ovirt.org/#/c/35906/2/packaging/services/ovirt-console-proxy/vmproxy_authkeys File packaging/services/ovirt-console-proxy/vmproxy_authkeys: Line 6: Line 7: import os Line 8: import sys Line 9: Line 10: authkeys_line = 'command="%s --vm-name \\"${SSH_ORIGINAL_COMMAND}\\" --user-guid \\"%s\\"",no-agent-forwarding,no-port-forwarding,no-user-rc,no-X11-forwarding %s' command must be quoted. not sure why original command should be applied here, you get it at environment anyway, and this command may be more than just vm name. Line 11: Line 12: def main(): Line 13: service.setupLogger() Line 14: Line 25: Line 26: users = get_json(encode_url(base_url, 'publickeys')) Line 27: Line 28: for guid, public_key in users: Line 29: print authkeys_line % (vmproxy_path, guid, public_key) please make sure you are python3 ready. Line 30: Line 31: if __name__ == "__main__": http://gerrit.ovirt.org/#/c/35906/2/packaging/setup/plugins/ovirt-engine-setup/console_proxy/config.py File packaging/setup/plugins/ovirt-engine-setup/console_proxy/config.py: Line 177: ), Line 178: condition=lambda self: self._enabled, Line 179: ) Line 180: def _misc_encrypted(self): Line 181: vdcoption.VdcOption( if you design to install this remotely you cannot assume you have database credentials nor access. Line 182: statement=self.environment[ Line 183: oenginecons.EngineDBEnv.STATEMENT Line 184: ] Line 185: ).updateVdcOptions( http://gerrit.ovirt.org/#/c/35906/2/packaging/setup/plugins/ovirt-engine-setup/console_proxy/pki.py File packaging/setup/plugins/ovirt-engine-setup/console_proxy/pki.py: Line 52: ), Line 53: condition=lambda self: self.environment[ Line 54: ocpcons.ConfigEnv.CONSOLE_PROXY_CONFIG Line 55: ], Line 56: ) all your closing ')' are misplaced, should be at same column of the opening expression. Line 57: def _misc(self): Line 58: rc, stdout, stderr = self.execute( Line 59: args=( Line 60: oenginecons.FileLocations.OVIRT_ENGINE_PKI_PKCS12_EXTRACT, -- To view, visit http://gerrit.ovirt.org/35906 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I034ef8e6d10da5dc93eda61e0c5c518ca13a5a28 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vitor de Lima <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
