From 5e5e43d1a3a7d27d128b327c0a544b434b529fae Mon Sep 17 00:00:00 2001
From: Adam Janikowski <12255597+ajanikow@users.noreply.github.com>
Date: Wed, 11 Oct 2023 10:18:28 +0200
Subject: [PATCH] [Feature] Fix numactl options (#1432)

---
 CHANGELOG.md                                  |   1 +
 pkg/deployment/deployment_core_test.go        | 184 ++++++++++++++++++
 pkg/deployment/images.go                      |  10 +-
 .../resources/pod_creator_arangod.go          |  21 +-
 pkg/deployment/resources/pod_creator_sync.go  |   7 +-
 pkg/util/k8sutil/interfaces/pod_creator.go    |   4 +-
 pkg/util/k8sutil/pair.go                      |  14 +-
 pkg/util/k8sutil/pods.go                      |   4 +-
 pkg/util/list.go                              |   8 +
 9 files changed, 231 insertions(+), 22 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0fae10f9f..968943eec 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@
 - (Improvement) Allow tcp:// and ssl:// protocols in endpoints for members
 - (Maintenance) Reorganize package imports / move common code to separate repos
 - (Maintenance) Remove support for RELATED_IMAGE_UBI, RELATED_IMAGE_DATABASE and RELATED_IMAGE_METRICSEXPORTER env vars
+- (Bugfix) Fix numactl options
  
 ## [1.2.33](https://github.com/arangodb/kube-arangodb/tree/1.2.33) (2023-09-27)
 - (Maintenance) Bump golang.org/x/net to v0.13.0
diff --git a/pkg/deployment/deployment_core_test.go b/pkg/deployment/deployment_core_test.go
index 2eb391d87..753dc3233 100644
--- a/pkg/deployment/deployment_core_test.go
+++ b/pkg/deployment/deployment_core_test.go
@@ -88,6 +88,190 @@ func TestEnsurePod_ArangoDB_Core(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name: "Agent Pod with numactl",
+			ArangoDeployment: &api.ArangoDeployment{
+				Spec: api.DeploymentSpec{
+					Image:           util.NewType[string](testImage),
+					Authentication:  noAuthentication,
+					TLS:             noTLS,
+					ImagePullPolicy: util.NewType[core.PullPolicy](core.PullAlways),
+
+					Agents: api.ServerGroupSpec{
+						Numactl: &api.ServerGroupSpecNumactl{
+							Enabled: util.NewType(true),
+						},
+					},
+				},
+			},
+			Helper: func(t *testing.T, deployment *Deployment, testCase *testCaseStruct) {
+				deployment.currentObjectStatus = &api.DeploymentStatus{
+					Members: api.DeploymentStatusMembers{
+						Agents: api.MemberStatusList{
+							firstAgentStatus,
+						},
+					},
+					Images: createTestImages(false),
+				}
+				testCase.createTestPodData(deployment, api.ServerGroupAgents, firstAgentStatus)
+			},
+			ExpectedEvent: "member agent is created",
+			ExpectedPod: core.Pod{
+				Spec: core.PodSpec{
+					Volumes: []core.Volume{
+						k8sutil.CreateVolumeEmptyDir(shared.ArangodVolumeName),
+					},
+					Containers: []core.Container{
+						{
+							Name:  shared.ServerContainerName,
+							Image: testImage,
+							Command: util.List[string]{api.ServerGroupSpecNumactlPathDefault}.
+								Append(createTestCommandForAgent(firstAgentStatus.ID, false, false, false)...).
+								List(),
+							Ports: createTestPorts(api.ServerGroupAgents),
+							VolumeMounts: []core.VolumeMount{
+								k8sutil.ArangodVolumeMount(),
+							},
+							Resources:       emptyResources,
+							LivenessProbe:   createTestLivenessProbe(httpProbe, false, "", shared.ServerPortName),
+							ImagePullPolicy: core.PullAlways,
+							SecurityContext: securityContext.NewSecurityContext(),
+						},
+					},
+					RestartPolicy:                 core.RestartPolicyNever,
+					TerminationGracePeriodSeconds: &defaultAgentTerminationTimeout,
+					Hostname:                      testDeploymentName + "-" + api.ServerGroupAgentsString + "-" + firstAgentStatus.ID,
+					Subdomain:                     testDeploymentName + "-int",
+					Affinity: k8sutil.CreateAffinity(testDeploymentName, api.ServerGroupAgentsString,
+						false, ""),
+				},
+			},
+		},
+		{
+			Name: "Agent Pod with numactl with opts and override",
+			ArangoDeployment: &api.ArangoDeployment{
+				Spec: api.DeploymentSpec{
+					Image:           util.NewType[string](testImage),
+					Authentication:  noAuthentication,
+					TLS:             noTLS,
+					ImagePullPolicy: util.NewType[core.PullPolicy](core.PullAlways),
+
+					Agents: api.ServerGroupSpec{
+						Numactl: &api.ServerGroupSpecNumactl{
+							Enabled: util.NewType(true),
+							Args: []string{
+								"--example=all",
+							},
+							Path: util.NewType("numactl-random-version"),
+						},
+					},
+				},
+			},
+			Helper: func(t *testing.T, deployment *Deployment, testCase *testCaseStruct) {
+				deployment.currentObjectStatus = &api.DeploymentStatus{
+					Members: api.DeploymentStatusMembers{
+						Agents: api.MemberStatusList{
+							firstAgentStatus,
+						},
+					},
+					Images: createTestImages(false),
+				}
+				testCase.createTestPodData(deployment, api.ServerGroupAgents, firstAgentStatus)
+			},
+			ExpectedEvent: "member agent is created",
+			ExpectedPod: core.Pod{
+				Spec: core.PodSpec{
+					Volumes: []core.Volume{
+						k8sutil.CreateVolumeEmptyDir(shared.ArangodVolumeName),
+					},
+					Containers: []core.Container{
+						{
+							Name:  shared.ServerContainerName,
+							Image: testImage,
+							Command: util.List[string]{"numactl-random-version", "--example=all"}.
+								Append(createTestCommandForAgent(firstAgentStatus.ID, false, false, false)...).
+								List(),
+							Ports: createTestPorts(api.ServerGroupAgents),
+							VolumeMounts: []core.VolumeMount{
+								k8sutil.ArangodVolumeMount(),
+							},
+							Resources:       emptyResources,
+							LivenessProbe:   createTestLivenessProbe(httpProbe, false, "", shared.ServerPortName),
+							ImagePullPolicy: core.PullAlways,
+							SecurityContext: securityContext.NewSecurityContext(),
+						},
+					},
+					RestartPolicy:                 core.RestartPolicyNever,
+					TerminationGracePeriodSeconds: &defaultAgentTerminationTimeout,
+					Hostname:                      testDeploymentName + "-" + api.ServerGroupAgentsString + "-" + firstAgentStatus.ID,
+					Subdomain:                     testDeploymentName + "-int",
+					Affinity: k8sutil.CreateAffinity(testDeploymentName, api.ServerGroupAgentsString,
+						false, ""),
+				},
+			},
+		},
+		{
+			Name: "Agent Pod with numactl with opts",
+			ArangoDeployment: &api.ArangoDeployment{
+				Spec: api.DeploymentSpec{
+					Image:           util.NewType[string](testImage),
+					Authentication:  noAuthentication,
+					TLS:             noTLS,
+					ImagePullPolicy: util.NewType[core.PullPolicy](core.PullAlways),
+
+					Agents: api.ServerGroupSpec{
+						Numactl: &api.ServerGroupSpecNumactl{
+							Enabled: util.NewType(true),
+							Args: []string{
+								"--example=all",
+							},
+						},
+					},
+				},
+			},
+			Helper: func(t *testing.T, deployment *Deployment, testCase *testCaseStruct) {
+				deployment.currentObjectStatus = &api.DeploymentStatus{
+					Members: api.DeploymentStatusMembers{
+						Agents: api.MemberStatusList{
+							firstAgentStatus,
+						},
+					},
+					Images: createTestImages(false),
+				}
+				testCase.createTestPodData(deployment, api.ServerGroupAgents, firstAgentStatus)
+			},
+			ExpectedEvent: "member agent is created",
+			ExpectedPod: core.Pod{
+				Spec: core.PodSpec{
+					Volumes: []core.Volume{
+						k8sutil.CreateVolumeEmptyDir(shared.ArangodVolumeName),
+					},
+					Containers: []core.Container{
+						{
+							Name:  shared.ServerContainerName,
+							Image: testImage,
+							Command: util.List[string]{api.ServerGroupSpecNumactlPathDefault, "--example=all"}.
+								Append(createTestCommandForAgent(firstAgentStatus.ID, false, false, false)...).
+								List(),
+							Ports: createTestPorts(api.ServerGroupAgents),
+							VolumeMounts: []core.VolumeMount{
+								k8sutil.ArangodVolumeMount(),
+							},
+							Resources:       emptyResources,
+							LivenessProbe:   createTestLivenessProbe(httpProbe, false, "", shared.ServerPortName),
+							ImagePullPolicy: core.PullAlways,
+							SecurityContext: securityContext.NewSecurityContext(),
+						},
+					},
+					RestartPolicy:                 core.RestartPolicyNever,
+					TerminationGracePeriodSeconds: &defaultAgentTerminationTimeout,
+					Hostname:                      testDeploymentName + "-" + api.ServerGroupAgentsString + "-" + firstAgentStatus.ID,
+					Subdomain:                     testDeploymentName + "-int",
+					Affinity: k8sutil.CreateAffinity(testDeploymentName, api.ServerGroupAgentsString,
+						false, ""),
+				},
+			},
+		},
 		{
 			Name: "Agent Pod with image pull policy",
 			ArangoDeployment: &api.ArangoDeployment{
diff --git a/pkg/deployment/images.go b/pkg/deployment/images.go
index 4e854bc81..e2c9eabe1 100644
--- a/pkg/deployment/images.go
+++ b/pkg/deployment/images.go
@@ -393,8 +393,8 @@ func (i *ImageUpdatePod) ApplyPodSpec(p *core.PodSpec) error {
 	return nil
 }
 
-func (a *ContainerIdentity) GetArgs() ([]string, error) {
-	return nil, nil
+func (a *ContainerIdentity) GetCommand() ([]string, error) {
+	return []string{a.GetExecutor()}, nil
 }
 
 // GetEnvs returns environment variables for identity containers.
@@ -449,8 +449,8 @@ func (a *ContainerIdentity) GetVolumeMounts() []core.VolumeMount {
 	return nil
 }
 
-// GetArgs returns the list of arguments for the ArangoD container identification.
-func (a *ArangoDIdentity) GetArgs() ([]string, error) {
+// GetCommand returns the command for the ArangoD container identification.
+func (a *ArangoDIdentity) GetCommand() ([]string, error) {
 	options := k8sutil.CreateOptionPairs(64)
 	options.Add("--server.authentication", "false")
 	options.Addf("--server.endpoint", "tcp://%s:%d", a.ipAddress, shared.ArangoPort)
@@ -460,7 +460,7 @@ func (a *ArangoDIdentity) GetArgs() ([]string, error) {
 	// Security
 	options.Merge(pod.Security().Args(a.input))
 
-	return options.Copy().Sort().AsArgs(), nil
+	return options.Copy().Sort().AsArgsWithCommand(a.GetExecutor()), nil
 }
 
 // GetEnvs returns environment variables for Arango identity containers.
diff --git a/pkg/deployment/resources/pod_creator_arangod.go b/pkg/deployment/resources/pod_creator_arangod.go
index 210ddc349..9dcb6c898 100644
--- a/pkg/deployment/resources/pod_creator_arangod.go
+++ b/pkg/deployment/resources/pod_creator_arangod.go
@@ -121,22 +121,23 @@ func (a *ArangoDContainer) GetPorts() []core.ContainerPort {
 	return ports
 }
 
-func (a *ArangoDContainer) GetArgs() ([]string, error) {
+func (a *ArangoDContainer) GetCommand() ([]string, error) {
+	cmd := make([]string, 0, 128)
+
+	if args := createArangodNumactl(a.groupSpec); len(args) > 0 {
+		cmd = append(cmd, args...)
+	}
+
+	cmd = append(cmd, a.GetExecutor())
+
 	args, err := createArangodArgs(a.cachedStatus, a.input)
 	if err != nil {
 		return nil, err
 	}
 
-	if nmargs := createArangodNumactl(a.groupSpec); len(nmargs) > 0 {
-		vs := make([]string, len(args)+len(nmargs))
-
-		copy(vs, nmargs)
-		copy(vs[len(nmargs):], args)
-
-		return vs, nil
-	}
+	cmd = append(cmd, args...)
 
-	return args, nil
+	return cmd, nil
 }
 
 func (a *ArangoDContainer) GetName() string {
diff --git a/pkg/deployment/resources/pod_creator_sync.go b/pkg/deployment/resources/pod_creator_sync.go
index 08f231db3..814c7de36 100644
--- a/pkg/deployment/resources/pod_creator_sync.go
+++ b/pkg/deployment/resources/pod_creator_sync.go
@@ -82,8 +82,11 @@ type MemberSyncPod struct {
 	cachedStatus           interfaces.Inspector
 }
 
-func (a *ArangoSyncContainer) GetArgs() ([]string, error) {
-	return createArangoSyncArgs(a.apiObject, a.spec, a.group, a.groupSpec, a.memberStatus), nil
+func (a *ArangoSyncContainer) GetCommand() ([]string, error) {
+	cmd := make([]string, 0, 128)
+	cmd = append(cmd, a.GetExecutor())
+	cmd = append(cmd, createArangoSyncArgs(a.apiObject, a.spec, a.group, a.groupSpec, a.memberStatus)...)
+	return cmd, nil
 }
 
 func (a *ArangoSyncContainer) GetName() string {
diff --git a/pkg/util/k8sutil/interfaces/pod_creator.go b/pkg/util/k8sutil/interfaces/pod_creator.go
index f0e866bd3..aa28aed45 100644
--- a/pkg/util/k8sutil/interfaces/pod_creator.go
+++ b/pkg/util/k8sutil/interfaces/pod_creator.go
@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2016-2022 ArangoDB GmbH, Cologne, Germany
+// Copyright 2016-2023 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -65,7 +65,7 @@ type PodCreator interface {
 }
 
 type ContainerCreator interface {
-	GetArgs() ([]string, error)
+	GetCommand() ([]string, error)
 	GetName() string
 	GetExecutor() string
 	GetProbes() (*core.Probe, *core.Probe, *core.Probe, error)
diff --git a/pkg/util/k8sutil/pair.go b/pkg/util/k8sutil/pair.go
index 949056fc3..576bd17fd 100644
--- a/pkg/util/k8sutil/pair.go
+++ b/pkg/util/k8sutil/pair.go
@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2016-2022 ArangoDB GmbH, Cologne, Germany
+// Copyright 2016-2023 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -126,6 +126,18 @@ func (o OptionPairs) Sort() OptionPairs {
 	return o
 }
 
+func (o OptionPairs) AsArgsWithCommand(command string) []string {
+	s := make([]string, len(o)+1)
+
+	s[0] = command
+
+	for id, pair := range o {
+		s[id+1] = pair.String()
+	}
+
+	return s
+}
+
 func (o OptionPairs) AsArgs() []string {
 	s := make([]string, len(o))
 
diff --git a/pkg/util/k8sutil/pods.go b/pkg/util/k8sutil/pods.go
index 4ef317544..2bb0507cf 100644
--- a/pkg/util/k8sutil/pods.go
+++ b/pkg/util/k8sutil/pods.go
@@ -545,7 +545,7 @@ func NewContainer(containerCreator interfaces.ContainerCreator) (core.Container,
 		return core.Container{}, err
 	}
 
-	args, err := containerCreator.GetArgs()
+	cmd, err := containerCreator.GetCommand()
 	if err != nil {
 		return core.Container{}, err
 	}
@@ -554,7 +554,7 @@ func NewContainer(containerCreator interfaces.ContainerCreator) (core.Container,
 	return core.Container{
 		Name:            containerCreator.GetName(),
 		Image:           containerCreator.GetImage(),
-		Command:         append([]string{containerCreator.GetExecutor()}, args...),
+		Command:         cmd,
 		Ports:           containerCreator.GetPorts(),
 		Env:             env,
 		EnvFrom:         envFrom,
diff --git a/pkg/util/list.go b/pkg/util/list.go
index 6fa207dd2..4d9319726 100644
--- a/pkg/util/list.go
+++ b/pkg/util/list.go
@@ -41,6 +41,14 @@ func (l List[T]) Count(fn func(T) bool) int {
 	return len(l.Filter(fn))
 }
 
+func (l List[T]) Append(items ...T) List[T] {
+	return append(l, items...)
+}
+
+func (l List[T]) List() []T {
+	return l
+}
+
 func (l List[T]) Sort(fn func(T, T) bool) List[T] {
 	clone := l
 	sort.Slice(clone, func(i, j int) bool {