[Bugfix] Fix resource propagation to InitContainers (#1418)

ajanikow · web-flow · commit 50a672c4e9aa · 2023-09-25T23:13:18.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,7 @@
 - (Feature) Add --deployment.feature.init-containers-copy-resources (default enabled)
 - (Feature) Add maxBackups option to ArangoBackupPolicy
 - (Improvement) Better detection for AllInSync condition for DC2DC sync status
+- (Bugfix) Fix resource propagation to InitContainers
 
 ## [1.2.32](https://github.com/arangodb/kube-arangodb/tree/1.2.32) (2023-08-07)
 - (Feature) Backup lifetime - remove Backup once its lifetime has been reached
diff --git a/pkg/deployment/deployment_core_test.go b/pkg/deployment/deployment_core_test.go
@@ -358,7 +358,7 @@ func TestEnsurePod_ArangoDB_Core(t *testing.T) {
 							Image:     testImage,
 							Command:   createTestCommandForDBServer(firstDBServerStatus.ID, false, false, false),
 							Ports:     createTestPorts(api.ServerGroupDBServers),
-							Resources: k8sutil.ExtractPodResourceRequirement(resourcesUnfiltered),
+							Resources: k8sutil.ExtractPodAcceptedResourceRequirement(resourcesUnfiltered),
 							VolumeMounts: []core.VolumeMount{
 								k8sutil.ArangodVolumeMount(),
 							},
@@ -415,7 +415,7 @@ func TestEnsurePod_ArangoDB_Core(t *testing.T) {
 							Image:     testImage,
 							Command:   createTestCommandForDBServer(firstDBServerStatus.ID, false, false, false),
 							Ports:     createTestPorts(api.ServerGroupDBServers),
-							Resources: k8sutil.ExtractPodResourceRequirement(resourcesUnfiltered),
+							Resources: k8sutil.ExtractPodAcceptedResourceRequirement(resourcesUnfiltered),
 							VolumeMounts: []core.VolumeMount{
 								k8sutil.ArangodVolumeMount(),
 							},
@@ -475,7 +475,7 @@ func TestEnsurePod_ArangoDB_Core(t *testing.T) {
 							Image:     testImage,
 							Command:   createTestCommandForDBServer(firstDBServerStatus.ID, false, false, false),
 							Ports:     createTestPorts(api.ServerGroupDBServers),
-							Resources: k8sutil.ExtractPodResourceRequirement(resourcesUnfiltered),
+							Resources: k8sutil.ExtractPodAcceptedResourceRequirement(resourcesUnfiltered),
 							VolumeMounts: []core.VolumeMount{
 								k8sutil.ArangodVolumeMount(),
 							},
@@ -1009,7 +1009,7 @@ func TestEnsurePod_ArangoDB_Core(t *testing.T) {
 							ImagePullPolicy: core.PullIfNotPresent,
 							SecurityContext: securityContext.NewSecurityContext(),
 						},
-						testArangodbInternalExporterContainer(false, true, k8sutil.ExtractPodResourceRequirement(resourcesUnfiltered)),
+						testArangodbInternalExporterContainer(false, true, k8sutil.ExtractPodAcceptedResourceRequirement(resourcesUnfiltered)),
 					},
 					RestartPolicy:                 core.RestartPolicyNever,
 					TerminationGracePeriodSeconds: &defaultDBServerTerminationTimeout,
@@ -1064,7 +1064,7 @@ func TestEnsurePod_ArangoDB_Core(t *testing.T) {
 						k8sutil.LifecycleVolume(),
 					},
 					InitContainers: []core.Container{
-						createTestLifecycleContainer(k8sutil.ExtractPodResourceRequirement(resourcesUnfiltered)),
+						createTestLifecycleContainer(k8sutil.ExtractPodAcceptedResourceRequirement(resourcesUnfiltered)),
 					},
 					Containers: []core.Container{
 						{
diff --git a/pkg/deployment/deployment_pod_resources_test.go b/pkg/deployment/deployment_pod_resources_test.go
@@ -134,7 +134,7 @@ func TestEnsurePod_ArangoDB_Resources(t *testing.T) {
 							Image:     testImage,
 							Command:   createTestCommandForDBServer(firstDBServerStatus.ID, false, false, false),
 							Ports:     createTestPorts(api.ServerGroupAgents),
-							Resources: k8sutil.ExtractPodResourceRequirement(resourcesUnfiltered),
+							Resources: k8sutil.ExtractPodAcceptedResourceRequirement(resourcesUnfiltered),
 							VolumeMounts: []core.VolumeMount{
 								k8sutil.ArangodVolumeMount(),
 							},
@@ -194,7 +194,7 @@ func TestEnsurePod_ArangoDB_Resources(t *testing.T) {
 							Image:     testImage,
 							Command:   createTestCommandForDBServer(firstDBServerStatus.ID, false, false, false),
 							Ports:     createTestPorts(api.ServerGroupAgents),
-							Resources: k8sutil.ExtractPodResourceRequirement(resourcesUnfiltered),
+							Resources: k8sutil.ExtractPodAcceptedResourceRequirement(resourcesUnfiltered),
 							VolumeMounts: []core.VolumeMount{
 								k8sutil.ArangodVolumeMount(),
 							},
@@ -252,7 +252,7 @@ func TestEnsurePod_ArangoDB_Resources(t *testing.T) {
 							Image:     testImage,
 							Command:   createTestCommandForDBServer(firstDBServerStatus.ID, false, false, false),
 							Ports:     createTestPorts(api.ServerGroupAgents),
-							Resources: k8sutil.ExtractPodResourceRequirement(resourcesUnfiltered),
+							Resources: k8sutil.ExtractPodAcceptedResourceRequirement(resourcesUnfiltered),
 							VolumeMounts: []core.VolumeMount{
 								k8sutil.ArangodVolumeMount(),
 							},
diff --git a/pkg/deployment/deployment_pod_sync_test.go b/pkg/deployment/deployment_pod_sync_test.go
@@ -1304,7 +1304,7 @@ func TestEnsurePod_Sync_Worker(t *testing.T) {
 							},
 							ImagePullPolicy: core.PullIfNotPresent,
 							Lifecycle:       createTestLifecycle(api.ServerGroupSyncMasters),
-							Resources:       k8sutil.ExtractPodResourceRequirement(resourcesUnfiltered),
+							Resources:       k8sutil.ExtractPodAcceptedResourceRequirement(resourcesUnfiltered),
 							SecurityContext: securityContext.NewSecurityContext(),
 							VolumeMounts: []core.VolumeMount{
 								k8sutil.LifecycleVolumeMount(),
@@ -1399,7 +1399,7 @@ func TestEnsurePod_Sync_Worker(t *testing.T) {
 							},
 							ImagePullPolicy: core.PullIfNotPresent,
 							Lifecycle:       createTestLifecycle(api.ServerGroupSyncMasters),
-							Resources:       k8sutil.ExtractPodResourceRequirement(resourcesUnfiltered),
+							Resources:       k8sutil.ExtractPodAcceptedResourceRequirement(resourcesUnfiltered),
 							SecurityContext: securityContext.NewSecurityContext(),
 							VolumeMounts: []core.VolumeMount{
 								k8sutil.LifecycleVolumeMount(),
diff --git a/pkg/deployment/resources/internal_exporter.go b/pkg/deployment/resources/internal_exporter.go
@@ -54,7 +54,7 @@ func ArangodbInternalExporterContainer(image string, args []string, livenessProb
 				Protocol:      core.ProtocolTCP,
 			},
 		},
-		Resources:       k8sutil.ExtractPodResourceRequirement(resources),
+		Resources:       k8sutil.ExtractPodAcceptedResourceRequirement(resources),
 		SecurityContext: k8sutil.CreateSecurityContext(groupSpec.SecurityContext),
 		ImagePullPolicy: core.PullIfNotPresent,
 		VolumeMounts:    []core.VolumeMount{k8sutil.LifecycleVolumeMount()},
diff --git a/pkg/deployment/resources/pod_creator_arangod.go b/pkg/deployment/resources/pod_creator_arangod.go
@@ -281,7 +281,7 @@ func (a *ArangoDContainer) GetEnvs() ([]core.EnvVar, []core.EnvFromSource) {
 }
 
 func (a *ArangoDContainer) GetResourceRequirements() core.ResourceRequirements {
-	return k8sutil.ExtractPodResourceRequirement(a.arangoMember.Spec.Overrides.GetResources(&a.groupSpec))
+	return k8sutil.ExtractPodAcceptedResourceRequirement(a.arangoMember.Spec.Overrides.GetResources(&a.groupSpec))
 }
 
 func (a *ArangoDContainer) GetLifecycle() (*core.Lifecycle, error) {
@@ -513,7 +513,7 @@ func (m *MemberArangoDPod) GetInitContainers(cachedStatus interfaces.Inspector)
 		}
 	}
 
-	return applyInitContainersResourceResources(initContainers, &m.groupSpec.Resources), nil
+	return applyInitContainersResourceResources(initContainers, k8sutil.ExtractPodInitContainerAcceptedResourceRequirement(m.GetContainerCreator().GetResourceRequirements())), nil
 }
 
 func (m *MemberArangoDPod) GetFinalizers() []string {
diff --git a/pkg/deployment/resources/pod_creator_sync.go b/pkg/deployment/resources/pod_creator_sync.go
@@ -148,7 +148,7 @@ func (a *ArangoSyncContainer) GetProbes() (*core.Probe, *core.Probe, *core.Probe
 }
 
 func (a *ArangoSyncContainer) GetResourceRequirements() core.ResourceRequirements {
-	return k8sutil.ExtractPodResourceRequirement(a.arangoMember.Spec.Overrides.GetResources(&a.groupSpec))
+	return k8sutil.ExtractPodAcceptedResourceRequirement(a.arangoMember.Spec.Overrides.GetResources(&a.groupSpec))
 }
 
 func (a *ArangoSyncContainer) GetLifecycle() (*core.Lifecycle, error) {
@@ -300,7 +300,7 @@ func (m *MemberSyncPod) GetInitContainers(cachedStatus interfaces.Inspector) ([]
 		initContainers = append(initContainers, c)
 	}
 
-	return applyInitContainersResourceResources(initContainers, &m.groupSpec.Resources), nil
+	return applyInitContainersResourceResources(initContainers, k8sutil.ExtractPodInitContainerAcceptedResourceRequirement(m.GetContainerCreator().GetResourceRequirements())), nil
 }
 
 func (m *MemberSyncPod) GetFinalizers() []string {
diff --git a/pkg/deployment/resources/pod_init_containers.go b/pkg/deployment/resources/pod_init_containers.go
@@ -30,18 +30,17 @@ import (
 
 // applyInitContainersResourceResources updates passed init containers to ensure that all resources are set (if such feature is enabled)
 // This is applied only to containers added by operator itself
-func applyInitContainersResourceResources(initContainers []core.Container, mainContainerResources *core.ResourceRequirements) []core.Container {
-	if !features.InitContainerCopyResources().Enabled() || mainContainerResources == nil {
+func applyInitContainersResourceResources(initContainers []core.Container, mainContainerResources core.ResourceRequirements) []core.Container {
+	if !features.InitContainerCopyResources().Enabled() {
 		return initContainers
 	}
 
-	for i, c := range initContainers {
-		if !api.IsReservedServerGroupInitContainerName(c.Name) {
+	for i := range initContainers {
+		if !api.IsReservedServerGroupInitContainerName(initContainers[i].Name) {
 			continue
 		}
 
-		k8sutil.EnsureAllResourcesNotEmpty(mainContainerResources.Limits, &initContainers[i].Resources.Limits)
-		k8sutil.EnsureAllResourcesNotEmpty(mainContainerResources.Requests, &initContainers[i].Resources.Requests)
+		k8sutil.ApplyContainerResourceRequirements(&initContainers[i], mainContainerResources)
 	}
 	return initContainers
 }
diff --git a/pkg/util/k8sutil/lifecycle.go b/pkg/util/k8sutil/lifecycle.go
@@ -74,7 +74,7 @@ func InitLifecycleContainer(image string, resources *core.ResourceRequirements,
 	}
 
 	if resources != nil {
-		c.Resources = ExtractPodResourceRequirement(*resources)
+		c.Resources = ExtractPodAcceptedResourceRequirement(*resources)
 	}
 	return c, nil
 }
diff --git a/pkg/util/k8sutil/pods.go b/pkg/util/k8sutil/pods.go
@@ -532,38 +532,6 @@ func operatorInitContainer(name, operatorImage string, command []string, securit
 	return c
 }
 
-// ExtractPodResourceRequirement filters resource requirements for Pods.
-func ExtractPodResourceRequirement(resources core.ResourceRequirements) core.ResourceRequirements {
-	filter := PodResourceRequirementsFilter(core.ResourceCPU, core.ResourceMemory, core.ResourceEphemeralStorage)
-
-	return core.ResourceRequirements{
-		Limits:   filter(resources.Limits),
-		Requests: filter(resources.Requests),
-	}
-}
-
-func PodResourceRequirementsFilter(filters ...core.ResourceName) func(in core.ResourceList) core.ResourceList {
-	return func(in core.ResourceList) core.ResourceList {
-		filtered := map[core.ResourceName]bool{}
-
-		for _, k := range filters {
-			filtered[k] = true
-		}
-
-		n := core.ResourceList{}
-
-		for k, v := range in {
-			if _, ok := filtered[k]; !ok {
-				continue
-			}
-
-			n[k] = v
-		}
-
-		return n
-	}
-}
-
 // NewContainer creates a container for specified creator
 func NewContainer(containerCreator interfaces.ContainerCreator) (core.Container, error) {
 
diff --git a/pkg/util/k8sutil/pods_test.go b/pkg/util/k8sutil/pods_test.go
@@ -22,17 +22,14 @@ package k8sutil
 
 import (
 	"context"
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	core "k8s.io/api/core/v1"
-	resource "k8s.io/apimachinery/pkg/api/resource"
 	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/arangodb/kube-arangodb/pkg/handlers/utils"
-	"github.com/arangodb/kube-arangodb/pkg/util"
 	"github.com/arangodb/kube-arangodb/pkg/util/constants"
 	"github.com/arangodb/kube-arangodb/pkg/util/kclient"
 )
@@ -419,61 +416,3 @@ func Test_EnsureFinalizer(t *testing.T) {
 		require.NotContains(t, pod.Finalizers, f)
 	})
 }
-
-func Test_ExtractPodResourceRequirement(t *testing.T) {
-	v, err := resource.ParseQuantity("1Gi")
-	require.NoError(t, err)
-
-	t.Run("Filter Storage", func(t *testing.T) {
-		in := core.ResourceRequirements{
-			Limits: map[core.ResourceName]resource.Quantity{
-				core.ResourceCPU:     v,
-				core.ResourceStorage: v,
-			},
-		}
-		require.Len(t, in.Limits, 2)
-		require.Len(t, in.Requests, 0)
-
-		out := ExtractPodResourceRequirement(in)
-		require.Len(t, out.Limits, 1)
-		require.Contains(t, out.Limits, core.ResourceCPU)
-		require.NotContains(t, out.Limits, core.ResourceStorage)
-		require.Len(t, out.Requests, 0)
-	})
-
-	t.Run("Ensure that all required Resources are filtered", func(t *testing.T) {
-		resources := map[core.ResourceName]bool{
-			core.ResourceCPU:              true,
-			core.ResourceMemory:           true,
-			core.ResourceStorage:          false,
-			core.ResourceEphemeralStorage: true,
-		}
-
-		in := core.ResourceRequirements{
-			Limits:   core.ResourceList{},
-			Requests: core.ResourceList{},
-		}
-
-		for k := range resources {
-			in.Limits[k] = v
-			in.Requests[k] = v
-		}
-
-		out := ExtractPodResourceRequirement(in)
-
-		for k, v := range resources {
-			t.Run(fmt.Sprintf("Resource %s should be %s", k, util.BoolSwitch(v, "present", "missing")), func(t *testing.T) {
-				require.Contains(t, in.Requests, k)
-				require.Contains(t, in.Limits, k)
-
-				if v {
-					require.Contains(t, out.Requests, k)
-					require.Contains(t, out.Limits, k)
-				} else {
-					require.NotContains(t, out.Requests, k)
-					require.NotContains(t, out.Limits, k)
-				}
-			})
-		}
-	})
-}
diff --git a/pkg/util/k8sutil/pvc.go b/pkg/util/k8sutil/pvc.go
@@ -60,17 +60,7 @@ func IsPersistentVolumeClaimResizing(pvc *core.PersistentVolumeClaim) bool {
 
 // ExtractStorageResourceRequirement filters resource requirements for Pods.
 func ExtractStorageResourceRequirement(resources core.ResourceRequirements) core.ResourceRequirements {
-
-	filterStorage := func(list core.ResourceList) core.ResourceList {
-		newlist := make(core.ResourceList)
-		for k, v := range list {
-			if k != core.ResourceStorage && k != "iops" {
-				continue
-			}
-			newlist[k] = v
-		}
-		return newlist
-	}
+	filterStorage := NewPodResourceListFilter(core.ResourceStorage, "iops")
 
 	return core.ResourceRequirements{
 		Limits:   filterStorage(resources.Limits),
diff --git a/pkg/util/k8sutil/resources.go b/pkg/util/k8sutil/resources.go
diff --git a/pkg/util/k8sutil/resources_test.go b/pkg/util/k8sutil/resources_test.go

Original file line number	Diff line number	Diff line change
`@@ -281,7 +281,7 @@ func (a *ArangoDContainer) GetEnvs() ([]core.EnvVar, []core.EnvFromSource) {`
`281`	`281`	`}`
`282`	`282`
`283`	`283`	`func (a *ArangoDContainer) GetResourceRequirements() core.ResourceRequirements {`
`284`		`- return k8sutil.ExtractPodResourceRequirement(a.arangoMember.Spec.Overrides.GetResources(&a.groupSpec))`
	`284`	`+ return k8sutil.ExtractPodAcceptedResourceRequirement(a.arangoMember.Spec.Overrides.GetResources(&a.groupSpec))`
`285`	`285`	`}`
`286`	`286`
`287`	`287`	`func (a ArangoDContainer) GetLifecycle() (core.Lifecycle, error) {`
`@@ -513,7 +513,7 @@ func (m *MemberArangoDPod) GetInitContainers(cachedStatus interfaces.Inspector)`
`513`	`513`	`}`
`514`	`514`	`}`
`515`	`515`
`516`		`- return applyInitContainersResourceResources(initContainers, &m.groupSpec.Resources), nil`
	`516`	`+ return applyInitContainersResourceResources(initContainers, k8sutil.ExtractPodInitContainerAcceptedResourceRequirement(m.GetContainerCreator().GetResourceRequirements())), nil`
`517`	`517`	`}`
`518`	`518`
`519`	`519`	`func (m *MemberArangoDPod) GetFinalizers() []string {`
Original file line number	Diff line number	Diff line change
`@@ -148,7 +148,7 @@ func (a ArangoSyncContainer) GetProbes() (core.Probe, core.Probe, core.Probe`
`148`	`148`	`}`
`149`	`149`
`150`	`150`	`func (a *ArangoSyncContainer) GetResourceRequirements() core.ResourceRequirements {`
`151`		`- return k8sutil.ExtractPodResourceRequirement(a.arangoMember.Spec.Overrides.GetResources(&a.groupSpec))`
	`151`	`+ return k8sutil.ExtractPodAcceptedResourceRequirement(a.arangoMember.Spec.Overrides.GetResources(&a.groupSpec))`
`152`	`152`	`}`
`153`	`153`
`154`	`154`	`func (a ArangoSyncContainer) GetLifecycle() (core.Lifecycle, error) {`
`@@ -300,7 +300,7 @@ func (m *MemberSyncPod) GetInitContainers(cachedStatus interfaces.Inspector) ([]`
`300`	`300`	`initContainers = append(initContainers, c)`
`301`	`301`	`}`
`302`	`302`
`303`		`- return applyInitContainersResourceResources(initContainers, &m.groupSpec.Resources), nil`
	`303`	`+ return applyInitContainersResourceResources(initContainers, k8sutil.ExtractPodInitContainerAcceptedResourceRequirement(m.GetContainerCreator().GetResourceRequirements())), nil`
`304`	`304`	`}`
`305`	`305`
`306`	`306`	`func (m *MemberSyncPod) GetFinalizers() []string {`
Original file line number	Diff line number	Diff line change
`@@ -30,18 +30,17 @@ import (`
`30`	`30`
`31`	`31`	`// applyInitContainersResourceResources updates passed init containers to ensure that all resources are set (if such feature is enabled)`
`32`	`32`	`// This is applied only to containers added by operator itself`
`33`		`-func applyInitContainersResourceResources(initContainers []core.Container, mainContainerResources *core.ResourceRequirements) []core.Container {`
`34`		`- if !features.InitContainerCopyResources().Enabled() \|\| mainContainerResources == nil {`
	`33`	`+func applyInitContainersResourceResources(initContainers []core.Container, mainContainerResources core.ResourceRequirements) []core.Container {`
	`34`	`+ if !features.InitContainerCopyResources().Enabled() {`
`35`	`35`	`return initContainers`
`36`	`36`	`}`
`37`	`37`
`38`		`- for i, c := range initContainers {`
`39`		`- if !api.IsReservedServerGroupInitContainerName(c.Name) {`
	`38`	`+ for i := range initContainers {`
	`39`	`+ if !api.IsReservedServerGroupInitContainerName(initContainers[i].Name) {`
`40`	`40`	`continue`
`41`	`41`	`}`
`42`	`42`
`43`		`- k8sutil.EnsureAllResourcesNotEmpty(mainContainerResources.Limits, &initContainers[i].Resources.Limits)`
`44`		`- k8sutil.EnsureAllResourcesNotEmpty(mainContainerResources.Requests, &initContainers[i].Resources.Requests)`
	`43`	`+ k8sutil.ApplyContainerResourceRequirements(&initContainers[i], mainContainerResources)`
`45`	`44`	`}`
`46`	`45`	`return initContainers`
`47`	`46`	`}`
Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ func InitLifecycleContainer(image string, resources *core.ResourceRequirements,`
`74`	`74`	`}`
`75`	`75`
`76`	`76`	`if resources != nil {`
`77`		`- c.Resources = ExtractPodResourceRequirement(*resources)`
	`77`	`+ c.Resources = ExtractPodAcceptedResourceRequirement(*resources)`
`78`	`78`	`}`
`79`	`79`	`return c, nil`
`80`	`80`	`}`