WIP: Refactory / Cleanup docker base (#73)

* Refactory / Cleanup docker base

* Check ID of running image

* Small bugs / lint

* Add log info

* Fix lint

* Add a real cleanup solution

* fix unused import

* Cleanup restart after updates

* Use restart callback

* rename callback

* Add info log for cleanup & fix lint

* Fix lint

* fix wrong id

* fix set addon as install

hassio/addons/__init__.py

@@ -191,15 +191,13 @@ class AddonManager(AddonsData):
             return False
 
         version = version or self.get_last_version(addon)
-        is_running = await self.dockers[addon].is_running()
 
         # update
-        if await self.dockers[addon].update(version):
+        if not await self.dockers[addon].update(version):
-            self.set_addon_update(addon, version)
+            return False
-            if is_running:
+
-                await self.start(addon)
+        self.set_addon_update(addon, version)
-            return True
+        return True
-        return False
 
     async def restart(self, addon):
         """Restart addon."""

hassio/config.py

@@ -21,7 +21,6 @@ HOMEASSISTANT_LAST = 'homeassistant_last'
 
 HASSIO_SSL = PurePath("ssl")
 HASSIO_LAST = 'hassio_last'
-HASSIO_CLEANUP = 'hassio_cleanup'
 
 ADDONS_CORE = PurePath("addons/core")
 ADDONS_LOCAL = PurePath("addons/local")
@@ -51,7 +50,6 @@ SCHEMA_CONFIG = vol.Schema({
     vol.Optional(TIMEZONE, default='UTC'): validate_timezone,
     vol.Optional(HOMEASSISTANT_LAST): vol.Coerce(str),
     vol.Optional(HASSIO_LAST): vol.Coerce(str),
-    vol.Optional(HASSIO_CLEANUP): vol.Coerce(str),
     vol.Optional(ADDONS_CUSTOM_LIST, default=[]): [vol.Url()],
     vol.Optional(SECURITY_INITIALIZE, default=False): vol.Boolean(),
     vol.Optional(SECURITY_TOTP): vol.Coerce(str),
@@ -148,20 +146,6 @@ class CoreConfig(Config):
         self._data[TIMEZONE] = value
         self.save()
 
-    @property
-    def hassio_cleanup(self):
-        """Return Version they need to cleanup."""
-        return self._data.get(HASSIO_CLEANUP)
-
-    @hassio_cleanup.setter
-    def hassio_cleanup(self, version):
-        """Set or remove cleanup flag."""
-        if version is None:
-            self._data.pop(HASSIO_CLEANUP, None)
-        else:
-            self._data[HASSIO_CLEANUP] = version
-        self.save()
-
     @property
     def homeassistant_image(self):
         """Return docker homeassistant repository."""

hassio/core.py

@@ -41,7 +41,7 @@ class HassIO(object):
 
         # init basic docker container
         self.supervisor = DockerSupervisor(
-            self.config, self.loop, self.dock, self)
+            self.config, self.loop, self.dock, self.stop)
         self.homeassistant = DockerHomeAssistant(
             self.config, self.loop, self.dock)
 
@@ -54,7 +54,8 @@ class HassIO(object):
     async def setup(self):
         """Setup HassIO orchestration."""
         # supervisor
-        await self.supervisor.attach()
+        if not await self.supervisor.attach():
+            _LOGGER.fatal("Can't attach to supervisor docker container!")
         await self.supervisor.cleanup()
 
         # set api endpoint
@@ -96,6 +97,8 @@ class HassIO(object):
             _LOGGER.info("No HomeAssistant docker found.")
             await homeassistant_setup(
                 self.config, self.loop, self.homeassistant)
+        else:
+            await self.homeassistant.attach()
 
         # Load addons
         arch = get_arch_from_image(self.supervisor.image)

hassio/dock/__init__.py

@@ -6,7 +6,6 @@ import logging
 import docker
 
 from ..const import LABEL_VERSION
-from ..tools import get_version_from_env
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -20,12 +19,11 @@ class DockerBase(object):
         self.loop = loop
         self.dock = dock
         self.image = image
-        self.container = None
         self.version = None
         self._lock = asyncio.Lock(loop=loop)
 
     @property
-    def docker_name(self):
+    def name(self):
         """Return name of docker container."""
         return None
 
@@ -34,18 +32,18 @@ class DockerBase(object):
         """Return True if a task is in progress."""
         return self._lock.locked()
 
-    def process_metadata(self, metadata=None, force=False):
+    def process_metadata(self, metadata, force=False):
         """Read metadata and set it to object."""
-        if not force and self.version:
+        # read image
-            return
+        if not self.image:
+            self.image = metadata['Config']['Image']
 
         # read metadata
-        metadata = metadata or self.container.attrs
+        need_version = force or not self.version
-        if LABEL_VERSION in metadata['Config']['Labels']:
+        if need_version and LABEL_VERSION in metadata['Config']['Labels']:
             self.version = metadata['Config']['Labels'][LABEL_VERSION]
-        else:
+        elif need_version:
-            # dedicated
+            _LOGGER.warning("Can't read version from %s", self.name)
-            self.version = get_version_from_env(metadata['Config']['Env'])
 
     async def install(self, tag):
         """Pull docker image."""
@@ -66,7 +64,7 @@ class DockerBase(object):
             image = self.dock.images.pull("{}:{}".format(self.image, tag))
 
             image.tag(self.image, tag='latest')
-            self.process_metadata(metadata=image.attrs, force=True)
+            self.process_metadata(image.attrs, force=True)
         except docker.errors.APIError as err:
             _LOGGER.error("Can't install %s:%s -> %s.", self.image, tag, err)
             return False
@@ -87,8 +85,7 @@ class DockerBase(object):
         Need run inside executor.
         """
         try:
-            image = self.dock.images.get(self.image)
+            self.dock.images.get(self.image)
-            self.process_metadata(metadata=image.attrs)
         except docker.errors.DockerException:
             return False
 
@@ -106,16 +103,21 @@ class DockerBase(object):
 
         Need run inside executor.
         """
-        if not self.container:
+        try:
-            try:
+            container = self.dock.containers.get(self.name)
-                self.container = self.dock.containers.get(self.docker_name)
+            image = self.dock.images.get(self.image)
-                self.process_metadata()
+        except docker.errors.DockerException:
-            except docker.errors.DockerException:
+            return False
-                return False
-        else:
-            self.container.reload()
 
-        return self.container.status == 'running'
+        # container is not running
+        if container.status != 'running':
+            return False
+
+        # we run on a old image, stop and start it
+        if container.image.id != image.id:
+            return False
+
+        return True
 
     async def attach(self):
         """Attach to running docker container."""
@@ -132,16 +134,17 @@ class DockerBase(object):
         Need run inside executor.
         """
         try:
-            self.container = self.dock.containers.get(self.docker_name)
+            if self.image:
-            self.image = self.container.attrs['Config']['Image']
+                obj_data = self.dock.images.get(self.image).attrs
-            self.process_metadata()
+            else:
-            _LOGGER.info("Attach to image %s with version %s",
+                obj_data = self.dock.containers.get(self.name).attrs
-                         self.image, self.version)
+        except docker.errors.DockerException:
-        except (docker.errors.DockerException, KeyError):
-            _LOGGER.fatal(
-                "Can't attach to %s docker container!", self.docker_name)
             return False
 
+        self.process_metadata(obj_data)
+        _LOGGER.info(
+            "Attach to image %s with version %s", self.image, self.version)
+
         return True
 
     async def run(self):
@@ -175,20 +178,19 @@ class DockerBase(object):
 
         Need run inside executor.
         """
-        if not self.container:
+        try:
+            container = self.dock.containers.get(self.name)
+        except docker.errors.DockerException:
             return
 
         _LOGGER.info("Stop %s docker application", self.image)
 
-        self.container.reload()
+        if container.status == 'running':
-        if self.container.status == 'running':
             with suppress(docker.errors.DockerException):
-                self.container.stop()
+                container.stop()
 
         with suppress(docker.errors.DockerException):
-            self.container.remove(force=True)
+            container.remove(force=True)
-
-        self.container = None
 
     async def remove(self):
         """Remove docker container."""
@@ -207,8 +209,8 @@ class DockerBase(object):
         if self._is_running():
             self._stop()
 
-        _LOGGER.info("Remove docker %s with latest and %s",
+        _LOGGER.info(
-                     self.image, self.version)
+            "Remove docker %s with latest and %s", self.image, self.version)
 
         try:
             with suppress(docker.errors.ImageNotFound):
@@ -239,23 +241,21 @@ class DockerBase(object):
 
         Need run inside executor.
         """
-        old_image = "{}:{}".format(self.image, self.version)
+        was_running = self._is_running()
 
-        _LOGGER.info("Update docker %s with %s:%s",
+        _LOGGER.info(
-                     old_image, self.image, tag)
+            "Update docker %s with %s:%s", self.version, self.image, tag)
 
         # update docker image
-        if self._install(tag):
+        if not self._install(tag):
-            _LOGGER.info("Cleanup old %s docker", old_image)
+            return False
-            self._stop()
-            try:
-                self.dock.images.remove(image=old_image, force=True)
-            except docker.errors.DockerException as err:
-                _LOGGER.warning(
-                    "Can't remove old image %s -> %s", old_image, err)
-            return True
 
-        return False
+        # cleanup old stuff
+        if was_running:
+            self._run()
+        self._cleanup()
+
+        return True
 
     async def logs(self):
         """Return docker logs of container."""
@@ -271,11 +271,13 @@ class DockerBase(object):
 
         Need run inside executor.
         """
-        if not self.container:
+        try:
-            return
+            container = self.dock.containers.get(self.name)
+        except docker.errors.DockerException:
+            return b""
 
         try:
-            return self.container.logs(tail=100, stdout=True, stderr=True)
+            return container.logs(tail=100, stdout=True, stderr=True)
         except docker.errors.DockerException as err:
             _LOGGER.warning("Can't grap logs from %s -> %s", self.image, err)
 
@@ -293,15 +295,45 @@ class DockerBase(object):
 
         Need run inside executor.
         """
-        if not self.container:
+        try:
+            container = self.dock.containers.get(self.name)
+        except docker.errors.DockerException:
             return False
 
         _LOGGER.info("Restart %s", self.image)
 
         try:
-            self.container.restart(timeout=30)
+            container.restart(timeout=30)
         except docker.errors.DockerException as err:
             _LOGGER.warning("Can't restart %s -> %s", self.image, err)
             return False
 
         return True
+
+    async def cleanup(self):
+        """Check if old version exists and cleanup."""
+        if self._lock.locked():
+            _LOGGER.error("Can't excute cleanup while a task is in progress")
+            return False
+
+        async with self._lock:
+            await self.loop.run_in_executor(None, self._cleanup)
+
+    def _cleanup(self):
+        """Check if old version exists and cleanup.
+
+        Need run inside executor.
+        """
+        try:
+            latest = self.dock.images.get(self.image)
+        except docker.errors.DockerException:
+            _LOGGER.warning("Can't find %s for cleanup", self.image)
+            return
+
+        for image in self.dock.images.list(name=self.image):
+            if latest.id == image.id:
+                continue
+
+            with suppress(docker.errors.DockerException):
+                _LOGGER.info("Cleanup docker images: %s", image.tags)
+                self.dock.images.remove(image.id, force=True)

hassio/dock/addon.py

@@ -24,7 +24,7 @@ class DockerAddon(DockerBase):
         self.addons_data = addons_data
 
     @property
-    def docker_name(self):
+    def name(self):
         """Return name of docker container."""
         return "addon_{}".format(self.addon)
 
@@ -96,13 +96,13 @@ class DockerAddon(DockerBase):
         if self._is_running():
             return
 
-        # cleanup old container
+        # cleanup
         self._stop()
 
         try:
-            self.container = self.dock.containers.run(
+            self.dock.containers.run(
                 self.image,
-                name=self.docker_name,
+                name=self.name,
                 detach=True,
                 network_mode=self.addons_data.get_network_mode(self.addon),
                 ports=self.addons_data.get_ports(self.addon),
@@ -112,42 +112,14 @@ class DockerAddon(DockerBase):
                 tmpfs=self.tmpfs
             )
 
-            self.process_metadata()
-            _LOGGER.info("Start docker addon %s with version %s",
-                         self.image, self.version)
-
         except docker.errors.DockerException as err:
             _LOGGER.error("Can't run %s -> %s", self.image, err)
             return False
 
+        _LOGGER.info(
+            "Start docker addon %s with version %s", self.image, self.version)
         return True
 
-    def _attach(self):
-        """Attach to running docker container.
-
-        Need run inside executor.
-        """
-        # read container
-        try:
-            self.container = self.dock.containers.get(self.docker_name)
-            self.process_metadata()
-
-            _LOGGER.info("Attach to container %s with version %s",
-                         self.image, self.version)
-            return
-        except (docker.errors.DockerException, KeyError):
-            pass
-
-        # read image
-        try:
-            image = self.dock.images.get(self.image)
-            self.process_metadata(metadata=image.attrs)
-
-            _LOGGER.info("Attach to image %s with version %s",
-                         self.image, self.version)
-        except (docker.errors.DockerException, KeyError):
-            _LOGGER.error("No container/image found for %s", self.image)
-
     def _install(self, tag):
         """Pull docker image or build it.
 
@@ -200,7 +172,7 @@ class DockerAddon(DockerBase):
                     path=str(build_dir), tag=build_tag, pull=True)
 
                 image.tag(self.image, tag='latest')
-                self.process_metadata(metadata=image.attrs, force=True)
+                self.process_metadata(image.attrs, force=True)
 
             except (docker.errors.DockerException, TypeError) as err:
                 _LOGGER.error("Can't build %s -> %s", build_tag, err)

hassio/dock/homeassistant.py

@@ -18,7 +18,7 @@ class DockerHomeAssistant(DockerBase):
         super().__init__(config, loop, dock, image=config.homeassistant_image)
 
     @property
-    def docker_name(self):
+    def name(self):
         """Return name of docker container."""
         return HASS_DOCKER_NAME
 
@@ -30,13 +30,13 @@ class DockerHomeAssistant(DockerBase):
         if self._is_running():
             return
 
-        # cleanup old container
+        # cleanup
         self._stop()
 
         try:
-            self.container = self.dock.containers.run(
+            self.dock.containers.run(
                 self.image,
-                name=self.docker_name,
+                name=self.name,
                 detach=True,
                 privileged=True,
                 network_mode='host',
@@ -53,15 +53,12 @@ class DockerHomeAssistant(DockerBase):
                         {'bind': '/share', 'mode': 'rw'},
                 })
 
-            self.process_metadata()
-
-            _LOGGER.info("Start docker addon %s with version %s",
-                         self.image, self.version)
-
         except docker.errors.DockerException as err:
             _LOGGER.error("Can't run %s -> %s", self.image, err)
             return False
 
+        _LOGGER.info(
+            "Start homeassistant %s with version %s", self.image, self.version)
         return True
 
     async def update(self, tag):
@@ -71,8 +68,4 @@ class DockerHomeAssistant(DockerBase):
             return False
 
         async with self._lock:
-            if await self.loop.run_in_executor(None, self._update, tag):
+            return await self.loop.run_in_executor(None, self._update, tag)
-                await self.loop.run_in_executor(None, self._run)
-                return True
-
-            return False

hassio/dock/supervisor.py

@@ -2,8 +2,6 @@
 import logging
 import os
 
-import docker
-
 from . import DockerBase
 from ..const import RESTART_EXIT_CODE
 
@@ -13,14 +11,13 @@ _LOGGER = logging.getLogger(__name__)
 class DockerSupervisor(DockerBase):
     """Docker hassio wrapper for HomeAssistant."""
 
-    def __init__(self, config, loop, dock, hassio, image=None):
+    def __init__(self, config, loop, dock, stop_callback, image=None):
         """Initialize docker base wrapper."""
         super().__init__(config, loop, dock, image=image)
-
+        self.stop_callback = stop_callback
-        self.hassio = hassio
 
     @property
-    def docker_name(self):
+    def name(self):
         """Return name of docker container."""
         return os.environ['SUPERVISOR_NAME']
 
@@ -31,41 +28,14 @@ class DockerSupervisor(DockerBase):
             return False
 
         _LOGGER.info("Update supervisor docker to %s:%s", self.image, tag)
-        old_version = self.version
 
         async with self._lock:
             if await self.loop.run_in_executor(None, self._install, tag):
-                self.config.hassio_cleanup = old_version
+                self.loop.create_task(self.stop_callback(RESTART_EXIT_CODE))
-                self.loop.create_task(self.hassio.stop(RESTART_EXIT_CODE))
                 return True
 
             return False
 
-    async def cleanup(self):
-        """Check if old supervisor version exists and cleanup."""
-        if not self.config.hassio_cleanup:
-            return
-
-        async with self._lock:
-            if await self.loop.run_in_executor(None, self._cleanup):
-                self.config.hassio_cleanup = None
-
-    def _cleanup(self):
-        """Remove old image.
-
-        Need run inside executor.
-        """
-        old_image = "{}:{}".format(self.image, self.config.hassio_cleanup)
-
-        _LOGGER.info("Old supervisor docker found %s", old_image)
-        try:
-            self.dock.images.remove(image=old_image, force=True)
-        except docker.errors.DockerException as err:
-            _LOGGER.warning("Can't remove old image %s -> %s", old_image, err)
-            return False
-
-        return True
-
     async def run(self):
         """Run docker image."""
         raise RuntimeError("Not support on supervisor docker container!")

hassio/tools.py

@@ -46,17 +46,6 @@ def get_arch_from_image(image):
         return found.group(1)
 
 
-def get_version_from_env(env_list):
-    """Extract Version from ENV list."""
-    for env in env_list:
-        found = _RE_VERSION.match(env)
-        if found:
-            return found.group(1)
-
-    _LOGGER.error("Can't find VERSION in env")
-    return None
-
-
 def get_local_ip(loop):
     """Retrieve local IP address.