cilium-cli icon indicating copy to clipboard operation
cilium-cli copied to clipboard

feat: Make Cilium CLI work with Cilium installed through helm with a non-default name.

Open matthewhembree opened this issue 1 year ago • 3 comments

This update introduces a new parameter across several functions and actions to allow the user to specify a Helm release name for the Cilium installation. This enables referencing installations installed via Helm directly. The default Helm release name remains "cilium" when no custom release name is provided.

The new global flag proposed here is: --helm-release-name.

Fixes: #2274 Signed-off-by: Matthew Hembree [email protected]

matthewhembree avatar Mar 19 '24 06:03 matthewhembree

Tested with a default installation and with a custom Helm release name installation.

The associated CLI sub-commands were tested across the two installations with and without the new flag.

Additionally, combination testing was done with sysdump specific flag --cilium-helm-release-name and the global flag --helm-release-name to validate the override priority. (i.e. more specific, if provided, overrides the global)

matthewhembree avatar Mar 19 '24 07:03 matthewhembree

Commit 7ec19cbaca7e76cfee7732d17e257352019d36b7 does not match "Signed-off-by".

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

The EKS (tunnel) workflow works fine when run from my account/cluster. It could be failing due to spot interruptions; even though those instance types report <5% interruption rate. The logs show that a pod can't be found.

matthewhembree avatar Mar 20 '24 06:03 matthewhembree

Adding @tklauser (one of the maintainers) as a reviewer to also get his eyes on this change, to make sure we're not missing anything

gandro avatar Mar 20 '24 10:03 gandro

@matthewhembree it looks like this PR picked up a merge conflict. Could you please rebase your branch on latest main?

tklauser avatar Mar 26 '24 21:03 tklauser

AKS workflow failed because of https://github.com/cilium/cilium-cli/pull/2446#discussion_r1540182453, fixed in https://github.com/cilium/cilium-cli/pull/2452. All other workflows passed. Merging.

tklauser avatar Mar 26 '24 23:03 tklauser