Skip to content

Commit

Permalink
Implement Force Removal Taint Logic (#246)
Browse files Browse the repository at this point in the history 
* Add force taint logic

* Comment updates

* Update controller_test

* Update controller_scale_node_group_test

* Update controller filter logic

* Update scale_down_test

* Add logic for forceTaints and dryMode in controller filter

* Add force tainted node check to controller tests

* Add force taint removal unit tests

* Add force taint docs
  • Loading branch information
@jackcasey-visier
jackcasey-visier authored Sep 27, 2024
1 parent f5e40a7 commit 650dda7
Show file tree
Hide file tree
Showing 9 changed files with 250 additions and 44 deletions.
12 changes: 12 additions & 0 deletions docs/node-termination.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,15 @@ metadata:
annotations:
atlassian.com/no-delete: "testing some long running bpf tracing"
```
## Force Tainting
Force Tainting provides a mechanism for Escalator to promptly remove nodes tainted by an external system, or administrator.
This is implemented by applying a taint to the node, with the following key:
```
atlassian.com/escalator-force
```

The node will be removed as soon as all non-daemonset pods are completed. This is useful when nodes must be removed asap, but running pods should not be killed.
88 changes: 60 additions & 28 deletions pkg/controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ type NodeGroupState struct {
scaleUpLock scaleLock

// used for tracking which nodes are tainted. testing when in dry mode
taintTracker []string
taintTracker []string
forceTaintTracker []string

// used for tracking scale delta across runs, useful for reducing hysteresis
scaleDelta int
Expand All @@ -54,11 +55,12 @@ type Opts struct {
// scaleOpts provides options for a scale function
// wraps options that would be passed as args
type scaleOpts struct {
nodes []*v1.Node
taintedNodes []*v1.Node
untaintedNodes []*v1.Node
nodeGroup *NodeGroupState
nodesDelta int
nodes []*v1.Node
taintedNodes []*v1.Node
forceTaintedNodes []*v1.Node
untaintedNodes []*v1.Node
nodeGroup *NodeGroupState
nodesDelta int
}

// NewController creates a new controller with the specified options
Expand Down Expand Up @@ -116,36 +118,52 @@ func (c *Controller) dryMode(nodeGroup *NodeGroupState) bool {
}

// filterNodes separates nodes between tainted and untainted nodes
func (c *Controller) filterNodes(nodeGroup *NodeGroupState, allNodes []*v1.Node) (untaintedNodes, taintedNodes, cordonedNodes []*v1.Node) {
func (c *Controller) filterNodes(nodeGroup *NodeGroupState, allNodes []*v1.Node) (untaintedNodes, taintedNodes, forceTaintedNodes, cordonedNodes []*v1.Node) {
untaintedNodes = make([]*v1.Node, 0, len(allNodes))
taintedNodes = make([]*v1.Node, 0, len(allNodes))
forceTaintedNodes = make([]*v1.Node, 0, len(allNodes))
cordonedNodes = make([]*v1.Node, 0, len(allNodes))

for _, node := range allNodes {
var tainted bool
var forceTainted bool
var untainted bool

if c.dryMode(nodeGroup) {
var contains bool
for _, name := range nodeGroup.taintTracker {
if node.Name == name {
contains = true
tainted = true
break
}
}
if !contains {
untaintedNodes = append(untaintedNodes, node)
} else {
taintedNodes = append(taintedNodes, node)

for _, name := range nodeGroup.forceTaintTracker {
if node.Name == name {
forceTainted = true
break
}
}

untainted = !forceTainted && !tainted

} else {
// If the node is Unschedulable (cordoned), separate it out from the tainted/untainted
if node.Spec.Unschedulable {
cordonedNodes = append(cordonedNodes, node)
continue
}
if _, tainted := k8s.GetToBeRemovedTaint(node); !tainted {
untaintedNodes = append(untaintedNodes, node)
} else {
taintedNodes = append(taintedNodes, node)
}

_, forceTainted = k8s.GetToBeForceRemovedTaint(node)
_, tainted = k8s.GetToBeRemovedTaint(node)
untainted = !forceTainted && !tainted
}

if forceTainted {
forceTaintedNodes = append(forceTaintedNodes, node)
} else if tainted {
taintedNodes = append(taintedNodes, node)
} else if untainted {
untaintedNodes = append(untaintedNodes, node)
}
}

Expand Down Expand Up @@ -210,20 +228,22 @@ func (c *Controller) scaleNodeGroup(nodegroup string, nodeGroup *NodeGroupState)
}

// Filter into untainted and tainted nodes
untaintedNodes, taintedNodes, cordonedNodes := c.filterNodes(nodeGroup, allNodes)
untaintedNodes, taintedNodes, forceTaintedNodes, cordonedNodes := c.filterNodes(nodeGroup, allNodes)

// Metrics and Logs
log.WithField("nodegroup", nodegroup).Infof("pods total: %v", len(pods))
log.WithField("nodegroup", nodegroup).Infof("nodes remaining total: %v", len(allNodes))
log.WithField("nodegroup", nodegroup).Infof("cordoned nodes remaining total: %v", len(cordonedNodes))
log.WithField("nodegroup", nodegroup).Infof("nodes remaining untainted: %v", len(untaintedNodes))
log.WithField("nodegroup", nodegroup).Infof("nodes remaining tainted: %v", len(taintedNodes))
log.WithField("nodegroup", nodegroup).Infof("nodes remaining force tainted: %v", len(forceTaintedNodes))
log.WithField("nodegroup", nodegroup).Infof("Minimum Node: %v", nodeGroup.Opts.MinNodes)
log.WithField("nodegroup", nodegroup).Infof("Maximum Node: %v", nodeGroup.Opts.MaxNodes)
metrics.NodeGroupNodes.WithLabelValues(nodegroup).Set(float64(len(allNodes)))
metrics.NodeGroupNodesCordoned.WithLabelValues(nodegroup).Set(float64(len(cordonedNodes)))
metrics.NodeGroupNodesUntainted.WithLabelValues(nodegroup).Set(float64(len(untaintedNodes)))
metrics.NodeGroupNodesTainted.WithLabelValues(nodegroup).Set(float64(len(taintedNodes)))
metrics.NodeGroupNodesForceTainted.WithLabelValues(nodegroup).Set(float64(len(forceTaintedNodes)))
metrics.NodeGroupPods.WithLabelValues(nodegroup).Set(float64(len(pods)))

// We want to be really simple right now so we don't do anything if we are outside the range of allowed nodes
Expand Down Expand Up @@ -288,11 +308,12 @@ func (c *Controller) scaleNodeGroup(nodegroup string, nodeGroup *NodeGroupState)
if len(untaintedNodes) < nodeGroup.Opts.MinNodes {
log.WithField("nodegroup", nodegroup).Warn("There are less untainted nodes than the minimum")
result, err := c.ScaleUp(scaleOpts{
nodes: allNodes,
nodesDelta: nodeGroup.Opts.MinNodes - len(untaintedNodes),
nodeGroup: nodeGroup,
taintedNodes: taintedNodes,
untaintedNodes: untaintedNodes,
nodes: allNodes,
nodesDelta: nodeGroup.Opts.MinNodes - len(untaintedNodes),
nodeGroup: nodeGroup,
taintedNodes: taintedNodes,
forceTaintedNodes: forceTaintedNodes,
untaintedNodes: untaintedNodes,
})
if err != nil {
log.WithField("nodegroup", nodegroup).Error(err)
Expand Down Expand Up @@ -382,10 +403,21 @@ func (c *Controller) scaleNodeGroup(nodegroup string, nodeGroup *NodeGroupState)
log.WithField("nodegroup", nodegroup).Debugf("Delta: %v", nodesDelta)

scaleOptions := scaleOpts{
nodes: allNodes,
taintedNodes: taintedNodes,
untaintedNodes: untaintedNodes,
nodeGroup: nodeGroup,
nodes: allNodes,
taintedNodes: taintedNodes,
forceTaintedNodes: forceTaintedNodes,
untaintedNodes: untaintedNodes,
nodeGroup: nodeGroup,
}

// Check for nodes tainted for force removal
var forceRemoved int
var forceActionErr error
forceRemoved, forceActionErr = c.TryRemoveForceTaintedNodes(scaleOptions)
log.WithField("nodegroup", nodegroup).Infof("Reaper: There were %v empty nodes force deleted this round", forceRemoved)

if forceActionErr != nil {
log.WithField("nodegroup", nodegroup).Error(forceActionErr)
}

// Perform a scale up, do nothing or scale down based on the nodes delta
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/controller_scale_node_group_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func TestUntaintNodeGroupMinNodes(t *testing.T) {
_, err = controller.scaleNodeGroup(nodeGroup.Name, nodeGroupsState[nodeGroup.Name])
assert.NoError(t, err)

untainted, tainted, _ := controller.filterNodes(nodeGroupsState[nodeGroup.Name], nodes)
untainted, tainted, _, _ := controller.filterNodes(nodeGroupsState[nodeGroup.Name], nodes)
// Ensure that the tainted nodes where untainted
assert.Equal(t, minNodes, len(untainted))
assert.Equal(t, 0, len(tainted))
Expand Down Expand Up @@ -193,7 +193,7 @@ func TestUntaintNodeGroupMaxNodes(t *testing.T) {
_, err = controller.scaleNodeGroup(nodeGroup.Name, nodeGroupsState[nodeGroup.Name])
require.NoError(t, err)

untainted, tainted, _ := controller.filterNodes(nodeGroupsState[nodeGroup.Name], nodes)
untainted, tainted, _, _ := controller.filterNodes(nodeGroupsState[nodeGroup.Name], nodes)
// Ensure that the tainted nodes where untainted
assert.Equal(t, maxNodes, len(untainted))
assert.Equal(t, 0, len(tainted))
Expand Down
23 changes: 16 additions & 7 deletions pkg/controller/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ func TestControllerFilterNodes(t *testing.T) {
Name: "n6",
Tainted: false,
}),
6: test.BuildTestNode(test.NodeOpts{
Name: "n7",
ForceTainted: true,
}),
}

type args struct {
Expand All @@ -113,11 +117,12 @@ func TestControllerFilterNodes(t *testing.T) {
master bool
}
tests := []struct {
name string
args args
wantUntaintedNodes []*v1.Node
wantTaintedNodes []*v1.Node
wantCordonedNodes []*v1.Node
name string
args args
wantUntaintedNodes []*v1.Node
wantTaintedNodes []*v1.Node
wantForceTaintedNodes []*v1.Node
wantCordonedNodes []*v1.Node
}{
{
"basic filter not drymode",
Expand All @@ -132,6 +137,7 @@ func TestControllerFilterNodes(t *testing.T) {
},
[]*v1.Node{nodes[1], nodes[3], nodes[5]},
[]*v1.Node{nodes[0], nodes[2], nodes[4]},
[]*v1.Node{nodes[6]},
[]*v1.Node{},
},
{
Expand All @@ -141,13 +147,15 @@ func TestControllerFilterNodes(t *testing.T) {
Opts: NodeGroupOptions{
DryMode: true,
},
taintTracker: []string{"n1", "n3", "n5"},
taintTracker: []string{"n1", "n3", "n5"},
forceTaintTracker: []string{"n7"},
},
nodes,
true,
},
[]*v1.Node{nodes[1], nodes[3], nodes[5]},
[]*v1.Node{nodes[0], nodes[2], nodes[4]},
[]*v1.Node{nodes[6]},
[]*v1.Node{},
},
}
Expand All @@ -158,9 +166,10 @@ func TestControllerFilterNodes(t *testing.T) {
DryMode: tt.args.master,
},
}
gotUntaintedNodes, gotTaintedNodes, gotCordonedNodes := c.filterNodes(tt.args.nodeGroup, tt.args.allNodes)
gotUntaintedNodes, gotTaintedNodes, gotForceTaintedNodes, gotCordonedNodes := c.filterNodes(tt.args.nodeGroup, tt.args.allNodes)
assert.Equal(t, tt.wantUntaintedNodes, gotUntaintedNodes)
assert.Equal(t, tt.wantTaintedNodes, gotTaintedNodes)
assert.Equal(t, tt.wantForceTaintedNodes, gotForceTaintedNodes)
assert.Equal(t, tt.wantCordonedNodes, gotCordonedNodes)
})
}
Expand Down
24 changes: 24 additions & 0 deletions pkg/controller/scale_down.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,26 @@ func safeFromDeletion(node *v1.Node) (string, bool) {
return "", false
}

// TryRemoveForceTaintedNodes attempts to remove nodes that are
// * Tainted with force remove and empty
func (c *Controller) TryRemoveForceTaintedNodes(opts scaleOpts) (int, error) {
var toBeDeleted []*v1.Node
drymode := c.dryMode(opts.nodeGroup)

for _, candidate := range opts.forceTaintedNodes {
if k8s.NodeEmpty(candidate, opts.nodeGroup.NodeInfoMap) {
if !drymode {
toBeDeleted = append(toBeDeleted, candidate)
}
log.WithField("drymode", drymode).WithField("nodegroup", opts.nodeGroup.Opts.Name).Infof("Node %v, %v ready to be force deleted", candidate.Name, candidate.Spec.ProviderID)
} else {
log.WithField("drymode", drymode).WithField("nodegroup", opts.nodeGroup.Opts.Name).Infof("Node %v, %v not ready to be force deleted", candidate.Name, candidate.Spec.ProviderID)
}
}

return TryDeleteNodes(c, opts, toBeDeleted)
}

// TryRemoveTaintedNodes attempts to remove nodes are
// * tainted and empty
// * have passed their grace period
Expand Down Expand Up @@ -98,6 +118,10 @@ func (c *Controller) TryRemoveTaintedNodes(opts scaleOpts) (int, error) {
}
}

return TryDeleteNodes(c, opts, toBeDeleted)
}

func TryDeleteNodes(c *Controller, opts scaleOpts, toBeDeleted []*v1.Node) (int, error) {
if len(toBeDeleted) > 0 {
podsRemaining := 0
for _, nodeToBeDeleted := range toBeDeleted {
Expand Down
Loading

0 comments on commit 650dda7

Please sign in to comment.