-
Notifications
You must be signed in to change notification settings - Fork 143
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.
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
base: main
Are you sure you want to change the base?
Conversation
Resolve: teemtee#3414 Signed-off-by: Xiaofeng Wang <[email protected]>
# 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" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unlink
should work as well:(DEFAULT_TMP_PATH / 'images/disk.qcow2').unlink(missing_ok=True)
- 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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
Plan with bootc provision builds its own qcow2 before test run. Each plan has different Containerfile or container image configured, so each plan should use its own qcow2 built from Contaienrfile or container image. The old or previous plan used qcow2 image should be removed when plan is done.
Resolve: #3414