Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump StateDB to v0.3.4 and refactor db command usages #36325

Merged
merged 4 commits into from
Dec 6, 2024

Conversation

joamaki
Copy link
Contributor

@joamaki joamaki commented Dec 3, 2024

To improve the ergonomics of the 'help' command in cilium shell I've decided to avoid nested
sub-commands (e.g. "db show") and instead go for Plan 9 style flat "db/show" commands.
This way the 'help' command can list all the commands and 'help -v' can give extended help
for the commands.

This PR bumps StateDB to v0.3.4 which comes with the new style and refactors the usages
of the commands in tests and mentions in docs.

While at it, I changed also the experimental LB to use the same style and to modify it so it won't
register its tables or commands unless enabled to avoid confusion.

Help now looks like this:

cilium> help
[stdout]

commands:

cat files...
        concatenate files and print to the script's stdout buffer
db
        Describe StateDB configuration
db/cmp [-timeout=<dur>] [-grep=<pattern>] table file
        Compare table
db/delete table path...
        Delete an object from the table
db/get [-o=<file>] [-columns=col1,...] [-format={table*,yaml,json}] [-index=<index>] table key
        Get the first matching object
db/initialized [-timeout=<duration>] table...
        Wait until all or specific tables have been initialized
db/insert table path...
        Insert object into a table
db/list [-o=<file>] [-columns=col1,...] [-format={table*,yaml,json}] [-index=<index>] table key
        List objects in the table
db/lowerbound [-o=<file>] [-columns=col1,...] [-format={table*,yaml,json}] [-index=<index>] table key
        Query table by lower bound search
db/prefix [-o=<file>] [-columns=col1,...] [-format={table*,yaml,json}] [-index=<index>] table key
        Query table by prefix
db/show [-o=<file>] [-columns=col1,...] [-format={table,yaml,json}] table
        Show the contents of a table
db/watch table
        Watch a table for changes
exec program [args...] [&]
        run an executable program with arguments
help [-v] name...
        log help text for commands and conditions
lb/prune
        Trigger pruning of LB maps
lb/maps-dump (output file)
        Dump the load-balancing BPF maps
lb/maps-restore
        Restore the load-balancing BPF maps from snapshot
lb/maps-snapshot
        Snapshot the load-balancing BPF maps

And we now can get verbose help for the commands:

cilium> help db/show
[stdout]
db/show [-o=<file>] [-columns=col1,...] [-format={table,yaml,json}] table
        Show the contents of a table

        Show the contents of a table.

        The contents are written to stdout, but can be written to
        a file instead with the -o flag.

        By default the table is shown in the table format.
        For YAML use '-format=yaml' and for JSON use '-format=json'

        To only show specific columns use the '-columns' flag. The
        columns are as specified by 'TableHeader()' method.
        This flag is only supported with 'table' formatting.

@joamaki joamaki added the release-note/misc This PR makes changes that have no direct user impact. label Dec 3, 2024
@joamaki joamaki force-pushed the pr/joamaki/bump-statedb branch from 87bbe4b to fcbf9fa Compare December 3, 2024 11:37
@joamaki
Copy link
Contributor Author

joamaki commented Dec 3, 2024

/test

@joamaki joamaki force-pushed the pr/joamaki/bump-statedb branch from fcbf9fa to 867ece5 Compare December 3, 2024 13:30
@joamaki
Copy link
Contributor Author

joamaki commented Dec 3, 2024

/test

@joamaki joamaki marked this pull request as ready for review December 3, 2024 13:55
@joamaki joamaki requested review from a team as code owners December 3, 2024 13:55
@joamaki joamaki enabled auto-merge December 4, 2024 08:10
@joamaki joamaki force-pushed the pr/joamaki/bump-statedb branch from 867ece5 to 8a37edb Compare December 4, 2024 12:35
@joamaki
Copy link
Contributor Author

joamaki commented Dec 5, 2024

/test

Copy link
Member

@pippolo84 pippolo84 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️

@joamaki joamaki force-pushed the pr/joamaki/bump-statedb branch from 8a37edb to 95d1ae9 Compare December 5, 2024 11:40
@joamaki
Copy link
Contributor Author

joamaki commented Dec 5, 2024

/test

Copy link
Member

@dylandreimerink dylandreimerink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link
Contributor

@tommyp1ckles tommyp1ckles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLI changes looks good

Bump to latest StateDB version with the new script commands and
improved help.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
Switch to the "db/<subcmd>" schema in tests, e.g. "db insert"
becomes "db/insert" and so on. This new schema for sub-commands
gives much better experience with 'help'.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
Since the loadbalancer implementation is still experimental in v1.17,
avoid confusion by not creating/registering the tables if not enabled.

Add 'cell_test.go' that checks that the defaults won't cause failure
during population since I messed up initially (ToTable() crashed when
RWTable[T] was nil).

Signed-off-by: Jussi Maki <jussi@isovalent.com>
Switch to same sub-command schema as in StateDB by flatting the lb-maps
commands. This makes it much easier to discover the commands using 'help'.

Remove the 'lb-maps cmp' command as the '* cmp' (cmp with retry) does the
same thing now.

Also since the new implementation is not ready for v1.17, avoid confusion
by not registering the lb* commands if not enabled.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki joamaki force-pushed the pr/joamaki/bump-statedb branch from 95d1ae9 to c526f3d Compare December 6, 2024 10:27
@joamaki
Copy link
Contributor Author

joamaki commented Dec 6, 2024

/test

@joamaki joamaki added this pull request to the merge queue Dec 6, 2024
Merged via the queue into cilium:main with commit 794f04b Dec 6, 2024
64 checks passed
@joamaki joamaki deleted the pr/joamaki/bump-statedb branch December 6, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants