[merged] Various syscontainers fixes

Atomic/delete.py

@@ -35,14 +35,15 @@ def prune_images(self):
         """
         enc = sys.getdefaultencoding()
 
+        self.syscontainers.prune_ostree_images()
+
         results = self.d.images(filters={"dangling":True}, quiet=True)
         if len(results) == 0:
             return 0
 
         for img in results:
             self.d.remove_image(img.decode(enc), force=True)
             util.write_out("Removed dangling Image {}".format(enc))
-        self.syscontainers.prune_ostree_images()
         return 0
 
     def _delete_remote(self, targets):
@@ -72,12 +73,15 @@ def _delete_remote(self, targets):
     def _delete_local(self, targets):
         results = 0
         for target in targets:
-            try:
-                self.d.remove_image(target)
-            except NotFound as e:
-                util.write_err("Failed to delete Image {}: {}".format(target, e))
-                results = 2
-            except APIError as e:
-                util.write_err("Failed operation for delete Image {}: {}".format(target, e))
-                results = 2
+            if self.syscontainers.has_system_container_image(target):
+                self.syscontainers.delete_image(target)
+            else:
+                try:
+                    self.d.remove_image(target)
+                except NotFound as e:
+                    util.write_err("Failed to delete Image {}: {}".format(target, e))
+                    results = 2
+                except APIError as e:
+                    util.write_err("Failed operation for delete Image {}: {}".format(target, e))
+                    results = 2
         return results

Atomic/ps.py

@@ -2,7 +2,9 @@
 
 from . import util
 from . import Atomic
+import subprocess
 from dateutil.parser import parse as dateparse
+from . import atomic
 
 class Ps(Atomic):
     def ps(self):
@@ -11,8 +13,11 @@ def ps(self):
         # Collect the system containers
         for i in self.syscontainers.get_system_containers():
             container = i["Id"]
-            inspect_stdout = util.check_output(["runc", "state", container])
-            ret = json.loads(inspect_stdout)
+            try:
+                inspect_stdout = util.check_output(["runc", "state", container], stderr=atomic.DEVNULL)
+                ret = json.loads(inspect_stdout.decode())
+            except (subprocess.CalledProcessError):
+                continue
             status = ret["status"]
             if not self.args.all and status != "running":
                 continue

Atomic/syscontainers.py

@@ -11,6 +11,7 @@
 import subprocess
 import time
 from .client import AtomicDocker
+from ctypes import cdll, CDLL
 
 try:
     import gi
@@ -58,6 +59,44 @@ def __init__(self):
     def get_atomic_config_item(self, config_item):
         return util.get_atomic_config_item(config_item, atomic_config=self.atomic_config)
 
+    def _do_syncfs(self, rootfs, rootfs_fd):
+        # Fallback to sync --file-system if loading it from libc fails.
+        try:
+            cdll.LoadLibrary("libc.so.6")
+            libc = CDLL("libc.so.6")
+            if libc.syncfs(rootfs_fd) == 0:
+                return
+        except (NameError, AttributeError, OSError):
+            pass
+
+        util.check_call(["sync", "--file-system", rootfs], stdin=DEVNULL,
+                        stdout=DEVNULL,
+                        stderr=DEVNULL)
+
+    def _checkout_layer(self, repo, rootfs_fd, rootfs, rev):
+        OSTREE_SAFE_GLIB_REPO_CHECKOUT_OPTIONS = False
+        # There is an issue in the way the RepoCheckoutOptions is mapped by glib, as the C
+        # struct is using bit fields that are not supported by the introspection.
+        # Accessing .disable_fsync and .process_whiteouts thus results in a segfault in
+        # libostree.  Re-enable this once it gets fixed.
+        if OSTREE_SAFE_GLIB_REPO_CHECKOUT_OPTIONS:
+            options = OSTree.RepoCheckoutOptions()
+            options.overwrite_mode = OSTree.RepoCheckoutOverwriteMode.UNION_FILES
+            options.process_whiteouts = True
+            options.disable_fsync = True
+            repo.checkout_tree_at(options, rootfs_fd, rootfs, rev)
+        else:
+            util.check_call(["ostree", "--repo=%s" % self._get_ostree_repo_location(),
+                             "checkout",
+                             "--union",
+                             "--whiteouts",
+                             "--fsync=no",
+                             rev,
+                             rootfs],
+                            stdin=DEVNULL,
+                            stdout=DEVNULL,
+                            stderr=DEVNULL)
+
     def set_args(self, args):
         self.args = args
 
@@ -148,19 +187,18 @@ def _checkout_system_container(self, repo, name, img, deployment, upgrade, value
         rev = repo.resolve_rev(imagebranch, False)[1]
 
         manifest = SystemContainers._get_commit_metadata(repo, rev, "docker.manifest")
-        options = OSTree.RepoCheckoutOptions()
-        options.overwrite_mode = OSTree.RepoCheckoutOverwriteMode.UNION_FILES
 
         rootfs_fd = None
         try:
             rootfs_fd = os.open(rootfs, os.O_DIRECTORY)
             if manifest is None:
-                repo.checkout_tree_at(options, rootfs_fd, rootfs, rev)
+                self._checkout_layer(repo, rootfs_fd, rootfs, rev)
             else:
                 layers = SystemContainers._get_layers_from_manifest(json.loads(manifest))
                 for layer in layers:
                     rev_layer = repo.resolve_rev("%s%s" % (OSTREE_OCIIMAGE_PREFIX, layer.replace("sha256:", "")), False)[1]
-                    repo.checkout_tree_at(options, rootfs_fd, rootfs, rev_layer)
+                    self._checkout_layer(repo, rootfs_fd, rootfs, rev_layer)
+            self._do_syncfs(rootfs, rootfs_fd)
         finally:
             if rootfs_fd:
                 os.close(rootfs_fd)
@@ -247,18 +285,20 @@ def _get_system_checkout_path(self):
             self.get_atomic_config_item(["checkout_path"]) or \
             "/var/lib/containers/atomic"
 
-    def _get_ostree_repo(self):
-        if not OSTREE_PRESENT:
-            return None
-
+    def _get_ostree_repo_location(self):
         if self.user:
             home_dir = os.getenv("HOME")
-            repo_location = os.path.expanduser("%s/ostree/repo" % home_dir)
+            return os.path.expanduser("%s/ostree/repo" % home_dir)
         else:
-            repo_location = os.environ.get("ATOMIC_OSTREE_REPO") or \
-                            self.get_atomic_config_item(["ostree_repository"]) or \
-                            "/ostree/repo"
+            return os.environ.get("ATOMIC_OSTREE_REPO") or \
+                self.get_atomic_config_item(["ostree_repository"]) or \
+                "/ostree/repo"
 
+    def _get_ostree_repo(self):
+        if not OSTREE_PRESENT:
+            return None
+
+        repo_location = self._get_ostree_repo_location()
         repo = OSTree.Repo.new(Gio.File.new_for_path(repo_location))
 
         # If the repository doesn't exist at the specified location, create it
@@ -318,6 +358,17 @@ def get_system_containers(self):
             ret.append(container)
         return ret
 
+    def delete_image(self, image):
+        repo = self._get_ostree_repo()
+        if not repo:
+            return
+        imagebranch = SystemContainers._get_ostree_image_branch(image)
+        commit_rev = repo.resolve_rev(imagebranch, True)
+        if not commit_rev[1]:
+            return
+        ref = OSTree.parse_refspec(imagebranch)
+        repo.set_ref_immediate(ref[1], ref[2], None)
+
     def get_system_images(self, repo=None):
         if repo is None:
             repo = self._get_ostree_repo()
@@ -361,12 +412,11 @@ def uninstall_system_container(self, name):
         except subprocess.CalledProcessError:
             pass
 
+        if os.path.lexists("%s/%s" % (self._get_system_checkout_path(), name)):
+            os.unlink("%s/%s" % (self._get_system_checkout_path(), name))
         for deploy in ["0", "1"]:
             if os.path.exists("%s/%s.%s" % (self._get_system_checkout_path(), name, deploy)):
                 shutil.rmtree("%s/%s.%s" % (self._get_system_checkout_path(), name, deploy))
-        if os.path.exists("%s/%s" % (self._get_system_checkout_path(), name)):
-            os.unlink("%s/%s" % (self._get_system_checkout_path(), name))
-
         if os.path.exists(os.path.join(SYSTEMD_UNIT_FILES_DEST, "%s.service" % name)):
             os.unlink(os.path.join(SYSTEMD_UNIT_FILES_DEST, "%s.service" % name))
 
@@ -551,24 +601,23 @@ def _check_system_oci_image(self, repo, img, upgrade):
                 missing_layers.append(layer)
                 util.write_out("Missing layer %s" % layer)
 
-        if len(missing_layers) == 0:
-            return True
-
-        layers_dir = self._skopeo_get_layers(img, missing_layers)
+        layers_dir = None
         try:
-            layers = {}
-            for root, _, files in os.walk(layers_dir):
-                for f in files:
-                    if f.endswith(".tar"):
-                        layer_file = os.path.join(root, f)
-                        layer = f.replace(".tar", "")
-                        if layer in missing_layers:
-                            layers[layer] = layer_file
-
-            if (len(layers)):
-                SystemContainers._import_layers_into_ostree(repo, imagebranch, manifest, layers)
+            layers_to_import = {}
+            if len(missing_layers):
+                layers_dir = self._skopeo_get_layers(img, missing_layers)
+                for root, _, files in os.walk(layers_dir):
+                    for f in files:
+                        if f.endswith(".tar"):
+                            layer_file = os.path.join(root, f)
+                            layer = f.replace(".tar", "")
+                            if layer in missing_layers:
+                                layers_to_import[layer] = layer_file
+
+            SystemContainers._import_layers_into_ostree(repo, imagebranch, manifest, layers_to_import)
         finally:
-            shutil.rmtree(layers_dir)
+            if layers_dir:
+                shutil.rmtree(layers_dir)
         return True
 
     @staticmethod

migrate.sh

@@ -130,20 +130,18 @@ container_export(){
 	echo $containerBaseImageID>>containerInfo.txt
 	echo $notruncContainerID>>containerInfo.txt
         "$GOTAR" -cf container-metadata.tar $dockerRootDir/containers/$notruncContainerID 2> /dev/null
-	if [[ ! -z $(docker diff $containerID) ]];then
-                imageName=$(echo $RANDOM)
-                docker commit $containerID $imageName 1>/dev/null||exit 1
-                mkdir -p $tmpDir/temp
-                docker save $imageName > $tmpDir/temp/image.tar||exit 1
-                $(cd $tmpDir/temp; "$GOTAR" -xf image.tar)
-                diffLayerID=$(python -c 'import json; f=open("temp/repositories"); j=json.load(f); print(j[j.keys()[0]]["latest"])')
-                cd $tmpDir/temp/$diffLayerID
-                cp layer.tar $tmpDir/container-diff.tar
-                cd $tmpDir
-                /usr/bin/tar --delete -f container-diff.tar run/gotar 2>/dev/null || true
-                rm -rf temp
-                docker rmi -f $imageName 1>/dev/null||exit 1
-	fi
+        imageName=$(echo $RANDOM)
+        docker commit $containerID $imageName 1>/dev/null||exit 1
+        mkdir -p $tmpDir/temp
+        docker save $imageName > $tmpDir/temp/image.tar||exit 1
+	$(cd $tmpDir/temp; "$GOTAR" -xf image.tar)
+        diffLayerID=$(python -c 'import json; f=open("temp/repositories"); j=json.load(f); print(j[j.keys()[0]]["latest"])')
+        cd $tmpDir/temp/$diffLayerID
+        cp layer.tar $tmpDir/container-diff.tar
+        cd $tmpDir
+        /usr/bin/tar --delete -f container-diff.tar run/gotar 2>/dev/null || true
+        rm -rf temp
+        docker rmi -f $imageName 1>/dev/null||exit 1
 }
 
 container_import(){
@@ -186,12 +184,8 @@ container_import(){
         fi
 
 	cd $importPath/containers/migrate-$containerID
-	dockerBaseImageID=$(sed -n '2p' containerInfo.txt)||exit 1
-        if [[ -f container-diff.tar ]];then
-                cat container-diff.tar|docker run -i -v "$GOTAR:/run/gotar" $dockerBaseImageID /run/gotar -xf -
-	else
-		docker run -i $dockerBaseImageID echo "container_import"
-	fi
+	dockerBaseImageID=$(sed -n '2p' containerInfo.txt)||exit 1	
+	cat container-diff.tar|docker run -i -v "$GOTAR:/run/gotar" $dockerBaseImageID /run/gotar -xf -
 	newContainerID=$(docker ps -lq)||exit 1
 	newContainerName=$(docker inspect -f '{{.Name}}' $newContainerID)||exit 1
 	newNotruncContainerID=$(docker ps -aq --no-trunc|grep $newContainerID)||exit 1					

tests/integration/test_migrate.sh

@@ -2,6 +2,13 @@
 set -euo pipefail
 IFS=$'\n\t'
 
+#With the inclusion of this PR (https://github.com/projectatomic/atomic/pull/294)
+#atomic storage export/import will only work with docker 1.10 support.
+#Skip this test, until we move to docker 1.10.
+
+echo "WARNING: skipping test_migrate.sh since it is only supported with docker 1.10 onwards."
+exit 0
+
 #
 # 'atomic storage' integration tests (non-live)
 # AUTHOR: Shishir Mahajan <shishir dot mahajan at redhat dot com>
@@ -18,13 +25,7 @@ if [ "$init" != "systemd" ];then
     exit 0
 fi
 
-if ! systemctl is-active docker >/dev/null; then
-     echo "Docker daemon is not running"
-     exit 1
-fi
-
-pid=$(systemctl show -p MainPID docker.service)
-dockerPid=$(echo ${pid#*=})
+dockerPid=$(ps -C docker -o pid=|xargs)
 dockerCmdline=$(cat /proc/$dockerPid/cmdline)
 if [[ $dockerCmdline =~ "-g=" ]] || [[ $dockerCmdline =~ "-g/" ]] || [[ $dockerCmdline =~ "--graph" ]];then
    echo "Docker is not located at the default (/var/lib/docker) root location. Skipping these tests."