Repository: koordinator-sh/koordinator

Issue:
[BUG] After deleting an ElasticQuota, its associated metrics can still be queried.
<!-- Please use this template while reporting a bug and provide as much info as possible. Not doing so may result in your bug not being addressed in a timely manner. Thanks!-->

**What happened**:

After deleting an ElasticQuota, its associated metrics can still be queried.

​​Reproduction Steps:​​
1. Fetch metrics for a specific ElasticQuota.
2. Delete the ElasticQuota CR.
3. Query the same ElasticQuota metrics again — they still persist.

**What you expected to happen**:

After deleting an ElasticQuota, its associated metrics should no longer be queryable. The metrics collection logic fails to listen for ElasticQuota deletion events, preventing the removal of associated metrics.

**How to reproduce it (as minimally and precisely as possible)**:

**Anything else we need to know?**:

**Environment**:
- App version: v1.16
- Kubernetes version (use `kubectl version`): 1.28
- Install details (e.g. helm install args): helm
- Node environment (for koordlet/runtime-proxy issue):
  - Containerd/Docker version:
  - OS version:
  - Kernal version:
  - Cgroup driver: cgroupfs/systemd
- Others:



Discussion:
@ZiMengSheng: Yep, maybe we need to manually delete the metric in the QuotaDelete event, like "ElasticQuotaSpecMetric.DeleteLabelValues(nodeName)"@qinfustu: @ZiMengSheng I'm attempting to fix this issue.@ZiMengSheng: /assign qinfustu

Solution patch:
diff --git a/pkg/scheduler/plugins/elasticquota/controller.go b/pkg/scheduler/plugins/elasticquota/controller.go
index 855c6626..31b8e8cf 100644
--- a/pkg/scheduler/plugins/elasticquota/controller.go
+++ b/pkg/scheduler/plugins/elasticquota/controller.go
@@ -270,6 +270,24 @@ func (ctrl *Controller) syncElasticQuotaStatusMetricsWorker() {
 }
 
 func syncElasticQuotaMetrics(eq *v1alpha1.ElasticQuota, summary *core.QuotaInfoSummary) {
+	resourceField, quotaLabels := getResourceFieldAndQuotaLabels(eq, summary)
+	for k, v := range resourceField {
+		decorateResource(eq, v)
+		k = strings.TrimPrefix(k, extension.QuotaKoordinatorPrefix+"/")
+		RecordElasticQuotaMetric(ElasticQuotaStatusMetric, v, k, quotaLabels)
+	}
+}
+
+func deleteElasticQuotaMetrics(eq *v1alpha1.ElasticQuota, summary *core.QuotaInfoSummary) {
+	resourceField, quotaLabels := getResourceFieldAndQuotaLabels(eq, summary)
+	for k, v := range resourceField {
+		decorateResource(eq, v)
+		k = strings.TrimPrefix(k, extension.QuotaKoordinatorPrefix+"/")
+		DeleteElasticQuotaMetric(ElasticQuotaStatusMetric, v, k, quotaLabels)
+	}
+}
+
+func getResourceFieldAndQuotaLabels(eq *v1alpha1.ElasticQuota, summary *core.QuotaInfoSummary) (map[string]v1.ResourceList, map[string]string) {
 	quotaLabels := map[string]string{
 		"name":      summary.Name,
 		"tree":      summary.Tree,
@@ -294,20 +312,9 @@ func syncElasticQuotaMetrics(eq *v1alpha1.ElasticQuota, summary *core.QuotaInfoS
 			m[extension.AnnotationUnschedulableResource] = unschedulable
 		}
 	}
-
-	for k, v := range m {
-		decorateResource(eq, v)
-
-		k = strings.TrimPrefix(k, extension.QuotaKoordinatorPrefix+"/")
-		RecordElasticQuotaMetric(ElasticQuotaStatusMetric, v, k, quotaLabels)
-	}
-	decorateResource(eq, summary.Used)
-	RecordElasticQuotaMetric(ElasticQuotaStatusMetric, summary.Used, "used", quotaLabels)
-
-	decorateResource(eq, summary.Min)
-	decorateResource(eq, summary.Max)
-	decorateResource(eq, summary.SharedWeight)
-	RecordElasticQuotaMetric(ElasticQuotaSpecMetric, summary.Min, "min", quotaLabels)
-	RecordElasticQuotaMetric(ElasticQuotaSpecMetric, summary.Max, "max", quotaLabels)
-	RecordElasticQuotaMetric(ElasticQuotaSpecMetric, summary.SharedWeight, "sharedWeight", quotaLabels)
+	m["used"] = summary.Used
+	m["min"] = summary.Min
+	m["max"] = summary.Max
+	m["sharedWeight"] = summary.SharedWeight
+	return m, quotaLabels
 }
diff --git a/pkg/scheduler/plugins/elasticquota/metrics.go b/pkg/scheduler/plugins/elasticquota/metrics.go
index 70ad6ddc..eeb28e6e 100644
--- a/pkg/scheduler/plugins/elasticquota/metrics.go
+++ b/pkg/scheduler/plugins/elasticquota/metrics.go
@@ -75,6 +75,17 @@ func RecordElasticQuotaMetric(gaugeVec *metrics.GaugeVec, resources corev1.Resou
 func recordResourceMetric(gaugeVec *metrics.GaugeVec, value int64, resourceName, field string, labels map[string]string) {
 	labels["resource"] = resourceName
 	labels["field"] = field
-
 	gaugeVec.With(labels).Set(float64(value))
 }
+
+func DeleteElasticQuotaMetric(gaugeVec *metrics.GaugeVec, resources corev1.ResourceList, field string, labels map[string]string) {
+	for resourceName := range resources {
+		deleteResourceMetric(gaugeVec, string(resourceName), field, labels)
+	}
+}
+
+func deleteResourceMetric(gaugeVec *metrics.GaugeVec, resourceName, field string, labels map[string]string) {
+	labels["resource"] = resourceName
+	labels["field"] = field
+	gaugeVec.Delete(labels)
+}
diff --git a/pkg/scheduler/plugins/elasticquota/quota_handler.go b/pkg/scheduler/plugins/elasticquota/quota_handler.go
index 3fc14d00..18d52031 100644
--- a/pkg/scheduler/plugins/elasticquota/quota_handler.go
+++ b/pkg/scheduler/plugins/elasticquota/quota_handler.go
@@ -104,7 +104,10 @@ func (g *Plugin) OnQuotaDelete(obj interface{}) {
 		klog.Errorf("quota is nil")
 		return
 	}
-
+	summary, _ := g.GetQuotaSummary(quota.Name, false)
+	if summary != nil {
+		deleteElasticQuotaMetrics(quota, summary)
+	}
 	klog.V(5).Infof("OnQuotaDeleteFunc delete quota: %v", quota.Name)
 	g.deleteQuotaToTreeMap(quota.Name)
 	mgr := g.GetGroupQuotaManagerForTree(quota.Labels[extension.LabelQuotaTreeID])



Test patch:
diff --git a/pkg/scheduler/plugins/elasticquota/controller_test.go b/pkg/scheduler/plugins/elasticquota/controller_test.go
index d90540e3..abe76df1 100644
--- a/pkg/scheduler/plugins/elasticquota/controller_test.go
+++ b/pkg/scheduler/plugins/elasticquota/controller_test.go
@@ -365,7 +365,53 @@ func Test_updateElasticQuotaStatusIfChanged(t *testing.T) {
 	}
 }
 
