Address new comments

pkg/scheduler/objects/queue.go

@@ -74,20 +74,22 @@ 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
-	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
+	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
 
 	locking.RWMutex
 }
@@ -1661,12 +1663,13 @@ func (sq *Queue) updateGuaranteedResourceMetrics() {
 		for k, v := range sq.guaranteedResource.Resources {
 			metrics.GetQueueMetrics(sq.QueuePath).SetQueueGuaranteedResourceMetrics(k, float64(v))
 		}
+		sq.previousGuaranteedResource = sq.guaranteedResource
 	} else {
-		// We need to update the resource, because we support update the guaranteed resource to nil or zero resources
-		// for the queue. We need to update the metrics to reflect the change.
-		// Here, we just set the guaranteed resource to zero for cpu and memory.
-		metrics.GetQueueMetrics(sq.QueuePath).SetQueueGuaranteedResourceMetrics("cpu", float64(0))
-		metrics.GetQueueMetrics(sq.QueuePath).SetQueueGuaranteedResourceMetrics("memory", float64(0))
+		if sq.previousGuaranteedResource != nil {
+			for k := range sq.previousGuaranteedResource.Resources {
+				metrics.GetQueueMetrics(sq.QueuePath).SetQueueGuaranteedResourceMetrics(k, float64(0))
+			}
+		}
 	}
 }
 
@@ -1676,12 +1679,13 @@ func (sq *Queue) updateMaxResourceMetrics() {
 		for k, v := range sq.maxResource.Resources {
 			metrics.GetQueueMetrics(sq.QueuePath).SetQueueMaxResourceMetrics(k, float64(v))
 		}
+		sq.previousMaxResource = sq.maxResource
 	} else {
-		// We need to update the resource, because we support update the max resource to nil or zero resources
-		// for the queue. We need to update the metrics to reflect the change.
-		// Here, we just set the max resource to zero for cpu and memory.
-		metrics.GetQueueMetrics(sq.QueuePath).SetQueueMaxResourceMetrics("cpu", float64(0))
-		metrics.GetQueueMetrics(sq.QueuePath).SetQueueMaxResourceMetrics("memory", float64(0))
+		if sq.previousMaxResource != nil {
+			for k := range sq.previousMaxResource.Resources {
+				metrics.GetQueueMetrics(sq.QueuePath).SetQueueMaxResourceMetrics(k, float64(0))
+			}
+		}
 	}
 }
 

pkg/scheduler/objects/queue_test.go

@@ -1753,6 +1753,8 @@ 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{
@@ -1762,6 +1764,8 @@ 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) {