From b70934fbc8773ec73c8634b1100b6afdb7748adf Mon Sep 17 00:00:00 2001 From: Julien Pinsonneau Date: Wed, 25 Mar 2026 10:38:45 +0100 Subject: [PATCH 1/2] help improvments --- commands/netobserv | 133 ++++++++++++++++++------------------------- scripts/functions.sh | 7 ++- scripts/help.sh | 40 ++++++++++++- 3 files changed, 98 insertions(+), 82 deletions(-) diff --git a/commands/netobserv b/commands/netobserv index 4a19c8d4d..8ba8f007d 100755 --- a/commands/netobserv +++ b/commands/netobserv @@ -58,18 +58,27 @@ maxTime="5m" maxBytes=50000000 # skip dependencies check for help or version -if [[ ! "$*" =~ ^(.*)help|version(.*) ]]; then +if [[ ! "$*" =~ (help|version) ]]; then check_dependencies "$required_yq_version" "$supported_archs" "$required_bash_version" fi # detect output yaml option before running the script to avoid any apply -if [[ "$*" =~ ^(.*)yaml=true|(yaml$)|(yaml )(.*) ]]; then +if [[ "$*" =~ (yaml=true|yaml$|yaml[[:space:]]) ]]; then echo "Output YAMLs without applying..." outputYAML="true" fi +contains_help() { + for arg in "$@"; do + case "$arg" in + *help) return 0 ;; + esac + done + return 1 +} + case "$1" in -*help) +""|*help) help exit 0 ;; @@ -78,100 +87,68 @@ case "$1" in exit 0 ;; *flows) - case "$2" in - *help) - flows_usage + shift + if contains_help "$@"; then + flows_usage | highlight_keywords flows $(get_help_keywords "$@") exit 0 - ;; - *) - shift # remove first argument - options=("$@") - # run flows command - command="flows" - ;; - esac + fi + options=("$@") + command="flows" ;; *packets) - case "$2" in - *help) - packets_usage + shift + if contains_help "$@"; then + packets_usage | highlight_keywords packets $(get_help_keywords "$@") exit 0 - ;; - *) - shift # remove first argument - options=("$@") - # run packets command - command="packets" - ;; - esac + fi + options=("$@") + command="packets" ;; *metrics) - case "$2" in - *help) - metrics_usage + shift + if contains_help "$@"; then + metrics_usage | highlight_keywords metrics $(get_help_keywords "$@") exit 0 - ;; - *) - shift # remove first argument - options=("$@") - # run metrics command - command="metrics" - # override maxTime default to 1h for metrics only - maxTime="1h" - ;; - esac + fi + options=("$@") + command="metrics" + maxTime="1h" ;; *follow) - case "$2" in - *help) - follow_usage - exit 0 - ;; - *) - # run follow command - follow + shift + if contains_help "$@"; then + follow_usage | highlight_keywords follow $(get_help_keywords "$@") exit 0 - ;; - esac + fi + follow + exit 0 ;; *stop) - case "$2" in - *help) - stop_usage - exit 0 - ;; - *) - # run deleteDaemonset command - deleteDaemonset + shift + if contains_help "$@"; then + stop_usage | highlight_keywords stop $(get_help_keywords "$@") exit 0 - ;; - esac + fi + deleteDaemonset + exit 0 ;; *copy) - case "$2" in - *help) - copy_usage - exit 0 - ;; - *) - # run copy output command - copyOutput + shift + if contains_help "$@"; then + copy_usage | highlight_keywords copy $(get_help_keywords "$@") exit 0 - ;; - esac + fi + copyOutput + exit 0 ;; *cleanup) - case "$2" in - *help) - cleanup_usage + shift + if contains_help "$@"; then + cleanup_usage | highlight_keywords cleanup $(get_help_keywords "$@") exit 0 - ;; - *) - # run cleanup command - cleanup - exit 0 - ;; - esac + fi + cleanup + exit 0 ;; *) echo "Unknown command $1. Use 'netobserv help' to display options" diff --git a/scripts/functions.sh b/scripts/functions.sh index 909548345..806e058a2 100755 --- a/scripts/functions.sh +++ b/scripts/functions.sh @@ -864,7 +864,12 @@ function check_args_and_apply() { fi ;; *interfaces) # Interfaces - edit_manifest "interfaces" "$value" + if [[ "$command" == "flows" || "$command" == "metrics" ]]; then + edit_manifest "interfaces" "$value" + else + echo "--interfaces is invalid option for packets" + exit 1 + fi ;; *enable_pkt_drop) # Enable packet drop if [[ "$command" == "flows" || "$command" == "metrics" ]]; then diff --git a/scripts/help.sh b/scripts/help.sh index f100fb78c..9d78629b0 100644 --- a/scripts/help.sh +++ b/scripts/help.sh @@ -3,6 +3,38 @@ # metrics includeList includeList="namespace_flows_total,node_ingress_bytes_total,node_egress_bytes_total,workload_ingress_bytes_total" +_BOLD_CYAN=$(printf '\033[1;36m') +_RESET=$(printf '\033[0m') + +# Extract keywords from args for help highlighting +# e.g. "--port=8080 --drops --help" → "--port" "--drops" +get_help_keywords() { + for arg in "$@"; do + case "$arg" in + *help|or) ;; + --*=*) printf '%s\n' "${arg%%=*}" ;; + *) printf '%s\n' "$arg" ;; + esac + done +} + +# Pipe help text through this to highlight keywords +# Usage: some_usage_fn | highlight_keywords --port --drops +highlight_keywords() { + if [ $# -eq 0 ]; then + cat + return + fi + + local sed_args=() + for keyword in "$@"; do + keyword="${keyword#--}" + sed_args+=(-e "s|${keyword}|${_BOLD_CYAN}&${_RESET}|g") + done + + sed "${sed_args[@]}" +} + # display main help function help { echo @@ -23,6 +55,8 @@ function help { echo " stop Stop collection by removing agent daemonset." echo " version Print software version." echo + echo "Use --help with any command for more details (e.g. netobserv flows --help)" + echo echo "Flow capture examples:" flows_examples echo @@ -210,7 +244,7 @@ function metrics_usage { function follow_usage { echo - echo "NetObserv allows you to capture flows and packets asyncronously using the --background option." + echo "NetObserv allows you to capture flows and packets asynchronously using the --background option." echo "While the capture is running in background, you can connect to the collector pod to see the progression using the follow command." echo "Find more information at: https://github.com/netobserv/network-observability-cli/" echo @@ -220,7 +254,7 @@ function follow_usage { function stop_usage { echo - echo "NetObserv allows you stop the collection and keep collector or dashboard for post analysis." + echo "NetObserv allows you to stop the collection and keep collector or dashboard for post analysis." echo "While the capture is running, use the stop command to remove the eBPF agents." echo "Find more information at: https://github.com/netobserv/network-observability-cli/" echo @@ -230,7 +264,7 @@ function stop_usage { function copy_usage { echo - echo "NetObserv allows you copy locally the captured flows or packets from the collector pod." + echo "NetObserv allows you to copy locally the captured flows or packets from the collector pod." echo "While the collector is running, use the copy command to copy the output file(s)." echo "To avoid modifications during the copy, it's recommended to stop the capture" echo "Find more information at: https://github.com/netobserv/network-observability-cli/" From 29f50700551ade977bc15609fde88affd98b6bdd Mon Sep 17 00:00:00 2001 From: Julien Pinsonneau Date: Thu, 26 Mar 2026 10:24:38 +0100 Subject: [PATCH 2/2] fix e2e tests --- e2e/script_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/e2e/script_test.go b/e2e/script_test.go index a04e8c13c..cc7d2098c 100644 --- a/e2e/script_test.go +++ b/e2e/script_test.go @@ -5,12 +5,15 @@ package e2e import ( "os" "path" + "regexp" "testing" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" ) +var ansiRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`) + var ( slog = logrus.WithField("component", "script_test") ) @@ -40,6 +43,8 @@ func TestHelpCommand(t *testing.T) { assert.Contains(t, output, "follow Follow collector logs when running in background.") assert.Contains(t, output, "stop Stop collection by removing agent daemonset.") assert.Contains(t, output, "version Print software version.") + // ensure help to display --help hint + assert.Contains(t, output, "Use --help with any command for more details") // ensure help to display examples assert.Contains(t, output, "Flow capture examples:") assert.Contains(t, output, "netobserv flows --drops") @@ -49,6 +54,52 @@ func TestHelpCommand(t *testing.T) { assert.Contains(t, output, "Capture flows in the background") assert.Contains(t, output, "Capture metrics in the background") }) + + t.Run("no argument shows help", func(t *testing.T) { + output, err := RunCommand(slog, "commands/oc-netobserv") + assert.Nil(t, err) + assert.NotEmpty(t, output) + assert.Contains(t, output, "Main commands:") + }) +} + +func TestSubcommandHelp(t *testing.T) { + t.Run("flows --help", func(t *testing.T) { + output, err := RunCommand(slog, "commands/oc-netobserv", "flows", "--help") + assert.Nil(t, err) + assert.NotEmpty(t, output) + plain := ansiRegex.ReplaceAllString(output, "") + assert.Contains(t, plain, "Syntax: netobserv flows [options]") + assert.Contains(t, plain, "Features:") + assert.Contains(t, plain, "Filters:") + assert.Contains(t, plain, "Options:") + assert.Contains(t, plain, "Examples:") + }) + + t.Run("flows --help at any position", func(t *testing.T) { + output, err := RunCommand(slog, "commands/oc-netobserv", "flows", "--port=8080", "--help") + assert.Nil(t, err) + assert.NotEmpty(t, output) + plain := ansiRegex.ReplaceAllString(output, "") + assert.Contains(t, plain, "Syntax: netobserv flows [options]") + assert.Contains(t, plain, "Filters:") + }) + + t.Run("packets --help", func(t *testing.T) { + output, err := RunCommand(slog, "commands/oc-netobserv", "packets", "--help") + assert.Nil(t, err) + assert.NotEmpty(t, output) + plain := ansiRegex.ReplaceAllString(output, "") + assert.Contains(t, plain, "Syntax: netobserv packets [options]") + }) + + t.Run("metrics --help", func(t *testing.T) { + output, err := RunCommand(slog, "commands/oc-netobserv", "metrics", "--help") + assert.Nil(t, err) + assert.NotEmpty(t, output) + plain := ansiRegex.ReplaceAllString(output, "") + assert.Contains(t, plain, "Syntax: netobserv metrics [options]") + }) } func TestVersionCommand(t *testing.T) {