Add hubble list namespaces command

[chancez]

Now that the Hubble Observer API supports GetNamespaces, implement it in Hubble CLI.

Relates to

• Add GetNamespaces support to Hubble cilium#25266
• hubble: Add GetNamespaces to observer API cilium#25563

[glibsm]

static check is acting starnge. It doesn't appear to pick up your function rename 🤔

[chancez]

static check is acting starnge. It doesn't appear to pick up your function rename 🤔

Oh I forgot to update the tests after changing the name of a function.

[sayboras]

LGTM ✔️

The release note label is major, but I leave it to Hubble team to decide 🤔

[kaworu]

Thanks @chancez! Tested the patch and LGTM.

One thing I noticed that bugged me:

% ./hubble list namespaces -o wide
NAMESPACE            CLUSTER
kube-system          N/A
local-path-storage   N/A

Here my cluster name is "default", but I see N/A. I wonder if we should consider removing this guard and consistently output "default" instead of empty string, although on the other hand it will go against this CFP.

[kaworu]

"throw" 😄

[chancez]

I copied the existing comments 😂

[chancez]

@kaworu I thought about this too, but I wasn't sure if the client should be doing that or not. I kinda feel like perhaps I should be extracting the cluster name from the flows inside hubble server, and configuring the cluster name there. What do you think?

[glibsm]

it's a minor nit, but i think w.Write() here is more typical without string() conversion

[chancez]

This was how it was already written, I just moved which file this function was in. I believe the goal is to get a newline at the end so that when it's logged it's not on the same line as the prompt.

[kaworu]

I wasn't sure if the client should be doing that or not.

Can the client disambiguate between "" and "default"?

I kinda feel like perhaps I should be extracting the cluster name from the flows inside hubble server, and configuring the cluster name there. What do you think?

I don't understand what you are suggesting, could you clarify?

[chancez]

Can the client disambiguate between "" and "default"?

We don't really define anywhere how they should be treated, so it's hard to say.

In the NodeNameFilter we basically treat empty as "default" for filtering purposes. But in the node_name we field we return in GetFlows doesn't get updated from "" to "default" IIRC. So basically, in the server, we always leave the flow alone, but when querying, we support treating "default" as "".

After writing that out, it makes me think the best option then is to just return "" instead of "N/A". What do you think?

I don't understand what you are suggesting, could you clarify?

Right now, the clusterName for a flow is determined by using nodeTypes.GetClusterName() which uses configuration to determine the cluster_name.

I was thinking that we could use the value from the flow itself (which is part of the node_name field), but I realize now it's not much better. I had thought we were adding "default/" to the node_name of flows so it would make sense to use the node_name field to figure out cluster_name, but I believe the node_name is just left alone unless cluster_name is configured explicitly.

[chancez]

Rebased and removed N/A in favor of leaving cluster empty if not set.