On 12/11/24 12:49, Dumitru Ceara wrote:
> On 12/10/24 12:45 PM, Ales Musil wrote:
>> On Tue, Nov 12, 2024 at 6:38 PM Alexandra Rukomoinikova
>> <[email protected]> wrote:
>>
>>> Added ability to output multiple individual chassis by their
>>> name, hostname and encap-ip.
>>>
>>> Signed-off-by: Alexandra Rukomoinikova <[email protected]>
>>> Tested-by: Ivan Burnin <[email protected]>
>>> ---
>>> v2: - removed all copy from ovs function
>>> - removed hash for hostnames
>>> ---
<snip>
>
> I'm not sure I like this too much unfortunately. It means we will have
> to write boilerplate code every time we want to add a new table to the
> "show" command.
>
> Isn't it a better option to enhance the OVS cmd_show() implementation
> [0] to accept an optional list of arguments to filter on?
>
> CC Ilya, in case he has ideas too.
Hi, Dumitru and Alexandra.
I've been thinking abut what we can do from the generic db-ctl-base perspective.
So, here is what I came up with:
Today, we have multiple commands like find, list and show. They have different
use cases and output formats.
- list TABLE
* Lists all database records in a specified table.
* Does not expand references, i.e. just lists UUIDs.
* Allows limiting the output to particular columns with --columns.
- find TABLE CONDITION...
* Same output format as in list.
* Prints only records that satisfy all the provided conditions.
* Allows limiting the output to particular columns with --columns.
- show
* Pretty-prints the table contents based on cmd_show_table array provided by
the application.
* Technically, still prints only one table, which is cmd_show_table[0], but
expands the references printing referenced records from other tables
recursively, if present in cmd_show_table.
* Can also enrich the output with some other tables that hold references to
the printed one.
* Doesn't allow limiting the list of printed columns or filtering the output
based on conditions dynamically (limiting is done statically via content of
cmd_show_table).
If I understand the purpose of this patch correctly:
1. We want to be able to find multiple records based on multiple conditions.
2. It should be possible to specify conditions as ANY as opposed to ALL,
i.e. we want records that match at least one condition.
3. We want to find records based on conditions applied to a referenced record,
e.g we want to print chassis if chassis->encaps->ip matches in any of
referenced encaps.
4. We want to pretty-print the found records with all the benefits of the show
command including the printing of records that hold references to the printed
one, but not necessarily being referenced from it.
We may be able to satisfy subsets or all the requirements above with some
modifications to the current commands.
Option 1:
We may improve the condition syntax for the 'find' command allowing matching
on referenced records and allowing ANY semantics of conditions. For example:
ovn-sbctl --columns=name,hostname,encaps find chassis \
name=ch0 or hostname="ch0_host" and encaps{ref-any}ip=1.2.3.4
This will require some basic expression evaluation, but should not be hard
to implement.
Pros: A powerful tool for database searches, especially in scripts and tests.
Cons: A little verbose, no pretty-printing, no printing of referenced tables.
Might be a good thing to implement regardless of it being chosen for the
currently discussed functionality in the patch.
Option 2:
We may try and add conditions to the 'show' command. Start with basic ones
that 'find' already supports and extend to the condition expressions from the
option 1. For example:
ovn-sbctl show name=ch0 or hostname="ch0_host" and encaps{ref-any}ip=1.2.3.4
I'm not really sure about this one, because while it's clear from the code
that 'show' only ever prints records from one table, it's not clear to the
user. And this also only applies to the generic 'show'. ovn-nbctl, for
example, implements a custom one that prints two tables and it's hard to write
conditions in this case due to potential column name difference. The conditions
may be extended to also specify the table name, but it will be even more verbose
and may also be further less intuitive to use. I suspect, users of the 'show'
command do not really know how the database schema looks like internally in
most cases. It's not the command for power users.
Pros: Pretty-printing.
Cons: Confusing to use, interface is too verbose for an "overview" command.
Option 3:
We may add a generic text filter. For example:
ovn-sbctl --columns=name,hostname,encaps --filter=ch0 list chassis
ovn-sbctl --filter=ch0,1.2.3.4 show
The first command would list all the chassis that have 'ch0' in a name or
hostname or external id or any other column. The second will show only the
chassis that have mentions of 'ch0' or '1.2.3.4' in a printed output for that
chassis, i.e. if one of the port binding records has 'ch0' referenced, the
chassis record that it references will be printed as well.
This can be implemented as a post-filter, i.e. as a simple strstr() on what
is going to be printed for the current record.
With that implementation, the filter can also work for UUIDs, numbers and
other things.
Pros: Simple to use, can work for any command like list, find, show,
should be easy to implement for custom 'show' commands as well,
doesn't require knowing the schema or modifying the commands
when the schema changes.
Cons: Prone to false-positive matches if the user is not careful.
I'm leaning towards options 1 and 3 as they seem to be generally useful and
not actually mutually exclusive. Both may also work together with the future
'show-table' command (like a 'list' but expanding references like 'show' via
dynamic generation of cmd_show_table) that Rosemarie worked on.
What do you think?
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
