Improve help message for --last option

[PriyaSharma9]

Fixes #861

Adds additional tip for the below flag

 --last uint      Get last N flows stored in Hubble's buffer (default 20). When querying against Hubble Relay, this gets N flows per instance of Hubble connected to that Relay

[kaworu]

Hi 👋 and thanks for the PR @PriyaSharma9!

[kaworu]

@PriyaSharma9 the tests need to be updated accordingly, please run:

% make && ./hubble observe --help > cmd/observe_help.txt

At that point git diff should show changes to the file related to your patch. The --config string line should not be changed and should be kept as

      --config string   Optional config file (default "%s")

Once make test is passing locally, you can update this PR by running

% git commit --amend
% git push --force-with-lease

[PriyaSharma9]

@kaworu I see this in the diff, Is this ok other than the --config string line

diff --git a/cmd/observe_help.txt b/cmd/observe_help.txt
index ed3ab187..fadf5788 100644
--- a/cmd/observe_help.txt
+++ b/cmd/observe_help.txt
@@ -130,15 +130,15 @@ Flow Format Flags:
 Server Flags:
       --server string                 Address of a Hubble server (default "localhost:4245")
       --timeout duration              Hubble server dialing timeout (default 5s)
-      --tls                           Specify that TLS must be used when establishing a connection to a Hubble server.
+      --tls                           Specify that TLS must be used when establishing a connection to a Hubble server.^M
                                       By default, TLS is only enabled if the server address starts with 'tls://'.
-      --tls-allow-insecure            Allows the client to skip verifying the server's certificate chain and host name.
-                                      This option is NOT recommended as, in this mode, TLS is susceptible to machine-in-the-middle attacks.
+      --tls-allow-insecure            Allows the client to skip verifying the server's certificate chain and host name.^M
+                                      This option is NOT recommended as, in this mode, TLS is susceptible to machine-in-the-middle attacks.^M
                                       See also the 'tls-server-name' option which allows setting the server name.
       --tls-ca-cert-files strings     Paths to custom Certificate Authority (CA) certificate files.The files must contain PEM encoded data.
-      --tls-client-cert-file string   Path to the public key file for the client certificate to connect to a Hubble server (implies TLS).
+      --tls-client-cert-file string   Path to the public key file for the client certificate to connect to a Hubble server (implies TLS).^M
                                       The file must contain PEM encoded data.
-      --tls-client-key-file string    Path to the private key file for the client certificate to connect a Hubble server (implies TLS).
+      --tls-client-key-file string    Path to the private key file for the client certificate to connect a Hubble server (implies TLS).^M
                                       The file must contain PEM encoded data.
       --tls-server-name string        Specify a server name to verify the hostname on the returned certificate (eg: 'instance.hubble-relay.cilium.io').

@@ -147,7 +147,7 @@ Other Flags:
   -s, --silent-errors       Silently ignores errors and warnings

 Global Flags:
-      --config string   Optional config file (default "%s")
+      --config string   Optional config file (default "/Users/psharma/Library/Application Support/hubble/config.yaml")
   -D, --debug           Enable debug messages

[chancez]

It's this part:

+      --config string   Optional config file (default "/Users/psharma/Library/Application Support/hubble/config.yaml")

The test code interpolates the expected value there, since it's dependent on the users-env.

[PriyaSharma9]

@chancez when I run % make && ./hubble observe --help > cmd/observe_help.txt
This is the diff

git diff
warning: in the working copy of 'cmd/observe_help.txt', LF will be replaced by CRLF the next time Git touches it
diff --git a/cmd/observe_help.txt b/cmd/observe_help.txt
index ed3ab187..9abe8330 100644
--- a/cmd/observe_help.txt
+++ b/cmd/observe_help.txt
@@ -147,7 +147,7 @@ Other Flags:
   -s, --silent-errors       Silently ignores errors and warnings

 Global Flags:
-      --config string   Optional config file (default "%s")
+      --config string   Optional config file (default "/Users/psharma/Library/Application Support/hubble/config.yaml")
   -D, --debug           Enable debug messages

 Get help:

Now when ran make test, it still returns the below

            	            	Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -156,2 +156,2 @@
            	            	 Use "hubble observe [command] --help" for more information about a command.
            	            	-%!(EXTRA string=/Users/psharma/Library/Application Support/hubble/config.yaml)
            	            	+
            	Test:       	TestTestHubbleObserve/observe_help
            	Messages:   	expected output does not match
FAIL

Not sure what I am missing here.

[chancez]

You can't simply just redirect the output and git add. You need to modify the line with --config to include "%s" instead of hardcoding your home directory. And you need to be careful about modifying whitespace, since it's doing a simple equals comparison. Use git add -p to incrementally add the changes that are relevant.

[glibsm]

I wonder why these lines are coming up in the diff 🤔

[chancez]

It's whitespace. I suggested using git add -p which makes it relatively easy to only update the relevant line that actually changed.

[PriyaSharma9]

Apologies! I had to work on certain priorities. Will try to get this sorted today.

[maintainer-s-little-helper]

Commit 9e0be4a does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/contributing/#developer-s-certificate-of-origin

[PriyaSharma9]

@kaworu I have removed those extra lines, Could you please help me with running the unit tests once.
I tried running it locally but the those extra lines get added as soon as I change the --config back to "%s"

[kaworu]

@PriyaSharma9 I git force pushed into your branch to remove the reverting commit and fix the observe help message, CI should pass now 🤞