[PATCH] [Kimchi 0/9] Virt-Viewer launcher backend
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch set adds the Virt-Viewer launcher backend to Kimchi. This feature consists of a new resource located in: **URI:** /plugins/kimchi/vms/*:name*/snapshots/current that retrieves a download link to a .vv file to be used by a Virt-Viewer compatible desktop app to connect to the remote virtual machine. This backend takes cares of handling firewall rules to allow the connection to be succesfull. Note that no firewall port will be opened unless a download is made - if the user decides to use noVNC or spice-html5 instead Kimchi will not touch the host firewall. Example: [danielhb@arthas kimchi]$ curl -u root -H "Content-Type: application/json" -H "Accept: application/json" -X GET "http://localhost:8010/plugins/kimchi/vms/OpenSUSE-Leap-42.1/virtviewerfile" Enter host password for user 'root': [virt-viewer] type=vnc host=localhost port=5904 After this call, port 5904 was opened in the host to allow for a virt-viewer connection. When shutting down the virtual machine or WoK, a cleanup is made to close any ports left opened. Daniel Henrique Barboza (9): Virt-Viewer launcher: docs and i18n changes Virt-Viewer launcher: Makefile and config changes Virt-Viewer launcher: control/vms.py and model/vms.py changes Virt-Viewer launcher: virtviewerfile module Virt-Viewer launcher: test changes Virt-Viewer launcher: adding FirewallManager class Virt-Viewer launcher: test changes for firewall manager Virt-Viewer launcher: libvirt events to control firewall Virt-Viewer launcher: changes after adding libvirt event listening Makefile.am | 2 + config.py.in | 10 ++- control/vms.py | 12 +++ docs/API.md | 6 ++ i18n.py | 2 + model/virtviewerfile.py | 234 ++++++++++++++++++++++++++++++++++++++++++++++++ model/vms.py | 9 +- tests/test_config.py.in | 6 ++ tests/test_model.py | 219 +++++++++++++++++++++++++++++++++++++++++++- 9 files changed, 494 insertions(+), 6 deletions(-) create mode 100644 model/virtviewerfile.py -- 2.5.5
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch adds the documentation and i18n changes for the new Virt-Viewer launcher API. URI: plugins/kimchi/vms/*:name*/virtviewerfile 'Retrieve the Virt-Viewer launcher file to connect to the graphics of this virtual machine. Virtual machine must be running.' Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- docs/API.md | 6 ++++++ i18n.py | 2 ++ 2 files changed, 8 insertions(+) diff --git a/docs/API.md b/docs/API.md index 84ef7c9..78c0574 100644 --- a/docs/API.md +++ b/docs/API.md @@ -286,6 +286,12 @@ Represents a snapshot of the Virtual Machine's primary monitor. **URI:** /plugins/kimchi/vms/*:name*/snapshots/current * **GET**: Retrieve current snapshot information for the virtual machine. +### Sub-resource: Virt-Viewer File +**URI:** /plugins/kimchi/vms/*:name*/virtviewerfile +* **GET**: Retrieve the Virt-Viewer launcher file to connect to the + graphics of this virtual machine. Virtual machine must + be running. + ### Collection: Templates **URI:** /plugins/kimchi/templates diff --git a/i18n.py b/i18n.py index 08a3ac8..deb0f8c 100644 --- a/i18n.py +++ b/i18n.py @@ -131,6 +131,8 @@ messages = { "KCHVM0080E": _("Cannot update Maximum Memory when guest is running."), "KCHVM0081E": _("Impossible to create %(dir)s directory."), "KCHVM0082E": _("Either the guest %(name)s did not start to listen to the serial or it is not configured to use the serial console."), + "KCHVM0083E": _("Unable to retrieve virt viever file for stopped virtual machine %(name)s"), + "KCHVM0084E": _("Error occured while retrieving the virt viewer file for virtual machine %(name)s : %(err)s"), "KCHVMHDEV0001E": _("VM %(vmid)s does not contain directly assigned host device %(dev_name)s."), "KCHVMHDEV0002E": _("The host device %(dev_name)s is not allowed to directly assign to VM."), -- 2.5.5
On 07-07-2016 09:57, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
This patch adds the documentation and i18n changes for the new Virt-Viewer launcher API.
URI: plugins/kimchi/vms/*:name*/virtviewerfile
'Retrieve the Virt-Viewer launcher file to connect to the graphics of this virtual machine. Virtual machine must be running.'
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- docs/API.md | 6 ++++++ i18n.py | 2 ++ 2 files changed, 8 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 84ef7c9..78c0574 100644 --- a/docs/API.md +++ b/docs/API.md @@ -286,6 +286,12 @@ Represents a snapshot of the Virtual Machine's primary monitor. **URI:** /plugins/kimchi/vms/*:name*/snapshots/current * **GET**: Retrieve current snapshot information for the virtual machine.
+### Sub-resource: Virt-Viewer File +**URI:** /plugins/kimchi/vms/*:name*/virtviewerfile +* **GET**: Retrieve the Virt-Viewer launcher file to connect to the + graphics of this virtual machine. Virtual machine must + be running. + ### Collection: Templates
**URI:** /plugins/kimchi/templates diff --git a/i18n.py b/i18n.py index 08a3ac8..deb0f8c 100644 --- a/i18n.py +++ b/i18n.py @@ -131,6 +131,8 @@ messages = { "KCHVM0080E": _("Cannot update Maximum Memory when guest is running."), "KCHVM0081E": _("Impossible to create %(dir)s directory."), "KCHVM0082E": _("Either the guest %(name)s did not start to listen to the serial or it is not configured to use the serial console."), + "KCHVM0083E": _("Unable to retrieve virt viever file for stopped virtual machine %(name)s"), viever -> viewer capitalize is a good idea too -> Virt Viewer
+ "KCHVM0084E": _("Error occured while retrieving the virt viewer file for virtual machine %(name)s : %(err)s"), capitalize Virt Viewer
"KCHVMHDEV0001E": _("VM %(vmid)s does not contain directly assigned host device %(dev_name)s."), "KCHVMHDEV0002E": _("The host device %(dev_name)s is not allowed to directly assign to VM."),
-- Lucio Correia Software Engineer IBM LTC Brazil
On 07/07/2016 10:45 AM, Lucio Correia wrote:
On 07-07-2016 09:57, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
This patch adds the documentation and i18n changes for the new Virt-Viewer launcher API.
URI: plugins/kimchi/vms/*:name*/virtviewerfile
'Retrieve the Virt-Viewer launcher file to connect to the graphics of this virtual machine. Virtual machine must be running.'
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- docs/API.md | 6 ++++++ i18n.py | 2 ++ 2 files changed, 8 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 84ef7c9..78c0574 100644 --- a/docs/API.md +++ b/docs/API.md @@ -286,6 +286,12 @@ Represents a snapshot of the Virtual Machine's primary monitor. **URI:** /plugins/kimchi/vms/*:name*/snapshots/current * **GET**: Retrieve current snapshot information for the virtual machine.
+### Sub-resource: Virt-Viewer File +**URI:** /plugins/kimchi/vms/*:name*/virtviewerfile +* **GET**: Retrieve the Virt-Viewer launcher file to connect to the + graphics of this virtual machine. Virtual machine must + be running. + ### Collection: Templates
**URI:** /plugins/kimchi/templates diff --git a/i18n.py b/i18n.py index 08a3ac8..deb0f8c 100644 --- a/i18n.py +++ b/i18n.py @@ -131,6 +131,8 @@ messages = { "KCHVM0080E": _("Cannot update Maximum Memory when guest is running."), "KCHVM0081E": _("Impossible to create %(dir)s directory."), "KCHVM0082E": _("Either the guest %(name)s did not start to listen to the serial or it is not configured to use the serial console."), + "KCHVM0083E": _("Unable to retrieve virt viever file for stopped virtual machine %(name)s"), viever -> viewer capitalize is a good idea too -> Virt Viewer
+ "KCHVM0084E": _("Error occured while retrieving the virt viewer file for virtual machine %(name)s : %(err)s"), capitalize Virt Viewer
Will fix these in v2
"KCHVMHDEV0001E": _("VM %(vmid)s does not contain directly assigned host device %(dev_name)s."), "KCHVMHDEV0002E": _("The host device %(dev_name)s is not allowed to directly assign to VM."),
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> The Virt-Viewer API will store the generated launcher files into the 'data/virtviewerfiles' directory. This patch adds this new dir to Makefile.am and config.py.in files. tests/test_config.py.in was also changed to reflect this new directory. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- Makefile.am | 2 ++ config.py.in | 10 +++++++++- tests/test_config.py.in | 6 ++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index 1b46773..eabf6ae 100644 --- a/Makefile.am +++ b/Makefile.am @@ -119,6 +119,7 @@ install-deb: install mkdir -p $(DESTDIR)/$(localstatedir)/lib/kimchi/ws-tokens mkdir -p $(DESTDIR)/$(localstatedir)/lib/kimchi/screenshots mkdir -p $(DESTDIR)/$(localstatedir)/lib/kimchi/isos + mkdir -p $(DESTDIR)/$(localstatedir)/lib/kimchi/virtviewerfiles deb: contrib/make-deb.sh @@ -168,6 +169,7 @@ install-data-local: mkdir -p $(DESTDIR)/$(localstatedir)/lib/kimchi/ws-tokens mkdir -p $(DESTDIR)/$(localstatedir)/lib/kimchi/screenshots mkdir -p $(DESTDIR)/$(localstatedir)/lib/kimchi/isos + mkdir -p $(DESTDIR)/$(localstatedir)/lib/kimchi/virviewerfiles uninstall-local: @if test -f $(DESTDIR)/etc/systemd/system/wokd.service.d/kimchi.conf; then \ diff --git a/config.py.in b/config.py.in index 937f1ec..a887670 100644 --- a/config.py.in +++ b/config.py.in @@ -57,6 +57,10 @@ def get_screenshot_path(): return os.path.join(PluginPaths('kimchi').state_dir, 'screenshots') +def get_virtviewerfiles_path(): + return os.path.join(PluginPaths('kimchi').state_dir, 'virtviewerfiles') + + def get_config(): plugin_conf = PluginPaths('kimchi').conf_file return Parser().dict_from_file(plugin_conf) @@ -152,7 +156,11 @@ class KimchiConfig(PluginConfig): '/data/screenshots': {'tools.nocache.on': False, 'tools.staticdir.dir': get_screenshot_path(), - 'tools.staticdir.on': True}} + 'tools.staticdir.on': True}, + '/data/virtviewerfiles': {'tools.nocache.on': False, + 'tools.staticdir.dir': + get_virtviewerfiles_path(), + 'tools.staticdir.on': True}} for uri, data in static_config.iteritems(): custom_config[uri] = {'tools.nocache.on': True, 'tools.wokauth.on': True} diff --git a/tests/test_config.py.in b/tests/test_config.py.in index a6728e8..48f5579 100644 --- a/tests/test_config.py.in +++ b/tests/test_config.py.in @@ -24,6 +24,7 @@ from wok.config import CACHEEXPIRES from wok.config import Paths, PluginPaths from wok.plugins.kimchi.config import get_screenshot_path +from wok.plugins.kimchi.config import get_virtviewerfiles_path from wok.plugins.kimchi.config import KimchiConfig, KimchiPaths @@ -151,6 +152,11 @@ class ConfigTests(unittest.TestCase): 'tools.staticdir.dir': get_screenshot_path(), 'tools.staticdir.on': True }, + '/data/virtviewerfiles': { + 'tools.nocache.on': False, + 'tools.staticdir.dir': get_virtviewerfiles_path(), + 'tools.staticdir.on': True + }, '/ui/config/tab-ext.xml': { 'tools.nocache.on': True, 'tools.staticfile.on': True, -- 2.5.5
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> control/vms.py: - added a new resource called VMVirtViewerFile that returns an URL to download the virt-viewer launcher file. - added a new URL called 'virtviewerfile' to allow access to the new VMVirtViewerFile resource. model/vms.py: - turned get_graphics() into a staticmethod to be used outside of the vms.py module. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- control/vms.py | 12 ++++++++++++ model/vms.py | 9 +++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/control/vms.py b/control/vms.py index bbf0fa4..645cb40 100644 --- a/control/vms.py +++ b/control/vms.py @@ -65,6 +65,7 @@ class VM(Resource): super(VM, self).__init__(model, ident) self.role_key = 'guests' self.screenshot = VMScreenShot(model, ident) + self.virtviewerfile = VMVirtViewerFile(model, ident) self.uri_fmt = '/vms/%s' for ident, node in sub_nodes.items(): setattr(self, ident, node(model, self.ident)) @@ -103,3 +104,14 @@ class VMScreenShot(Resource): self.lookup() internal_uri = self.info.replace('plugins/kimchi', '') raise internal_redirect(internal_uri) + + +class VMVirtViewerFile(Resource): + def __init__(self, model, ident): + super(VMVirtViewerFile, self).__init__(model, ident) + self.role_key = 'guests' + + @property + def data(self): + internal_uri = self.info.replace('plugins/kimchi', '') + raise internal_redirect(internal_uri) diff --git a/model/vms.py b/model/vms.py index 6a309fc..e0b0a37 100644 --- a/model/vms.py +++ b/model/vms.py @@ -1154,7 +1154,7 @@ class VMModel(object): state = DOM_STATE_MAP[info[0]] screenshot = None # (type, listen, port, passwd, passwdValidTo) - graphics = self._vm_get_graphics(name) + graphics = self.get_graphics(name, self.conn) graphics_port = graphics[2] graphics_port = graphics_port if state == 'running' else None try: @@ -1426,8 +1426,9 @@ class VMModel(object): return True - def _vm_get_graphics(self, name): - dom = self.get_vm(name, self.conn) + @staticmethod + def get_graphics(name, conn): + dom = VMModel.get_vm(name, conn) xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE) expr = "/domain/devices/graphics/@type" @@ -1492,7 +1493,7 @@ class VMModel(object): def connect(self, name): # (type, listen, port, passwd, passwdValidTo) - graphics_port = self._vm_get_graphics(name)[2] + graphics_port = self.get_graphics(name, self.conn)[2] if graphics_port is not None: websocket.add_proxy_token(name.encode('utf-8'), graphics_port) else: -- 2.5.5
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This new module contains functions to create virt viewer files and a new model called VMVirtViewerFileModel that will return a link to the new generated script file in the lookup() method. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- model/virtviewerfile.py | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 model/virtviewerfile.py diff --git a/model/virtviewerfile.py b/model/virtviewerfile.py new file mode 100644 index 0000000..baccc8a --- /dev/null +++ b/model/virtviewerfile.py @@ -0,0 +1,100 @@ +# +# Project Kimchi +# +# Copyright IBM Corp, 2016 +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# + +import cherrypy +import libvirt +import os + +from wok.config import config as wok_config +from wok.exception import NotFoundError, OperationFailed +from wok.plugins.kimchi import config as kimchi_config +from wok.plugins.kimchi.model.vms import VMModel + + +def write_virt_viewer_file(params): + file_template = """\ +[virt-viewer] +type=%(type)s +host=%(host)s +port=%(graphics_port)s +""" + file_contents = file_template % params + + if params.get('graphics_passwd'): + file_contents += 'password=%s\n' % params['graphics_passwd'] + + try: + with open(params.get('path'), 'w') as vv_file: + vv_file.write(file_contents) + except Exception: + raise + + +def _get_request_host(): + host = cherrypy.request.headers.get('Host') + if not host: + host = wok_config.get("server", "host") + host = host.split(':')[0] + return host + + +def create_virt_viewer_file(vm_name, graphics_info): + graphics_type = graphics_info[0] + graphics_port = graphics_info[2] + graphics_passwd = graphics_info[3] + + try: + host = _get_request_host() + + default_dir = kimchi_config.get_virtviewerfiles_path() + file_path = os.path.join(default_dir, '%s-access.vv' % vm_name) + + file_params = { + 'type': graphics_type, + 'graphics_port': graphics_port, + 'graphics_passwd': graphics_passwd, + 'host': host, + 'path': file_path + } + write_virt_viewer_file(file_params) + return file_path + + except Exception as e: + raise OperationFailed("KCHVM0084E", + {'name': vm_name}, {'err': e.message}) + + +class VMVirtViewerFileModel(object): + def __init__(self, **kargs): + self.conn = kargs['conn'] + + def _check_if_vm_running(self, name): + dom = VMModel.get_vm(name, self.conn) + d_info = dom.info() + if d_info[0] != libvirt.VIR_DOMAIN_RUNNING: + raise NotFoundError("KCHVM0083E", {'name': name}) + + def lookup(self, name): + self._check_if_vm_running(name) + graphics_info = VMModel.get_graphics(name, self.conn) + file_path = create_virt_viewer_file(name, graphics_info) + + return 'plugins/kimchi/data/virtviewerfiles/%s' %\ + os.path.basename(file_path) -- 2.5.5
On 07-07-2016 09:57, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
This new module contains functions to create virt viewer files and a new model called VMVirtViewerFileModel that will return a link to the new generated script file in the lookup() method.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- model/virtviewerfile.py | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 model/virtviewerfile.py
diff --git a/model/virtviewerfile.py b/model/virtviewerfile.py new file mode 100644 index 0000000..baccc8a --- /dev/null +++ b/model/virtviewerfile.py @@ -0,0 +1,100 @@ +# +# Project Kimchi +# +# Copyright IBM Corp, 2016 +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# + +import cherrypy +import libvirt +import os + +from wok.config import config as wok_config +from wok.exception import NotFoundError, OperationFailed +from wok.plugins.kimchi import config as kimchi_config +from wok.plugins.kimchi.model.vms import VMModel + + +def write_virt_viewer_file(params): + file_template = """\ +[virt-viewer] +type=%(type)s +host=%(host)s +port=%(graphics_port)s +""" + file_contents = file_template % params + + if params.get('graphics_passwd'): + file_contents += 'password=%s\n' % params['graphics_passwd'] + + try: + with open(params.get('path'), 'w') as vv_file: + vv_file.write(file_contents) + except Exception: + raise + + +def _get_request_host(): + host = cherrypy.request.headers.get('Host') + if not host: + host = wok_config.get("server", "host") + host = host.split(':')[0] + return host + + +def create_virt_viewer_file(vm_name, graphics_info): + graphics_type = graphics_info[0] + graphics_port = graphics_info[2] + graphics_passwd = graphics_info[3] + + try: + host = _get_request_host() + + default_dir = kimchi_config.get_virtviewerfiles_path() + file_path = os.path.join(default_dir, '%s-access.vv' % vm_name) + + file_params = { + 'type': graphics_type, + 'graphics_port': graphics_port, + 'graphics_passwd': graphics_passwd, + 'host': host, + 'path': file_path + } + write_virt_viewer_file(file_params) + return file_path + + except Exception as e: + raise OperationFailed("KCHVM0084E", + {'name': vm_name}, {'err': e.message}) Why 2 dicts here? Shouldn't it be only one dict?
+ + +class VMVirtViewerFileModel(object): + def __init__(self, **kargs): + self.conn = kargs['conn'] + + def _check_if_vm_running(self, name): + dom = VMModel.get_vm(name, self.conn) + d_info = dom.info() + if d_info[0] != libvirt.VIR_DOMAIN_RUNNING: + raise NotFoundError("KCHVM0083E", {'name': name}) + + def lookup(self, name): + self._check_if_vm_running(name) + graphics_info = VMModel.get_graphics(name, self.conn) + file_path = create_virt_viewer_file(name, graphics_info) + + return 'plugins/kimchi/data/virtviewerfiles/%s' %\ + os.path.basename(file_path)
-- Lucio Correia Software Engineer IBM LTC Brazil
On 07/07/2016 11:32 AM, Lucio Correia wrote:
On 07-07-2016 09:57, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
This new module contains functions to create virt viewer files and a new model called VMVirtViewerFileModel that will return a link to the new generated script file in the lookup() method.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- model/virtviewerfile.py | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 model/virtviewerfile.py
diff --git a/model/virtviewerfile.py b/model/virtviewerfile.py new file mode 100644 index 0000000..baccc8a --- /dev/null +++ b/model/virtviewerfile.py @@ -0,0 +1,100 @@ +# +# Project Kimchi +# +# Copyright IBM Corp, 2016 +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# + +import cherrypy +import libvirt +import os + +from wok.config import config as wok_config +from wok.exception import NotFoundError, OperationFailed +from wok.plugins.kimchi import config as kimchi_config +from wok.plugins.kimchi.model.vms import VMModel + + +def write_virt_viewer_file(params): + file_template = """\ +[virt-viewer] +type=%(type)s +host=%(host)s +port=%(graphics_port)s +""" + file_contents = file_template % params + + if params.get('graphics_passwd'): + file_contents += 'password=%s\n' % params['graphics_passwd'] + + try: + with open(params.get('path'), 'w') as vv_file: + vv_file.write(file_contents) + except Exception: + raise + + +def _get_request_host(): + host = cherrypy.request.headers.get('Host') + if not host: + host = wok_config.get("server", "host") + host = host.split(':')[0] + return host + + +def create_virt_viewer_file(vm_name, graphics_info): + graphics_type = graphics_info[0] + graphics_port = graphics_info[2] + graphics_passwd = graphics_info[3] + + try: + host = _get_request_host() + + default_dir = kimchi_config.get_virtviewerfiles_path() + file_path = os.path.join(default_dir, '%s-access.vv' % vm_name) + + file_params = { + 'type': graphics_type, + 'graphics_port': graphics_port, + 'graphics_passwd': graphics_passwd, + 'host': host, + 'path': file_path + } + write_virt_viewer_file(file_params) + return file_path + + except Exception as e: + raise OperationFailed("KCHVM0084E", + {'name': vm_name}, {'err': e.message}) Why 2 dicts here? Shouldn't it be only one dict? Why 2? Because I messed up! hehehe
Will fix in v2
+ + +class VMVirtViewerFileModel(object): + def __init__(self, **kargs): + self.conn = kargs['conn'] + + def _check_if_vm_running(self, name): + dom = VMModel.get_vm(name, self.conn) + d_info = dom.info() + if d_info[0] != libvirt.VIR_DOMAIN_RUNNING: + raise NotFoundError("KCHVM0083E", {'name': name}) + + def lookup(self, name): + self._check_if_vm_running(name) + graphics_info = VMModel.get_graphics(name, self.conn) + file_path = create_virt_viewer_file(name, graphics_info) + + return 'plugins/kimchi/data/virtviewerfiles/%s' %\ + os.path.basename(file_path)
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch adds 3 new unit tests in test_model.py to test the new VirtViewerModel. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- tests/test_model.py | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/tests/test_model.py b/tests/test_model.py index 9a6a1aa..2fc1d38 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -18,15 +18,20 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +import __builtin__ as builtins + import grp import lxml.etree as ET import os import pwd +import mock import re import shutil import time import unittest +from mock import call, mock_open, patch + import tests.utils as utils import wok.objectstore @@ -43,6 +48,7 @@ from wok.plugins.kimchi import osinfo from wok.plugins.kimchi.config import kimchiPaths as paths from wok.plugins.kimchi.model import model from wok.plugins.kimchi.model.libvirtconnection import LibvirtConnection +from wok.plugins.kimchi.model.virtviewerfile import VMVirtViewerFileModel from wok.plugins.kimchi.model.vms import VMModel import iso_gen @@ -108,7 +114,8 @@ class ModelTests(unittest.TestCase): # test_rest or test_mockmodel to avoid overriding problems LibvirtConnection._connections['test:///default'] = {} - os.unlink(self.tmp_store) + if os.path.isfile(self.tmp_store): + os.unlink(self.tmp_store) def test_vm_info(self): inst = model.Model('test:///default', self.tmp_store) @@ -338,6 +345,94 @@ class ModelTests(unittest.TestCase): inst.template_delete('test') + @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') + def test_vm_virtviewerfile_vmnotrunning(self): + inst = model.Model(objstore_loc=self.tmp_store) + params = {'name': 'test', + 'source_media': {'type': 'disk', 'path': UBUNTU_ISO}} + inst.templates_create(params) + + with RollbackContext() as rollback: + params = {'name': 'kimchi-vnc', + 'template': '/plugins/kimchi/templates/test'} + task1 = inst.vms_create(params) + inst.task_wait(task1['id']) + rollback.prependDefer(inst.vm_delete, 'kimchi-vnc') + + expected_error_msg = "KCHVM0083E" + with self.assertRaisesRegexp(NotFoundError, expected_error_msg): + vvmodel = VMVirtViewerFileModel(conn=inst.conn) + vvmodel.lookup('kimchi-vnc') + + inst.template_delete('test') + + @mock.patch('wok.plugins.kimchi.model.virtviewerfile._get_request_host') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'VMModel.get_graphics') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'VMVirtViewerFileModel._check_if_vm_running') + def test_vm_virtviewerfile_vnc(self, mock_vm_running, mock_get_graphics, + mock_get_host): + + mock_get_host.return_value = 'kimchi-test-host' + mock_get_graphics.return_value = ['vnc', 'listen', '5999', None] + mock_vm_running.return_value = True + + vvmodel = VMVirtViewerFileModel(conn=None) + + open_ = mock_open(read_data='') + with patch.object(builtins, 'open', open_): + vvfilepath = vvmodel.lookup('kimchi-vm') + + self.assertEqual( + vvfilepath, + 'plugins/kimchi/data/virtviewerfiles/kimchi-vm-access.vv' + ) + + expected_write_content = "[virt-viewer]\ntype=vnc\n"\ + "host=kimchi-test-host\nport=5999\n" + self.assertEqual( + open_().write.mock_calls, [call(expected_write_content)] + ) + + mock_get_graphics.assert_called_once_with('kimchi-vm', None) + mock_vm_running.assert_called_once_with('kimchi-vm') + + @mock.patch('wok.plugins.kimchi.model.virtviewerfile._get_request_host') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'VMModel.get_graphics') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'VMVirtViewerFileModel._check_if_vm_running') + def test_vm_virtviewerfile_spice_passwd(self, mock_vm_running, + mock_get_graphics, + mock_get_host): + + mock_get_host.return_value = 'kimchi-test-host' + mock_get_graphics.return_value = [ + 'spice', 'listen', '6660', 'spicepasswd' + ] + mock_vm_running.return_value = True + + vvmodel = VMVirtViewerFileModel(conn=None) + + open_ = mock_open(read_data='') + with patch.object(builtins, 'open', open_): + vvfilepath = vvmodel.lookup('kimchi-vm') + + self.assertEqual( + vvfilepath, + 'plugins/kimchi/data/virtviewerfiles/kimchi-vm-access.vv' + ) + + expected_write_content = "[virt-viewer]\ntype=spice\n"\ + "host=kimchi-test-host\nport=6660\npassword=spicepasswd\n" + self.assertEqual( + open_().write.mock_calls, [call(expected_write_content)] + ) + + mock_get_graphics.assert_called_once_with('kimchi-vm', None) + mock_vm_running.assert_called_once_with('kimchi-vm') + @unittest.skipUnless(utils.running_as_root(), "Must be run as root") def test_vm_serial(self): inst = model.Model(objstore_loc=self.tmp_store) -- 2.5.5
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> The FirewallManager class opens and closes firewall ports to allow for virt viewer connections in the graphics server of the VM. For Fedora distros and Ubuntu, 'firewall-cmd' and 'ufw' is used respectively. For all other distros, 'iptables' is used. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- model/virtviewerfile.py | 94 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/model/virtviewerfile.py b/model/virtviewerfile.py index baccc8a..398b8a3 100644 --- a/model/virtviewerfile.py +++ b/model/virtviewerfile.py @@ -26,6 +26,7 @@ from wok.config import config as wok_config from wok.exception import NotFoundError, OperationFailed from wok.plugins.kimchi import config as kimchi_config from wok.plugins.kimchi.model.vms import VMModel +from wok.utils import run_command, wok_log def write_virt_viewer_file(params): @@ -98,3 +99,96 @@ class VMVirtViewerFileModel(object): return 'plugins/kimchi/data/virtviewerfiles/%s' %\ os.path.basename(file_path) + + +class FirewallManager(object): + + @staticmethod + def check_if_firewall_cmd_enabled(): + _, _, r_code = run_command(['firewall-cmd', '--state', '-q']) + return r_code == 0 + + @staticmethod + def check_if_ufw_enabled(): + _, _, r_code = run_command(['ufw', 'status']) + return r_code == 0 + + def __init__(self): + self.opened_ports = {} + self.firewall_provider = None + + if self.check_if_firewall_cmd_enabled(): + self.firewall_provider = FirewallCMDProvider() + elif self.check_if_ufw_enabled(): + self.firewall_provider = UFWProvider() + else: + self.firewall_provider = IPTablesProvider() + + def add_vm_graphics_port(self, vm_name, port): + self.firewall_provider.enable_tcp_port(port) + self.opened_ports[vm_name] = port + + def remove_vm_graphics_port(self, vm_name): + port = self.opened_ports.pop(vm_name, None) + if port: + self.firewall_provider.disable_tcp_port(port) + + def remove_all_vms_ports(self): + for port in self.opened_ports.values(): + self.firewall_provider.disable_tcp_port(port) + + self.opened_ports = {} + + +class FirewallCMDProvider(object): + + @staticmethod + def enable_tcp_port(port): + _, err, r_code = run_command( + ['firewall-cmd', '--add-port=%s/tcp' % port] + ) + if r_code != 0: + wok_log.error('Error when adding port to firewall-cmd: %s' % err) + + @staticmethod + def disable_tcp_port(port): + _, err, r_code = run_command( + ['firewall-cmd', '--remove-port=%s/tcp' % port] + ) + if r_code != 0: + wok_log.error('Error when removing port from ' + 'firewall-cmd: %s' % err) + + +class UFWProvider(object): + + @staticmethod + def enable_tcp_port(port): + _, err, r_code = run_command(['ufw', 'allow', '%s/tcp' % port]) + if r_code != 0: + wok_log.error('Error when adding port to ufw: %s' % err) + + @staticmethod + def disable_tcp_port(port): + _, err, r_code = run_command(['ufw', 'deny', '%s/tcp' % port]) + if r_code != 0: + wok_log.error('Error when removing port from ufw: %s' % err) + + +class IPTablesProvider(object): + + @staticmethod + def enable_tcp_port(port): + cmd = ['iptables', '-I', 'INPUT', '-p', 'tcp', '--dport', + port, '-j', 'ACCEPT'] + _, err, r_code = run_command(cmd) + if r_code != 0: + wok_log.error('Error when adding port to iptables: %s' % err) + + @staticmethod + def disable_tcp_port(port): + cmd = ['iptables', '-D', 'INPUT', '-p', 'tcp', '--dport', + port, '-j', 'ACCEPT'] + _, err, r_code = run_command(cmd) + if r_code != 0: + wok_log.error('Error when removing port from itables: %s' % err) -- 2.5.5
This is very nice code and IMHO looks like a server feature, to be added to Wok. So my suggestion is to split this patch between Wok and Kimchi, keeping on Kimchi, as an extension of Wok class, only:
+ def add_vm_graphics_port(self, vm_name, port): + def remove_vm_graphics_port(self, vm_name): + def remove_all_vms_ports(self):
Also, isn't necessary a --reload after por is opened by firewall_cmd? On 07-07-2016 09:57, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
The FirewallManager class opens and closes firewall ports to allow for virt viewer connections in the graphics server of the VM.
For Fedora distros and Ubuntu, 'firewall-cmd' and 'ufw' is used respectively. For all other distros, 'iptables' is used.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- model/virtviewerfile.py | 94 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+)
diff --git a/model/virtviewerfile.py b/model/virtviewerfile.py index baccc8a..398b8a3 100644 --- a/model/virtviewerfile.py +++ b/model/virtviewerfile.py @@ -26,6 +26,7 @@ from wok.config import config as wok_config from wok.exception import NotFoundError, OperationFailed from wok.plugins.kimchi import config as kimchi_config from wok.plugins.kimchi.model.vms import VMModel +from wok.utils import run_command, wok_log
def write_virt_viewer_file(params): @@ -98,3 +99,96 @@ class VMVirtViewerFileModel(object):
return 'plugins/kimchi/data/virtviewerfiles/%s' %\ os.path.basename(file_path) + + +class FirewallManager(object): + + @staticmethod + def check_if_firewall_cmd_enabled(): + _, _, r_code = run_command(['firewall-cmd', '--state', '-q']) + return r_code == 0 + + @staticmethod + def check_if_ufw_enabled(): + _, _, r_code = run_command(['ufw', 'status']) + return r_code == 0 + + def __init__(self): + self.opened_ports = {} + self.firewall_provider = None + + if self.check_if_firewall_cmd_enabled(): + self.firewall_provider = FirewallCMDProvider() + elif self.check_if_ufw_enabled(): + self.firewall_provider = UFWProvider() + else: + self.firewall_provider = IPTablesProvider() + + def add_vm_graphics_port(self, vm_name, port): + self.firewall_provider.enable_tcp_port(port) + self.opened_ports[vm_name] = port + + def remove_vm_graphics_port(self, vm_name): + port = self.opened_ports.pop(vm_name, None) + if port: + self.firewall_provider.disable_tcp_port(port) + + def remove_all_vms_ports(self): + for port in self.opened_ports.values(): + self.firewall_provider.disable_tcp_port(port) + + self.opened_ports = {} + + +class FirewallCMDProvider(object): + + @staticmethod + def enable_tcp_port(port): + _, err, r_code = run_command( + ['firewall-cmd', '--add-port=%s/tcp' % port] + ) + if r_code != 0: + wok_log.error('Error when adding port to firewall-cmd: %s' % err) + + @staticmethod + def disable_tcp_port(port): + _, err, r_code = run_command( + ['firewall-cmd', '--remove-port=%s/tcp' % port] + ) + if r_code != 0: + wok_log.error('Error when removing port from ' + 'firewall-cmd: %s' % err) + + +class UFWProvider(object): + + @staticmethod + def enable_tcp_port(port): + _, err, r_code = run_command(['ufw', 'allow', '%s/tcp' % port]) + if r_code != 0: + wok_log.error('Error when adding port to ufw: %s' % err) + + @staticmethod + def disable_tcp_port(port): + _, err, r_code = run_command(['ufw', 'deny', '%s/tcp' % port]) + if r_code != 0: + wok_log.error('Error when removing port from ufw: %s' % err) + + +class IPTablesProvider(object): + + @staticmethod + def enable_tcp_port(port): + cmd = ['iptables', '-I', 'INPUT', '-p', 'tcp', '--dport', + port, '-j', 'ACCEPT'] + _, err, r_code = run_command(cmd) + if r_code != 0: + wok_log.error('Error when adding port to iptables: %s' % err) + + @staticmethod + def disable_tcp_port(port): + cmd = ['iptables', '-D', 'INPUT', '-p', 'tcp', '--dport', + port, '-j', 'ACCEPT'] + _, err, r_code = run_command(cmd) + if r_code != 0: + wok_log.error('Error when removing port from itables: %s' % err)
-- Lucio Correia Software Engineer IBM LTC Brazil
On 07/07/2016 12:10 PM, Lucio Correia wrote:
This is very nice code and IMHO looks like a server feature, to be added to Wok.
So my suggestion is to split this patch between Wok and Kimchi, keeping on Kimchi, as an extension of Wok class, only:
+ def add_vm_graphics_port(self, vm_name, port): + def remove_vm_graphics_port(self, vm_name): + def remove_all_vms_ports(self):
Yeah we can discuss if this feature can be deployed @ WoK. There's a feature request for it in Ginger github too.
Also, isn't necessary a --reload after por is opened by firewall_cmd?
No. In fact a '--reload' iwill reload the firewall with its permanent rules set, overwriting any 'transient' changes done.
On 07-07-2016 09:57, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
The FirewallManager class opens and closes firewall ports to allow for virt viewer connections in the graphics server of the VM.
For Fedora distros and Ubuntu, 'firewall-cmd' and 'ufw' is used respectively. For all other distros, 'iptables' is used.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- model/virtviewerfile.py | 94 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+)
diff --git a/model/virtviewerfile.py b/model/virtviewerfile.py index baccc8a..398b8a3 100644 --- a/model/virtviewerfile.py +++ b/model/virtviewerfile.py @@ -26,6 +26,7 @@ from wok.config import config as wok_config from wok.exception import NotFoundError, OperationFailed from wok.plugins.kimchi import config as kimchi_config from wok.plugins.kimchi.model.vms import VMModel +from wok.utils import run_command, wok_log
def write_virt_viewer_file(params): @@ -98,3 +99,96 @@ class VMVirtViewerFileModel(object):
return 'plugins/kimchi/data/virtviewerfiles/%s' %\ os.path.basename(file_path) + + +class FirewallManager(object): + + @staticmethod + def check_if_firewall_cmd_enabled(): + _, _, r_code = run_command(['firewall-cmd', '--state', '-q']) + return r_code == 0 + + @staticmethod + def check_if_ufw_enabled(): + _, _, r_code = run_command(['ufw', 'status']) + return r_code == 0 + + def __init__(self): + self.opened_ports = {} + self.firewall_provider = None + + if self.check_if_firewall_cmd_enabled(): + self.firewall_provider = FirewallCMDProvider() + elif self.check_if_ufw_enabled(): + self.firewall_provider = UFWProvider() + else: + self.firewall_provider = IPTablesProvider() + + def add_vm_graphics_port(self, vm_name, port): + self.firewall_provider.enable_tcp_port(port) + self.opened_ports[vm_name] = port + + def remove_vm_graphics_port(self, vm_name): + port = self.opened_ports.pop(vm_name, None) + if port: + self.firewall_provider.disable_tcp_port(port) + + def remove_all_vms_ports(self): + for port in self.opened_ports.values(): + self.firewall_provider.disable_tcp_port(port) + + self.opened_ports = {} + + +class FirewallCMDProvider(object): + + @staticmethod + def enable_tcp_port(port): + _, err, r_code = run_command( + ['firewall-cmd', '--add-port=%s/tcp' % port] + ) + if r_code != 0: + wok_log.error('Error when adding port to firewall-cmd: %s' % err) + + @staticmethod + def disable_tcp_port(port): + _, err, r_code = run_command( + ['firewall-cmd', '--remove-port=%s/tcp' % port] + ) + if r_code != 0: + wok_log.error('Error when removing port from ' + 'firewall-cmd: %s' % err) + + +class UFWProvider(object): + + @staticmethod + def enable_tcp_port(port): + _, err, r_code = run_command(['ufw', 'allow', '%s/tcp' % port]) + if r_code != 0: + wok_log.error('Error when adding port to ufw: %s' % err) + + @staticmethod + def disable_tcp_port(port): + _, err, r_code = run_command(['ufw', 'deny', '%s/tcp' % port]) + if r_code != 0: + wok_log.error('Error when removing port from ufw: %s' % err) + + +class IPTablesProvider(object): + + @staticmethod + def enable_tcp_port(port): + cmd = ['iptables', '-I', 'INPUT', '-p', 'tcp', '--dport', + port, '-j', 'ACCEPT'] + _, err, r_code = run_command(cmd) + if r_code != 0: + wok_log.error('Error when adding port to iptables: %s' % err) + + @staticmethod + def disable_tcp_port(port): + cmd = ['iptables', '-D', 'INPUT', '-p', 'tcp', '--dport', + port, '-j', 'ACCEPT'] + _, err, r_code = run_command(cmd) + if r_code != 0: + wok_log.error('Error when removing port from itables: %s' % err)
Lucio, after doing some thinking I believe that this firewall class should remain in Kimchi for now. There is no immediate gain into adding the FirewallManager somewhere else while keeping this patch set waiting - specially considering that this is the only feature in all WoK plug-ins that is using it. Note that this does not mean that we can't start a RFC proposing that the FirewallManager should be hosted in WoK (perhaps Gingerbase?). We can send a RFC with a more robust proposal that can also deal with firewall zones configuration (Ginger has a request for this specific feature at https://github.com/kimchi-project/ginger/issues/107) and with more generic rules (adding udp ports for example). After this feature is discussed and contributed we can go back to this code and change it to use this new firewall backend. Daniel On 07/07/2016 02:44 PM, Daniel Henrique Barboza wrote:
On 07/07/2016 12:10 PM, Lucio Correia wrote:
This is very nice code and IMHO looks like a server feature, to be added to Wok.
So my suggestion is to split this patch between Wok and Kimchi, keeping on Kimchi, as an extension of Wok class, only:
+ def add_vm_graphics_port(self, vm_name, port): + def remove_vm_graphics_port(self, vm_name): + def remove_all_vms_ports(self):
Yeah we can discuss if this feature can be deployed @ WoK. There's a feature request for it in Ginger github too.
Also, isn't necessary a --reload after por is opened by firewall_cmd?
No. In fact a '--reload' iwill reload the firewall with its permanent rules set, overwriting any 'transient' changes done.
On 07-07-2016 09:57, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
The FirewallManager class opens and closes firewall ports to allow for virt viewer connections in the graphics server of the VM.
For Fedora distros and Ubuntu, 'firewall-cmd' and 'ufw' is used respectively. For all other distros, 'iptables' is used.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- model/virtviewerfile.py | 94 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+)
diff --git a/model/virtviewerfile.py b/model/virtviewerfile.py index baccc8a..398b8a3 100644 --- a/model/virtviewerfile.py +++ b/model/virtviewerfile.py @@ -26,6 +26,7 @@ from wok.config import config as wok_config from wok.exception import NotFoundError, OperationFailed from wok.plugins.kimchi import config as kimchi_config from wok.plugins.kimchi.model.vms import VMModel +from wok.utils import run_command, wok_log
def write_virt_viewer_file(params): @@ -98,3 +99,96 @@ class VMVirtViewerFileModel(object):
return 'plugins/kimchi/data/virtviewerfiles/%s' %\ os.path.basename(file_path) + + +class FirewallManager(object): + + @staticmethod + def check_if_firewall_cmd_enabled(): + _, _, r_code = run_command(['firewall-cmd', '--state', '-q']) + return r_code == 0 + + @staticmethod + def check_if_ufw_enabled(): + _, _, r_code = run_command(['ufw', 'status']) + return r_code == 0 + + def __init__(self): + self.opened_ports = {} + self.firewall_provider = None + + if self.check_if_firewall_cmd_enabled(): + self.firewall_provider = FirewallCMDProvider() + elif self.check_if_ufw_enabled(): + self.firewall_provider = UFWProvider() + else: + self.firewall_provider = IPTablesProvider() + + def add_vm_graphics_port(self, vm_name, port): + self.firewall_provider.enable_tcp_port(port) + self.opened_ports[vm_name] = port + + def remove_vm_graphics_port(self, vm_name): + port = self.opened_ports.pop(vm_name, None) + if port: + self.firewall_provider.disable_tcp_port(port) + + def remove_all_vms_ports(self): + for port in self.opened_ports.values(): + self.firewall_provider.disable_tcp_port(port) + + self.opened_ports = {} + + +class FirewallCMDProvider(object): + + @staticmethod + def enable_tcp_port(port): + _, err, r_code = run_command( + ['firewall-cmd', '--add-port=%s/tcp' % port] + ) + if r_code != 0: + wok_log.error('Error when adding port to firewall-cmd: %s' % err) + + @staticmethod + def disable_tcp_port(port): + _, err, r_code = run_command( + ['firewall-cmd', '--remove-port=%s/tcp' % port] + ) + if r_code != 0: + wok_log.error('Error when removing port from ' + 'firewall-cmd: %s' % err) + + +class UFWProvider(object): + + @staticmethod + def enable_tcp_port(port): + _, err, r_code = run_command(['ufw', 'allow', '%s/tcp' % port]) + if r_code != 0: + wok_log.error('Error when adding port to ufw: %s' % err) + + @staticmethod + def disable_tcp_port(port): + _, err, r_code = run_command(['ufw', 'deny', '%s/tcp' % port]) + if r_code != 0: + wok_log.error('Error when removing port from ufw: %s' % err) + + +class IPTablesProvider(object): + + @staticmethod + def enable_tcp_port(port): + cmd = ['iptables', '-I', 'INPUT', '-p', 'tcp', '--dport', + port, '-j', 'ACCEPT'] + _, err, r_code = run_command(cmd) + if r_code != 0: + wok_log.error('Error when adding port to iptables: %s' % err) + + @staticmethod + def disable_tcp_port(port): + cmd = ['iptables', '-D', 'INPUT', '-p', 'tcp', '--dport', + port, '-j', 'ACCEPT'] + _, err, r_code = run_command(cmd) + if r_code != 0: + wok_log.error('Error when removing port from itables: %s' % err)
On 07-07-2016 23:26, Daniel Henrique Barboza wrote:
Lucio, after doing some thinking I believe that this firewall class should remain in Kimchi for now. There is no immediate gain into adding the FirewallManager somewhere else while keeping this patch set waiting - specially considering that this is the only feature in all WoK plug-ins that is using it.
Note that this does not mean that we can't start a RFC proposing that the FirewallManager should be hosted in WoK (perhaps Gingerbase?). We can send a RFC with a more robust proposal that can also deal with firewall zones configuration (Ginger has a request for this specific feature at https://github.com/kimchi-project/ginger/issues/107) and with more generic rules (adding udp ports for example). After this feature is discussed and contributed we can go back to this code and change it to use this new firewall backend.
OK.
Daniel
On 07/07/2016 02:44 PM, Daniel Henrique Barboza wrote:
On 07/07/2016 12:10 PM, Lucio Correia wrote:
This is very nice code and IMHO looks like a server feature, to be added to Wok.
So my suggestion is to split this patch between Wok and Kimchi, keeping on Kimchi, as an extension of Wok class, only:
+ def add_vm_graphics_port(self, vm_name, port): + def remove_vm_graphics_port(self, vm_name): + def remove_all_vms_ports(self):
Yeah we can discuss if this feature can be deployed @ WoK. There's a feature request for it in Ginger github too.
Also, isn't necessary a --reload after por is opened by firewall_cmd?
No. In fact a '--reload' iwill reload the firewall with its permanent rules set, overwriting any 'transient' changes done.
On 07-07-2016 09:57, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
The FirewallManager class opens and closes firewall ports to allow for virt viewer connections in the graphics server of the VM.
For Fedora distros and Ubuntu, 'firewall-cmd' and 'ufw' is used respectively. For all other distros, 'iptables' is used.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- model/virtviewerfile.py | 94 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+)
diff --git a/model/virtviewerfile.py b/model/virtviewerfile.py index baccc8a..398b8a3 100644 --- a/model/virtviewerfile.py +++ b/model/virtviewerfile.py @@ -26,6 +26,7 @@ from wok.config import config as wok_config from wok.exception import NotFoundError, OperationFailed from wok.plugins.kimchi import config as kimchi_config from wok.plugins.kimchi.model.vms import VMModel +from wok.utils import run_command, wok_log
def write_virt_viewer_file(params): @@ -98,3 +99,96 @@ class VMVirtViewerFileModel(object):
return 'plugins/kimchi/data/virtviewerfiles/%s' %\ os.path.basename(file_path) + + +class FirewallManager(object): + + @staticmethod + def check_if_firewall_cmd_enabled(): + _, _, r_code = run_command(['firewall-cmd', '--state', '-q']) + return r_code == 0 + + @staticmethod + def check_if_ufw_enabled(): + _, _, r_code = run_command(['ufw', 'status']) + return r_code == 0 + + def __init__(self): + self.opened_ports = {} + self.firewall_provider = None + + if self.check_if_firewall_cmd_enabled(): + self.firewall_provider = FirewallCMDProvider() + elif self.check_if_ufw_enabled(): + self.firewall_provider = UFWProvider() + else: + self.firewall_provider = IPTablesProvider() + + def add_vm_graphics_port(self, vm_name, port): + self.firewall_provider.enable_tcp_port(port) + self.opened_ports[vm_name] = port + + def remove_vm_graphics_port(self, vm_name): + port = self.opened_ports.pop(vm_name, None) + if port: + self.firewall_provider.disable_tcp_port(port) + + def remove_all_vms_ports(self): + for port in self.opened_ports.values(): + self.firewall_provider.disable_tcp_port(port) + + self.opened_ports = {} + + +class FirewallCMDProvider(object): + + @staticmethod + def enable_tcp_port(port): + _, err, r_code = run_command( + ['firewall-cmd', '--add-port=%s/tcp' % port] + ) + if r_code != 0: + wok_log.error('Error when adding port to firewall-cmd: %s' % err) + + @staticmethod + def disable_tcp_port(port): + _, err, r_code = run_command( + ['firewall-cmd', '--remove-port=%s/tcp' % port] + ) + if r_code != 0: + wok_log.error('Error when removing port from ' + 'firewall-cmd: %s' % err) + + +class UFWProvider(object): + + @staticmethod + def enable_tcp_port(port): + _, err, r_code = run_command(['ufw', 'allow', '%s/tcp' % port]) + if r_code != 0: + wok_log.error('Error when adding port to ufw: %s' % err) + + @staticmethod + def disable_tcp_port(port): + _, err, r_code = run_command(['ufw', 'deny', '%s/tcp' % port]) + if r_code != 0: + wok_log.error('Error when removing port from ufw: %s' % err) + + +class IPTablesProvider(object): + + @staticmethod + def enable_tcp_port(port): + cmd = ['iptables', '-I', 'INPUT', '-p', 'tcp', '--dport', + port, '-j', 'ACCEPT'] + _, err, r_code = run_command(cmd) + if r_code != 0: + wok_log.error('Error when adding port to iptables: %s' % err) + + @staticmethod + def disable_tcp_port(port): + cmd = ['iptables', '-D', 'INPUT', '-p', 'tcp', '--dport', + port, '-j', 'ACCEPT'] + _, err, r_code = run_command(cmd) + if r_code != 0: + wok_log.error('Error when removing port from itables: %s' % err)
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Lucio Correia Software Engineer IBM LTC Brazil
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch adds unit tests for the FirewallManager class. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- tests/test_model.py | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/tests/test_model.py b/tests/test_model.py index 2fc1d38..1162d2c 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -48,6 +48,7 @@ from wok.plugins.kimchi import osinfo from wok.plugins.kimchi.config import kimchiPaths as paths from wok.plugins.kimchi.model import model from wok.plugins.kimchi.model.libvirtconnection import LibvirtConnection +from wok.plugins.kimchi.model.virtviewerfile import FirewallManager from wok.plugins.kimchi.model.virtviewerfile import VMVirtViewerFileModel from wok.plugins.kimchi.model.vms import VMModel @@ -433,6 +434,76 @@ class ModelTests(unittest.TestCase): mock_get_graphics.assert_called_once_with('kimchi-vm', None) mock_vm_running.assert_called_once_with('kimchi-vm') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.run_command') + def test_firewall_provider_firewallcmd(self, mock_run_cmd): + mock_run_cmd.side_effect = [ + ['', '', 0], ['', '', 0], ['', '', 0] + ] + + fw_manager = FirewallManager() + fw_manager.add_vm_graphics_port('vm-name', 5905) + self.assertEqual(fw_manager.opened_ports, {'vm-name': 5905}) + + fw_manager.remove_vm_graphics_port('vm-name') + self.assertEqual(fw_manager.opened_ports, {}) + + mock_run_cmd.assert_has_calls( + [ + call(['firewall-cmd', '--state', '-q']), + call(['firewall-cmd', '--add-port=5905/tcp']), + call(['firewall-cmd', '--remove-port=5905/tcp']) + ] + ) + + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.run_command') + def test_firewall_provider_ufw(self, mock_run_cmd): + mock_run_cmd.side_effect = [ + ['', '', 1], ['', '', 0], ['', '', 0], ['', '', 0] + ] + + fw_manager = FirewallManager() + fw_manager.add_vm_graphics_port('vm-name', 5905) + self.assertEqual(fw_manager.opened_ports, {'vm-name': 5905}) + + fw_manager.remove_vm_graphics_port('vm-name') + self.assertEqual(fw_manager.opened_ports, {}) + + mock_run_cmd.assert_has_calls( + [ + call(['firewall-cmd', '--state', '-q']), + call(['ufw', 'status']), + call(['ufw', 'allow', '5905/tcp']), + call(['ufw', 'deny', '5905/tcp']) + ] + ) + + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.run_command') + def test_firewall_provider_iptables(self, mock_run_cmd): + mock_run_cmd.side_effect = [ + ['', '', 1], ['', '', 1], ['', '', 0], ['', '', 0] + ] + + fw_manager = FirewallManager() + fw_manager.add_vm_graphics_port('vm-name', 5905) + self.assertEqual(fw_manager.opened_ports, {'vm-name': 5905}) + + fw_manager.remove_vm_graphics_port('vm-name') + self.assertEqual(fw_manager.opened_ports, {}) + + iptables_add = ['iptables', '-I', 'INPUT', '-p', 'tcp', '--dport', + 5905, '-j', 'ACCEPT'] + + iptables_del = ['iptables', '-D', 'INPUT', '-p', 'tcp', '--dport', + 5905, '-j', 'ACCEPT'] + + mock_run_cmd.assert_has_calls( + [ + call(['firewall-cmd', '--state', '-q']), + call(['ufw', 'status']), + call(iptables_add), call(iptables_del) + ] + ) + @unittest.skipUnless(utils.running_as_root(), "Must be run as root") def test_vm_serial(self): inst = model.Model(objstore_loc=self.tmp_store) -- 2.5.5
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> In this patch, the FirewallManager class is used together with libvirt events to: - open the firewall port to the graphics server when a call to lookup() is made; - close the firewall port when the virtual machine is turned off. A cleanup() method was also added to the cherrypy 'exit' engine to ensure that no firewall port previously opened by the virtviewerfile module will be left opened when Kimchi/WoK exits. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- model/virtviewerfile.py | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/model/virtviewerfile.py b/model/virtviewerfile.py index 398b8a3..e3f60e6 100644 --- a/model/virtviewerfile.py +++ b/model/virtviewerfile.py @@ -85,6 +85,16 @@ def create_virt_viewer_file(vm_name, graphics_info): class VMVirtViewerFileModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] + self.firewall_mngr = FirewallManager() + self.vm_event_callbacks = {} + cherrypy.engine.subscribe('exit', self.cleanup) + + def cleanup(self): + wok_log.info('Closing any VNC/SPICE firewall ports ' + 'opened by Kimchi ...') + self.firewall_mngr.remove_all_vms_ports() + for cb_id in self.vm_event_callbacks.values(): + self.conn.get().domainEventDeregisterAny(cb_id) def _check_if_vm_running(self, name): dom = VMModel.get_vm(name, self.conn) @@ -92,11 +102,41 @@ class VMVirtViewerFileModel(object): if d_info[0] != libvirt.VIR_DOMAIN_RUNNING: raise NotFoundError("KCHVM0083E", {'name': name}) + def event_vmshutdown_cb(self, conn, dom, event, detail, *args): + if event == libvirt.VIR_DOMAIN_EVENT_STOPPED: + vm_name = dom.name() + self.firewall_mngr.remove_vm_graphics_port(vm_name) + cb_id = self.vm_event_callbacks.pop(vm_name, None) + self.conn.get().domainEventDeregisterAny(cb_id) + + def handleVMShutdownPowerOff(self, vm_name): + try: + dom = VMModel.get_vm(vm_name, self.conn) + cb_id = self.conn.get().domainEventRegisterAny( + dom, + libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE, + self.event_vmshutdown_cb, + None + ) + self.vm_event_callbacks[vm_name] = cb_id + + except (libvirt.libvirtError, AttributeError) as e: + if type(e) == AttributeError: + reason = 'Libvirt service is not running' + else: + reason = e.message + wok_log.error("Register of LIFECYCLE event failed: %s" % reason) + def lookup(self, name): self._check_if_vm_running(name) graphics_info = VMModel.get_graphics(name, self.conn) file_path = create_virt_viewer_file(name, graphics_info) + if not self.vm_event_callbacks.get(name, None): + graphics_port = graphics_info[2] + self.firewall_mngr.add_vm_graphics_port(name, graphics_port) + self.handleVMShutdownPowerOff(name) + return 'plugins/kimchi/data/virtviewerfiles/%s' %\ os.path.basename(file_path) -- 2.5.5
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch changes existing tests and add a new test to assert that the latest changes with FirewallManager and libvirt event listening in virtviewerfile.py is working properly. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- tests/test_model.py | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/tests/test_model.py b/tests/test_model.py index 1162d2c..44358a5 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -371,8 +371,13 @@ class ModelTests(unittest.TestCase): @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' 'VMModel.get_graphics') @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'FirewallManager.add_vm_graphics_port') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'VMVirtViewerFileModel.handleVMShutdownPowerOff') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' 'VMVirtViewerFileModel._check_if_vm_running') - def test_vm_virtviewerfile_vnc(self, mock_vm_running, mock_get_graphics, + def test_vm_virtviewerfile_vnc(self, mock_vm_running, mock_handleVMOff, + mock_add_port, mock_get_graphics, mock_get_host): mock_get_host.return_value = 'kimchi-test-host' @@ -398,13 +403,21 @@ class ModelTests(unittest.TestCase): mock_get_graphics.assert_called_once_with('kimchi-vm', None) mock_vm_running.assert_called_once_with('kimchi-vm') + mock_handleVMOff.assert_called_once_with('kimchi-vm') + mock_add_port.assert_called_once_with('kimchi-vm', '5999') @mock.patch('wok.plugins.kimchi.model.virtviewerfile._get_request_host') @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' 'VMModel.get_graphics') @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'FirewallManager.add_vm_graphics_port') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'VMVirtViewerFileModel.handleVMShutdownPowerOff') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' 'VMVirtViewerFileModel._check_if_vm_running') def test_vm_virtviewerfile_spice_passwd(self, mock_vm_running, + mock_handleVMOff, + mock_add_port, mock_get_graphics, mock_get_host): @@ -433,6 +446,8 @@ class ModelTests(unittest.TestCase): mock_get_graphics.assert_called_once_with('kimchi-vm', None) mock_vm_running.assert_called_once_with('kimchi-vm') + mock_handleVMOff.assert_called_once_with('kimchi-vm') + mock_add_port.assert_called_once_with('kimchi-vm', '6660') @mock.patch('wok.plugins.kimchi.model.virtviewerfile.run_command') def test_firewall_provider_firewallcmd(self, mock_run_cmd): @@ -504,6 +519,42 @@ class ModelTests(unittest.TestCase): ] ) + @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'FirewallManager.remove_vm_graphics_port') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'FirewallManager.add_vm_graphics_port') + def test_vm_virtviewerfile_vmlifecycle(self, mock_add_port, + mock_remove_port): + + inst = model.Model(objstore_loc=self.tmp_store) + params = {'name': 'test', + 'source_media': {'type': 'disk', 'path': UBUNTU_ISO}} + inst.templates_create(params) + + with RollbackContext() as rollback: + params = {'name': 'kimchi-vnc', + 'template': '/plugins/kimchi/templates/test'} + task1 = inst.vms_create(params) + inst.task_wait(task1['id']) + rollback.prependDefer(inst.vm_delete, 'kimchi-vnc') + + inst.vm_start('kimchi-vnc') + + graphics_info = VMModel.get_graphics('kimchi-vnc', inst.conn) + graphics_port = graphics_info[2] + + vvmodel = VMVirtViewerFileModel(conn=inst.conn) + vvmodel.lookup('kimchi-vnc') + + inst.vm_poweroff('kimchi-vnc') + time.sleep(5) + + mock_add_port.assert_called_once_with('kimchi-vnc', graphics_port) + mock_remove_port.assert_called_once_with('kimchi-vnc') + + inst.template_delete('test') + @unittest.skipUnless(utils.running_as_root(), "Must be run as root") def test_vm_serial(self): inst = model.Model(objstore_loc=self.tmp_store) -- 2.5.5
My suggestion is to test it with a vm with special characters in name, since there is name reading from libvirt in backend. On 07-07-2016 09:57, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
This patch changes existing tests and add a new test to assert that the latest changes with FirewallManager and libvirt event listening in virtviewerfile.py is working properly.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- tests/test_model.py | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-)
diff --git a/tests/test_model.py b/tests/test_model.py index 1162d2c..44358a5 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -371,8 +371,13 @@ class ModelTests(unittest.TestCase): @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' 'VMModel.get_graphics') @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'FirewallManager.add_vm_graphics_port') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'VMVirtViewerFileModel.handleVMShutdownPowerOff') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' 'VMVirtViewerFileModel._check_if_vm_running') - def test_vm_virtviewerfile_vnc(self, mock_vm_running, mock_get_graphics, + def test_vm_virtviewerfile_vnc(self, mock_vm_running, mock_handleVMOff, + mock_add_port, mock_get_graphics, mock_get_host):
mock_get_host.return_value = 'kimchi-test-host' @@ -398,13 +403,21 @@ class ModelTests(unittest.TestCase):
mock_get_graphics.assert_called_once_with('kimchi-vm', None) mock_vm_running.assert_called_once_with('kimchi-vm') + mock_handleVMOff.assert_called_once_with('kimchi-vm') + mock_add_port.assert_called_once_with('kimchi-vm', '5999')
@mock.patch('wok.plugins.kimchi.model.virtviewerfile._get_request_host') @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' 'VMModel.get_graphics') @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'FirewallManager.add_vm_graphics_port') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'VMVirtViewerFileModel.handleVMShutdownPowerOff') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' 'VMVirtViewerFileModel._check_if_vm_running') def test_vm_virtviewerfile_spice_passwd(self, mock_vm_running, + mock_handleVMOff, + mock_add_port, mock_get_graphics, mock_get_host):
@@ -433,6 +446,8 @@ class ModelTests(unittest.TestCase):
mock_get_graphics.assert_called_once_with('kimchi-vm', None) mock_vm_running.assert_called_once_with('kimchi-vm') + mock_handleVMOff.assert_called_once_with('kimchi-vm') + mock_add_port.assert_called_once_with('kimchi-vm', '6660')
@mock.patch('wok.plugins.kimchi.model.virtviewerfile.run_command') def test_firewall_provider_firewallcmd(self, mock_run_cmd): @@ -504,6 +519,42 @@ class ModelTests(unittest.TestCase): ] )
+ @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'FirewallManager.remove_vm_graphics_port') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'FirewallManager.add_vm_graphics_port') + def test_vm_virtviewerfile_vmlifecycle(self, mock_add_port, + mock_remove_port): + + inst = model.Model(objstore_loc=self.tmp_store) + params = {'name': 'test', + 'source_media': {'type': 'disk', 'path': UBUNTU_ISO}} + inst.templates_create(params) + + with RollbackContext() as rollback: + params = {'name': 'kimchi-vnc', + 'template': '/plugins/kimchi/templates/test'} + task1 = inst.vms_create(params) + inst.task_wait(task1['id']) + rollback.prependDefer(inst.vm_delete, 'kimchi-vnc') + + inst.vm_start('kimchi-vnc') + + graphics_info = VMModel.get_graphics('kimchi-vnc', inst.conn) + graphics_port = graphics_info[2] + + vvmodel = VMVirtViewerFileModel(conn=inst.conn) + vvmodel.lookup('kimchi-vnc') + + inst.vm_poweroff('kimchi-vnc') + time.sleep(5) + + mock_add_port.assert_called_once_with('kimchi-vnc', graphics_port) + mock_remove_port.assert_called_once_with('kimchi-vnc') + + inst.template_delete('test') + @unittest.skipUnless(utils.running_as_root(), "Must be run as root") def test_vm_serial(self): inst = model.Model(objstore_loc=self.tmp_store)
-- Lucio Correia Software Engineer IBM LTC Brazil
On 07/07/2016 12:25 PM, Lucio Correia wrote:
My suggestion is to test it with a vm with special characters in name, since there is name reading from libvirt in backend.
I'll edit this unit test to use non-ASCII chars in the VM name.
On 07-07-2016 09:57, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
This patch changes existing tests and add a new test to assert that the latest changes with FirewallManager and libvirt event listening in virtviewerfile.py is working properly.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- tests/test_model.py | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-)
diff --git a/tests/test_model.py b/tests/test_model.py index 1162d2c..44358a5 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -371,8 +371,13 @@ class ModelTests(unittest.TestCase): @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' 'VMModel.get_graphics') @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'FirewallManager.add_vm_graphics_port') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'VMVirtViewerFileModel.handleVMShutdownPowerOff') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' 'VMVirtViewerFileModel._check_if_vm_running') - def test_vm_virtviewerfile_vnc(self, mock_vm_running, mock_get_graphics, + def test_vm_virtviewerfile_vnc(self, mock_vm_running, mock_handleVMOff, + mock_add_port, mock_get_graphics, mock_get_host):
mock_get_host.return_value = 'kimchi-test-host' @@ -398,13 +403,21 @@ class ModelTests(unittest.TestCase):
mock_get_graphics.assert_called_once_with('kimchi-vm', None) mock_vm_running.assert_called_once_with('kimchi-vm') + mock_handleVMOff.assert_called_once_with('kimchi-vm') + mock_add_port.assert_called_once_with('kimchi-vm', '5999')
@mock.patch('wok.plugins.kimchi.model.virtviewerfile._get_request_host') @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' 'VMModel.get_graphics') @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'FirewallManager.add_vm_graphics_port') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'VMVirtViewerFileModel.handleVMShutdownPowerOff') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' 'VMVirtViewerFileModel._check_if_vm_running') def test_vm_virtviewerfile_spice_passwd(self, mock_vm_running, + mock_handleVMOff, + mock_add_port, mock_get_graphics, mock_get_host):
@@ -433,6 +446,8 @@ class ModelTests(unittest.TestCase):
mock_get_graphics.assert_called_once_with('kimchi-vm', None) mock_vm_running.assert_called_once_with('kimchi-vm') + mock_handleVMOff.assert_called_once_with('kimchi-vm') + mock_add_port.assert_called_once_with('kimchi-vm', '6660')
@mock.patch('wok.plugins.kimchi.model.virtviewerfile.run_command') def test_firewall_provider_firewallcmd(self, mock_run_cmd): @@ -504,6 +519,42 @@ class ModelTests(unittest.TestCase): ] )
+ @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'FirewallManager.remove_vm_graphics_port') + @mock.patch('wok.plugins.kimchi.model.virtviewerfile.' + 'FirewallManager.add_vm_graphics_port') + def test_vm_virtviewerfile_vmlifecycle(self, mock_add_port, + mock_remove_port): + + inst = model.Model(objstore_loc=self.tmp_store) + params = {'name': 'test', + 'source_media': {'type': 'disk', 'path': UBUNTU_ISO}} + inst.templates_create(params) + + with RollbackContext() as rollback: + params = {'name': 'kimchi-vnc', + 'template': '/plugins/kimchi/templates/test'} + task1 = inst.vms_create(params) + inst.task_wait(task1['id']) + rollback.prependDefer(inst.vm_delete, 'kimchi-vnc') + + inst.vm_start('kimchi-vnc') + + graphics_info = VMModel.get_graphics('kimchi-vnc', inst.conn) + graphics_port = graphics_info[2] + + vvmodel = VMVirtViewerFileModel(conn=inst.conn) + vvmodel.lookup('kimchi-vnc') + + inst.vm_poweroff('kimchi-vnc') + time.sleep(5) + + mock_add_port.assert_called_once_with('kimchi-vnc', graphics_port) + mock_remove_port.assert_called_once_with('kimchi-vnc') + + inst.template_delete('test') + @unittest.skipUnless(utils.running_as_root(), "Must be run as root") def test_vm_serial(self): inst = model.Model(objstore_loc=self.tmp_store)
participants (3)
-
Daniel Henrique Barboza -
dhbarboza82＠gmail.com
-
Lucio Correia