[virt-tools-list] [virt-bootstrap 4/4] docker: Add support for whiteout files

[Radostin Stoyanov]

On 06/08/2019 19:21, Cole Robinson wrote:
> On 8/4/19 2:06 PM, Radostin Stoyanov wrote:
>> A whiteout is an empty file with a special name that signifies
>> a path that should not be present in upper layers.
>>
>> When container image layer contains whiteouts (described in [2])
>> virt-bootstrap should handle them appropriately:
>>
>> .wh.PATH     : PATH should be deleted
>> .wh..wh..opq : all children (including sub-directories and all
>> 	       descendants) of the folder containing this file
>> 	       should be removed.
>>
>> [1] https://github.com/opencontainers/image-spec/blob/master/layer.md#whiteouts
>> [2] https://github.com/moby/moby/blob/d1f470946/pkg/archive/whiteouts.go
>>
>> Signed-off-by: Radostin Stoyanov <rstoyanov1 at gmail.com>
>> ---
>>   src/virtBootstrap/utils.py    | 12 ++++-
>>   src/virtBootstrap/whiteout.py | 93 +++++++++++++++++++++++++++++++++++
>>   2 files changed, 103 insertions(+), 2 deletions(-)
>>   create mode 100644 src/virtBootstrap/whiteout.py
>>
>> diff --git a/src/virtBootstrap/utils.py b/src/virtBootstrap/utils.py
>> index d6031f1..245d8e0 100644
>> --- a/src/virtBootstrap/utils.py
>> +++ b/src/virtBootstrap/utils.py
>> @@ -35,6 +35,7 @@ import logging
>>   import shutil
>>   
>>   import passlib.hosts
>> +from virtBootstrap import whiteout
>>   
>>   try:
>>       import guestfs
>> @@ -279,8 +280,12 @@ def safe_untar(src, dest):
>>       # Note: Here we use --absolute-names flag to get around the error message
>>       # "Cannot open: Permission denied" when symlynks are extracted, with the
>>       # qemu:/// driver. This flag must not be used outside virt-sandbox.
>> -    params = ['--', '/bin/tar', 'xf', src, '-C', '/mnt', '--exclude', 'dev/*',
>> -              '--overwrite', '--absolute-names']
>> +    params = ['--', '/bin/tar', 'xf', src,
>> +              '-C', '/mnt',
>> +              '--exclude', 'dev/*',
>> +              '--exclude', '*/%s*' % whiteout.PREFIX,
>> +              '--overwrite',
>> +              '--absolute-names']
>>       # Preserve file attributes following the specification in
>>       # https://github.com/opencontainers/image-spec/blob/master/layer.md
>>       if os.geteuid() == 0:
>> @@ -341,6 +346,9 @@ def untar_layers(layers_list, dest_dir, progress):
>>           tar_file, tar_size = layer
>>           log_layer_extract(tar_file, tar_size, index + 1, nlayers, progress)
>>   
>> +        # Apply whiteout changes with respect to parent layers
>> +        whiteout.apply_whiteout_changes(tar_file, dest_dir)
>> +
>>           # Extract layer tarball into destination directory
>>           safe_untar(tar_file, dest_dir)
>>   
>> diff --git a/src/virtBootstrap/whiteout.py b/src/virtBootstrap/whiteout.py
>> new file mode 100644
>> index 0000000..1f9efd4
>> --- /dev/null
>> +++ b/src/virtBootstrap/whiteout.py
>> @@ -0,0 +1,93 @@
>> +# -*- coding: utf-8 -*-
>> +# Authors: Radostin Stoyanov <rstoyanov1 at gmail.com>
>> +#
>> +# Copyright (c) 2019 Radostin Stoyanov
>> +#
>> +# This program is free software: you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation, either version 3 of the License, or
>> +# (at your option) any later version.
>> +
>> +# This program 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 General Public License for more details.
>> +
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +"""
>> +Whiteouts are files with a special meaning for a layered filesystem.
>> +The should not be extracted in the destination directory.
>> +
> * They should
>
>> +Whiteout prefix (.wh.) followed by a filename means that the file has
>> +been removed from.
>> +
> Ending with 'from' sounds a bit weird here. Maybe 'means that the file
> has been removed ' but I'm not sure that's totally accurate
>
>> +Whiteout meta prefix (.wh..wh.) has special meaning which is not for
>> +removing a file.
>> +"""
> This reads a little weird too. Maybe it's fine to just drop this text
> and add a link to the file(s) you mention in the commit message. The
> process_whiteout function below already describes the behavior well, not
> sure if it needs to be duplicated here.
>
>
>> +
>> +import logging
>> +import os
>> +import shutil
>> +import tarfile
>> +
>> +
>> +PREFIX = ".wh."
>> +METAPREFIX = PREFIX + PREFIX
>> +OPAQUE = METAPREFIX + ".opq"
>> +
>> +# pylint: disable=invalid-name
>> +logger = logging.getLogger(__name__)
>> +
>> +
>> +def apply_whiteout_changes(tar_file, dest_dir):
>> +    """
>> +    Process files with whiteout prefix and apply
>> +    changes in destination folder.
>> +    """
>> +    for path in get_whiteout_files(tar_file):
>> +        basename = os.path.basename(path)
>> +        dirname = os.path.dirname(path)
>> +        dirname = os.path.join(dest_dir, dirname)
>> +
>> +        process_whiteout(dirname, basename)
>> +
>> +
>> +def get_whiteout_files(filepath):
>> +    """
>> +    Return a list of whiteout files from tar file
>> +    """
>> +    whiteout_files = []
>> +    with tarfile.open(filepath) as tar:
>> +        for path in tar.getnames():
>> +            if os.path.basename(path).startswith(PREFIX):
>> +                whiteout_files.append(path)
>> +    return whiteout_files
>> +
>> +
>> +def process_whiteout(dirname, basename):
>> +    """
>> +    Process a whiteout file:
>> +
>> +    .wh.PATH     : PATH should be deleted
>> +    .wh..wh..opq : all children (including sub-directories and all
>> +                   descendants) of the folder containing this file
>> +                   should be removed
>> +
>> +    When a folder is first created in a layer an opq file will be
>> +    generated. In such case, there is nothing to remove we can simply
>> +    ignore the opque whiteout file.
>> +    """
>> +    if basename == OPAQUE:
>> +        if os.path.isdir(dirname):
>> +            shutil.rmtree(dirname)
>> +            os.makedirs(dirname)
>> +    elif not basename.startswith(METAPREFIX):
>> +        target = os.path.join(dirname, basename[len(PREFIX):])
>> +        if os.path.isfile(target):
>> +            os.remove(target)
>> +        elif os.path.isdir(target):
>> +            shutil.rmtree(target)
>> +        else:
>> +            logger.error("%s is not a file or directory", target)
>>
> I applied it but didn't test it, but the code looks fine to me.
>
> Reviewed-by: Cole Robinson <crobinso at redhat.com>
Thank you for the code review. I fixed the comment above and pushed this 
patch series.

Radostin