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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 8 commits into from
Mar 26, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4ec5d70
feat: Add customizable Helm release name for Cilium
matthewhembree Mar 19, 2024
e858b37
style: Improve log message formatting. Make consistent/conventional.
matthewhembree Mar 19, 2024
0aead5c
feat: Add helmReleaseName to hubble disable parameters.
matthewhembree Mar 19, 2024
d4d8808
feat: Add helmReleaseName to Hubble uninstall pre-phase of the core u…
matthewhembree Mar 19, 2024
75f46d2
fix: Add WaitDuration from defaults for hubble enable|disable for cil…
matthewhembree Mar 19, 2024
b8b789a
style: Reorganize parameters struct to enhance readability.
matthewhembree Mar 19, 2024
23cd774
refactor: Move default Helm release name to sysdump package.
matthewhembree Mar 20, 2024
b7f1c50
refactor: Remove unused constants in defaults.go.
matthewhembree Mar 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions cli/clustermesh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ func newCmdClusterMeshEnableWithHelm() *cobra.Command {
Long: ``,
RunE: func(_ *cobra.Command, _ []string) error {
params.Namespace = namespace
params.HelmReleaseName = helmReleaseName
ctx := context.Background()
if err := clustermesh.EnableWithHelm(ctx, k8sClient, params); err != nil {
fatalf("Unable to enable ClusterMesh: %s", err)
Expand All @@ -266,6 +267,7 @@ func newCmdClusterMeshDisableWithHelm() *cobra.Command {
Long: ``,
RunE: func(_ *cobra.Command, _ []string) error {
params.Namespace = namespace
params.HelmReleaseName = helmReleaseName
ctx := context.Background()
if err := clustermesh.DisableWithHelm(ctx, k8sClient, params); err != nil {
fatalf("Unable to disable ClusterMesh: %s", err)
Expand All @@ -288,6 +290,7 @@ func newCmdClusterMeshConnectWithHelm() *cobra.Command {
Long: ``,
RunE: func(_ *cobra.Command, _ []string) error {
params.Namespace = namespace
params.HelmReleaseName = helmReleaseName
cm := clustermesh.NewK8sClusterMesh(k8sClient, params)
if err := cm.ConnectWithHelm(context.Background()); err != nil {
fatalf("Unable to connect cluster: %s", err)
Expand All @@ -311,6 +314,7 @@ func newCmdClusterMeshDisconnectWithHelm() *cobra.Command {
Short: "Disconnect from a remote cluster",
Run: func(_ *cobra.Command, _ []string) {
params.Namespace = namespace
params.HelmReleaseName = helmReleaseName
cm := clustermesh.NewK8sClusterMesh(k8sClient, params)
if err := cm.DisconnectWithHelm(context.Background()); err != nil {
fatalf("Unable to disconnect clusters: %s", err)
Expand Down
6 changes: 4 additions & 2 deletions cli/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ import (
)

var (
contextName string
namespace string
contextName string
namespace string
helmReleaseName string

k8sClient *k8s.Client
)
Expand Down Expand Up @@ -78,6 +79,7 @@ cilium connectivity test`,

cmd.PersistentFlags().StringVar(&contextName, "context", "", "Kubernetes configuration context")
cmd.PersistentFlags().StringVarP(&namespace, "namespace", "n", "kube-system", "Namespace Cilium is running in")
cmd.PersistentFlags().StringVar(&helmReleaseName, "helm-release-name", "cilium", "Helm release name")

cmd.AddCommand(
newCmdBgp(),
Expand Down
2 changes: 2 additions & 0 deletions cli/hubble.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ func newCmdHubbleEnableWithHelm() *cobra.Command {
Long: ``,
RunE: func(_ *cobra.Command, _ []string) error {
params.Namespace = namespace
params.HelmReleaseName = helmReleaseName
ctx := context.Background()
if err := hubble.EnableWithHelm(ctx, k8sClient, params); err != nil {
fatalf("Unable to enable Hubble: %s", err)
Expand All @@ -119,6 +120,7 @@ func newCmdHubbleDisableWithHelm() *cobra.Command {
Long: ``,
RunE: func(_ *cobra.Command, _ []string) error {
params.Namespace = namespace
params.HelmReleaseName = helmReleaseName
ctx := context.Background()
if err := hubble.DisableWithHelm(ctx, k8sClient, params); err != nil {
fatalf("Unable to disable Hubble: %s", err)
Expand Down
9 changes: 7 additions & 2 deletions cli/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ cilium install --context kind-cluster1 --set cluster.id=1 --set cluster.name=clu
`,
RunE: func(cmd *cobra.Command, _ []string) error {
params.Namespace = namespace
params.HelmReleaseName = helmReleaseName
// Don't log anything if it's a dry run so that the dry run output can easily be piped to other commands.
if params.IsDryRun() {
params.Writer = io.Discard
Expand Down Expand Up @@ -101,6 +102,7 @@ func newCmdUninstallWithHelm() *cobra.Command {
Long: ``,
RunE: func(_ *cobra.Command, _ []string) error {
params.Namespace = namespace
params.HelmReleaseName = helmReleaseName
ctx := context.Background()

cc, err := check.NewConnectivityTest(k8sClient, check.Parameters{
Expand All @@ -116,8 +118,10 @@ func newCmdUninstallWithHelm() *cobra.Command {
}
uninstaller := install.NewK8sUninstaller(k8sClient, params)
var hubbleParams = hubble.Parameters{
Writer: os.Stdout,
Wait: true,
Writer: os.Stdout,
Wait: true,
Namespace: namespace,
HelmReleaseName: helmReleaseName,
}

if params.Wait {
Expand Down Expand Up @@ -181,6 +185,7 @@ cilium upgrade --set cluster.id=1 --set cluster.name=cluster1
`,
RunE: func(cmd *cobra.Command, _ []string) error {
params.Namespace = namespace
params.HelmReleaseName = helmReleaseName
// Don't log anything if it's a dry run so that the dry run output can easily be piped to other commands.
if params.IsDryRun() {
params.Writer = io.Discard
Expand Down
4 changes: 4 additions & 0 deletions cli/sysdump.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ func newCmdSysdump(hooks sysdump.Hooks) *cobra.Command {
if sysdumpOptions.CiliumNamespace == "" && cmd.Flags().Changed("namespace") {
sysdumpOptions.CiliumNamespace = namespace
}
// Honor --helm-release-name global flag in case it is set and --cilium-helm-release-name is not set
if sysdumpOptions.CiliumHelmReleaseName == "" && cmd.Flags().Changed("helm-release-name") {
Copy link
Member

Choose a reason for hiding this comment

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

Could we deprecate cilium-helm-release-name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering where a change in this would break things. IMO, for a manual diagnostic tool I'd have a lower level of caution. But sysdump can be called by connectivity test, so there's three places to set the helmReleaseName.

Would the process be:

  1. Open an issue noting the deprecation.
  2. Put a // TODO: Deprecated by #nnnn on these lines.

sysdumpOptions.CiliumHelmReleaseName = helmReleaseName
}
// Silence klog to avoid displaying "throttling" messages - those are expected.
klog.SetOutput(io.Discard)
// Collect the sysdump.
Expand Down
2 changes: 1 addition & 1 deletion cli/version.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func newCmdVersion() *cobra.Command {
if clientOnly {
return nil
}
version, err := k8sClient.GetRunningCiliumVersion()
version, err := k8sClient.GetRunningCiliumVersion(helmReleaseName)
if err != nil {
fmt.Printf("cilium image (running): unknown. Unable to obtain cilium version. Reason: %s\n", err.Error())
} else {
Expand Down
25 changes: 15 additions & 10 deletions clustermesh/clustermesh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ type Parameters struct {
// EnableKVStoreMesh indicates whether kvstoremesh should be enabled.
// For Helm mode only.
EnableKVStoreMesh bool

// HelmReleaseName specifies the Helm release name for the Cilium CLI.
// Useful for referencing Cilium installations installed directly through Helm
// or overriding the Cilium CLI for install/upgrade/enable.
HelmReleaseName string
}

func (p Parameters) waitTimeout() time.Duration {
Expand Down Expand Up @@ -1387,7 +1392,7 @@ func EnableWithHelm(ctx context.Context, k8sClient *k8s.Client, params Parameter
}
upgradeParams := helm.UpgradeParameters{
Namespace: params.Namespace,
Name: defaults.HelmReleaseName,
Name: params.HelmReleaseName,
Values: helmVals,
ResetValues: false,
ReuseValues: true,
Expand All @@ -1407,7 +1412,7 @@ func DisableWithHelm(ctx context.Context, k8sClient *k8s.Client, params Paramete
}
upgradeParams := helm.UpgradeParameters{
Namespace: params.Namespace,
Name: defaults.HelmReleaseName,
Name: params.HelmReleaseName,
Values: vals,
ResetValues: false,
ReuseValues: true,
Expand All @@ -1416,8 +1421,8 @@ func DisableWithHelm(ctx context.Context, k8sClient *k8s.Client, params Paramete
return err
}

func getRelease(kc *k8s.Client) (*release.Release, error) {
return kc.HelmActionConfig.Releases.Last(defaults.HelmReleaseName)
func getRelease(kc *k8s.Client, params Parameters) (*release.Release, error) {
return kc.HelmActionConfig.Releases.Last(params.HelmReleaseName)
}

// validateCAMatch determines if the certificate authority certificate being
Expand Down Expand Up @@ -1457,7 +1462,7 @@ func (k *K8sClusterMesh) validateCAMatch(aiLocal, aiRemote *accessInformation) (
// (certgen) mode. As with classic mode, only autodetected IP-based
// clustermesh-apiserver Service endpoints are currently supported.
func (k *K8sClusterMesh) ConnectWithHelm(ctx context.Context) error {
localRelease, err := getRelease(k.client.(*k8s.Client))
localRelease, err := getRelease(k.client.(*k8s.Client), k.params)
if err != nil {
k.Log("❌ Unable to find Helm release for the target cluster")
return err
Expand Down Expand Up @@ -1495,7 +1500,7 @@ func (k *K8sClusterMesh) ConnectWithHelm(ctx context.Context) error {
}

// Get existing helm values for the remote cluster
remoteRelease, err := getRelease(remoteClient)
remoteRelease, err := getRelease(remoteClient, k.params)
if err != nil {
k.Log("❌ Unable to find Helm release for the remote cluster")
return err
Expand All @@ -1509,7 +1514,7 @@ func (k *K8sClusterMesh) ConnectWithHelm(ctx context.Context) error {

upgradeParams := helm.UpgradeParameters{
Namespace: k.params.Namespace,
Name: defaults.HelmReleaseName,
Name: k.params.HelmReleaseName,
Values: localHelmValues,
ResetValues: false,
ReuseValues: true,
Expand Down Expand Up @@ -1537,7 +1542,7 @@ func (k *K8sClusterMesh) ConnectWithHelm(ctx context.Context) error {
}

func (k *K8sClusterMesh) DisconnectWithHelm(ctx context.Context) error {
localRelease, err := getRelease(k.client.(*k8s.Client))
localRelease, err := getRelease(k.client.(*k8s.Client), k.params)
if err != nil {
k.Log("❌ Unable to find Helm release for the target cluster")
return err
Expand All @@ -1563,7 +1568,7 @@ func (k *K8sClusterMesh) DisconnectWithHelm(ctx context.Context) error {
}

// Get existing helm values for the remote cluster
remoteRelease, err := getRelease(remoteClient)
remoteRelease, err := getRelease(remoteClient, k.params)
if err != nil {
k.Log("❌ Unable to find Helm release for the remote cluster")
return err
Expand All @@ -1576,7 +1581,7 @@ func (k *K8sClusterMesh) DisconnectWithHelm(ctx context.Context) error {

upgradeParams := helm.UpgradeParameters{
Namespace: k.params.Namespace,
Name: defaults.HelmReleaseName,
Name: k.params.HelmReleaseName,
Values: localHelmValues,
ResetValues: false,
ReuseValues: true,
Expand Down
6 changes: 1 addition & 5 deletions defaults/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,7 @@ const (

IngressClassName = "cilium"

// HelmReleaseName is the default Helm release name for Cilium.
HelmReleaseName = "cilium"
HelmValuesSecretName = "cilium-cli-helm-values"
HelmValuesSecretKeyName = "io.cilium.cilium-cli"
HelmChartVersionSecretKeyName = "io.cilium.chart-version"
HelmValuesSecretName = "cilium-cli-helm-values"
tklauser marked this conversation as resolved.
Show resolved Hide resolved

CiliumNoScheduleLabel = "cilium.io/no-schedule"

Expand Down
26 changes: 15 additions & 11 deletions hubble/hubble.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ type Parameters struct {

// Wait will cause Helm upgrades related to disabling Hubble to wait.
Wait bool

HelmReleaseName string
}

func (p *Parameters) Log(format string, a ...interface{}) {
Expand All @@ -53,11 +55,12 @@ func EnableWithHelm(ctx context.Context, k8sClient *k8s.Client, params Parameter
return err
}
upgradeParams := helm.UpgradeParameters{
Namespace: params.Namespace,
Name: defaults.HelmReleaseName,
Values: vals,
ResetValues: false,
ReuseValues: true,
Namespace: params.Namespace,
Name: params.HelmReleaseName,
Values: vals,
ResetValues: false,
ReuseValues: true,
WaitDuration: defaults.UninstallTimeout,
}
_, err = helm.Upgrade(ctx, k8sClient.HelmActionConfig, upgradeParams)
return err
Expand All @@ -72,12 +75,13 @@ func DisableWithHelm(ctx context.Context, k8sClient *k8s.Client, params Paramete
return err
}
upgradeParams := helm.UpgradeParameters{
Namespace: params.Namespace,
Name: defaults.HelmReleaseName,
Values: vals,
ResetValues: false,
ReuseValues: true,
Wait: params.Wait,
Namespace: params.Namespace,
Name: params.HelmReleaseName,
Values: vals,
ResetValues: false,
ReuseValues: true,
Wait: params.Wait,
WaitDuration: defaults.UninstallTimeout,
tklauser marked this conversation as resolved.
Show resolved Hide resolved
}
_, err = helm.Upgrade(ctx, k8sClient.HelmActionConfig, upgradeParams)
return err
Expand Down
23 changes: 14 additions & 9 deletions install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ type Parameters struct {
// Useful to test from upstream where a helm release is not available yet.
HelmChartDirectory string

// HelmRepository specifies the Helm repository to download Cilium Helm charts from.
HelmRepository string

// HelmReleaseName specifies the Helm release name for the Cilium CLI.
// Useful for referencing Cilium installations installed directly through Helm
// or overriding the Cilium CLI for install/upgrade/enable.
HelmReleaseName string

// HelmOpts are all the options the user used to pass into the Cilium cli
// template.
HelmOpts values.Options
Expand All @@ -114,12 +122,6 @@ type Parameters struct {
// specified by other flags. This options take precedence over the HelmResetValues option.
HelmReuseValues bool

// ListVersions lists all the available versions for install without actually installing.
ListVersions bool

// NodesWithoutCilium enables the affinities to avoid scheduling Cilium components on nodes labeled with cilium.io/no-schedule
NodesWithoutCilium bool

// DryRun writes resources to be installed to stdout without actually installing them. For Helm
// installation mode only.
DryRun bool
Expand All @@ -128,8 +130,11 @@ type Parameters struct {
// For Helm installation mode only.
DryRunHelmValues bool

// HelmRepository specifies the Helm repository to download Cilium Helm charts from.
HelmRepository string
// ListVersions lists all the available versions for install without actually installing.
ListVersions bool

// NodesWithoutCilium enables the affinities to avoid scheduling Cilium components on nodes labeled with cilium.io/no-schedule
NodesWithoutCilium bool
}

func (p *Parameters) IsDryRun() bool {
Expand Down Expand Up @@ -253,7 +258,7 @@ func (k *K8sInstaller) InstallWithHelm(ctx context.Context, k8sClient *k8s.Clien
return err
}
helmClient := action.NewInstall(k8sClient.HelmActionConfig)
helmClient.ReleaseName = defaults.HelmReleaseName
helmClient.ReleaseName = k.params.HelmReleaseName
helmClient.Namespace = k.params.Namespace
helmClient.Wait = k.params.Wait
helmClient.Timeout = k.params.WaitDuration
Expand Down
5 changes: 2 additions & 3 deletions install/uninstall.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ import (
"helm.sh/helm/v3/pkg/action"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"

"github.com/cilium/cilium-cli/defaults"
)

type UninstallParameters struct {
Expand All @@ -27,6 +25,7 @@ type UninstallParameters struct {
HelmChartDirectory string
WorkerCount int
Timeout time.Duration
HelmReleaseName string
}

type K8sUninstaller struct {
Expand All @@ -52,7 +51,7 @@ func (k *K8sUninstaller) UninstallWithHelm(ctx context.Context, actionConfig *ac
helmClient.DeletionPropagation = "foreground"
}
helmClient.Timeout = k.params.Timeout
if _, err := helmClient.Run(defaults.HelmReleaseName); err != nil {
if _, err := helmClient.Run(k.params.HelmReleaseName); err != nil {
return err
}
// If aws-node daemonset exists, remove io.cilium/aws-node-enabled node selector.
Expand Down
3 changes: 1 addition & 2 deletions install/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"helm.sh/helm/v3/pkg/getter"
"sigs.k8s.io/yaml"

"github.com/cilium/cilium-cli/defaults"
"github.com/cilium/cilium-cli/internal/helm"
"github.com/cilium/cilium-cli/k8s"
)
Expand All @@ -31,7 +30,7 @@ func (k *K8sInstaller) UpgradeWithHelm(ctx context.Context, k8sClient *k8s.Clien

upgradeParams := helm.UpgradeParameters{
Namespace: k.params.Namespace,
Name: defaults.HelmReleaseName,
Name: k.params.HelmReleaseName,
Chart: k.chart, // k.chart was initialized in NewK8sInstaller, based on Version and HelmChartDirectory
Values: vals,
ResetValues: k.params.HelmResetValues,
Expand Down
2 changes: 1 addition & 1 deletion internal/helm/helm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,5 +298,5 @@ func Upgrade(
helmClient.Timeout = params.WaitDuration
helmClient.DryRun = params.IsDryRun()

return helmClient.RunWithContext(ctx, defaults.HelmReleaseName, params.Chart, params.Values)
return helmClient.RunWithContext(ctx, params.Name, params.Chart, params.Values)
}
4 changes: 2 additions & 2 deletions k8s/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,8 +843,8 @@ func (c *Client) GetCiliumVersion(ctx context.Context, p *corev1.Pod) (*semver.V
return &podVersion, nil
}

func (c *Client) GetRunningCiliumVersion() (string, error) {
release, err := action.NewGet(c.HelmActionConfig).Run(defaults.HelmReleaseName)
func (c *Client) GetRunningCiliumVersion(ciliumHelmReleaseName string) (string, error) {
release, err := action.NewGet(c.HelmActionConfig).Run(ciliumHelmReleaseName)
if err != nil {
return "", err
}
Expand Down