From 6628afe15583169fed64d1e597511720d671fa20 Mon Sep 17 00:00:00 2001 From: Alon Braymok <138359965+alonkeyval@users.noreply.github.com> Date: Tue, 4 Feb 2025 10:31:15 +0200 Subject: [PATCH] [GEN-2309] fix: config set command (#2372) This pull request introduces several changes to the `cli/cmd/config.go` file to enhance configuration management in Odigos. The changes include importing new packages, updating command arguments, and modifying the configuration update process. ### Imports and Dependencies: * Added imports for `resources`, `odigospro`, `common`, and `consts` packages to support new functionalities. ### Command Argument Handling: * Changed the `Args` for the `setConfigCmd` command from `cobra.ExactArgs(2)` to `cobra.MinimumNArgs(2)` to allow for multiple values. ### Configuration Update Process: * Replaced the `updateConfigProperty` function with `setConfigProperty` to handle a broader range of configuration properties and to increment the `ConfigVersion`. * Introduced a new process to fetch the current configuration and apply changes using `resources.GetCurrentConfig`, `resources.CreateResourceManagers`, and `resources.ApplyResourceManagers`. * Added error handling for deprecated configurations and cleanup of old Odigos resources. These changes improve the flexibility and robustness of the configuration management in Odigos. --------- Co-authored-by: alonkeyval --- cli/cmd/config.go | 141 ++++++++++++++++++++++++++----------- docs/cli/odigos_config.mdx | 2 +- 2 files changed, 99 insertions(+), 44 deletions(-) diff --git a/cli/cmd/config.go b/cli/cmd/config.go index 887860d7fb..898f097ae3 100644 --- a/cli/cmd/config.go +++ b/cli/cmd/config.go @@ -1,87 +1,142 @@ package cmd import ( - "context" "fmt" "os" + "strconv" + "github.com/odigos-io/odigos/cli/cmd/resources" + "github.com/odigos-io/odigos/cli/cmd/resources/odigospro" cmdcontext "github.com/odigos-io/odigos/cli/pkg/cmd_context" - "github.com/odigos-io/odigos/cli/pkg/kube" "github.com/odigos-io/odigos/cli/pkg/log" + "github.com/odigos-io/odigos/common" + "github.com/odigos-io/odigos/common/consts" "github.com/spf13/cobra" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var configCmd = &cobra.Command{ Use: "config", Short: "Manage Odigos configuration", - Long: "Manage Odigos configuration settings, including central backend URL and other properties", + Long: "Manage Odigos configuration settings to customize system behavior.", } // `odigos config set ` var setConfigCmd = &cobra.Command{ Use: "set ", Short: "Set a configuration property in Odigos", - Args: cobra.ExactArgs(2), + Args: cobra.MinimumNArgs(2), Run: func(cmd *cobra.Command, args []string) { property := args[0] - value := args[1] + value := args[1:] ctx := cmd.Context() client := cmdcontext.KubeClientFromContextOrExit(ctx) - ns, _ := cmd.Flags().GetString("namespace") - if ns == "" { - ns = "odigos-system" - } + ns, err := resources.GetOdigosNamespace(client, ctx) l := log.Print(fmt.Sprintf("Updating %s to %s...", property, value)) - err := updateConfigProperty(ctx, client, ns, property, value) + + config, err := resources.GetCurrentConfig(ctx, client, ns) + if err != nil { + odigosConfig, err := resources.GetDeprecatedConfig(ctx, client, ns) + if err != nil { + l.Error(fmt.Errorf("unable to read the current Odigos configuration: %w", err)) + os.Exit(1) + } + config = odigosConfig.ToCommonConfig() + } + + config.ConfigVersion += 1 + err = setConfigProperty(config, property, value) if err != nil { l.Error(err) os.Exit(1) } - l.Success() - if property == "central-backend-url" { - l = log.Print("Restarting UI pod to apply changes...") - err = restartUIPod(ctx, client, ns) - if err != nil { - l.Error(err) - os.Exit(1) - } - l.Success() + currentTier, err := odigospro.GetCurrentOdigosTier(ctx, client, ns) + if err != nil { + l.Error(fmt.Errorf("unable to read the current Odigos tier: %w", err)) + os.Exit(1) } - }, -} -func updateConfigProperty(ctx context.Context, client *kube.Client, ns, property, value string) error { - configMapName := "odigos-config" + resourceManagers := resources.CreateResourceManagers(client, ns, currentTier, nil, config, "Updating Config") + err = resources.ApplyResourceManagers(ctx, client, resourceManagers, "Updating Config") + if err != nil { + l.Error(fmt.Errorf("failed to apply updated configuration: %w", err)) + os.Exit(1) + } - cm, err := client.CoreV1().ConfigMaps(ns).Get(ctx, configMapName, metav1.GetOptions{}) - if err != nil { - return fmt.Errorf("failed to get ConfigMap: %w", err) - } + err = resources.DeleteOldOdigosSystemObjects(ctx, client, ns, config) + if err != nil { + fmt.Println("Odigos config update failed - unable to cleanup old Odigos resources.") + os.Exit(1) + } - cm.Data[property] = value - _, err = client.CoreV1().ConfigMaps(ns).Update(ctx, cm, metav1.UpdateOptions{}) - if err != nil { - return fmt.Errorf("failed to update ConfigMap: %w", err) - } + l.Success() - return nil + }, } -func restartUIPod(ctx context.Context, client *kube.Client, ns string) error { - pods, err := client.CoreV1().Pods(ns).List(ctx, metav1.ListOptions{LabelSelector: "app=odigos-ui"}) - if err != nil { - return fmt.Errorf("failed to list UI pods: %w", err) - } +func setConfigProperty(config *common.OdigosConfiguration, property string, value []string) error { + switch property { + case "central-backend-url": + if len(value) != 1 { + return fmt.Errorf("%s expects exactly one value", property) + } + config.CentralBackendURL = value[0] - for _, pod := range pods.Items { - err := client.CoreV1().Pods(ns).Delete(ctx, pod.Name, metav1.DeleteOptions{}) + case "telemetry-enabled", "openshift-enabled", "psp", "skip-webhook-issuer-creation", "allow-concurrent-agents": + if len(value) != 1 { + return fmt.Errorf("%s expects exactly one value (true/false)", property) + } + boolValue, err := strconv.ParseBool(value[0]) if err != nil { - return fmt.Errorf("failed to delete pod %s: %w", pod.Name, err) + return fmt.Errorf("invalid boolean value for %s: %s", property, value[0]) } + + switch property { + case "telemetry-enabled": + config.TelemetryEnabled = boolValue + case "openshift-enabled": + config.OpenshiftEnabled = boolValue + case "psp": + config.Psp = boolValue + case "skip-webhook-issuer-creation": + config.SkipWebhookIssuerCreation = boolValue + case "allow-concurrent-agents": + config.AllowConcurrentAgents = &boolValue + } + + case "image-prefix", "odiglet-image", "instrumentor-image", "autoscaler-image", "ui-mode": + if len(value) != 1 { + return fmt.Errorf("%s expects exactly one value", property) + } + switch property { + case "image-prefix": + config.ImagePrefix = value[0] + case "odiglet-image": + config.OdigletImage = value[0] + case "instrumentor-image": + config.InstrumentorImage = value[0] + case "autoscaler-image": + config.AutoscalerImage = value[0] + case "ui-mode": + config.UiMode = common.UiMode(value[0]) + } + + case "ignored-namespaces": + if len(value) < 1 { + return fmt.Errorf("%s expects at least one value", property) + } + config.IgnoredNamespaces = value + + case "ignored-containers": + if len(value) < 1 { + return fmt.Errorf("%s expects at least one value", property) + } + config.IgnoredContainers = value + + default: + return fmt.Errorf("invalid property: %s", property) } return nil @@ -91,5 +146,5 @@ func init() { rootCmd.AddCommand(configCmd) configCmd.AddCommand(setConfigCmd) - setConfigCmd.Flags().StringP("namespace", "n", "odigos-system", "Namespace where Odigos is installed") + setConfigCmd.Flags().StringP("namespace", "n", consts.DefaultOdigosNamespace, "Namespace where Odigos is installed") } diff --git a/docs/cli/odigos_config.mdx b/docs/cli/odigos_config.mdx index 06bb8afbfc..8a3decedf4 100644 --- a/docs/cli/odigos_config.mdx +++ b/docs/cli/odigos_config.mdx @@ -8,7 +8,7 @@ Manage Odigos configuration ### Synopsis -Manage Odigos configuration settings, including central backend URL and other properties +Manage Odigos configuration settings to customize system behavior. ### Options