Skip to content

Commit 506c50d

Browse files
committed
changed deployments map key to be va.Namespace/va.scaleTargetRef.name
1 parent d94ddb2 commit 506c50d

5 files changed

Lines changed: 26 additions & 20 deletions

File tree

internal/collector/replica_metrics.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func NewReplicaMetricsCollector(metricsSource source.MetricsSource, k8sClient cl
6666
// - ctx: Context for the operation
6767
// - modelID: The model identifier to collect metrics for
6868
// - namespace: The namespace where the model is deployed
69-
// - deployments: Map of VariantAutoscaling namespace/name to Deployment
69+
// - deployments: Map of Deployment namespace/name to Deployment
7070
// - variantAutoscalings: Map of VariantAutoscaling namespace/name to VariantAutoscaling object
7171
// - variantCosts: Map of VariantAutoscaling namespace/name to cost value
7272
//

internal/collector/source/pod_va_mapper.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func (m *PodVAMapper) findDeploymentForPod(
6868

6969
pod := &corev1.Pod{}
7070
if err := m.k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: podName}, pod); err != nil {
71-
logger.Error(err, "failed to get pod", "pod", podName, "namespace", namespace)
71+
logger.V(logging.DEBUG).Error(err, "failed to get pod", "pod", podName, "namespace", namespace)
7272
return ""
7373
}
7474

@@ -80,7 +80,7 @@ func (m *PodVAMapper) findDeploymentForPod(
8080

8181
rs := &appsv1.ReplicaSet{}
8282
if err := m.k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: owner.Name}, rs); err != nil {
83-
logger.Error(err, "failed to get ReplicaSet", "replicaset", owner.Name, "namespace", namespace)
83+
logger.V(logging.DEBUG).Error(err, "failed to get ReplicaSet", "replicaset", owner.Name, "namespace", namespace)
8484
return ""
8585
}
8686

@@ -91,10 +91,9 @@ func (m *PodVAMapper) findDeploymentForPod(
9191
}
9292

9393
// Verify the Deployment is in our map of tracked Deployments
94-
for _, deploy := range deployments {
95-
if deploy != nil && deploy.Name == rsOwner.Name && deploy.Namespace == namespace {
96-
return rsOwner.Name
97-
}
94+
deploymentKey := namespace + "/" + rsOwner.Name
95+
if deploy, ok := deployments[deploymentKey]; ok && deploy != nil && deploy.Namespace == namespace {
96+
return rsOwner.Name
9897
}
9998
return ""
10099
}

internal/collector/source/pod_va_mapper_test.go

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ var _ = Describe("PodVAMapper", func() {
116116
},
117117
},
118118
}
119-
deployments["default/llama-va"] = deployment
119+
deployments["default/llama-deploy"] = deployment
120120

121121
va := createVA("llama-va", "default", "llama-deploy")
122122
rs := createReplicaSet("llama-deploy-abc123", "default", "llama-deploy")
@@ -153,7 +153,7 @@ var _ = Describe("PodVAMapper", func() {
153153
},
154154
},
155155
}
156-
deployments["default/orphan-va"] = deployment
156+
deployments["default/orphan-deploy"] = deployment
157157

158158
rs := createReplicaSet("orphan-deploy-abc123", "default", "orphan-deploy")
159159
pod := createPod("orphan-deploy-abc123-xyz", "default", "orphan-deploy-abc123", map[string]string{"app": "orphan"})
@@ -180,7 +180,7 @@ var _ = Describe("PodVAMapper", func() {
180180
},
181181
},
182182
}
183-
deployments["default/llama-va"] = deployment
183+
deployments["default/llama-deploy"] = deployment
184184

