Conversation
Some tests will have multiple instances of QEMU VMs, one example is a migration test. Some files are not created with names unique enough to identify each instance. There's some prior art to name them after the QEMU process (if that has been created already). This adds the seabios log files to the same pattern, so it's easier to correlate them. Before this: $ avocado run migrate.default.tcp.default $ ls -1 $TEST_RESULTS | grep avocado-vt-vm1 | grep \.log\$ catch_monitor-avocado-vt-vm1-pid-564872.log catch_monitor-avocado-vt-vm1-pid-565316.log qmpmonitor1-avocado-vt-vm1-pid-564872.log qmpmonitor1-avocado-vt-vm1-pid-565316.log seabios-avocado-vt-vm1.log After this: ... catch_monitor-avocado-vt-vm1-pid-562753.log catch_monitor-avocado-vt-vm1-pid-563200.log qmpmonitor1-avocado-vt-vm1-pid-562753.log qmpmonitor1-avocado-vt-vm1-pid-563200.log seabios-avocado-vt-vm1-pid-562753.log seabios-avocado-vt-vm1-pid-563200.log ... Signed-off-by: Cleber Rosa <crosa@redhat.com>
Larger changes are necessary to allow for Avocado-VT to transparently support running multiple tp-qemu tests in parallel. But, enabling automatic cloning of images and making them unique (based on the prior art of one file per VM and process) goes a long way. This follows a naming pattern used for log files, and although the default directories for data are not the same for log files, it makes them *somewhat* easier to correlate. The PIDs won't match given that the creation of the image files happen before the creation of the QEMU process, but their parent PIDs should match. Signed-off-by: Cleber Rosa <crosa@redhat.com>
Signed-off-by: Cleber Rosa <crosa@redhat.com>
|
Warning Review limit reached
Next review available in: 30 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
pevogam
left a comment
pevogam
left a comment
There was a problem hiding this comment.
Hmmm, I don't understand the goal of this pull request. We already have not some but fully parallel test execution. This is the entire reason we developed LXC and even remote spanwers on the first place in avocado core. So having a "partial support or parallelism" like this considering we already have complete support and documenting this as the way to run in parallel seems orthogonal to me.
@clebergnu Could you elaborate more on the use cases you actually need?
Hi @pevogam , the reason for this is quite simple. When you're testing the virtualization stack itself (for instance QEMU/KVM) using other isolation technologies (like LXC) is not an option. Also, many developers on our side have access to one machine with lots of resources that shouldn't be wasted. And if you take the first two patches at their face value, they simply add more naming consistency and easier correlation to which instance created the logs and image files. Do you see any regressions? |
| outfile = os.path.join( | ||
| utils_logfile.get_log_file_dir(), "%s-%s.log" % (key, name) | ||
| utils_logfile.get_log_file_dir(), | ||
| "%s-%s-pid-%s.log" % (key, name, self.get_pid()), |
There was a problem hiding this comment.
While being at it can we also s/seabios/firmware/ ?
Could you elaborate more on this statement? Why would that be the case? How does LXC effect the validated Qemu functionality and how does that differ from e.g. a podman isolated test validating some custom software functionality?
Are you suggesting that LXC adds too much overhead? If these developers have enough resources to run more than one VT test at a time then I don't think LXC will add much overhead, containers are designed to be lightweight. Also, properly isolating test environments with VT tests is just as important as isolating python unit tests with podman, and is supposed to be just as lightweight (minimum overhead).
This could be additive in isolation yes, however I would not consider this as as a solution of what is claimed here.
I have not tested yet because at first I want to hear what the motivation for this is. So far I still haven't fully grasped why not live a simpler live and use the canonical way we all run parallel tests with proper isolation. My concern with all of this is that a lot of users might settle for partial workarounds like these and convert that to technical debt for the maintainers instead of trying a more established way that is more compatible with the avocado framework design on the first place. Of course I am willing to help and coordinate in any way possible so that such functionality becomes easy to set up and use and am already taking steps (now within maintenance) to improve such integration. See e.g. the epic issue #4388 but even without it we could open issues regarding unclear points on how to run Avocado VT in parallel. We could note down an issue where I will add documentation similar to the one you gave here about how to run tests in parallel but canonical and not a partial implementation like this one which would mislead new users about the non-minesweeper way of running VT tests in parallel. |
|
Regarding the pure additivity of the mentioned separation, it also has cases of clear disadvantages. Consider for instance simple automation that grabs on a specific qemu log using a standard name. If one uses proper isolation which is already needed on the first place they can run their log grep and other automation scripts to just grab the standard log file name like |
Running in a namespace simply isn't the same thing. Most obvious case is where the virtualization stack uses namespaces itself, passt networking comes to mind. It also adds overhead + complexity, you now not only have to manage the host, but also the containers (make sure they have the qemu / firmware / libvirt / ... packages installed you want test). "Here is a lxe setup example script" doesn't cut it. |
Yes, for what it is worth it is a better thing since it decouples from your own environment. Especially for very environmentally-coupled tests providing identical reproducible environment is a must.
I am not familiar with
Right, the same applies for any container out there one has to set up be it docker, podman, etc. and is a very standard thing for literally any CI or environment preparation for a test. So I don't see how this is an argument against using LXC isolation around the vms. Worse yet, I don't see how not having any test isolation and being careful about which tests are run is lower complexity than such standard procedure.
Not sure but sentences like these are increasingly abstract and don't elaborate on any detail or specific argument. Just "doesn't cut it" doesn't tell me what the problem you are point at is. Such script is plenty helpful like any kind of automation of sequence of imperative steps to set up an environment, just like a YAML workflow to install dependencies in your CI. So all in all I have heard just two arguments here, one of which is partial and maybe something can be done to address it while the other one doesn't make sense with contemporary test isolation approaches. |
Not everything. Specifically containers do by design not decouple the kernel, which is in virtualization testing a rather important piece of the software stack because kvm lives there.
Indeed. Using a container isn't the only solution to that problem though.
Granted, probably there are many test cases where it does not make much of a difference whenever they run in a container or not. That still doesn't make "just run everything in a container" a workable approach.
There is such a standard setup procedure, it just doesn't use containers. It's roughly this (simplified):
There is /zero/ integration with avocado it seems. I have to prepare the containers beforehand so avocado can use them. I guess when looking at the larger scope the underlying problem is that avocado simply does not care about machine preparation. We have a non-trivial script collection handling that because avocado doesn't. |
Right, if you need to isolate from the kernel you would likely have to use some virt-in-virt concept but in any case the point here remains that:
I don't recall claiming it was the only solution. This being said, I would be glad if you provide concrete alternatives since at present at least for our production testing LXC is the most isolating way we know of and use to be able to run VT tests in parallel.
It still seems fairly ubiquitous and commonly established way, not to mention that in this particular case it provides the greatest isolation available if we don't consider some virt-in-virt setting.
Hmmm, do I understand it right that you claim that setting up a complete host machine (with all previously needed dependencies, OS install on top) is lower complexity than setting up one potentially clonable container and reusing + decoupling from your host environment?
Again, "zero integration" is overly strong wording as the bash script is already part of avocado-vt, installs necessary dependencies for running VT tests and mostly focuses on preparing an LXC environment.
Same applies to your own host or even host + host OS. And in the same way you reuse your host OS you can also reuse the container for as long as you like, difference being that you can have 5+ containers instead of needing five host machines and can easily destroy and clone these if needed.
Indeed and we have an upcoming epic PR that plans to help with at least one scope of this - preparing vms and reusing vm states at #4388. I am not sure how the podman situation is with creating containers there and it is likely much more automated than the LXC option but these are application-centric containers that are easy created and immediately destroyed. The idea with LXC containers is more like the host idea you described - to create them once and then if you like forget about recreating them and run your test for as long as you like (the "slots" idea in the documentation). If of course you are still more interested in using direct hardware and multiple hosts, this is also doable and achieves parallelism with avocado VT by using the remote spawner instead. We also do distributed testing for some of our more complex simulated network topologies and use the remote spawner instead of the LXC spawner for that. |
First, as already mentioned testing in containers does not work in all cases for various different reasons. You might not like that fact as repeated attempts to discuss it away show, it is still there though. Second, it's not like we are starting from scratch, there is a large number of test cases and quite a number of them predate any kind of container support in avocado. So, to come back to this PR, collecting some low-hanging fruit when it comes to enable parallel execution without switching the whole thing into container mode (which is a much larger project) makes total sense to me.
Scripting for that is there today, and it will continue to be there because there are enough test cases which we can't switch into container mode. So container support would have to be added to that, which clearly increases complexity. We would need to either somehow clone the host into a container, or update the setup scripts so they work on both bare metal and in containers, ...
"integration" would be avocado doing the container setup for me. I'd envision something like the 'prepare' steps supported by tmt test plans, where you can essentially declare everything needed to prepare the test environment (install packages, run scripts, enable FIPS mode, ...). Steps can be conditional (do this only on fedora, do that only in containers, ...). So tmt can do the full setup for you, no matter what environment (bare metal / container / VM) you are using. Oh, and tmt creates a work directory for every test plan run, and it is no problem to run multiple test plans in parallel even without using containers. At least as long as all test cases play nice and keep their stuff inside the work directory, so there are no file name clashes. And if I understand this PR correctly @clebergnu essentially wants that for avocado too. What exactly is so bad about that?
I think I've already mentioned that the kernel is an important part of virtualization testing. So if I want test five different host OSes (in parallel) I actually need five host machines. You are still trying to sell me advantages of containers which simply don't apply when it comes to virtualization testing.
That is not the typical case btw. The common case is that the test machines are frequently reinstalled + used for test runs (fully automatic). It is also possible to do the setup once, then run tests manually multiple times, but that is more common when developing test cases, not for the regular test runs. |
Sorry but this is not about "like" so let's stop adding personal taste here without concrete rational detail as it seems to have been done multiple times in your previous comments. These are my repeated attempts to stir this in an impartial tone and arguments that actually build on top of each other or actually counter each other.
Right, and all of them are serial tests and have only been run in a serial mode. I don't see what this historical argument serves, maybe not move into parallel mode because we didn't start in parallel mode or something like that.
Yes, this might have a point but does not address the namespacing concerns I pointed out above in my original review. Feel free to address those if you would like.
I assume you are referring here to some privately owned/developed setup within your company? I could always assist when it comes to upstreamed code and setup here. So if by "increased complexity" you mean moving from one environment to the other then yes, there could be such temporary differential. What I am referring though is the long term comparison of two static in time setups - one where you set up a bunch of hosts and one where you set up a bunch of containers. In this more global setting I tend to disagree here and I believe most readers and commenters here would consider the complexity of managing bare metal hosts much worse.
There are at least included scripts which is at least a step in that direction. And in my previous reply I point to some further attempts to unify environments like this.
As you point out here too though, I don't see much of a difference between containers and hosts using any PoC like this with the exception of containers providing more flexibility.
As Cleber correctly points out you have to be careful which VT tests you restrict to in your filter since this is only partial support of parallelism. There is no need for such careful narration with proper test isolation which is already a must in modern test environments anyway. Also another pointer to my earlier concerns in my first review here.
Yes, and just one host machine in the case I mentioned. I am starting to lose the thread of what you are arguing about to be honest.
I am not trying to "sell you" since you are not a customer of mine so one final time I kindly ask you to keep a good and respectful tone here. And this is another generic sentence without anything specific.
Right but the typical cases also contain various forms of caching when and where possible and containers as well as qemu vms support various forms of snapshots that represent a beneficial and more sophisticated step in caching. Anyway, this is already likely burning significant brain cycles for both of us so I would kindly ask for everyone else to also share their opinion here hopefully after reading both sides of the argumentation in good detail. |
Whatever your motivation is, you keep pushing for containers. I tell you containers have their limits and we simply can't move all testing into containers. That message seems to not arrive. Or you don't believe me. So we run in circles.
See? You are arguing as if we can just move /everything/ to containers. We can't. The need to setup hosts is not going away. So it is not "hosts" vs. "containers", it actually is "hosts" vs. "hosts + containers". |
Please back read again in more detail and with more care. If you do you will spot me writing exactly this. We run in circles because you don't build on arguments any more and have repeated the same vague pointers over and over again.
Sorry I don't see what you claim I say in the quote sentence you included. Anyway please let's include other people in this conversation and patiently wait for them since I don't find this ongoing discussion with you fruitful past the first few posts. |
Ok, sounds like you simply don't believe that moving testing into containers can be problematic (depending on test case). Sorry, but that is not up for discussion. It's a fact. Running in a container simply is not the same thing. Sometimes it doesn't make much of a difference. There are enough cases where it does make a difference. Incomplete list:
|
Strange that you keep writing and telling me what I think and suggest. I never said "I don't believe that moving testing into containers can be problematic", I keep asking you to quote me appropriately if possible but to no avail. I already explained that no solution is perfect but some isolation (validity and independence of test result) is better than no isolation at all. And I kindly asked you to let someone else speak next instead of adding more to this and pulling me back but you decided to disrespect that request. I will reply one last time and if we drag this further due to lack of resources to constantly deal with just one frequent unhappy poster telling me what I like, what I think, and what I try to sell, I will have to lock this discussion until other maintainers take part (and unlock it).
Indeed, there is a "maybe" I clearly implied here. Same apples to the "maybe not" too so just like I wouldn't like speculating I would prefer we avoid both sides of the speculation here.
We use nested containers all the time to define "swarms" of parallel VT runs and LXC has very good support for nesting. We didn't need any special setup for this other than duplicating the same original setup so that we can run both serially in a supercontainer and parallely in subcontainers. Not sure if I even mentioned the increased security of running even serial VT tests in a single container so we even disallow running VT tests outside containers at present.
We have used nesting for many years.
If this is not an option for you, you are still free to use remote spawners (mentioned above) or serial runs. We have been using container nesting together with virtualization for many years in our own production testing that I can say I don't have any trouble with that.
Yes which again nicely decouples from the host and he have more control over what is accessed, what network configuration wraps the tests and vms, etc. Again an advantage rather than a disadvantage.
We have passed such devices through containers and vms multuple times in the past. No issue I can recount here.
We have some of those too and at least for our own needs the performance penalty has been negligible, not to mention that a near constant margin or systematic error is easily subtractable.
We run LXC as root containers.
Right, and there could be cases where it is not possible, yet I believe more people will fall into the cases where they need VT tests for testing software inside of the vms as well as virtualization software more often than cases right at the container boundary. Areas outstrip boundaries in general. Anyway, I hope this helps. I am sorry but we all have limited resources here and cannot entertain excessively long and non-constructive at times conversations. Thanks! |
|
Hi @pevogam , This discussion ran way longer than it should have, and not in a productive way. Not because there were technical aspects that must be addressed, but because there's a lack of understanding on what Avocado-VT as a framework (or any framework) is, especially an Open Source framework. In case you need to be reminded, a framework should provide mechanisms and almost never implement policies that restrict users. The fact that you personally don't have a use case for what's being proposed here, or even disklike it in favor of your own ideal scenario, is NOT a reason for blocking a proposal that brings enhancements to others, as long as they don't bring regressions or extra maintenance burdens. LXC won't work for the team involved here. Neither will remote runners. Please get over it. Just like the podman container support in Avocado wouldn't work for you like LXC and remote would. Let me be clear: I'll take every single one of your points, considering they pinpoint a coding mistake, or provide comment on how to code something better, or how it brings regressions to a supported behavior or API. When it comes to regressions, please do not duck the fact that only users that opt in (by changing the number of tasks to be run at once) will be affected by limitations that their tests can have. @kraxel has already provided some feedback that falls within this sort of acceptable and constructive review. In case positive review(s) are provided, even if not unanimously, I'll reserve the right as a maintainer to merge this. |
|
Hi @clebergnu,
I will kindly ask you as well to reason and be rational here and also not use "dislike" as a word since @kraxel was the only one adding taste here and worse yet putting words in my mouth. You can read my arguments in more detail, they are rational arguments after all. Not in any place did I use the words "like", "prefer", "dislike" and so on.
Alright, I guess I just wanted to know a concrete technical detail because maybe I could help address it in some way. It is of course just a best effort, I don't claim any of this is panacea and solves everyone's problems. My earlier point was that isolation is a good thing to have and at the very least with LXC we don't have to be careful what tests are being run concurrently. Indeed, I would like if it remains a few simple claims like this that could be refuted with concrete counter-evidence if any.
Let me point you to something specific:
Could you confirm that this PR does suggest a partial parallelism and that one has to carefully consider what tests are being run concurrently? If this is that case won't you at least agree that adding some level of isolation allows for full parallelism at least for some users (non-boundary users in my earlier claims, see above)? In that case can't I at least ask why advocate for partial parallelism if we can have full such, at least for a wide range of use cases (not I never claim for all possible use cases)? Also could you address my questions about the double prefixing above? I believe both of these are legitimate questions that should be addressed if we want to collaborate on an open source project and to me they go deeper than pointing specific coding style and error and into how such changes integrate into the overall picture. Please bear in mind that we are l involved in this and affected by this and many of us have been using these products for many years as well. We deserve equal rights as maintainers to ask questions about the direction here for as long as we remain constructive. |
While it's my opinion (prone to error) that you dislike this, based not on an explicit affirmation but on a repetitive defense of another approach closer to you in authorship and experience, I'll say that I meant that in generic, universally true terms. So, I apologize if that sentence hit you unfairly, but I meant that (s/you/one/): The fact that one personally doesn't have a use case for what's being proposed here, or even disklikes it in favor of your own ideal scenario, is NOT a reason for blocking a proposal that brings enhancements to others, as long as they don't bring regressions or extra maintenance burdens.
I'll give you one example of why I chose to use the fairest words possible in my description of what this PR brings. Suppose you're running tests in parallel under an environment which you believe are fully isolated, for instance, using the LXC spawner. Now suppose the tests interact with a network filesystem. Now suppose they create files that can clash. Even the "fully isolated" environment can fail if tests are not carefully written. When a framework enters the scene and provides some utilities for tests, it's a must that those utilities do not introduce clashes themselves. This is what this PR is doing: going one step further into the direction of not creating files that clash with each other. Hopefully this effort, beyond this PR, will go from "allows some level of parallel test execution" to "allows for parallel test execution unless your test is really ill-written".
Sure. Like one of the commits shows, there's already prior art on using the double prefix. This PR simply expands it, and as @kraxel noted, the first version doesn't expand it as much as it could/should. Also, let's not really try to stretch the concept of behavior/API stability towards not allowing for more specific (and clash-free) log file names. If third-party scripts are looking for log files, they can be updated to use a simple glob instead.
Absolutely. That's why I gave you the example of LCX and remote spawners. I don't think the people who reviewed the PRs that introduced them (myself included) had any immediate use for them, but they would not pass on the privileges of receiving a contribution that augments the framework's capability and provides equal rights to interested people to shape the framework. Another example is PR #4388, which introduces features a lot of (maybe most) Avocado-VT users may not be looking for. I'm looking forward to making Avocado-VT better for you, for us, and for all. |
Alright, if I have to say what I feel it would not be that I dislike this, I am rather curious about the new example use case with passt (to be concrete) mentioned here and how exactly it manages to fall between the cracks of the current alternative I proposed here. Clearly LXC will not cover all use cases and there will be some out of reach which makes it even more interesting to explore such cases.
Never claimed that scenario is ideal, it is simply a
Indeed, we have a few tests that need to download licenses from a shared LAN location like this and they could clash indeed. So is this what you mean by partial parallelism only? Does this imply that assuming strictly local VT tests (maybe what could at present be called "the standard VT tests we know") would work just fine in any order given your changes proposed here? The impression I was left with in your original statement is that we only isolate a few features for Qemu and if I boldly run the entire qemu test suite (for instance) some tests would clash entirely locally since only some attributes have been namespaced here and not all. Is the former really the case or the latter?
At the very least at this point I could claim this for the LXC isolation - we only have very few tests accessing networking resources and we are fully aware of their clashing nature. For a typical VT test that does not involve any remotely shared resources we never worry about the needed isolation any longer. So I guess what I am trying to understand here is if the state of your PR here is "very initial parallelism of a few carefully narrated VT tests" or "proper parallelism for most of the standard VT tests currently in existence". Because if it is the latter I could have misunderstood your original description.
I see. This definitely adds two useful counter-arguments then.
I have seen numerous times users here proposing VT architectures to achieve distributed testing, various forms of parallelism, and other white papers, so I tend to believe a lot of this is due to the lack of information and documentation rather than anything else. I think you would still agree that having a proper multi-vm support, automation for various simulated network topologies, as well as snapshot-based vm recreation and reuse are all major facets of what we could do with virtualized testing. I was also never given a chance to present to the rest of you directly a lot of the functionality there. I recall years ago in 2020 you invited me to a Red Hat conference where I would have loved to attend this but then covid struck. So the "introduces features a lot of (maybe most) Avocado-VT users may not be looking for" is also something that sounds pretty harsh making me wonder why should I even push through with things no one cares about then. I am not looking for or asking for your tolerance this this repo to add code you will never need to the code base and always appreciate feedback on usefulness or directions that could save us both unnecessary effort. The main reason I opened the #4388 epic is because from what I have heard the avocado organization is short on maintainers and I joined to help out but then wanted lower maintenance costs for myself from also maintaining a third party plugin on top of this. Then, I believed, if I improve documentation and introduce more people to these concepts they will actually turn really useful. I started the migration but instead I had to spend extra weeks recovering the entirely disabled and as a follow up extremely bit-rotten CI of the VT project as well as a lot of failing routine and non-routine workflows of the avocado core project just so that I can have enough gate keeping and acceptance testing to test my own migration (see recent commits in both repos for details if you would like). All of this then contributes to the potentially wasted time recovering things that must have been there, just to introduce features no one is interested in. And it seems on your side, the main reason you would onboard these (and past features of mine) would be to give me chance at equal access for my own use cases while in reality they are of little value to the wider community. At least this is what I learn from one of your last statements here. Of course you might be completely right and I just have to deal with the reality of producing worthless features with you pointing out some of my own blind spots to me (which is always appreciated). Or you may be wrong and have judged the value of these features without even knowing them with also the wider community never even knowing them yet. I guess only time will tell, either outcome will teach us something new. |
pevogam
left a comment
pevogam
left a comment
There was a problem hiding this comment.
LGTM, please consider placing the documentation section in https://github.com/avocado-framework/avocado-vt/pull/4399/changes and the devoted documents for parallel VT jobs.
3 participants
Some QEMU VM's attribute are global (system wide) in Avocado-VT. This makes some of them unique and possible to coexist in a given system.
This allows for some level of parallel test execution.