Divide assigned limits with replicas (#725)

* Divide assigned limits with replicas * Fix unit test * Add changelog * Update Resources interface Update Resources interface to accept replicas.
grafana · Jan 8, 2024 · 105a022 · 105a022
1 parent 505a7eb
commit 105a022
Show file tree

Hide file tree

Showing 11 changed files with 106 additions and 17 deletions.
diff --git a/.chloggen/divide_limit_with_replicas.yaml b/.chloggen/divide_limit_with_replicas.yaml
@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. operator, github action)
+component: operator
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Divide assigned limits with replicas
+
+# One or more tracking issues related to the change
+issues: [721]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
diff --git a/internal/manifests/compactor/compactor.go b/internal/manifests/compactor/compactor.go
@@ -108,7 +108,7 @@ func deployment(params manifestutils.Params) (*v1.Deployment, error) {
 									MountPath: manifestutils.TmpStoragePath,
 								},
 							},
-							Resources:       manifestutils.Resources(tempo, manifestutils.CompactorComponentName),
+							Resources:       manifestutils.Resources(tempo, manifestutils.CompactorComponentName, tempo.Spec.Template.Compactor.Replicas),
 							SecurityContext: manifestutils.TempoContainerSecurityContext(),
 						},
 					},

diff --git a/internal/manifests/compactor/compactor_test.go b/internal/manifests/compactor/compactor_test.go
@@ -154,12 +154,12 @@ func TestBuildCompactor(t *testing.T) {
 							},
 							Resources: corev1.ResourceRequirements{
 								Limits: corev1.ResourceList{
-									corev1.ResourceCPU:    *resource.NewMilliQuantity(160, resource.BinarySI),
-									corev1.ResourceMemory: *resource.NewQuantity(386547072, resource.BinarySI),
+									corev1.ResourceCPU:    *resource.NewMilliQuantity(80, resource.BinarySI),
+									corev1.ResourceMemory: *resource.NewQuantity(193273536, resource.BinarySI),
 								},
 								Requests: corev1.ResourceList{
-									corev1.ResourceCPU:    *resource.NewMilliQuantity(48, resource.BinarySI),
-									corev1.ResourceMemory: *resource.NewQuantity(115964128, resource.BinarySI),
+									corev1.ResourceCPU:    *resource.NewMilliQuantity(24, resource.BinarySI),
+									corev1.ResourceMemory: *resource.NewQuantity(57982064, resource.BinarySI),
 								},
 							},
 							SecurityContext: manifestutils.TempoContainerSecurityContext(),

diff --git a/internal/manifests/distributor/distributor.go b/internal/manifests/distributor/distributor.go
@@ -232,7 +232,7 @@ func deployment(params manifestutils.Params) *v1.Deployment {
 									MountPath: manifestutils.TmpStoragePath,
 								},
 							},
-							Resources:       manifestutils.Resources(tempo, manifestutils.DistributorComponentName),
+							Resources:       manifestutils.Resources(tempo, manifestutils.DistributorComponentName, tempo.Spec.Template.Distributor.Replicas),
 							SecurityContext: manifestutils.TempoContainerSecurityContext(),
 						},
 					},

diff --git a/internal/manifests/gateway/gateway.go b/internal/manifests/gateway/gateway.go
@@ -235,7 +235,7 @@ func deployment(params manifestutils.Params, rbacCfgHash string, tenantsCfgHash
 									SubPath:   manifestutils.GatewayTenantFileName,
 								},
 							},
-							Resources:       manifestutils.Resources(tempo, manifestutils.GatewayComponentName),
+							Resources:       manifestutils.Resources(tempo, manifestutils.GatewayComponentName, nil),
 							SecurityContext: manifestutils.TempoContainerSecurityContext(),
 						},
 					},

diff --git a/internal/manifests/ingester/ingester.go b/internal/manifests/ingester/ingester.go
@@ -124,7 +124,7 @@ func statefulSet(params manifestutils.Params) (*v1.StatefulSet, error) {
 								},
 							},
 							ReadinessProbe:  manifestutils.TempoReadinessProbe(params.CtrlConfig.Gates.HTTPEncryption),
-							Resources:       manifestutils.Resources(tempo, manifestutils.IngesterComponentName),
+							Resources:       manifestutils.Resources(tempo, manifestutils.IngesterComponentName, tempo.Spec.Template.Ingester.Replicas),
 							SecurityContext: manifestutils.TempoContainerSecurityContext(),
 						},
 					},