185185
// VA in different namespace should not match
186186
va := createVA("llama-va", "production", "llama-deploy")
@@ -201,8 +201,7 @@ var _ = Describe("PodVAMapper", func() {
201201
// Setup multiple deployments
202202
var objects []client.Object
203203
for _, name := range []string{"deploy-a", "deploy-b", "deploy-c"} {
204-
vaName := "va-" + name[len("deploy-"):]
205-
deployments["default/"+vaName] = &appsv1.Deployment{
204+
deployments["default/"+name] = &appsv1.Deployment{
206205
ObjectMeta: metav1.ObjectMeta{
207206
Name: name,
208207
Namespace: "default",
@@ -247,7 +246,7 @@ var _ = Describe("PodVAMapper", func() {
247246
},
248247
},
249248
}
250-
deployments["default/cached-va"] = deployment
249+
deployments["default/cached-deploy"] = deployment
251250

252251
va := createVA("cached-va", "default", "cached-deploy")
253252
rs := createReplicaSet("cached-deploy-rs", "default", "cached-deploy")
@@ -281,7 +280,7 @@ var _ = Describe("PodVAMapper", func() {
281280
},
282281
},
283282
}
284-
deployments["default/removable-va"] = deployment
283+
deployments["default/removable-deploy"] = deployment
285284

286285
va := createVA("removable-va", "default", "removable-deploy")
287286
rs := createReplicaSet("removable-deploy-rs", "default", "removable-deploy")
@@ -297,7 +296,7 @@ var _ = Describe("PodVAMapper", func() {
297296
Expect(result1).To(Equal("removable-va"))
298297

299298
// Remove deployment from map
300-
delete(deployments, "default/removable-va")
299+
delete(deployments, "default/removable-deploy")
301300

302301
// Second lookup - should return empty since deployment is gone from tracked map
303302
result2 := mapper.FindVAForPod(ctx, "removable-deploy-pod-xyz", "default", deployments)
@@ -311,7 +310,7 @@ var _ = Describe("PodVAMapper", func() {
311310
Namespace: "default",
312311
},
313312
}
314-
deployments["default/standalone-va"] = deployment
313+
deployments["default/standalone-deploy"] = deployment
315314

316315
// Pod without owner references (standalone pod)
317316
pod := &corev1.Pod{
@@ -337,7 +336,7 @@ var _ = Describe("PodVAMapper", func() {
337336
Namespace: "namespace-a",
338337
},
339338
}
340-
deployments["namespace-a/va-a"] = deploymentA
339+
deployments["namespace-a/shared-deploy"] = deploymentA
341340

342341
// Deployment in namespace-b (same deployment name, different namespace)
343342
deploymentB := &appsv1.Deployment{
@@ -346,7 +345,7 @@ var _ = Describe("PodVAMapper", func() {
346345
Namespace: "namespace-b",
347346
},
348347
}
349-
deployments["namespace-b/va-b"] = deploymentB
348+
deployments["namespace-b/shared-deploy"] = deploymentB
350349

351350
// VA in namespace-a targeting shared-deploy
352351
vaA := createVA("va-a", "namespace-a", "shared-deploy")

internal/engines/saturation/engine.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ func (e *Engine) BuildVariantStates(
330330

331331
// Try to look up in provided map first (optimization)
332332
if deployments != nil {
333-
deploy, found = deployments[utils.GetVariantKey(va.Namespace, va.Name)]
333+
deploy, found = deployments[utils.GetDeploymentKey(va.Namespace, va.GetScaleTargetName())]
334334
}
335335

336336
if !found {
@@ -528,9 +528,12 @@ func (e *Engine) RunSaturationAnalysis(
528528
}
529529
}
530530

531+
// Populate maps indexed by Deployment namespace/name
532+
deploymentKey := utils.GetDeploymentKey(va.Namespace, va.GetScaleTargetName())
533+
deployments[deploymentKey] = &deploy
534+
531535
// Populate maps indexed by VariantAutoscaling namespace/name
532536
variantKey := utils.GetVariantKey(va.Namespace, va.Name)
533-
deployments[variantKey] = &deploy
534537
variantAutoscalings[variantKey] = va
535538
variantCosts[variantKey] = cost
536539
}

internal/utils/variant.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,8 @@ func GetDesiredReplicas(deploy *appsv1.Deployment) int32 {
216216
func GetVariantKey(namespace, name string) string {
217217
return namespace + "/" + name
218218
}
219+
220+
// GetDeploymentKey returns a unique key for a Deployment combining namespace and name.
221+
func GetDeploymentKey(namespace, name string) string {
222+
return namespace + "/" + name
223+
}

0 commit comments

Comments
 (0)