Hmm, since this is completely changing how "cilium-dbg statedb" command works, we could also just change the command to "cilium-dbg db". This way it'll also match the script command, and the help matches as well.

Or another option would be to kill the statedb command completely and instead just invoke these with cilium-dbg shell db ....

Also one annoying thing with this wrapper is that flag parsing will get mixed and requires quotes:

root@kind-worker:/home/cilium# cilium-dbg statedb "prefix -index=name devices cilium"
Name           Index   Selected   Type    MTU    HWAddr              Flags                    Addresses
cilium_host    3       false      veth    1500   c2:f6:99:50:af:71   up|broadcast|multicast   10.244.1.105, fe80::c0f6:99ff:fe50:af71
cilium_net     2       false      veth    1500   5e:70:20:4d:8a:bc   up|broadcast|multicast   fe80::5c70:20ff:fe4d:8abc
cilium_vxlan   4       false      vxlan   1500   b2:c6:10:14:48:47   up|broadcast|multicast   fe80::b0c6:10ff:fe14:4847

Not quite sure what to do about this. One option I guess is to not use flags in the "db" command and instead do some sort of query language. The flags are convenient to implement though and provide some command line usage help without much extra work.

I'm somewhat starting to lean towards just killing the "cilium-dbg statedb" command completely and exposing these via "cilium-dbg shell" only. If one is interested in poking at the database then you'll likely anyway want to go via cilium-dbg shell and run multiple commands in row. This way there's less confusion with mixing up the "cilium-dbg" pflag parsing with the script command parsing.

Hmm, since this is completely changing how "cilium-dbg statedb" command works, we could also just change the command to "cilium-dbg db". This way it'll also match the script command, and the help matches as well.

Or another option would be to kill the statedb command completely and instead just invoke these with cilium-dbg shell db ....

Also one annoying thing with this wrapper is that flag parsing will get mixed and requires quotes:

root@kind-worker:/home/cilium# cilium-dbg statedb "prefix -index=name devices cilium"
Name           Index   Selected   Type    MTU    HWAddr              Flags                    Addresses
cilium_host    3       false      veth    1500   c2:f6:99:50:af:71   up|broadcast|multicast   10.244.1.105, fe80::c0f6:99ff:fe50:af71
cilium_net     2       false      veth    1500   5e:70:20:4d:8a:bc   up|broadcast|multicast   fe80::5c70:20ff:fe4d:8abc
cilium_vxlan   4       false      vxlan   1500   b2:c6:10:14:48:47   up|broadcast|multicast   fe80::b0c6:10ff:fe14:4847

Not quite sure what to do about this. One option I guess is to not use flags in the "db" command and instead do some sort of query language. The flags are convenient to implement though and provide some command line usage help without much extra work.

That's unfortunate but I agree that that having them as flags with easy usage documentation provides a lot of benefits. We should just make sure to document the fact that flags needs to be quoted.

If the need arises we could still switch to a query language.

I'm somewhat starting to lean towards just killing the "cilium-dbg statedb" command completely and exposing these via "cilium-dbg shell" only. If one is interested in poking at the database then you'll likely anyway want to go via cilium-dbg shell and run multiple commands in row. This way there's less confusion with mixing up the "cilium-dbg" pflag parsing with the script command parsing.

+1 IMO this way would provide a more streamlined to the user and it also makes it easier to "transfer" commands used during an interactive session to a single non-interactive cilium-dbg shell invocation.

Looks good, nice and clean! Two nits inline.

I've left my comments wrt. killing the statedb subcommand in a separate reply and I'll let you decide on how you want to move forward with this.

Thanks! I think simplifying is the way to go, but I don't want to leave folks used to "cilium-dbg statedb" confused. I'll make the "cilium-dbg statedb" just spit out a "deprecated message" that tells how to do the same things with cilium-dbg shell -- db show <table> etc. so we have a smooth transition. And then kill the command in v1.17.

/test

/test