Libertine

Merge lp:~townsend/libertine/use-lxc-manager into lp:libertine

use-lxc-manager
Merge into devel

Proposed by Christopher Townsend on 2016-10-06

Status:

Merged

Approved by:

Larry Price on 2016-10-07

Approved revision:

309

Merged at revision:

310

Proposed branch:

lp:~townsend/libertine/use-lxc-manager

Merge into:

lp:libertine

Diff against target:

385 lines (+155/-97)

5 files modified

python/libertine/Libertine.py (+2/-3)
python/libertine/LxcContainer.py (+76/-56)
python/libertine/utils.py (+17/-0)
tools/libertine-lxc-manager (+59/-22)
tools/update-puritine-containers (+1/-16)

To merge this branch:

bzr merge lp:~townsend/libertine/use-lxc-manager

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Larry Price		2016-10-06	Approve on 2016-10-07
Libertine CI Bot	continuous-integration		Approve on 2016-10-07
Review via email: mp+307864@code.launchpad.net

Commit message

Use libertine-lxc-manager when starting/stopping a container for libertine-container-manager operations if the session DBus is available. If not, fallback to l-c-m starting/stopping the container.

Description of the change

If a user is logged into a U8 session, we want to be careful about running l-c-m operations, especially if the user has an app running in the container.

Some example scenarios to try:

1. Run an app in a container. Ensure l-c-m operations won't run.
2. Run an l-c-m operation and then try to start an app. Ensure the app fails to start and some meaningful message is put in the upstart log for the app.
3. Run l-c-m in an ssh session and not logged in to the U8 desktop. Ensure l-c-m will still work when the libertine-lxc-manager isn't running.
4. Run l-c-m in an ssh session while logged into the U8 session. Ensure l-c-m will work by automatically discovering the session DBus address and run through libertine-lxc-manager.

Known issue (not necessarily relevant to this MP):
1. If libertine-lxc-manager is killed, either intentionally or unintentionally, it will not know the state of running/stopped containers. This will be addressed later.

Revision history for this message

Libertine CI Bot (libertine-ci-bot) wrote on 2016-10-06:

Click here to trigger a rebuild:
https://jenkins.canonical.com/libertine/job/lp-libertine-ci/150/rebuild

review: Approve (continuous-integration)

lp:~townsend/libertine/use-lxc-manager updated on 2016-10-07

308. By Christopher Townsend on 2016-10-07: Only bind-mount user dirs when launching an app.

Revision history for this message

Libertine CI Bot (libertine-ci-bot) wrote on 2016-10-07:

Click here to trigger a rebuild:
https://jenkins.canonical.com/libertine/job/lp-libertine-ci/152/rebuild

review: Approve (continuous-integration)

Revision history for this message

Larry Price (larryprice) wrote on 2016-10-07:

Couple of optional inlines, and now I'm off to actually test it...

review: Needs Information

lp:~townsend/libertine/use-lxc-manager updated on 2016-10-07

309. By Christopher Townsend on 2016-10-07: Changes based on review.

Revision history for this message

Libertine CI Bot (libertine-ci-bot) wrote on 2016-10-07:

Click here to trigger a rebuild:
https://jenkins.canonical.com/libertine/job/lp-libertine-ci/154/rebuild

review: Approve (continuous-integration)

Revision history for this message

Larry Price (larryprice) wrote on 2016-10-07:

This works really well, lgtm.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Christopher Townsend

Libertine Developers

 === modified file 'python/libertine/Libertine.py'
 --- python/libertine/Libertine.py	2016-09-30 18:55:22 +0000
 +++ python/libertine/Libertine.py	2016-10-07 15:43:27 +0000
