Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bootc: remove qcow2 when plan is done #3419

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

bootc: remove qcow2 when plan is done #3419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions tests/provision/bootc/test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
IMAGE_NEEDS_DEPS="localhost/tmt-bootc-needs-deps"
IMAGE_INCLUDES_DEPS="localhost/tmt-bootc-includes-deps"

TESTCLOUD_IMAGE="/var/tmp/tmt/testcloud/images/disk.qcow2"


rlJournalStart
rlPhaseStartSetup
Expand All @@ -19,23 +17,19 @@ rlJournalStart
rlPhaseStartTest "Image that needs dependencies"
rlRun "podman build . -f needs-deps.containerfile -t $IMAGE_NEEDS_DEPS"
rlRun "tmt -vvv run --scratch -i $run plan --name /plans/image/needs-deps"
rlRun "rm -rf $TESTCLOUD_IMAGE"
rlPhaseEnd

rlPhaseStartTest "Image that already includes dependencies"
rlRun "podman build . -f includes-deps.containerfile -t $IMAGE_INCLUDES_DEPS"
rlRun "tmt -vvv run --scratch -i $run plan --name /plans/image/includes-deps"
rlRun "rm -rf $TESTCLOUD_IMAGE"
rlPhaseEnd

rlPhaseStartTest "Containerfile that needs dependencies"
rlRun "tmt -vvv run --scratch -i $run plan --name /plans/containerfile/needs-deps"
rlRun "rm -rf $TESTCLOUD_IMAGE"
rlPhaseEnd

rlPhaseStartTest "Containerfile that already includes dependencies"
rlRun "tmt -vvv run --scratch -i $run plan --name /plans/containerfile/includes-deps"
rlRun "rm -rf $TESTCLOUD_IMAGE"
rlPhaseEnd

rlPhaseStartCleanup
Expand Down
6 changes: 6 additions & 0 deletions tmt/steps/provision/bootc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ def remove(self) -> None:
logger=self._logger,
env=PODMAN_ENV if self._rootless else None)

# plan with bootc provision should build its own qcow2 before test run
# test qcow2 image should be removed when plan is done
tmt.utils.Command(
"rm", "-vf", f"{DEFAULT_TMP_PATH}/images/disk.qcow2"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. unlink should work as well: (DEFAULT_TMP_PATH / 'images/disk.qcow2').unlink(missing_ok=True)
  2. but what caught my eye: is this a single path shared by all plans and all bootc plugin invocations? That seems like a huge risk of conflict, being a shared, mutable state without proper synchronization. Can the name be changed, to reflect which run, plan, and guest it belongs to? Because this is not good, if all runs and plans all compete over the same file, we are just asking for trouble. Either its name or path should be different, perhaps the file should be moved under a workdir, something like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment.

The bib build disk.qocw2 and save into {self.workdir}/qcow2/disk.qcow2. The workdir is different for each plan.

The disk.qcow2 should be copied to DEFAULT_TMP_PATH}/images by virtual.testcloud when testcloud run VM. Is this correct?

What happens when defined qcow2 path https://github.com/teemtee/tmt/blob/5dc0a1f4441a7a3727fc36160302355d7d737363/tmt/steps/provision/bootc.py#L318C31-L318C62 for each plan and run it

tmt/tmt/steps/provision/bootc.py

Line 344 in 5dc0a1f

self._guest = GuestBootc(
?

The name disk.qcow2 is hardcoded by bib.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data.image = f"file://{self.workdir}/qcow2/disk.qcow2" looks good, that's what I would expect to exist.

The disk.qcow2 should be copied to DEFAULT_TMP_PATH}/images by virtual.testcloud when testcloud run VM. Is this correct?

I see. No, I think this spells trouble. I don't mean now, bootc plugin is basically a tech preview, but once more people start playing with it and someone tries more than one provision phase in their plan, multiple bootc, it may easily become a problem as virtual would use the same qcow2 file for all of them. Which may not necessarily be the right choice...

I think it's something left to resolve, sooner rather than later, and we might need help from testcloud owners.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @happz! I'm not tmt expert, so anyone can help on this?

).run(cwd=self.workdir, stream_output=True, logger=self._logger)

try:
tmt.utils.Command(
"podman", "machine", "rm", "-f", PODMAN_MACHINE_NAME
Expand Down