From d3f1892665250c61da87aa1b7c11958c4058e7bd Mon Sep 17 00:00:00 2001
From: Alexey Makhov <makhov.alex@gmail.com>
Date: Fri, 12 Jul 2024 17:14:12 +0300
Subject: [PATCH] Set extraArgs in the end of args processing (#250)

Set extraArgs in the end of args processing to let them overwrite
default values
---
 internal/controller/factory/statefulset.go    | 68 ++++++++++---------
 .../controller/factory/statefulset_test.go    | 14 +++-
 2 files changed, 48 insertions(+), 34 deletions(-)

diff --git a/internal/controller/factory/statefulset.go b/internal/controller/factory/statefulset.go
index a2671ae..e8d4836 100644
--- a/internal/controller/factory/statefulset.go
+++ b/internal/controller/factory/statefulset.go
@@ -23,7 +23,6 @@ import (
 	"slices"
 	"strconv"
 
-	"github.com/aenix-io/etcd-operator/internal/log"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -34,6 +33,7 @@ import (
 
 	etcdaenixiov1alpha1 "github.com/aenix-io/etcd-operator/api/v1alpha1"
 	"github.com/aenix-io/etcd-operator/internal/k8sutils"
+	"github.com/aenix-io/etcd-operator/internal/log"
 )
 
 const (
@@ -256,36 +256,6 @@ func generateEtcdCommand() []string {
 func generateEtcdArgs(cluster *etcdaenixiov1alpha1.EtcdCluster) []string {
 	args := []string{}
 
-	if value, ok := cluster.Spec.Options["quota-backend-bytes"]; !ok || value == "" {
-		var size resource.Quantity
-		if cluster.Spec.Storage.EmptyDir != nil {
-			if cluster.Spec.Storage.EmptyDir.SizeLimit != nil {
-				size = *cluster.Spec.Storage.EmptyDir.SizeLimit
-			}
-		} else {
-			size = *cluster.Spec.Storage.VolumeClaimTemplate.Spec.Resources.Requests.Storage()
-		}
-		quota := float64(size.Value()) * defaultBackendQuotaBytesFraction
-		quota = math.Floor(quota)
-		if quota > 0 {
-			if cluster.Spec.Options == nil {
-				cluster.Spec.Options = make(map[string]string, 1)
-			}
-			cluster.Spec.Options["quota-backend-bytes"] = strconv.FormatInt(int64(quota), 10)
-		}
-	}
-
-	for name, value := range cluster.Spec.Options {
-		flag := "--" + name
-		if len(value) == 0 {
-			args = append(args, flag)
-
-			continue
-		}
-
-		args = append(args, fmt.Sprintf("%s=%s", flag, value))
-	}
-
 	peerTlsSettings := []string{"--peer-auto-tls"}
 
 	if cluster.Spec.Security != nil && cluster.Spec.Security.TLS.PeerSecret != "" {
@@ -335,7 +305,41 @@ func generateEtcdArgs(cluster *etcdaenixiov1alpha1.EtcdCluster) []string {
 	args = append(args, clientTlsSettings...)
 	args = append(args, autoCompactionSettings...)
 
-	slices.Sort(args)
+	extraArgs := []string{}
+
+	if value, ok := cluster.Spec.Options["quota-backend-bytes"]; !ok || value == "" {
+		var size resource.Quantity
+		if cluster.Spec.Storage.EmptyDir != nil {
+			if cluster.Spec.Storage.EmptyDir.SizeLimit != nil {
+				size = *cluster.Spec.Storage.EmptyDir.SizeLimit
+			}
+		} else {
+			size = *cluster.Spec.Storage.VolumeClaimTemplate.Spec.Resources.Requests.Storage()
+		}
+		quota := float64(size.Value()) * defaultBackendQuotaBytesFraction
+		quota = math.Floor(quota)
+		if quota > 0 {
+			if cluster.Spec.Options == nil {
+				cluster.Spec.Options = make(map[string]string, 1)
+			}
+			cluster.Spec.Options["quota-backend-bytes"] = strconv.FormatInt(int64(quota), 10)
+		}
+	}
+
+	for name, value := range cluster.Spec.Options {
+		flag := "--" + name
+		if len(value) == 0 {
+			extraArgs = append(extraArgs, flag)
+
+			continue
+		}
+
+		extraArgs = append(extraArgs, fmt.Sprintf("%s=%s", flag, value))
+	}
+
+	// Sort the extra args to ensure a deterministic order
+	slices.Sort(extraArgs)
+	args = append(args, extraArgs...)
 
 	return args
 }
diff --git a/internal/controller/factory/statefulset_test.go b/internal/controller/factory/statefulset_test.go
index 0cdce04..64d4660 100644
--- a/internal/controller/factory/statefulset_test.go
+++ b/internal/controller/factory/statefulset_test.go
@@ -66,6 +66,11 @@ var _ = Describe("CreateOrUpdateStatefulSet handler", func() {
 				},
 				Spec: etcdaenixiov1alpha1.EtcdClusterSpec{
 					Replicas: ptr.To(int32(3)),
+					Options: map[string]string{
+						"foo":  "bar",
+						"key1": "value1",
+						"key2": "value2",
+					},
 				},
 			}
 			Expect(k8sClient.Create(ctx, &etcdcluster)).Should(Succeed())
@@ -181,9 +186,14 @@ var _ = Describe("CreateOrUpdateStatefulSet handler", func() {
 			By("Checking the extraArgs", func() {
 				Expect(statefulSet.Spec.Template.Spec.Containers[0].Args).To(Equal(generateEtcdArgs(&etcdcluster)))
 				By("Checking args are sorted", func() {
-					argsClone := slices.Clone(statefulSet.Spec.Template.Spec.Containers[0].Args)
+					// Check that only the extra args are sorted, which means we need to check elements starting from n,
+					// where n is the length of the default args. So we subtract the length of the extra args from the all args.
+					// For example: if we have 3 extra args and 10 total args, we need to check elements starting from 10-3 = 7,
+					// because the first 7 elements are default args and the elements args[7], args[8], and args[9] are extra args.
+					n := len(statefulSet.Spec.Template.Spec.Containers[0].Args) - len(etcdcluster.Spec.Options)
+					argsClone := slices.Clone(statefulSet.Spec.Template.Spec.Containers[0].Args[n:])
 					slices.Sort(argsClone)
-					Expect(statefulSet.Spec.Template.Spec.Containers[0].Args).To(Equal(argsClone))
+					Expect(statefulSet.Spec.Template.Spec.Containers[0].Args[n:]).To(Equal(argsClone))
 				})
 			})