From e56a323a17da6c555c177424a7b28ef722881b61 Mon Sep 17 00:00:00 2001 From: phanama Date: Wed, 22 Nov 2023 09:37:48 +0700 Subject: [PATCH 1/3] fix(cli): pass redis compression to cluster stats and shards commands Co-authored-by: clavinjune <24659468+clavinjune@users.noreply.github.com> Signed-off-by: phanama --- cmd/argocd/commands/admin/cluster.go | 42 ++++++++++++++++++---------- util/cache/cache.go | 7 ++++- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/cmd/argocd/commands/admin/cluster.go b/cmd/argocd/commands/admin/cluster.go index a72aaebc201a0..a7353b77c9c54 100644 --- a/cmd/argocd/commands/admin/cluster.go +++ b/cmd/argocd/commands/admin/cluster.go @@ -69,7 +69,7 @@ type ClusterWithInfo struct { Namespaces []string } -func loadClusters(ctx context.Context, kubeClient *kubernetes.Clientset, appClient *versioned.Clientset, replicas int, namespace string, portForwardRedis bool, cacheSrc func() (*appstatecache.Cache, error), shard int, redisName string, redisHaProxyName string) ([]ClusterWithInfo, error) { +func loadClusters(ctx context.Context, kubeClient *kubernetes.Clientset, appClient *versioned.Clientset, replicas int, namespace string, portForwardRedis bool, cacheSrc func() (*appstatecache.Cache, error), shard int, redisName string, redisHaProxyName string, redisCompressionStr string) ([]ClusterWithInfo, error) { settingsMgr := settings.NewSettingsManager(ctx, kubeClient, namespace) argoDB := db.NewDB(namespace, settingsMgr, kubeClient) @@ -88,7 +88,11 @@ func loadClusters(ctx context.Context, kubeClient *kubernetes.Clientset, appClie return nil, err } client := redis.NewClient(&redis.Options{Addr: fmt.Sprintf("localhost:%d", port)}) - cache = appstatecache.NewCache(cacheutil.NewCache(cacheutil.NewRedisCache(client, time.Hour, cacheutil.RedisCompressionNone)), time.Hour) + compressionType, err := cacheutil.CompressionTypeFromString(redisCompressionStr) + if err != nil { + return nil, err + } + cache = appstatecache.NewCache(cacheutil.NewCache(cacheutil.NewRedisCache(client, time.Hour, compressionType)), time.Hour) } else { cache, err = cacheSrc() if err != nil { @@ -161,11 +165,12 @@ func getControllerReplicas(ctx context.Context, kubeClient *kubernetes.Clientset func NewClusterShardsCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command { var ( - shard int - replicas int - clientConfig clientcmd.ClientConfig - cacheSrc func() (*appstatecache.Cache, error) - portForwardRedis bool + shard int + replicas int + clientConfig clientcmd.ClientConfig + cacheSrc func() (*appstatecache.Cache, error) + portForwardRedis bool + redisCompressionStr string ) var command = cobra.Command{ Use: "shards", @@ -190,7 +195,7 @@ func NewClusterShardsCommand(clientOpts *argocdclient.ClientOptions) *cobra.Comm return } - clusters, err := loadClusters(ctx, kubeClient, appClient, replicas, namespace, portForwardRedis, cacheSrc, shard, clientOpts.RedisName, clientOpts.RedisHaProxyName) + clusters, err := loadClusters(ctx, kubeClient, appClient, replicas, namespace, portForwardRedis, cacheSrc, shard, clientOpts.RedisName, clientOpts.RedisHaProxyName, redisCompressionStr) errors.CheckError(err) if len(clusters) == 0 { return @@ -204,6 +209,10 @@ func NewClusterShardsCommand(clientOpts *argocdclient.ClientOptions) *cobra.Comm command.Flags().IntVar(&replicas, "replicas", 0, "Application controller replicas count. Inferred from number of running controller pods if not specified") command.Flags().BoolVar(&portForwardRedis, "port-forward-redis", true, "Automatically port-forward ha proxy redis from current namespace?") cacheSrc = appstatecache.AddCacheFlagsToCmd(&command) + + // parse all added flags and get the redis-compression flag that was added by parent command + command.Flags().Parse(os.Args[1:]) + redisCompressionStr, _ = command.Flags().GetString(cacheutil.CLIFlagRedisCompress) return &command } @@ -439,11 +448,12 @@ func NewClusterDisableNamespacedMode() *cobra.Command { func NewClusterStatsCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command { var ( - shard int - replicas int - clientConfig clientcmd.ClientConfig - cacheSrc func() (*appstatecache.Cache, error) - portForwardRedis bool + shard int + replicas int + clientConfig clientcmd.ClientConfig + cacheSrc func() (*appstatecache.Cache, error) + portForwardRedis bool + redisCompressionStr string ) var command = cobra.Command{ Use: "stats", @@ -464,7 +474,7 @@ func NewClusterStatsCommand(clientOpts *argocdclient.ClientOptions) *cobra.Comma replicas, err = getControllerReplicas(ctx, kubeClient, namespace, clientOpts.AppControllerName) errors.CheckError(err) } - clusters, err := loadClusters(ctx, kubeClient, appClient, replicas, namespace, portForwardRedis, cacheSrc, shard, clientOpts.RedisName, clientOpts.RedisHaProxyName) + clusters, err := loadClusters(ctx, kubeClient, appClient, replicas, namespace, portForwardRedis, cacheSrc, shard, clientOpts.RedisName, clientOpts.RedisHaProxyName, redisCompressionStr) errors.CheckError(err) w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) @@ -480,6 +490,10 @@ func NewClusterStatsCommand(clientOpts *argocdclient.ClientOptions) *cobra.Comma command.Flags().IntVar(&replicas, "replicas", 0, "Application controller replicas count. Inferred from number of running controller pods if not specified") command.Flags().BoolVar(&portForwardRedis, "port-forward-redis", true, "Automatically port-forward ha proxy redis from current namespace?") cacheSrc = appstatecache.AddCacheFlagsToCmd(&command) + + // parse all added flags and get the redis-compression flag that was added by parent command + command.Flags().Parse(os.Args[1:]) + redisCompressionStr, _ = command.Flags().GetString(cacheutil.CLIFlagRedisCompress) return &command } diff --git a/util/cache/cache.go b/util/cache/cache.go index fdea46cdea0d2..c9cb8c3b8607a 100644 --- a/util/cache/cache.go +++ b/util/cache/cache.go @@ -29,6 +29,11 @@ const ( defaultRedisRetryCount = 3 ) +const ( + // CLIFlagRedisCompress is a cli flag name to define the redis compression setting for data sent to redis + CLIFlagRedisCompress = "redis-compress" +) + func NewCache(client CacheClient) *Cache { return &Cache{client} } @@ -96,7 +101,7 @@ func AddCacheFlagsToCmd(cmd *cobra.Command, opts ...func(client *redis.Client)) cmd.Flags().StringVar(&redisClientKey, "redis-client-key", "", "Path to Redis client key (e.g. /etc/certs/redis/client.crt).") cmd.Flags().BoolVar(&insecureRedis, "redis-insecure-skip-tls-verify", false, "Skip Redis server certificate validation.") cmd.Flags().StringVar(&redisCACertificate, "redis-ca-certificate", "", "Path to Redis server CA certificate (e.g. /etc/certs/redis/ca.crt). If not specified, system trusted CAs will be used for server certificate validation.") - cmd.Flags().StringVar(&compressionStr, "redis-compress", env.StringFromEnv("REDIS_COMPRESSION", string(RedisCompressionGZip)), "Enable compression for data sent to Redis with the required compression algorithm. (possible values: gzip, none)") + cmd.Flags().StringVar(&compressionStr, CLIFlagRedisCompress, env.StringFromEnv("REDIS_COMPRESSION", string(RedisCompressionGZip)), "Enable compression for data sent to Redis with the required compression algorithm. (possible values: gzip, none)") return func() (*Cache, error) { var tlsConfig *tls.Config = nil if redisUseTLS { From dd02e7ec1c25e140389b5305dc6cee8ccbce896b Mon Sep 17 00:00:00 2001 From: phanama Date: Wed, 22 Nov 2023 11:03:42 +0700 Subject: [PATCH 2/3] fix(cli): check command.Parse error in cluster command Co-authored-by: clavinjune <24659468+clavinjune@users.noreply.github.com> Signed-off-by: phanama --- cmd/argocd/commands/admin/cluster.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/argocd/commands/admin/cluster.go b/cmd/argocd/commands/admin/cluster.go index a7353b77c9c54..d45ed23cf3be2 100644 --- a/cmd/argocd/commands/admin/cluster.go +++ b/cmd/argocd/commands/admin/cluster.go @@ -211,7 +211,8 @@ func NewClusterShardsCommand(clientOpts *argocdclient.ClientOptions) *cobra.Comm cacheSrc = appstatecache.AddCacheFlagsToCmd(&command) // parse all added flags and get the redis-compression flag that was added by parent command - command.Flags().Parse(os.Args[1:]) + err := command.Flags().Parse(os.Args[1:]) + errors.CheckError(err) redisCompressionStr, _ = command.Flags().GetString(cacheutil.CLIFlagRedisCompress) return &command } @@ -492,7 +493,8 @@ func NewClusterStatsCommand(clientOpts *argocdclient.ClientOptions) *cobra.Comma cacheSrc = appstatecache.AddCacheFlagsToCmd(&command) // parse all added flags and get the redis-compression flag that was added by parent command - command.Flags().Parse(os.Args[1:]) + err := command.Flags().Parse(os.Args[1:]) + errors.CheckError(err) redisCompressionStr, _ = command.Flags().GetString(cacheutil.CLIFlagRedisCompress) return &command } From 1bc26ced7f93d9323b251a74a22b1e5c25a341f0 Mon Sep 17 00:00:00 2001 From: phanama Date: Wed, 22 Nov 2023 14:11:30 +0700 Subject: [PATCH 3/3] fix(cli): change to ParseFlags and skip errcheck lint Co-authored-by: clavinjune <24659468+clavinjune@users.noreply.github.com> Signed-off-by: phanama --- cmd/argocd/commands/admin/cluster.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/cmd/argocd/commands/admin/cluster.go b/cmd/argocd/commands/admin/cluster.go index d45ed23cf3be2..6625787de7d71 100644 --- a/cmd/argocd/commands/admin/cluster.go +++ b/cmd/argocd/commands/admin/cluster.go @@ -210,9 +210,10 @@ func NewClusterShardsCommand(clientOpts *argocdclient.ClientOptions) *cobra.Comm command.Flags().BoolVar(&portForwardRedis, "port-forward-redis", true, "Automatically port-forward ha proxy redis from current namespace?") cacheSrc = appstatecache.AddCacheFlagsToCmd(&command) - // parse all added flags and get the redis-compression flag that was added by parent command - err := command.Flags().Parse(os.Args[1:]) - errors.CheckError(err) + // parse all added flags so far to get the redis-compression flag that was added by AddCacheFlagsToCmd() above + // we can ignore unchecked error here as the command will be parsed again and checked when command.Execute() is run later + // nolint:errcheck + command.ParseFlags(os.Args[1:]) redisCompressionStr, _ = command.Flags().GetString(cacheutil.CLIFlagRedisCompress) return &command } @@ -492,9 +493,10 @@ func NewClusterStatsCommand(clientOpts *argocdclient.ClientOptions) *cobra.Comma command.Flags().BoolVar(&portForwardRedis, "port-forward-redis", true, "Automatically port-forward ha proxy redis from current namespace?") cacheSrc = appstatecache.AddCacheFlagsToCmd(&command) - // parse all added flags and get the redis-compression flag that was added by parent command - err := command.Flags().Parse(os.Args[1:]) - errors.CheckError(err) + // parse all added flags so far to get the redis-compression flag that was added by AddCacheFlagsToCmd() above + // we can ignore unchecked error here as the command will be parsed again and checked when command.Execute() is run later + // nolint:errcheck + command.ParseFlags(os.Args[1:]) redisCompressionStr, _ = command.Flags().GetString(cacheutil.CLIFlagRedisCompress) return &command }