diff --git a/internal/manifests/manifestutils/resources.go b/internal/manifests/manifestutils/resources.go
@@ -33,7 +33,7 @@ var (
 )
 
 // Resources calculates the resource requirements of a specific component.
-func Resources(tempo v1alpha1.TempoStack, component string) corev1.ResourceRequirements {
+func Resources(tempo v1alpha1.TempoStack, component string, replicas *int32) corev1.ResourceRequirements {
 
 	resourcesMap := resourcesMapNoGateway
 	if tempo.Spec.Template.Gateway.Enabled {
@@ -49,6 +49,9 @@ func Resources(tempo v1alpha1.TempoStack, component string) corev1.ResourceRequi
 	if ok {
 		totalCpuInt := totalCpu.MilliValue()
 		cpu := float32(totalCpuInt) * componentResources.cpu
+		if replicas != nil && *replicas > 1 {
+			cpu /= float32(*replicas)
+		}
 
 		resources.Limits = corev1.ResourceList{
 			corev1.ResourceCPU: *resource.NewMilliQuantity(int64(cpu), resource.BinarySI),
@@ -68,6 +71,9 @@ func Resources(tempo v1alpha1.TempoStack, component string) corev1.ResourceRequi
 		}
 		totalMemoryInt := totalMemory.Value()
 		mem := float32(totalMemoryInt) * componentResources.memory
+		if replicas != nil && *replicas > 1 {
+			mem /= float32(*replicas)
+		}
 		resources.Limits[corev1.ResourceMemory] = *resource.NewQuantity(int64(mem), resource.BinarySI)
 		resources.Requests[corev1.ResourceMemory] = *resource.NewQuantity(int64(mem*0.3), resource.BinarySI)
 	}

diff --git a/internal/manifests/manifestutils/resources_test.go b/internal/manifests/manifestutils/resources_test.go
@@ -6,6 +6,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
+	"k8s.io/utils/ptr"
 
 	"github.com/grafana/tempo-operator/apis/tempo/v1alpha1"
 )
@@ -37,6 +38,7 @@ func TestResources(t *testing.T) {
 		resources corev1.ResourceRequirements
 		name      string
 		tempo     v1alpha1.TempoStack
+		replicas  *int32
 	}{
 		{
 			name: "resources not set",
@@ -70,6 +72,37 @@ func TestResources(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "cpu, memory resources set with replicas",
+			tempo: v1alpha1.TempoStack{
+				Spec: v1alpha1.TempoStackSpec{
+					Resources: v1alpha1.Resources{
+						Total: &corev1.ResourceRequirements{
+							Limits: map[corev1.ResourceName]resource.Quantity{
+								corev1.ResourceMemory: resource.MustParse("2Gi"),
+								corev1.ResourceCPU:    resource.MustParse("1000m"),
+							},
+						},
+					},
+					Template: v1alpha1.TempoTemplateSpec{
+						Compactor: v1alpha1.TempoComponentSpec{
+							Replicas: ptr.To(int32(2)),
+						},
+					},
+				},
+			},
+			resources: corev1.ResourceRequirements{
+				Limits: corev1.ResourceList{
+					corev1.ResourceCPU:    *resource.NewMilliQuantity(135, resource.BinarySI),
+					corev1.ResourceMemory: *resource.NewQuantity(128849016, resource.BinarySI),
+				},
+				Requests: corev1.ResourceList{
+					corev1.ResourceCPU:    *resource.NewMilliQuantity(40, resource.BinarySI),
+					corev1.ResourceMemory: *resource.NewQuantity(38654708, resource.BinarySI),
+				},
+			},
+			replicas: ptr.To(int32(2)),
+		},
 		{
 			name: "cpu, memory resources set and gateway enable",
 			tempo: v1alpha1.TempoStack{
@@ -100,6 +133,40 @@ func TestResources(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "cpu, memory resources set with replicas and gateway enable",
+			tempo: v1alpha1.TempoStack{
+				Spec: v1alpha1.TempoStackSpec{
+					Template: v1alpha1.TempoTemplateSpec{
+						Gateway: v1alpha1.TempoGatewaySpec{
+							Enabled: true,
+						},
+						Compactor: v1alpha1.TempoComponentSpec{
+							Replicas: ptr.To(int32(2)),
+						},
+					},
+					Resources: v1alpha1.Resources{
+						Total: &corev1.ResourceRequirements{
+							Limits: map[corev1.ResourceName]resource.Quantity{
+								corev1.ResourceMemory: resource.MustParse("2Gi"),
+								corev1.ResourceCPU:    resource.MustParse("1000m"),
+							},
+						},
+					},
+				},
+			},
+			resources: corev1.ResourceRequirements{
+				Limits: corev1.ResourceList{
+					corev1.ResourceCPU:    *resource.NewMilliQuantity(130, resource.BinarySI),
+					corev1.ResourceMemory: *resource.NewQuantity(118111600, resource.BinarySI),
+				},
+				Requests: corev1.ResourceList{
+					corev1.ResourceCPU:    *resource.NewMilliQuantity(39, resource.BinarySI),
+					corev1.ResourceMemory: *resource.NewQuantity(35433480, resource.BinarySI),
+				},
+			},
+			replicas: ptr.To(int32(2)),
+		},
 		{
 			name: "missing cpu resources",
 			tempo: v1alpha1.TempoStack{
@@ -150,7 +217,7 @@ func TestResources(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			resources := Resources(test.tempo, "distributor")
+			resources := Resources(test.tempo, "distributor", test.replicas)
 			assert.Equal(t, test.resources, resources)
 		})
 	}

diff --git a/internal/manifests/querier/querier.go b/internal/manifests/querier/querier.go
@@ -112,7 +112,7 @@ func deployment(params manifestutils.Params) (*v1.Deployment, error) {
 									MountPath: manifestutils.TmpStoragePath,
 								},
 							},
-							Resources:       manifestutils.Resources(tempo, manifestutils.QuerierComponentName),
+							Resources:       manifestutils.Resources(tempo, manifestutils.QuerierComponentName, tempo.Spec.Template.Querier.Replicas),
 							SecurityContext: manifestutils.TempoContainerSecurityContext(),
 						},
 					},

diff --git a/internal/manifests/querier/querier_test.go b/internal/manifests/querier/querier_test.go
@@ -161,12 +161,12 @@ func TestBuildQuerier(t *testing.T) {
 							},
 							Resources: corev1.ResourceRequirements{
 								Limits: corev1.ResourceList{
-									corev1.ResourceCPU:    *resource.NewMilliQuantity(100, resource.BinarySI),
-									corev1.ResourceMemory: *resource.NewQuantity(322122560, resource.BinarySI),
+									corev1.ResourceCPU:    *resource.NewMilliQuantity(33, resource.BinarySI),
+									corev1.ResourceMemory: *resource.NewQuantity(107374184, resource.BinarySI),
 								},
 								Requests: corev1.ResourceList{
-									corev1.ResourceCPU:    *resource.NewMilliQuantity(30, resource.BinarySI),
-									corev1.ResourceMemory: *resource.NewQuantity(96636768, resource.BinarySI),
+									corev1.ResourceCPU:    *resource.NewMilliQuantity(10, resource.BinarySI),
+									corev1.ResourceMemory: *resource.NewQuantity(32212256, resource.BinarySI),
 								},
 							},
 							SecurityContext: manifestutils.TempoContainerSecurityContext(),

diff --git a/internal/manifests/queryfrontend/query_frontend.go b/internal/manifests/queryfrontend/query_frontend.go
@@ -166,7 +166,7 @@ func deployment(params manifestutils.Params) (*appsv1.Deployment, error) {
 									MountPath: manifestutils.TmpStoragePath,
 								},
 							},
-							Resources:       manifestutils.Resources(tempo, manifestutils.QueryFrontendComponentName),
+							Resources:       manifestutils.Resources(tempo, manifestutils.QueryFrontendComponentName, tempo.Spec.Template.QueryFrontend.Replicas),
 							SecurityContext: manifestutils.TempoContainerSecurityContext(),
 						},
 					},
@@ -231,7 +231,7 @@ func deployment(params manifestutils.Params) (*appsv1.Deployment, error) {
 					MountPath: manifestutils.TmpStoragePath,
 				},
 			},
-			Resources: manifestutils.Resources(tempo, manifestutils.QueryFrontendComponentName),
+			Resources: manifestutils.Resources(tempo, manifestutils.QueryFrontendComponentName, tempo.Spec.Template.QueryFrontend.Replicas),
 		}
 		jaegerQueryVolume := corev1.Volume{
 			Name: manifestutils.TmpStorageVolumeName + "-query",