Implementing the functionality of the 'named' argument of the 'Image.save' method

[milanbalazs]

In the Image.save method, the named argument was ignored. The self.id argument was always used, resulting in an empty repositories file in the tarball. As a result, if the tarball was loaded into Podman, the image was not given a name (only None). This limitation can cause problems in some cases, since the name of the imported image is not known.

The used minimal script:

import podman

with podman.PodmanClient() as client:
    with open("hello_image.tgz", "wb") as file:
        image = client.images.get("hello:latest")
        for chunk in image.save(named=True):
            file.write(chunk)

Before my change:

>>> tar -xvf hello_image.tgz

c13e6cd75347b97d0c518979dfa54033e6582b8915fc222c44491438f5c33791.tar
633eb872fc662b2f070514927075745206904c96442aace19754cdab171dc4c7.json
acd44ca82826b9348d7229d430214aec57fd7b54a5f099a7b4d08aa23a806568/layer.tar
acd44ca82826b9348d7229d430214aec57fd7b54a5f099a7b4d08aa23a806568/VERSION
acd44ca82826b9348d7229d430214aec57fd7b54a5f099a7b4d08aa23a806568/json
manifest.json
repositories

>>> cat repositories

{}

>>> podman load < hello_image.tgz

Getting image source signatures
Copying blob c13e6cd75347 skipped: already exists
Copying config 633eb872fc done
Writing manifest to image destination
Storing signatures
Loaded image: sha256:633eb872fc662b2f070514927075745206904c96442aace19754cdab171dc4c7

It's visible in the last line the Image doesn't have name.

With my change:

>>> tar -xvf hello_image.tgz

c13e6cd75347b97d0c518979dfa54033e6582b8915fc222c44491438f5c33791.tar
633eb872fc662b2f070514927075745206904c96442aace19754cdab171dc4c7.json
acd44ca82826b9348d7229d430214aec57fd7b54a5f099a7b4d08aa23a806568/layer.tar
acd44ca82826b9348d7229d430214aec57fd7b54a5f099a7b4d08aa23a806568/VERSION
acd44ca82826b9348d7229d430214aec57fd7b54a5f099a7b4d08aa23a806568/json
manifest.json
repositories

>>> cat repositories

{"quay.io/podman/hello":{"latest":"acd44ca82826b9348d7229d430214aec57fd7b54a5f099a7b4d08aa23a806568"}}

>>> podman load < hello_image.tgz

Getting image source signatures
Copying blob c13e6cd75347 skipped: already exists
Copying config 633eb872fc done
Writing manifest to image destination
Storing signatures
Loaded image: quay.io/podman/hello:latest

The last line shows the loaded Image has name and tag based on the repositories file.

Note:

• I have checked the Docker Python SDK implementation as well, and my change is compatible with it, so it's shouldn't cause incompatibility issues.

Signed-off-by: Milan Balazs [email protected]

[rhatdan]

Please squash your commits.

@jwhonce @umohnani8 @mwhahaha PTAL

[milanbalazs]

Hi @rhatdan,

Thanks, I have just squashed my commits.

[jwhonce]

@milanbalazs Please add tests to support your change. Thank you for your support of this repository.

@lsm5 Please review the packit issues, TIA.

[lsm5]

/packit rebuild-failed

[lsm5]

@inknos PTAL

[lsm5]

/packit rebuild-failed

[milanbalazs]

Hi @jwhonce,

I have extended the test cases with the "named" save option.

[milanbalazs]

I have tried to deliver the extended Integration Tests but it seems the podman/tests/integration/test_images.py:116 fails constantly. Honestly, I don't see how it's connected to my change. I am not able to reproduce it in local. Could you help me out about it, please?

[milanbalazs]

/packit rebuild-failed

[milanbalazs]

Hi @lsm5,

I tried to restart the failed tests, but they seem to have stalled. Can you check, please? (And I think the function tests are still having problems, which I mentioned in my previous post.)

[lsm5]

Hi @lsm5,

I tried to restart the failed tests, but they seem to have stalled. Can you check, please? (And I think the function tests are still having problems, which I mentioned in my previous post.)

The build jobs have passed but there's an issue with status reporting on the fedora jobs. We can ignore the in-progress rpm-build jobs.

Don't know about the Test on Fedora job though. @inknos @jwhonce wdyt ?

[lsm5]

/packit rebuild-failed

[jwhonce]

@lsm5 I have not had a chance to look at what is happening with the failing test, but the expected container is not the one being reported during the prune. I suspect the new tests are bleeding into the prune test.

[milanbalazs]

@lsm5 I have not had a chance to look at what is happening with the failing test, but the expected container is not the one being reported during the prune. I suspect the new tests are bleeding into the prune test.

@jwhonce You are right, the actual Image ID is not in the pruned (deleted) image list. But I didn't really touch the prune or other parts of the code/test, this is the reason why I don't understand how this test failure connected to my change. I have tried to reproduce this behavior in local but I couldn't. Perhaps somebody from your side who has deeper understanding of the system can provide some hints.

[umohnani8]

I think I know what is happening with the test. Now that the image has a name when loaded back from the tarball, it is no longer an unnamed image so podman image prune doesn't end up deleting it and that is why it is not in the list of deleted images. Setting named=False should make the test happy again.

We should probably add another test to test the name functionality instead of updating the existing test so that we can avoid any future regressions.

[milanbalazs]

I think I know what is happening with the test. Now that the image has a name when loaded back from the tarball, it is no longer an unnamed image so podman image prune doesn't end up deleting it and that is why it is not in the list of deleted images. Setting named=False should make the test happy again.

We should probably add another test to test the name functionality instead of updating the existing test so that we can avoid any future regressions.

@umohnani8, Thank you for your comment. I have created a separate test case for the named image saving. It seems that currently the integration tests are running successfully.

[umohnani8]

LGTM
Thank you for the PR @milanbalazs!

@jwhonce @rhatdan @inknos PTAL

[jwhonce]

/lgtm

[jwhonce]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jwhonce, milanbalazs

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [jwhonce]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment