Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Add Snapshot() method to the NodeDeletionTracker
  • Loading branch information
yaroslava-serdiuk committed Mar 3, 2023
commit dec813640cd6fa40f31a2d0e1e643b3026d02886
7 changes: 2 additions & 5 deletions cluster-autoscaler/core/scaledown/actuation/actuator.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,9 @@ func NewActuator(ctx *context.AutoscalingContext, csr *clusterstate.ClusterState
}
}

// CheckStatus should return an immutable snapshot of ongoing deletions. Before the TODO is addressed, a live object
// is returned instead of an immutable snapshot.
// CheckStatus should returns an immutable snapshot of ongoing deletions.
func (a *Actuator) CheckStatus() scaledown.ActuationStatus {
// TODO: snapshot information from the tracker instead of keeping live
// updated object.
return a.nodeDeletionTracker
return a.nodeDeletionTracker.Snapshot()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ideally we'd just replace ActuationStatus interface with a simple struct that doesn't have the mutex overhead. It would also be simpler to mock in tests. Can you add a TODO for this somewhere? Perhaps next to the interface definition.

}

// ClearResultsNotNewerThan removes information about deletions finished before or exactly at the provided timestamp.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,26 @@ func (n *NodeDeletionTracker) ClearResultsNotNewerThan(t time.Time) {
defer n.Unlock()
n.deletionResults.DropNotNewerThan(t)
}

// Snapshot return a copy of NodeDeletionTracker.
func (n *NodeDeletionTracker) Snapshot() *NodeDeletionTracker {
n.Lock()
defer n.Unlock()
snapshot := NewNodeDeletionTracker(n.evictionsTTL)
for k, val := range n.emptyNodeDeletions {
snapshot.emptyNodeDeletions[k] = val
}
for k, val := range n.drainedNodeDeletions {
snapshot.drainedNodeDeletions[k] = val
}
for k, val := range n.deletionsPerNodeGroup {
snapshot.deletionsPerNodeGroup[k] = val
}
for _, eviction := range n.evictions.ToSlice() {
snapshot.evictions.RegisterElement(eviction)
}
for _, result := range n.deletionResults.ToSlice() {
snapshot.deletionResults.RegisterElement(result)
}
return snapshot
}
7 changes: 1 addition & 6 deletions cluster-autoscaler/core/scaledown/scaledown.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ type Actuator interface {
}

// ActuationStatus is used for feeding Actuator status back into Planner
// TODO: Replace ActuationStatus with simple struct with getter methods.
type ActuationStatus interface {
// DeletionsInProgress returns two lists of node names that are
// currently undergoing deletion, for empty and non-empty (i.e. drained)
Expand All @@ -77,10 +78,4 @@ type ActuationStatus interface {
// the Actuator and hence are likely to get recreated elsewhere in the
// cluster.
RecentEvictions() (pods []*apiv1.Pod)
// DeletionResults returns a map of recent node deletion results, keyed
// by the node name. Note: if node deletion was scheduled more than
// once, only the latest result will be present.
// The timestamp returned as the second value indicates the time at
// which the last result was collected.
DeletionResults() (map[string]status.NodeDeleteResult, time.Time)
}