@@ -265,9 +265,8 @@
      """
      def __init__(self, container):
          super().__init__()
--        if not container.is_running():
--            container.start_container()
--            self.callback(lambda: container.stop_container())
++        container.start_container()
++        self.callback(lambda: container.stop_container())
  class LibertineContainer(object):
 === modified file 'python/libertine/LxcContainer.py'
 --- python/libertine/LxcContainer.py	2016-09-20 13:58:52 +0000
 +++ python/libertine/LxcContainer.py	2016-10-07 15:43:27 +0000
@@ -56,6 +56,14 @@
      return os.path.join(home_path, '.config', 'lxc')
++def get_lxc_manager_dbus_name():
++    return LIBERTINE_LXC_MANAGER_NAME
++
++
++def get_lxc_manager_dbus_path():
++    return LIBERTINE_LXC_MANAGER_PATH
++
++
  def lxc_container(container_id):
      config_path = utils.get_libertine_containers_dir_path()
      if not os.path.exists(config_path):
@@ -66,6 +74,28 @@
      return container
++def lxc_start(container, lxc_log_file):
++    container.append_config_item("lxc.logfile", lxc_log_file)
++    container.append_config_item("lxc.logpriority", "3")
++
++    if not container.start():
++        return (False, "Container failed to start")
++
++    if not container.wait("RUNNING", 10):
++        return (False, "Container failed to enter the RUNNING state")
++
++    if not container.get_ips(timeout=30):
++        lxc_stop(container)
++        return (False, "Not able to connect to the network.")
++
++    return (True, "")
++
++
++def lxc_stop(container):
++    if container.running:
++        container.stop()
++
++
  def get_host_timezone():
      with open(os.path.join('/', 'etc', 'timezone'), 'r') as fd:
          host_timezone = fd.read().strip('\n')
@@ -107,6 +137,17 @@
          super().__init__(container_id)
          self.container_type = "lxc"
          self.container = lxc_container(container_id)
++        self._set_lxc_log()
++        self.lxc_manager_interface = None
++
++        utils.set_session_dbus_env_var()
++
++        try:
++            bus = dbus.SessionBus()
++            lxc_mgr_service = bus.get_object(get_lxc_manager_dbus_name(), get_lxc_manager_dbus_path())
++            self.lxc_manager_interface = dbus.Interface(lxc_mgr_service, get_lxc_manager_dbus_name())
++        except dbus.exceptions.DBusException:
++            pass
      def is_running(self):
          return self.container.running
@@ -122,40 +163,26 @@
          else:
              return True
--    def _dynamic_bind_mounts(self):
--        for user_dir in utils.get_common_xdg_user_directories():
--            xdg_user_dir_entry = (
--                "%s %s/%s none bind,create=dir,optional"
--                % (user_dir[0], home_path.strip('/'), user_dir[1])
--            )
--            self.container.append_config_item("lxc.mount.entry", xdg_user_dir_entry)
--
--    def start_container(self, launchable = False):
--        if not self.container.defined:
--            raise RuntimeError("Container %s is not valid" % self.container_id)
--
--        if not self.container.running:
--            self._set_lxc_log()
--            self._dynamic_bind_mounts()
--            if not self.container.start():
--                self._dump_lxc_log()
--                raise RuntimeError("Container failed to start")
--            if not self.container.wait("RUNNING", 10):
--                self._dump_lxc_log()
--                raise RuntimeError("Container failed to enter the RUNNING state")
--
--        if not self.container.get_ips(timeout=30):
--            self.stop_container()
--            raise RuntimeError("Not able to connect to the network.")
--
--        if not launchable:
--            if self.run_in_container("mountpoint -q /tmp/.X11-unix") == 0:
--                self.run_in_container("umount /tmp/.X11-unix")
--            if self.run_in_container("mountpoint -q /usr/lib/locale") == 0:
--                self.run_in_container("umount -l /usr/lib/locale")
++    def start_container(self):
++        if self.lxc_manager_interface:
++            (result, error) = self.lxc_manager_interface.operation_start(self.container_id, self.lxc_log_file)
++        else:
++            (result, error) = lxc_start(self.container, self.lxc_log_file)
++
++        if not result:
++            self._dump_lxc_log()
++            raise RuntimeError(error)
++
++        if self.run_in_container("mountpoint -q /tmp/.X11-unix") == 0:
++            self.run_in_container("umount /tmp/.X11-unix")
++        if self.run_in_container("mountpoint -q /usr/lib/locale") == 0:
++            self.run_in_container("umount -l /usr/lib/locale")
      def stop_container(self):
--        self.container.stop()
++        if self.lxc_manager_interface:
++            self.lxc_manager_interface.operation_stop(self.container_id)
++        else:
++            lxc_stop(self.container)
      def run_in_container(self, command_string):
          cmd_args = shlex.split(command_string)
@@ -283,21 +310,15 @@
          self.container.save_config()
      def launch_application(self, app_exec_line):
--        bus = dbus.SessionBus()
--        lxc_mgr_service = bus.get_object(LIBERTINE_LXC_MANAGER_NAME, LIBERTINE_LXC_MANAGER_PATH)
--        lxc_manager_interface = dbus.Interface(lxc_mgr_service, LIBERTINE_LXC_MANAGER_NAME)
--
--        if lxc_manager_interface.app_start(self.container_id):
--            if not self.container.wait("RUNNING", 10):
--                print("Container failed to enter the RUNNING state")
--                return
--
--            if not self.container.get_ips(timeout=30):
--                print("Not able to connect to the network.")
--                return
--
--        else:
--            print("Failure detected from libertine-lxc-manager")
++        if self.lxc_manager_interface == None:
++            print("No interface to libertine-lxc-manager.  Failing application launch.")
++            return
++
++        (result, error) = self.lxc_manager_interface.app_start(self.container_id, self.lxc_log_file)
++
++        if not result:
++            self._dump_lxc_log()
++            print("%s" % error)
              return
          window_manager = self.container.attach(lxc.attach_run_command,
@@ -314,17 +335,16 @@
          utils.terminate_window_manager(psutil.Process(window_manager))
          # Tell libertine-lxc-manager that the app has stopped.
--        lxc_manager_interface.app_stop(self.container_id)
++        self.lxc_manager_interface.app_stop(self.container_id)
      def _set_lxc_log(self):
          self.lxc_log_file = os.path.join(tempfile.mkdtemp(), 'lxc-start.log')
--        self.container.append_config_item("lxc.logfile", self.lxc_log_file)
--        self.container.append_config_item("lxc.logpriority", "3")
      def _dump_lxc_log(self):
--        try:
--            with open(self.lxc_log_file, 'r') as fd:
--                for line in fd:
--                    print(line.lstrip())
--        except Exception as ex:
--            print("Could not open LXC log file: %s" % ex, file=sys.stderr)
++        if os.path.exists(self.lxc_log_file):
++            try:
++                with open(self.lxc_log_file, 'r') as fd:
++                    for line in fd:
++                        print(line.lstrip())
++            except Exception as ex:
++                print("Could not open LXC log file: %s" % ex, file=sys.stderr)
 === modified file 'python/libertine/utils.py'
 --- python/libertine/utils.py	2016-09-06 12:36:47 +0000
 +++ python/libertine/utils.py	2016-10-07 15:43:27 +0000
@@ -173,3 +173,20 @@
                   (scopes_object_path, invalidate_signal, libertine_scope_id))
      subprocess.Popen(shlex.split(gdbus_cmd), stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
++
++
++def set_session_dbus_env_var():
++    if not 'DBUS_SESSION_BUS_ADDRESS' in os.environ:
++        dbus_session_path = os.path.join('/', 'run', 'user', str(os.getuid()), 'dbus-session')
++
++        if os.path.exists(dbus_session_path):
++            with open(dbus_session_path, 'r') as fd:
++                dbus_session_str = fd.read()
++
++                os.environ['DBUS_SESSION_BUS_ADDRESS'] = dbus_session_str.partition('DBUS_SESSION_BUS_ADDRESS=')[2].rstrip('\n')
++
++                return True
++        else:
++            return False
++
++    return True
 === modified file 'tools/libertine-lxc-manager'
 --- tools/libertine-lxc-manager	2016-09-06 12:36:47 +0000
 +++ tools/libertine-lxc-manager	2016-10-07 15:43:27 +0000
@@ -30,8 +30,10 @@
  from gi.repository import GLib
--LIBERTINE_LXC_MANAGER_NAME = "com.canonical.libertine.LxcManager"
--LIBERTINE_LXC_MANAGER_PATH = "/LxcManager"
++home_path = os.environ['HOME']
++
++LIBERTINE_LXC_MANAGER_NAME = libertine.LxcContainer.get_lxc_manager_dbus_name()
++LIBERTINE_LXC_MANAGER_PATH = libertine.LxcContainer.get_lxc_manager_dbus_path()
  class Service(dbus.service.Object):
@@ -39,6 +41,7 @@
      def __init__(self):
          self.is_pulse_setup = False
          self.app_counter = Counter()
++        self.operation_running = {}
          DBusGMainLoop(set_as_default=True)
          try:
@@ -51,15 +54,18 @@
          super().__init__(bus_name, LIBERTINE_LXC_MANAGER_PATH)
      @dbus.service.method(LIBERTINE_LXC_MANAGER_NAME,
--                         in_signature='s',
--                         out_signature='b')
--    def app_start(self, container_id):
--        started = self._launch_lxc_container(container_id)
++                         in_signature='ss',
++                         out_signature='(bs)')
++    def app_start(self, container_id, lxc_logfile):
++        if container_id in self.operation_running and self.operation_running[container_id]:
++            return (False, "Libertine container operation already running: cannot launch application.")
++
++        (started, reason) = self._launch_lxc_container(container_id, lxc_logfile)
          if started:
              self.app_counter[container_id] += 1
--        return started
++        return (started, reason)
      @dbus.service.method(LIBERTINE_LXC_MANAGER_NAME,
                           in_signature='s')
@@ -70,6 +76,26 @@
              self._stop_lxc_container(container_id)
              del self.app_counter[container_id]
++    @dbus.service.method(LIBERTINE_LXC_MANAGER_NAME,
++                         in_signature='ss',
++                         out_signature='(bs)')
++    def operation_start(self, container_id, lxc_log_file):
++        if self.app_counter[container_id] != 0:
++            return (False, "Application already running in container: cannot run operation.")
++
++        (started, reason) = self._launch_lxc_container(container_id, lxc_log_file, launchable=False)
++
++        if started:
++            self.operation_running[container_id] = True
++
++        return (started, reason)
++
++    @dbus.service.method(LIBERTINE_LXC_MANAGER_NAME,
++                         in_signature='s')
++    def operation_stop(self, container_id):
++        self._stop_lxc_container(container_id)
++        del self.operation_running[container_id]
++
      def _setup_pulse(self):
          pulse_socket_path = os.path.join(libertine.utils.get_libertine_runtime_dir(), 'pulse_socket')
@@ -87,25 +113,36 @@
          self.is_pulse_setup = True
--    def _launch_lxc_container(self, container_id):
--        container = libertine.LxcContainer.LibertineLXC(container_id)
--
--        if not self.is_pulse_setup:
++    def _launch_lxc_container(self, container_id, lxc_log_file, launchable=True):
++        container = libertine.LxcContainer.lxc_container(container_id)
++
++        if not container.defined:
++            return (False, "Container {} is not valid".format(container_id))
++
++        if launchable and not self.is_pulse_setup:
              self._setup_pulse()
--        if not container.is_running():
--            try:
--                container.start_container(True)
--            except RuntimeError as e:
--                print("%s" % e)
--                return False
--
--        return True
++        if not container.running:
++            if launchable:
++                self._dynamic_bind_mounts(container)
++
++            return libertine.LxcContainer.lxc_start(container, lxc_log_file)
++
++        return (True, "")
      def _stop_lxc_container(self, container_id):
--        container = libertine.LxcContainer.LibertineLXC(container_id)
--        if container.is_running():
--            container.stop_container()
++        container = libertine.LxcContainer.lxc_container(container_id)
++
++        libertine.LxcContainer.lxc_stop(container)
++
++
++    def _dynamic_bind_mounts(self, container):
++        for user_dir in libertine.utils.get_common_xdg_user_directories():
++            xdg_user_dir_entry = (
++                "%s %s/%s none bind,create=dir,optional"
++                % (user_dir[0], home_path.strip('/'), user_dir[1])
++            )
++            container.append_config_item("lxc.mount.entry", xdg_user_dir_entry)
  def sigterm(self):
 === modified file 'tools/update-puritine-containers'
 --- tools/update-puritine-containers	2016-09-06 12:36:47 +0000
 +++ tools/update-puritine-containers	2016-10-07 15:43:27 +0000
@@ -29,21 +29,6 @@
  puritine_symlink_farm_file = os.path.join(puritine_hook_dir, 'PuritineSymlinkFarm.json')
  puritine_click_config_file = os.path.join('libertine-config', 'libertine', 'ContainersConfig.json')
--def set_session_dbus_env_var():
--    if not 'DBUS_SESSION_BUS_ADDRESS' in os.environ:
--        dbus_session_path = os.path.join('/', 'run', 'user', str(os.getuid()), 'dbus-session')
--
--        if os.path.exists(dbus_session_path):
--            with open(dbus_session_path, 'r') as fd:
--                dbus_session_str = fd.read()
--
--                os.environ['DBUS_SESSION_BUS_ADDRESS'] = dbus_session_str.partition('DBUS_SESSION_BUS_ADDRESS=')[2].rstrip('\n')
--
--                return True
--        else:
--            return False
--
--    return True
  def symlink_farm_entries_count():
      if (puritine_symlink_farm_list and
@@ -168,7 +153,7 @@
  if __name__ == '__main__':
      puritine_symlink_farm_list = {}
--    update_libertine_scope = set_session_dbus_env_var()
++    update_libertine_scope = libertine.utils.set_session_dbus_env_var()
      if not os.path.exists(puritine_hook_dir):
          os.makedirs(puritine_hook_dir)