Address new comments

pkg/metrics/queue.go

@@ -301,12 +301,64 @@ func (m *QueueMetrics) AddReleasedContainers(value int) {
 	m.containerMetrics.WithLabelValues(ContainerReleased).Add(float64(value))
 }
 
-func (m *QueueMetrics) SetQueueGuaranteedResourceMetrics(resourceName string, value float64) {
-	m.setQueueResource(QueueGuaranteed, resourceName, value)
+func (m *QueueMetrics) SetQueueResourceMetrics(state string, resourceName string, value float64) {
+	m.setQueueResource(state, resourceName, value)
 }
 
-func (m *QueueMetrics) SetQueueMaxResourceMetrics(resourceName string, value float64) {
-	m.setQueueResource(QueueMax, resourceName, value)
+func (m *QueueMetrics) SetQueueNilResourceMetrics(state string) {
+	currResource, err := m.GetAllQueueStateMetrics(state)
+	if err != nil {
+		log.Log(log.Metrics).Warn("failed to get current guaranteed resource metrics",
+			zap.Error(err))
+		return
+	}
+	for k := range currResource {
+		m.SetQueueResourceMetrics(state, k, float64(0))
+	}
+}
+
+func (m *QueueMetrics) GetQueueMaxResourceMetrics(resourceName string) (float64, error) {
+	metricDto := &dto.Metric{}
+	err := m.resourceMetricsLabel.WithLabelValues(QueueMax, resourceName).Write(metricDto)
+	if err == nil {
+		return *metricDto.Gauge.Value, nil
+	}
+	return -1, err
+}
+
+func (m *QueueMetrics) GetAllQueueStateMetrics(state string) (map[string]float64, error) {
+	metrics := make(map[string]float64)
+	metricFamilies, err := prometheus.DefaultGatherer.Gather()
+	if err != nil {
+		return nil, err
+	}
+
+	for _, metricFamily := range metricFamilies {
+		for _, metric := range metricFamily.Metric {
+			// Check if this metric belongs to the state type.
+			for _, label := range metric.Label {
+				if *label.Name == "state" && *label.Value == state {
+					// Extract the `resource` label and its corresponding value.
+					for _, subLabel := range metric.Label {
+						if *subLabel.Name == "resource" {
+							metrics[*subLabel.Value] = metric.GetGauge().GetValue()
+						}
+					}
+				}
+			}
+		}
+	}
+
+	return metrics, nil
+}
+
+func (m *QueueMetrics) GetQueueGuaranteedResourceMetrics(resourceName string) (float64, error) {
+	metricDto := &dto.Metric{}
+	err := m.resourceMetricsLabel.WithLabelValues(QueueGuaranteed, resourceName).Write(metricDto)
+	if err == nil {
+		return *metricDto.Gauge.Value, nil
+	}
+	return -1, err
 }
 
 func (m *QueueMetrics) SetQueueAllocatedResourceMetrics(resourceName string, value float64) {

pkg/metrics/queue_test.go

@@ -196,16 +196,38 @@ func TestQueueGuaranteedResourceMetrics(t *testing.T) {
 	qm = getQueueMetrics()
 	defer unregisterQueueMetrics()
 
-	qm.SetQueueGuaranteedResourceMetrics("cpu", 1)
+	qm.SetQueueResourceMetrics(QueueGuaranteed, "cpu", 1)
 	verifyResourceMetrics(t, "guaranteed", "cpu")
+
+	res, err := qm.GetAllQueueStateMetrics(QueueGuaranteed)
+	assert.NilError(t, err)
+	assert.Equal(t, len(res), 1)
+	assert.DeepEqual(t, res, map[string]float64{"cpu": 1})
+
+	qm.SetQueueNilResourceMetrics(QueueGuaranteed)
+	res, err = qm.GetAllQueueStateMetrics(QueueGuaranteed)
+	assert.NilError(t, err)
+	assert.Equal(t, len(res), 1)
+	assert.DeepEqual(t, res, map[string]float64{"cpu": 0})
 }
 
 func TestQueueMaxResourceMetrics(t *testing.T) {
 	qm = getQueueMetrics()
 	defer unregisterQueueMetrics()
 
-	qm.SetQueueMaxResourceMetrics("cpu", 1)
+	qm.SetQueueResourceMetrics(QueueMax, "cpu", 1)
 	verifyResourceMetrics(t, "max", "cpu")
+
+	res, err := qm.GetAllQueueStateMetrics(QueueMax)
+	assert.NilError(t, err)
+	assert.Equal(t, len(res), 1)
+	assert.DeepEqual(t, res, map[string]float64{"cpu": 1})
+
+	qm.SetQueueNilResourceMetrics(QueueMax)
+	res, err = qm.GetAllQueueStateMetrics(QueueMax)
+	assert.NilError(t, err)
+	assert.Equal(t, len(res), 1)
+	assert.DeepEqual(t, res, map[string]float64{"cpu": 0})
 }
 
 func TestQueueAllocatedResourceMetrics(t *testing.T) {

pkg/scheduler/objects/queue.go

@@ -74,22 +74,20 @@ type Queue struct {
 	// The queue properties should be treated as immutable the value is a merge of the
 	// parent properties with the config for this queue only manipulated during creation
 	// of the queue or via a queue configuration update.
-	properties                 map[string]string
-	adminACL                   security.ACL        // admin ACL
-	submitACL                  security.ACL        // submit ACL
-	previousMaxResource        *resources.Resource // Previous max resource for the queue before update
-	previousGuaranteedResource *resources.Resource // Previous guaranteed resource for the queue before update
-	maxResource                *resources.Resource // When not set, max = nil
-	guaranteedResource         *resources.Resource // When not set, Guaranteed == 0
-	isLeaf                     bool                // this is a leaf queue or not (i.e. parent)
-	isManaged                  bool                // queue is part of the config, not auto created
-	stateMachine               *fsm.FSM            // the state of the queue for scheduling
-	stateTime                  time.Time           // last time the state was updated (needed for cleanup)
-	maxRunningApps             uint64
-	runningApps                uint64
-	allocatingAcceptedApps     map[string]bool
-	template                   *template.Template
-	queueEvents                *schedEvt.QueueEvents
+	properties             map[string]string
+	adminACL               security.ACL        // admin ACL
+	submitACL              security.ACL        // submit ACL
+	maxResource            *resources.Resource // When not set, max = nil
+	guaranteedResource     *resources.Resource // When not set, Guaranteed == 0
+	isLeaf                 bool                // this is a leaf queue or not (i.e. parent)
+	isManaged              bool                // queue is part of the config, not auto created
+	stateMachine           *fsm.FSM            // the state of the queue for scheduling
+	stateTime              time.Time           // last time the state was updated (needed for cleanup)
+	maxRunningApps         uint64
+	runningApps            uint64
+	allocatingAcceptedApps map[string]bool
+	template               *template.Template
+	queueEvents            *schedEvt.QueueEvents
 
 	locking.RWMutex
 }
@@ -1661,27 +1659,21 @@ func (sq *Queue) SupportTaskGroup() bool {
 func (sq *Queue) updateGuaranteedResourceMetrics() {
 	if sq.guaranteedResource != nil {
 		for k, v := range sq.guaranteedResource.Resources {
-			metrics.GetQueueMetrics(sq.QueuePath).SetQueueGuaranteedResourceMetrics(k, float64(v))
-		}
-		sq.previousGuaranteedResource = sq.guaranteedResource
-	} else if sq.previousGuaranteedResource != nil {
-		for k := range sq.previousGuaranteedResource.Resources {
-			metrics.GetQueueMetrics(sq.QueuePath).SetQueueGuaranteedResourceMetrics(k, float64(0))
+			metrics.GetQueueMetrics(sq.QueuePath).SetQueueResourceMetrics(metrics.QueueGuaranteed, k, float64(v))
 		}
+	} else {
+		metrics.GetQueueMetrics(sq.QueuePath).SetQueueNilResourceMetrics(metrics.QueueGuaranteed)
 	}
 }
 
 // updateMaxResourceMetrics updates max resource metrics.
 func (sq *Queue) updateMaxResourceMetrics() {
 	if sq.maxResource != nil {
 		for k, v := range sq.maxResource.Resources {
-			metrics.GetQueueMetrics(sq.QueuePath).SetQueueMaxResourceMetrics(k, float64(v))
-		}
-		sq.previousMaxResource = sq.maxResource
-	} else if sq.previousMaxResource != nil {
-		for k := range sq.previousMaxResource.Resources {
-			metrics.GetQueueMetrics(sq.QueuePath).SetQueueMaxResourceMetrics(k, float64(0))
+			metrics.GetQueueMetrics(sq.QueuePath).SetQueueResourceMetrics(metrics.QueueMax, k, float64(v))
 		}
+	} else {
+		metrics.GetQueueMetrics(sq.QueuePath).SetQueueNilResourceMetrics(metrics.QueueMax)
 	}
 }
 

pkg/scheduler/objects/queue_test.go

@@ -1753,8 +1753,6 @@ func TestSetResources(t *testing.T) {
 	assert.NilError(t, err, "failed to set resources: %v", err)
 	assert.DeepEqual(t, queue.guaranteedResource, nilResource)
 	assert.DeepEqual(t, queue.maxResource, nilResource)
-	assert.DeepEqual(t, queue.previousGuaranteedResource, expectedGuaranteedResource)
-	assert.DeepEqual(t, queue.previousMaxResource, expectedMaxResource)
 
 	// case 2: zero resource won't change the queue resources as it is 'nil' already
 	err = queue.setResourcesFromConf(configs.Resources{
@@ -1764,8 +1762,6 @@ func TestSetResources(t *testing.T) {
 	assert.NilError(t, err, "failed to set resources: %v", err)
 	assert.DeepEqual(t, queue.guaranteedResource, nilResource)
 	assert.DeepEqual(t, queue.maxResource, nilResource)
-	assert.DeepEqual(t, queue.previousGuaranteedResource, expectedGuaranteedResource)
-	assert.DeepEqual(t, queue.previousMaxResource, expectedMaxResource)
 }
 
 func TestPreemptingResource(t *testing.T) {