[Avocado-devel] (potential) design issue in multiplexer
Lukáš Doktor
ldoktor at redhat.com
Fri Aug 19 11:29:33 UTC 2016
Dne 17.8.2016 v 13:30 Ademar Reis napsal(a):
> On Wed, Aug 17, 2016 at 09:43:34AM +0200, Lukáš Doktor wrote:
>> Hello guys,
>>
>> While looking at the
>> https://github.com/avocado-framework/avocado-virt/pull/90 I noticed quite
>> serious issue in the current multiplexer design and if I understand it
>> correctly even in the ideal multiplexer design (avocado still does not
>> support the full specification). I'm not sure whether I just don't see
>> something obvious so please take a look at it.
>>
>> The problem is, that in that PR Fam assigns the value of non-leaf item. That
>> works well, the problem is when one tries to multiplex it, because he
>> changes the non-leaf object to !mux and it adds the other branches as
>> different mux domains.
>>
>> Let's make it concrete. The idea is to expect `kvm = on|off` in
>> `/plugins/virt/qemu/*` path. This path contains several children already
>> (`/plugins/virt/qemu/paths` and `/plugins/virt/qemu/migrate`), which still
>> works well (one can generate similar output with `avocado multiplex
>> --system-wide ...` when avocado-virt is installed; note you can't use `-s`
>> as it clashes with `--silent`, PR to fix it has been sent):
>
> It's been a while since I thought about the multiplexer at this
> level and I'm having trouble understanding the issue... Could
> you please provide an example using the avocado command line and
> yaml files where we can see the issue by running it and comparing
> the results?
Sure, I thought this suffices, but you're right that real examples are
easier to fiddle with.
>
> Thanks.
> - Ademar
>
>>
Let's create a `basic.yaml` (avocado does not accept empty multiplex
files as they don't provide mapping)
```
foo: bar
```
and run
```
avocado multiplex --system-wide -t -i empty.yaml
```
>> ```
>> ┗━━ plugins
>> ┗━━ virt
>> ┣━━ qemu
>> ┃ ┣━━ paths
>> ┃ ┃ → qemu_dst_bin: None
>> ┃ ┃ → qemu_img_bin: None
>> ┃ ┃ → kvm: on
>> ┃ ┃ → qemu_bin: None
>> ┃ ┗━━ migrate
>> ┃ → kvm: on
>> ┃ → timeout: 60.0
>> ```
>>
>> Now when you want to vary between different values, you might want to inject
>> something like:
>>
Let's call this one `qemu.yaml`
>> ```
>> plugins:
>> virt:
>> qemu: !mux
>> enabled:
>> kvm: on
>> disabled:
>> qemu: off
>> ```
>>
>> The problem is, that it affects the whole `/plugins/virt/qemu` node and all
>> it's children become multiplexed:
>>
and execute
```
avocado multiplex --system-wide -t -i /:qemu.yaml
```
(the `/` is important, otherwise it gets injected into `/run`)
>> ```
>> ┗━━ plugins
>> ┗━━ virt
>> ┣━━ qemu
>> ┃ ╠══ enabled
>> ┃ ║ → kvm: on
>> ┃ ╠══ disabled
>> ┃ ║ → kvm: off
>> ┃ ╠══ paths
>> ┃ ║ → qemu_dst_bin: None
>> ┃ ║ → qemu_img_bin: None
>> ┃ ║ → qemu_bin: None
>> ┃ ╚══ migrate
>> ┃ → timeout: 60.0
>> ```
>>
>> The original multiplexer RFC wanted to always allow left side to be
>> optional. This currently works only for paths inside mux-paths (like /run).
>> Anyway let's assume it works and explore the consequences. The query path
>> would change to `*/plugins/virt/qemu/*` and we'd inject the values on a
>> different place:
>>
```
avocado multiplex --system-wide -t -i /my/kvm:qemu.yaml
```
>> ```
>> ┣━━ my
>> ┃ ┗━━ kvm
>> ┃ ┗━━ plugins
>> ┃ ┗━━ virt
>> ┃ ┗━━ qemu
>> ┃ ╠══ enabled
>> ┃ ║ → kvm: on
>> ┃ ╚══ disabled
>> ┃ → kvm: off
>> ┗━━ plugins
>> ┗━━ virt
>> ┣━━ qemu
>> ┃ ┣━━ paths
>> ┃ ┃ → qemu_dst_bin: None
>> ┃ ┃ → qemu_img_bin: None
>> ┃ ┃ → qemu_bin: None
>> ┃ ┗━━ migrate
>> ┃ → timeout: 60.0
>> ```
>>
>> 1. mux looks into mux-path (`/run`) first, can't find the
>> `kvm/plugins/virt/qemu` there
>> 2. it looks into `/` and finds it in `/my/kvm/plugins/virt/qemu` -> success
>>
>> The problem is, when we use defaults (eg. the way we define default values
>> in avocado-virt):
>>
To get this result you need to insert the `Something_else` value. Here
I'm using `--mux-inject` but imagine it's defined in the `--system-wide`
(as was proposed in the original pull request and as is with the other
values here like `qemu_dst_bin`).
```
avocado multiplex --system-wide -t -i /my/kvm:qemu.yaml --mux-inject
/plugins/virt/qemu/paths:kvm:Something_else
```
>> ```
>> ┣━━ my
>> ┃ ┗━━ kvm
>> ┃ ┗━━ plugins
>> ┃ ┗━━ virt
>> ┃ ┗━━ qemu
>> ┃ ╠══ enabled
>> ┃ ║ → kvm: on
>> ┃ ╚══ disabled
>> ┃ → kvm: off
>> ┗━━ plugins
>> ┗━━ virt
>> ┣━━ qemu
>> ┃ ┣━━ paths
>> ┃ ┃ → qemu_dst_bin: None
>> ┃ ┃ → qemu_img_bin: None
>> ┃ ┃ → kvm: Something_else
>> ┃ ┃ → qemu_bin: None
>> ┃ ┗━━ migrate
>> ┃ → kvm: Something_else
>> ┃ → timeout: 60.0
>> ```
which produces following params:
```
$ avocado multiplex /my/kvm:qemu.yaml -c --system-wide
Variants generated:
Variant 1: /my/kvm/plugins/virt/qemu/enabled,
/plugins/virt/qemu/paths, /plugins/virt/qemu/migrate,
/plugins/virt/paths, /plugins/virt/guest, /plugins/virt/screendumps,
/plugins/virt/videos
/my/kvm/plugins/virt/qemu/enabled:kvm => True
/plugins/virt/guest:disable_restore_image_test => False
/plugins/virt/guest:image_path => None
/plugins/virt/guest:password => None
/plugins/virt/guest:user => None
/plugins/virt/paths:qemu_io_bin => None
/plugins/virt/qemu/migrate:timeout => 60.0
/plugins/virt/qemu/paths:qemu_bin => None
/plugins/virt/qemu/paths:qemu_dst_bin => None
/plugins/virt/qemu/paths:qemu_img_bin => None
/plugins/virt/screendumps:enable => None
/plugins/virt/screendumps:interval => 0.5
/plugins/virt/videos:enable => None
/plugins/virt/videos:jpeg_quality => 95
Variant 2: /my/kvm/plugins/virt/qemu/disabled,
/plugins/virt/qemu/paths, /plugins/virt/qemu/migrate,
/plugins/virt/paths, /plugins/virt/guest, /plugins/virt/screendumps,
/plugins/virt/videos
/my/kvm/plugins/virt/qemu/disabled:kvm => False
/plugins/virt/guest:disable_restore_image_test => False
/plugins/virt/guest:image_path => None
/plugins/virt/guest:password => None
/plugins/virt/guest:user => None
/plugins/virt/paths:qemu_io_bin => None
/plugins/virt/qemu/migrate:timeout => 60.0
/plugins/virt/qemu/paths:qemu_bin => None
/plugins/virt/qemu/paths:qemu_dst_bin => None
/plugins/virt/qemu/paths:qemu_img_bin => None
/plugins/virt/screendumps:enable => None
/plugins/virt/screendumps:interval => 0.5
/plugins/virt/videos:enable => None
/plugins/virt/videos:jpeg_quality => 95
```
>>
>> 1. mux looks into mux-path (`/run`) first, can't find the
>> `kvm/plugins/virt/qemu` there
>> 2. it looks into `/` and finds it in `/my/kvm/plugins/virt/qemu` and in
>> `/plugins/virt/qemu` -> failure
>>
>> Yes, one could solve it by defining another `mux-path` to `/my` or even
>> `/my/kvm`, but that just adds the complexity.
>>
>> Let me also mention why do we like to extend nodes from right. Imagine we
>> expect `disk_type` in `/virt/hw/disk/*`. The yaml file might look like this:
>>
This one is `disks.yaml`
>> ```
>> virt:
>> hw:
>> disk: !mux
>> virtio_blk:
>> disk_type: virtio_blk
>> virtio_scsi:
>> disk_type: virtio_scsi
>> ```
>>
>> Now the user develops `virtio_scsi_next` and he wants to compare them. Today
>> he simply merges this config with the above:
>>
This one `mydisks.yaml`
>> ```
>> virt:
>> hw:
>> disk: !mux
>> virtio_scsi_debug:
>> disk_type: virtio_scsi
>> enable_next: True
>> ```
we can try it out by running `avocado multiplex disks.yaml mydisks.yaml -c`
>> and avocado produces 3 variants, where `params.get("disk_type",
>> "/virt/hw/disk/*")` reports the 3 defined variants. If we try to do the same
>> with `*/virt/hw/disk` we have to modify the first file:
>>
`relative_disks.yaml`
>> ```
>> !mux
>> virtio_blk:
>> virt:
>> hw:
>> disk:
>> disk_type: virtio_blk
>> virtio_scsi:
>> virt:
>> hw:
>> disk:
>> disk_type: virtio_scsi
>> ```
>>
Could be tested:
```
avocado multiplex relative_disks.yaml
```
But in reality (to avoid multiplexing the root, which contains other
values like `/run` and `/plugins`) one would use:
```
avocado multiplex /my_disks:relative_disks.yaml
```
You are free to extend this, but the `my_disks` is the global variable
shared across all your disk types and you must put them all there.
>> One would want to prepend yet another node in front of it, because we don't
>> want to vary over disk types only, but also over other items (like cpus,
>> ...). The problem is, that the first category has to again be unique to the
>> whole multiplex tree in order to not clash with the other items. And that is
>> what the tree path was actually introduced, to get rid of this
>> global-namespace.
>>
>> Right now the only solution I see is to change the way `!mux` works.
>> Currently it multiplexes all the children, but (not sure if easily done) it
>> should only define the children, which mix together. Therefor (back to the
>> original example) one would be able to say:
>>
No example here as it's not supported.
>> ```
>> plugins:
>> virt:
>> qemu:
>> enabled: !newmux
>> kvm: on
>> disabled: !newmux
>> kvm: off
>> paths:
>> qemu_dst_bin: None
>> qemu_img_bin: None
>> qemu_bin: None
>> migrate:
>> timeout: 60.0
>> ```
>>
>> which would produce:
>>
>> ```
>> ┗━━ plugins
>> ┗━━ virt
>> ┣━━ qemu
>> ┃ ╠══ enabled
>> ┃ ║ → kvm: on
>> ┃ ╠══ disabled
>> ┃ ┃ → kvm: off
>> ┃ ┣━━ paths
>> ┃ ┃ → qemu_dst_bin: None
>> ┃ ┃ → qemu_img_bin: None
>> ┃ ┃ → qemu_bin: None
>> ┃ ┗━━ migrate
>> ┃ → timeout: 60.0
>> ```
>>
>> and in terms of variants:
>>
>> ```
>> Variant 1: /plugins/virt/qemu/enabled, /plugins/virt/paths,
>> /plugins/virt/migrate
>> Variant 2: /plugins/virt/qemu/disabled, /plugins/virt/paths,
>> /plugins/virt/migrate
>> ```
>>
>> I'm looking forward to your suggestions and I hope I'm wrong and that the
>> multiplexer (at least the full-spec) can handle this nicely.
>>
>> Kind regards,
>> Lukáš
>>
After thinking about this a bit more, one way to solve it is also to
restrict `!mux` definition on exiting non-mux nodes with children. In
such case one would not be able to multiplex the `kvm/nokvm` as defined
here without the change to the main structure (moving the `kvm=on|off`
into a leaf node). This solutions still suffers one issue and that is
multiple files where first defines the node without children, then one
adds custom node redefining it to !mux and last another mux file which
assumes the node is non-mux adds other children nodes resulting in
unexpected behavior. This and the fact that it requires changes to the
code when clash is found makes me think that it's not a good idea (also
valid simplification).
I hope it's clearer with the examples,
Lukáš
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 502 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/avocado-devel/attachments/20160819/fdd3464d/attachment.sig>
More information about the Avocado-devel
mailing list