Hello Cleber, few clarifications (hopefully) before you come up with v2
...
Basically what I ask here is whether you considered the possibility to define the `streams` not using the default params path, but from a different path:Another option would be to allow `path` of the Avocado Test `params` and the class would get the details from the provided params+path. The cons would be it'd be hard to use anything but `params` for that: streams: first: type: localhost second: hostname: 192.168.122.10 user: test password: 123456 third: type: docker create: yes ...I like this structure better.get_implementation(self.params, "/streams/first/*") => would use `params.get(..., "/streams/first/*")` to get all necessary parametersThe idea of `get_implementation` is pretty simple: get an Execution Stream implementation by its name. What you're describing here (with regards to getting the "right" parameters and instantiating/activating the execution stream easily/automatically) is exactly what I propose later.This is not really about the automatic creation. As for today it's impossible to map tests-to-variants and I saw some people are actually writing tests which depends on certain path so they can supply different params to different tests:First of all, the design shouldn't be limited by current limitations that can be addressed (although it's a very good thing to mention and keep track of them). Furthermore, I'd say the problem here is not mapping tests to variants at all. We're using YAML (yaml_to_mux) simply as a didactic way of setting variables at specific paths, and that is the essence here. Maybe I'm missing something, but I don't see a reason why the same "*/avocado/stream/*" parameters couldn't be available to all tests as "Default params": http://avocado-framework.readthedocs.io/en/48.0/TestParameters.html#default-params And still be overridden by specific tests. If you agree that's achievable but currently limited or non functional because of addressable limitations, then I believe we're fine, and just have another work item to deliver multi stream test support.
so instead of just:
avocado:
streams:
- 1:
type: remote
host: foo.example.com
I'd like to be able to pass:
First:
streams:
- 1:
type: remote
host: foo.example.com
Second:
streams:
- 1:
type: remote
host: foo.example.com
avocado:
streams:
- 1:
type: remote
host: foo.example.com
And in test to say this time please use `/Second/*` instead of
`/avocado/*` to define the default streams, because I'm running multiple
different tests in the single execution and I want to define different
streams for each of them.
Sure, my aim is to provide very simple way for most cases while still allowing the hard-core users to use our libraries directly to be able to do everything. Anyway I agree `get_stream` could be implemented on top of `get_implementation` but right now I don't see much points in having `get_implementation` at all. Looking forward to the v2 to hopefully learn about it's benefits.boot: timeout: 10 check: timeout: 1 shutdown: timeout: 5 and in boot.py: timeout = self.params.get("timeout", "*/boot/*") in check.py: timeout = self.params.get("timeout", "*/check/*") ... So the comment above tried to describe how one would create descriptions of streams on different places and be still able to get them in various tests from different locations. Imagine in test1.py: get_implementation(self.params, "*/test1/streams/first/*") in test2.py: get_implementation(self.params, "*/test2/streams/server/*") and so on. The automatic way would be shared for all tests, while this would allow the same but using different locations. Anyway this is really painful and I'd prefer using the single-line definition, or the list of tuples as described later...if klass is not None: stream = klass(host=self.params.get("remote_hostname"), username=self.params.get("remote_username") password=self.params.get("remote_password")) cmd = "ping -c 1 %s" % self.params.get("test_host_hostname") stream.run(shell(cmd))I do like the rest of the example (only the klass would be already initialized by the `get_implementation`.This conflicts with the idea of "utility library to be used by test developer that wants full control of the individual creation and use of the execution streams". I mean, if some implementations are made available at the `avocado.utils` namespace, then they could even be referred to directly by module/name.Yes, by `get_implementation("docker://create=yes,image=fedora,remove=always") or `get_implementation(host=host, username=username, ...)`, which returns already initialized object of the detected type. Alternatively the user would import directly the specific class and use `DockerStream(image="fedora")`. Again, the better name for `get_implementation` in this sense would be `get_stream`.I think you missed the point about `get_implementation()`. Again, `get_stream()` could be implemented on top of it. And remember that this is the "verbose, lengthy and boilerplate-full" method that we hope most users won't need or use. I'd really focus most of our energy on giving the majority of users (the majority of use cases) a very easy to use set of tools.
Sure, this was more-like a brainstorm of the possible scalability of this design...Again, since we're talking about making whatever makes sense in the utility namespace, there could a function such as "get_stream_parameters(name)" that would return the parameters to an Execution Stream, that is: stream_name = "server" path = "/avocado/streams/%s/*" % stream_name impl = self.params.get("type", path=path) stream = get_implementation(impl)(**get_stream_parameters(path))Actually it should probably be `self.streams.get_stream()` as we do want to register it in the `Streams` to get it cleaned automatically. This also works for libraries as you'd first initialize `Streams` object and then do the `get_stream()`, `close()` and so on directly on the `Streams` object.Can you explain a use case where you'd write code to manually create a stream using the object that is supposed to contain the ready to use streams? If you need to manually create it, then it's probably a good idea for you to manually destroy it. Of course we could add mechanisms to register streams created outside of `self.streams`, but I don't feel the urge to do so unless proven wrong. Anyway, this seems a corner case to me, and the most difficult and important task here is to try to get the "recommended" way right, so that most multi stream tests are really straightforward to read and write.
Currently defined streams (as I'd like to be able to register new ones for automatic postprocess. It should be simple while powerful)Please note that this is not the intended end result of this proposal, but a side effect of implementing it using different software layers. Most users should favor the simplified (higher level) interface. Writing a Multi-Stream test =========================== As mentioned before, users have not yet been given tools **and guidelines** for writing multi-host (multi-stream in Avocado lingo) tests. By setting a standard and supported way to use the available tools, we can certainly expect advanced multi-stream tests to become easier to write and then much more common, robust and better supported by Avocado itself. Mapping from parameters ----------------------- The separation of stream definitions and test is a very important goal of this proposal. Avocado already has a advanced parameter system, inof this proposal. Avocado already has __an__ advanced parameter system, inwhich a test received parameters from various sources.The most common way of passing parameters at this point is by means of YAML files, so these will be used as the example format.Well this might be quite hard to understand, how about just saying: "Avocado supports test parametrisation via Test Parameters system and the most common way is to use a YAML file by using `yaml_to_mux` plugin."Parameters that match a predefined schema (based on paths and node names) will be by evaluated by a tests' ``streams`` instance (available as ``self.streams`` within a test). For instance, the following snippet of test code:: from avocado import Test class MyTest(Test): def test(self): self.streams[1].run(python("import mylib; mylib.action()")) Together with the following YAML file fed as input to the parameter system:: avocado: streams: - 1: type: remote host: foo.example.comThis is currently not supported by our yaml parser as any dictionary is mapped to multiplex structure and I'm not sure it'd be possible (in a sane manner) to treat dict inside lists differently. Anyway as I mentioned earlier we could use:Oops, I may have used an incorrect syntax or idea (or both).avocado: streams: 1: ssh://foo.example.com or: avocado: streams: 1: type: remote host: foo.example.comThis one looks good IMO. I'm all in for more explicitly naming **when** you go to the lengths of defining them.Yes, plus we do use the OrderedDicts so we can still let people use: self.streams[0:4] together with: self.streams["server"] just beware the: self.streams["1"] is different then: self.streams[1] and the node-names (therefor even the 1: in streams) becomes string in Avocado, so it actually works well and in the example above the: self.streams["1"] == self.streams[0]Nice, looks like OrderedDicts, or an even "smarter" version, could be the basis or our slicing support.or: avocado: streams: - type: remote host: foo.example.com Another thing is I'd probably prefer names to ints so "1" or "server" or "worker1" etc, which goes nicely with the first 2 examples. The last example goes well with indexes, but it starts with 0 (which would be my recommendation anyway if we decided to go with indexes).I think I mentioned somewhere "if only integers"... I actually share your fondness of names. I did not say it explicitly, but I think we can support both, as the slicing examples make a lot of sense to me. But, the slicing examples can be expanded to its own dialect, such as supporting regexes. For instance, `self.streams["client-\d+"]` makes a lot of sense IMO.Yes, this makes sense and is quite simple to do...Would result in the execution of ``import mylib; mylib.action()`` in a Python interpreter on host ``foo.example.com``. If test environments are refered to on a test, but have not been definedIf test environments are __referred__ to on a test, but have not been definedOK, thanks.in the outlined schema, Avocado's ``streams`` attribute implementation can use a default Execution Stream implementation, such as a local process based one. This default implementation can, of course, also be configured at the system and user level by means of configuration files, command line arguments and so on. Another possibility is an "execution stream strict mode", in which no default implementation would be used, but an error condition would be generated. This may be useful on environments or tests that are really tied to their execution stream types.I'd solve this by supporting `__len__` where `len(self.streams)` should report number of defined streams. Note the number of defined streams changes based on how many streams are defined __OR__ used by the test. So: avocado: streams: - 0: - 1: len(self.streams) => 2 self.streams[5].run(cmd) len(self.streams) => 6 where the streams 2-5 are the default streams.I have mixed feelings here. In my understanding, all of the streams are created on demand, that is, when they're used. A configuration that defines thousands of them will not cause an empty test (think of `passtest.py`) to initialize them.Yes, that is the point. `__len__` reports the "defined", not necessarily "initialized" streams. Adding streams dynamically is definitely must-have-feature and one note the code above would actually only have the `streams[5]` initialized, the rest would wait for the first usage.OK, I'm fine with that.But, the meaning of `len(self.streams)`, that is, defined or initialized, is something we can further discuss later.Sure, another possibility would be to support `streams.defined_streams` but I think the `__len__` would make more sense. It's similar to a list: streams = list(params.get("streams", "/avocado/*") len(streams) == 2 streams.extend([2,3,4,5]) len(streams) == 6Sure, as long as we document that `len()` returns the defined streams, it makes perfect sense to me.
If you say the name is implicit than it means you are only talking about all-to-all single barrier. This is (usually) true for 2 streams, but with 3+ streams you usually need different named barriers as not all of the streams have to sync all of the time. Please take a deeper look at the example I posted and try to incorporate it in your v2.Intercommunication Test Example ------------------------------- This is a simple example that exercises the most important aspects proposed here. The use case is to check that different hosts can communicate among themselves. To do that, we define two streams as parameters (using YAML here), backed by a "remote" implementation:: avocado: streams: - 1: type: remote host: foo.example.com - 2: type: remote host: bar.example.com Then, the following Avocado Test code makes use of them:: from avocado import Test from avocado.streams.code import shell class InterCommunication(Test): def test(self): self.streams[1].run(shell("ping -c 1 %s" % self.streams[2].host)) self.streams[2].run(shell("ping -c 1 %s" % self.streams[1].host)) self.streams.wait() self.assertTrue(self.streams.success)Brainstorming here, how about letting `wait` raise exception when it fails unless we use `wait(ignore_failure)`. The exception would contain all the information so it'd be THE exception which failed the test?Yep, this is a valid question. I think the answer will depend on how much we want the **test** result to be bound to what happens on the streams. Right now it's obvious that I decided to keep them pretty much separate.Sure, the more I think about it I also share your vision.Good!As for the `streams.success`, I guess it'd be a property, which would go through all streams results, and report `any(_.failure for _ in self.streams)`, right?Exactly.The ``streams`` attribute provide a aggregated interface for all the streams. Calling ``self.streams.wait()`` waits for all execution streams (and their block of code) to finish execution. Support for slicing, if execution streams names based on integers only could be added, allowing for writing tests such as:: avocado: streams: - 1: type: remote host: foo.example.com - 2: type: remote host: bar.example.com - 3: type: remote host: blackhat.example.com - 4: type: remote host: pentest.example.com from avocado import Test from avocado.streams.code import shell class InterCommunication(Test): def test(self): self.streams[1].run(shell("ping -c 1 %s" % self.streams[2].host)) self.streams[2].run(shell("ping -c 1 %s" % self.streams[1].host)) self.streams[3].run(shell("ping -c 1 %s" % self.streams[1].host)) self.streams[4].run(shell("ping -c 1 %s" % self.streams[1].host)) self.streams.wait() self.assertTrue(self.streams[1:2].success) self.assertFalse(self.streams[3:4].success)As mentioned earlier I'd prefer names to indexes, anyway I see the indexes useful as well. How about supporting a name or index?Yep, also thought of that.As for the slices, I'd prefer list-like slice to stream-like slices as it'd be more natural to me to interact with a list of individual streams rather than a Stream object with a limited subset of streams. Anyway that's a matter of taste and I can definitely live with this as well.See my previous comments.Now about this example, it's really limited. Again you are hard-coding the scenario and changing it is really complicated. I'd prefer something like: self.streams[0].run(server_cmd) self.streams[1:].run(contact_server_cmd) self.assertTrue(self.streams.success)Real tests will probably (hopefully) use better (symbolic) names. The goal here is to focus on the mechanisms, which on yours and on my version are identical.Support for synchronized execution also maps really well to the slicing example. For instance, consider this:: from avocado import Test from avocado.streams.code import shell class InterCommunication(Test): def test(self): self.streams[1].run(shell("ping -c 60 %s" % self.streams[2].host) self.streams[2].run(shell("ping -c 60 %s" % self.streams[1].host)) ddos = shell("ddos --target %s" self.streams[1].host) self.streams[3:4].run(ddos, synchronized=True) self.streams[1:2].wait() self.assertTrue(self.streams.success) This instructs streams 1 and 2 to start connectivity checks as soon as they **individually** can, while, for a full DDOS effect, streams 3 and 4 would start only when they are both ready to do so.OK so this is about before-start-synchronisation. Well again, I'm not much fond of boundling the streams so I'd prefer allowing to define the workload (which returns when the workload is ready), trigger it (which triggers it and reports immediately) and then wait for it. The difference in usage is: self.streams[3].run(ddos, stopped=True) self.streams[4].run(ddos, stopped=True) self.streams[3].start() self.streams[4].start() The result is the same (unless you create processes per each stream and in `self.streams[3:4]` you use signals to synchronize the execution) but it allows greater flexibility like synchronizing other tasks then just streams... Or we can create methods `establish(cmd)`, `start()` and `wait()` which might better describe the actions.I think you missed my point here. Streams #3 and #4, in my example, wait for *each other*. Using a mechanism such as barriers.Oh you mean that the `synchronized=True` means it'd make a `barrier` mechanism available between those two streams and you'd be able to use this barrier from the streams? That would be rather limiting as you might want to create multiple barriers between several streams. I think you should re-describe your synchronization here as well as the structure changed a bit... In my RFC I basically passed the information about barriers by test params, which is obviously not available here as the Code could be anything. Anyway a barrier requires: 1. nameName on my example is implicit, as it can be created from the unique names of the stream themselves.
2. serverAlso implicit, as most use cases should be able to use the default barrier server provided by `self.streams`. To be more precise, an also on-demand barrier server running alongside the avocado test process (what you refer to as as "main", later).3. number of clientsSure, but I don't see how **code** run on streams have to care about this. The idea is that they will only have their `run()` methods called by the respective execution streams when the barrier is lifted (say, all clients have checked in). The point here is that individual stream execution in a "slice" will be **synchronized** from the outside of the **block of code**. How the synchronization is done, is a different matter.When creating synchronized streams by a slice, you have all of these available, but it might not be always like that and you might want to define different groups of barriers, for example: ssh: start_ssh() barrier("ssh_started", worker, 2) barrier("worker_finished", main, 3)These 3 lies, IIUC, are "block of code" content, right?
Yes, this is a stream1 called "ssh"
http: start_http() barrier("http_started", worker, 2) barrier("worker_finished", main, 3)Same here.
Yep, stream2 called "http"
worker: barrier("ssh_started", worker, 2) barrier("http_started", worker, 2) connect_to_both() barrier("worker_finished", main, 3)Same here for these 4 lines, right?
Yes, stream3 called "worker".Basically what is expected here to happen is that the worker waits for "ssh" to be started, then it waits for "http" to be started, then it uses them and after finishing it notifies all of the clients to let them finish.
The main difference between the first 2 barriers and the last one is that the first 2 barriers are independent on the other process while the last one is all-to-all.
To demonstrate the need for other than all-to-all barriers let me modify the example:
worker:
barrier("ssh_started", worker, 2)
connect_to_ssh_and_do_some_business()
barrier("http_started", worker, 2)
connect_to_both()
barrier("worker_finished", main, 3)
Now imagine the "http" start takes a long time as it can consist of
configuring a database, bootstraping the server etc. In this example you
have 3 streams where concurrently "ssh" and "http" are starting. When
"ssh" starts you immediately start doing some business there (as it also
can be time-expensive task) and only when this finishes you check
whether the "http" already started.
This particular case is still possible to write with all-to-all barriers, it just becomes sub-optimal:
worker:
barrier()
connect_to_ssh_and_do_some_business()
connect_to_both()
barrier()
As now the `connect_to_ssh_and_do_some_business` waits for "http" to be
started even though it does not need it.
Anyway I'm looking forward to the v2, I'd be fine with implicit all-to-all barriers as a standard feature initialized automatically (if asked for) if we put the generic implementation in `avocado.utils` so users could use it freely in advanced use-cases like the one I explained here.
... Thank you for the follow up, Lukáš
signature.asc
Description: OpenPGP digital signature
