Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Force Removal Taint Logic #246

Merged
merged 10 commits into from
Sep 27, 2024
12 changes: 12 additions & 0 deletions docs/node-termination.md
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
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
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
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
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
Loading