+func Test_deleteElasticQuotaMetrics(t *testing.T) {
+	eq, summary := getEqAndSummary()
+	syncElasticQuotaMetrics(eq, summary)
+	dtoMetrics := getMetrics(t)
+	assert.Equal(t, 22, len(dtoMetrics))
+	deleteElasticQuotaMetrics(eq, summary)
+	dtoMetricsD := getMetrics(t)
+	assert.Equal(t, 0, len(dtoMetricsD))
+}
+
 func Test_syncElasticQuotaMetrics(t *testing.T) {
+	eq, summary := getEqAndSummary()
+	syncElasticQuotaMetrics(eq, summary)
+	dtoMetrics := getMetrics(t)
+	expect := []string{
+		`{"label":[{"name":"field","value":"allocated"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":7000}}`,
+		`{"label":[{"name":"field","value":"allocated"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
+		`{"label":[{"name":"field","value":"child-request"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":6000}}`,
+		`{"label":[{"name":"field","value":"child-request"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
+		`{"label":[{"name":"field","value":"guaranteed"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":8000}}`,
+		`{"label":[{"name":"field","value":"guaranteed"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
+		`{"label":[{"name":"field","value":"max"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":4000}}`,
+		`{"label":[{"name":"field","value":"max"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":200}}`,
+		`{"label":[{"name":"field","value":"min"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":2000}}`,
+		`{"label":[{"name":"field","value":"min"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":100}}`,
+		`{"label":[{"name":"field","value":"non-preemptible-request"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":3000}}`,
+		`{"label":[{"name":"field","value":"non-preemptible-request"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
+		`{"label":[{"name":"field","value":"non-preemptible-used"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":2000}}`,
+		`{"label":[{"name":"field","value":"non-preemptible-used"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
+		`{"label":[{"name":"field","value":"request"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":4000}}`,
+		`{"label":[{"name":"field","value":"request"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
+		`{"label":[{"name":"field","value":"runtime"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":5000}}`,
+		`{"label":[{"name":"field","value":"runtime"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
+		`{"label":[{"name":"field","value":"unschedulable-resource"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":4000}}`,
+		`{"label":[{"name":"field","value":"unschedulable-resource"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":8}}`,
+		`{"label":[{"name":"field","value":"used"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":1000}}`,
+		`{"label":[{"name":"field","value":"used"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
+	}
+	assert.Equal(t, len(expect), len(dtoMetrics))
+	for i := range dtoMetrics {
+		v := dtoMetrics[i]
+		jsonStrBytes, _ := json.Marshal(v)
+		assert.Equal(t, expect[i], string(jsonStrBytes))
+	}
+}
+
+func getEqAndSummary() (*v1alpha1.ElasticQuota, *core.QuotaInfoSummary) {
 	eq := &v1alpha1.ElasticQuota{
 		ObjectMeta: metav1.ObjectMeta{
 			Labels: map[string]string{
@@ -396,7 +442,10 @@ func Test_syncElasticQuotaMetrics(t *testing.T) {
 		Allocated:             MakeResourceList().CPU(7).Mem(50).Obj(),
 		Guaranteed:            MakeResourceList().CPU(8).Mem(50).Obj(),
 	}
-	syncElasticQuotaMetrics(eq, summary)
+	return eq, summary
+}
+
+func getMetrics(t *testing.T) []*dto.Metric {
 	metricsCh := make(chan prometheus.Metric, 10)
 	go func() {
 		ElasticQuotaSpecMetric.Collect(metricsCh)
@@ -424,36 +473,7 @@ loopChan:
 	sort.Slice(dtoMetrics, func(i, j int) bool {
 		return dtoMetrics[i].String() < dtoMetrics[j].String()
 	})
-	expect := []string{
-		`{"label":[{"name":"field","value":"allocated"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":7000}}`,
-		`{"label":[{"name":"field","value":"allocated"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
-		`{"label":[{"name":"field","value":"child-request"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":6000}}`,
-		`{"label":[{"name":"field","value":"child-request"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
-		`{"label":[{"name":"field","value":"guaranteed"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":8000}}`,
-		`{"label":[{"name":"field","value":"guaranteed"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
-		`{"label":[{"name":"field","value":"max"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":4000}}`,
-		`{"label":[{"name":"field","value":"max"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":200}}`,
-		`{"label":[{"name":"field","value":"min"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":2000}}`,
-		`{"label":[{"name":"field","value":"min"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":100}}`,
-		`{"label":[{"name":"field","value":"non-preemptible-request"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":3000}}`,
-		`{"label":[{"name":"field","value":"non-preemptible-request"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
-		`{"label":[{"name":"field","value":"non-preemptible-used"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":2000}}`,
-		`{"label":[{"name":"field","value":"non-preemptible-used"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
-		`{"label":[{"name":"field","value":"request"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":4000}}`,
-		`{"label":[{"name":"field","value":"request"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
-		`{"label":[{"name":"field","value":"runtime"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":5000}}`,
-		`{"label":[{"name":"field","value":"runtime"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
-		`{"label":[{"name":"field","value":"unschedulable-resource"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":4000}}`,
-		`{"label":[{"name":"field","value":"unschedulable-resource"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":8}}`,
-		`{"label":[{"name":"field","value":"used"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"cpu"},{"name":"tree","value":"tree-1"}],"gauge":{"value":1000}}`,
-		`{"label":[{"name":"field","value":"used"},{"name":"is_parent","value":"true"},{"name":"name","value":"test-eq"},{"name":"parent","value":"root"},{"name":"resource","value":"memory"},{"name":"tree","value":"tree-1"}],"gauge":{"value":50}}`,
-	}
-	assert.Equal(t, len(expect), len(dtoMetrics))
-	for i := range dtoMetrics {
-		v := dtoMetrics[i]
-		jsonStrBytes, _ := json.Marshal(v)
-		assert.Equal(t, expect[i], string(jsonStrBytes))
-	}
+	return dtoMetrics
 }
 
 type eqWrapper struct{ *v1alpha1.ElasticQuota }