testutil: fix make_container() cleanup

During the work on PR#1752 Florian discovered that make_containers()
is broken for nested containers like:
```
with make_container(tmp_path, {"file1": "file1 from base"}) as base_tag:
    with make_container(tmp_path, {"file1": "file1 from final layer"}, base_tag) as cont_tag:
```
It errors with:
```
Error: 5b947de461ee21b858dd5b4224e80442b2f65b6410189147f2445884d9e4e3d8: image not known
```
The reason is that we work with hashes for the image and then call
`podman image rm` which by default will also remove all dangling
references. Those are defined by not having a tag and not referenced
anymore. So the inner container cleanup also removes the outter.

There are many ways to fix this, I went with re-adding tags to the
test containers because it also makes it easy for the user to see if
we left any containers (accidently) around.
This commit is contained in:
Michael Vogt 2024-04-25 13:09:27 +02:00
parent 15e969c4c6
commit a3f86a0736
2 changed files with 14 additions and 16 deletions

View file

@ -5,9 +5,11 @@ import contextlib
import inspect import inspect
import os import os
import pathlib import pathlib
import random
import re import re
import shutil import shutil
import socket import socket
import string
import subprocess import subprocess
import tempfile import tempfile
import textwrap import textwrap
@ -125,6 +127,7 @@ def mock_command(cmd_name: str, script: str):
@contextlib.contextmanager @contextlib.contextmanager
def make_container(tmp_path, fake_content, base="scratch"): def make_container(tmp_path, fake_content, base="scratch"):
fake_container_tag = "osbuild-test-" + "".join(random.choices(string.digits, k=12))
fake_container_src = tmp_path / "fake-container-src" fake_container_src = tmp_path / "fake-container-src"
fake_container_src.mkdir(exist_ok=True) fake_container_src.mkdir(exist_ok=True)
make_fake_tree(fake_container_src, fake_content) make_fake_tree(fake_container_src, fake_content)
@ -134,22 +137,16 @@ def make_container(tmp_path, fake_content, base="scratch"):
COPY . . COPY . .
""" """
fake_containerfile_path.write_text(container_file_content, encoding="utf8") fake_containerfile_path.write_text(container_file_content, encoding="utf8")
p = subprocess.Popen([ subprocess.check_call([
"podman", "build", "podman", "build",
"--no-cache", "--no-cache",
"-t", fake_container_tag,
"-f", os.fspath(fake_containerfile_path), "-f", os.fspath(fake_containerfile_path),
], universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) ])
while True:
line = p.stdout.readline()
if line == "":
break
print(line)
container_id = line.strip()
p.wait()
try: try:
yield container_id yield fake_container_tag
finally: finally:
subprocess.check_call(["podman", "image", "rm", container_id]) subprocess.check_call(["podman", "image", "rm", fake_container_tag])
@contextlib.contextmanager @contextlib.contextmanager

View file

@ -9,7 +9,7 @@ import pytest
from osbuild.testutil import has_executable, make_container, mock_command from osbuild.testutil import has_executable, make_container, mock_command
def test_make_container_bad_podman_prints_podman_output(tmp_path, capsys): def test_make_container_bad_podman_prints_podman_output(tmp_path, capfd):
fake_broken_podman = textwrap.dedent("""\ fake_broken_podman = textwrap.dedent("""\
#!/bin/sh #!/bin/sh
echo fake-broken-podman echo fake-broken-podman
@ -19,11 +19,12 @@ def test_make_container_bad_podman_prints_podman_output(tmp_path, capsys):
with pytest.raises(subprocess.CalledProcessError): with pytest.raises(subprocess.CalledProcessError):
with make_container(tmp_path, {}) as _: with make_container(tmp_path, {}) as _:
pass pass
assert "fake-broken-podman" in capsys.readouterr().out assert "fake-broken-podman" in capfd.readouterr().out
@pytest.mark.skipif(not has_executable("podman"), reason="no podman executable") @pytest.mark.skipif(not has_executable("podman"), reason="no podman executable")
def test_make_container_integration(tmp_path, capsys): def test_make_container_integration(tmp_path, capfd):
with make_container(tmp_path, {"/etc/foo": "foo-content"}) as cref: with make_container(tmp_path, {"/etc/foo": "foo-content"}) as cref:
assert len(cref) == 64 # names have the form "osubild-test-<random-number-of-len12>"
assert "COMMIT" in capsys.readouterr().out assert len(cref) == len("osbuild-test-123456789012")
assert "COMMIT" in capfd.readouterr().out