Re: [Qemu-devel] [PATCH] docker: Improved image checksum

[Alex Bennée]

Philippe Mathieu-Daudé <address@hidden> writes:

> Thanks Fam, this fixes Shippable builds :)
>
> On 11/03/2017 10:12 AM, Fam Zheng wrote:
>> When a base image locally defined by QEMU, such as in the debian images,
>> is updated, the dockerfile checksum mechanism in docker.py still skips
>> updating the derived image, because it only looks at the literal content
>> of the dockerfile, without considering changes to the base image.
>>
>> For example we have a recent fix e58c1f9b35e81 that fixed
>> debian-win64-cross by updating its base image, debian8-mxe, but due to
>> above "feature" of docker.py the image in question is automatically NOT
>> rebuilt unless you add NOCACHE=1. It is noticed on Shippable:
>>
>> https://app.shippable.com/github/qemu/qemu/runs/541/2/console
>>
>> because after the fix is merged, the error still occurs, and the log
>> shows the container image is, as explained above, not updated.
>>
>> This is because at the time docker.py was written, there wasn't any
>> dependencies between QEMU's docker images.
>>
>> Now improve this to preprocess any "FROM qemu:*" directives in the
>> dockerfiles while doing checksum, and inline the base image's dockerfile
>> content, recursively. This ensures any changes on the depended _QEMU_
>> images are taken into account.
>>
>> This means for external images that we expect to retrieve from docker
>> registries, we still do it as before. It is not perfect, because
>> registry images can get updated too. Technically we could substitute the
>> image name with its hex ID as obtained with $(docker images $IMAGE
>> --format="{{.Id}}"), but --format is not supported by RHEL 7, so leave
>> it for now.
>>
>> Reported-by: Philippe Mathieu-Daudé <address@hidden>
>> Signed-off-by: Fam Zheng <address@hidden>
>
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
> Tested-by: Philippe Mathieu-Daudé <address@hidden>

Reviewed-by: Alex Bennée <address@hidden>

>
>> ---
>>  tests/docker/docker.py | 24 +++++++++++++++++++++++-
>>  1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
>> index 08122ca17d..1246ba9578 100755
>> --- a/tests/docker/docker.py
>> +++ b/tests/docker/docker.py
>> @@ -105,6 +105,28 @@ def _copy_binary_with_libs(src, dest_dir):
>>              so_path = os.path.dirname(l)
>>              _copy_with_mkdir(l , dest_dir, so_path)
>>
>> +def _read_qemu_dockerfile(img_name):
>> +    df = os.path.join(os.path.dirname(__file__), "dockerfiles",
>> +                      img_name + ".docker")
>> +    return open(df, "r").read()
>> +
>> +def _dockerfile_preprocess(df):
>> +    out = ""
>> +    for l in df.splitlines():
>> +        if len(l.strip()) == 0 or l.startswith("#"):
>> +            continue
>> +        from_pref = "FROM qemu:"
>> +        if l.startswith(from_pref):
>> +            # TODO: Alternatively we could replace this line with "FROM $ID"
>> +            # where $ID is the image's hex id obtained with
>> +            #    $ docker images $IMAGE --format="{{.Id}}"
>> +            # but unfortunately that's not supported by RHEL 7.
>> +            inlining = _read_qemu_dockerfile(l[len(from_pref):])
>> +            out += _dockerfile_preprocess(inlining)
>> +            continue
>> +        out += l + "\n"
>> +    return out
>> +
>>  class Docker(object):
>>      """ Running Docker commands """
>>      def __init__(self):
>> @@ -196,7 +218,7 @@ class Docker(object):
>>              checksum = self.get_image_dockerfile_checksum(tag)
>>          except Exception:
>>              return False
>> -        return checksum == _text_checksum(dockerfile)
>> +        return checksum == 
>> _text_checksum(_dockerfile_preprocess(dockerfile))
>>
>>      def run(self, cmd, keep, quiet):
>>          label = uuid.uuid1().hex
>>


--
Alex Bennée