Skip to content

Commit e95a33a

Browse files
authored
[CASCL-1386] (9/11) Evict standalone EC2 nodes (#3176)
* [CASCL-1386] Implement shared node cordon and drain primitives Introduce the reusable node-cordon and pod-eviction/drain primitives that the per-manager evictors (EKS managed node groups, ASG, Karpenter, standalone) build on: - cordonNodes/cordonNode: idempotent, conflict-retrying cordon that treats an already-gone node as a silent skip and returns the cordoned Node objects. - drainNode: evicts every evictable pod (skipping mirror, DaemonSet, completed and terminating pods), then waits for the node to empty. - evictPodWithRetry: PDB-aware eviction that retries on 429 and treats 404 as success. - listPodsOnNode, waitForNodeEmpty and the pod-classification predicates (podOccupiesNode/shouldSkipEviction/isMirrorPod/isDaemonSetPod/isCompleted). These primitives have no production caller yet; the first (evictEKSManagedNode- Group) lands in the next PR in the stack. A temporary blank-identifier reference keeps golangci-lint's `unused` quiet until then (the linter runs with tests = false, so the unit tests do not count as use) and is removed by that PR. * [CASCL-1386] Implement EKS managed node group eviction Part of a stack splitting #3026 (too large to review in one piece) into small pieces that each build and pass tests on their own. The command is fully functional only once the whole stack lands. * [CASCL-1386] Implement ASG eviction Part of a stack splitting #3026 (too large to review in one piece) into small pieces that each build and pass tests on their own. The command is fully functional only once the whole stack lands. * [CASCL-1386] Implement standalone EC2 node eviction Part of a stack splitting #3026 (too large to review in one piece) into small pieces that each build and pass tests on their own. The command is fully functional only once the whole stack lands.
1 parent 110b9f5 commit e95a33a

2 files changed

Lines changed: 229 additions & 1 deletion

File tree

cmd/kubectl-datadog/autoscaling/cluster/evict/standalone.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,45 @@ package evict
22

33
import (
44
"context"
5+
"errors"
6+
"fmt"
7+
"log"
58

69
"github.com/aws/aws-sdk-go-v2/service/ec2"
710
"k8s.io/client-go/kubernetes"
11+
12+
commonaws "github.com/DataDog/datadog-operator/cmd/kubectl-datadog/autoscaling/cluster/common/aws"
813
)
914

1015
type EC2API interface {
1116
TerminateInstances(ctx context.Context, in *ec2.TerminateInstancesInput, opts ...func(*ec2.Options)) (*ec2.TerminateInstancesOutput, error)
1217
}
1318

1419
func evictStandalone(ctx context.Context, clientset kubernetes.Interface, ec2API EC2API, nodes []string, drainOpts nodeDrainOptions) error {
15-
panic("TODO: evictStandalone — implemented in PR https://github.com/DataDog/datadog-operator/pull/3176")
20+
cordoned, errs := cordonNodes(ctx, clientset, nodes, drainOpts.DryRun)
21+
for _, node := range cordoned {
22+
nodeName := node.Name
23+
if err := drainNode(ctx, clientset, nodeName, drainOpts); err != nil {
24+
errs = append(errs, fmt.Errorf("drain node %s: %w", nodeName, err))
25+
continue // do NOT terminate this instance: workloads are still on it
26+
}
27+
28+
id, hasInstanceID := commonaws.ExtractEC2InstanceID(node)
29+
if !hasInstanceID {
30+
log.Printf("Warning: node %s has unexpected providerID %q; cannot terminate the underlying instance", nodeName, node.Spec.ProviderID)
31+
continue
32+
}
33+
if drainOpts.DryRun {
34+
log.Printf("[dry-run] would terminate standalone instance %s", id)
35+
continue
36+
}
37+
if _, err := ec2API.TerminateInstances(ctx, &ec2.TerminateInstancesInput{
38+
InstanceIds: []string{id},
39+
}); err != nil {
40+
errs = append(errs, fmt.Errorf("terminate instance %s: %w", id, err))
41+
} else {
42+
log.Printf("Terminated standalone EC2 instance %s.", id)
43+
}
44+
}
45+
return errors.Join(errs...)
1646
}
Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
package evict
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
"time"
8+
9+
"github.com/aws/aws-sdk-go-v2/service/ec2"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
corev1 "k8s.io/api/core/v1"
13+
policyv1 "k8s.io/api/policy/v1"
14+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
15+
"k8s.io/apimachinery/pkg/runtime"
16+
"k8s.io/client-go/kubernetes/fake"
17+
clienttesting "k8s.io/client-go/testing"
18+
)
19+
20+
type stubEC2 struct {
21+
terminated []string
22+
err error
23+
}
24+
25+
func (s *stubEC2) TerminateInstances(_ context.Context, in *ec2.TerminateInstancesInput, _ ...func(*ec2.Options)) (*ec2.TerminateInstancesOutput, error) {
26+
s.terminated = append(s.terminated, in.InstanceIds...)
27+
return &ec2.TerminateInstancesOutput{}, s.err
28+
}
29+
30+
func TestEvictStandalone(t *testing.T) {
31+
ec2Node := func(name, az, id string) *corev1.Node {
32+
return &corev1.Node{
33+
ObjectMeta: metav1.ObjectMeta{Name: name},
34+
Spec: corev1.NodeSpec{ProviderID: "aws:///" + az + "/" + id},
35+
}
36+
}
37+
stuckPod := func() *corev1.Pod {
38+
return &corev1.Pod{
39+
ObjectMeta: metav1.ObjectMeta{Name: "blocker", Namespace: "default"},
40+
Spec: corev1.PodSpec{NodeName: "ip-stuck"},
41+
}
42+
}
43+
gceNode := &corev1.Node{
44+
ObjectMeta: metav1.ObjectMeta{Name: "ip-1"},
45+
Spec: corev1.NodeSpec{ProviderID: "gce:///zone/instance"},
46+
}
47+
fastDrain := nodeDrainOptions{
48+
EvictionTimeout: 50 * time.Millisecond,
49+
NodeTimeout: 100 * time.Millisecond,
50+
PollInterval: 10 * time.Millisecond,
51+
}
52+
53+
for _, tc := range []struct {
54+
name string
55+
objects []runtime.Object
56+
installEvictionReactor bool
57+
nodes []string
58+
opts nodeDrainOptions
59+
// stubErr, when set, makes every TerminateInstances call fail.
60+
stubErr error
61+
wantErr bool
62+
// wantTerminated is the expected set of instance IDs in
63+
// stubEC2.terminated. nil ⇒ TerminateInstances must not be called.
64+
wantTerminated []string
65+
// wantUnschedulable: per-node expected `Spec.Unschedulable`.
66+
wantUnschedulable map[string]bool
67+
}{
68+
{
69+
name: "happy path terminates and cordons two nodes",
70+
objects: []runtime.Object{ec2Node("ip-1", "eu-west-3a", "i-aaa"), ec2Node("ip-2", "eu-west-3b", "i-bbb")},
71+
nodes: []string{"ip-1", "ip-2"},
72+
opts: newDrainOpts(false),
73+
wantTerminated: []string{"i-aaa", "i-bbb"},
74+
wantUnschedulable: map[string]bool{"ip-1": true, "ip-2": true},
75+
},
76+
{
77+
name: "dry-run touches nothing",
78+
objects: []runtime.Object{ec2Node("ip-1", "eu-west-3a", "i-aaa")},
79+
nodes: []string{"ip-1"},
80+
opts: newDrainOpts(true),
81+
wantUnschedulable: map[string]bool{"ip-1": false},
82+
},
83+
{
84+
// Node already gone from K8s ⇒ no instance ID extracted ⇒
85+
// no TerminateInstances call.
86+
name: "node already gone skips terminate",
87+
nodes: []string{"ip-1"},
88+
opts: newDrainOpts(false),
89+
},
90+
{
91+
// Non-EC2 providerID can't yield an instance ID, but the
92+
// node is still cordoned and drained.
93+
name: "non-EC2 providerID skips terminate but still cordons",
94+
objects: []runtime.Object{gceNode},
95+
nodes: []string{"ip-1"},
96+
opts: newDrainOpts(false),
97+
wantUnschedulable: map[string]bool{"ip-1": true},
98+
},
99+
{
100+
// Safety regression: drain failure must prevent the
101+
// instance from being terminated (otherwise the EC2 dies
102+
// mid-grace-period).
103+
name: "drain failure prevents terminate",
104+
objects: []runtime.Object{ec2Node("ip-stuck", "eu-west-3a", "i-aaaaaaaaaaaaaaaaa"), stuckPod()},
105+
installEvictionReactor: true,
106+
nodes: []string{"ip-stuck"},
107+
opts: fastDrain,
108+
wantErr: true,
109+
},
110+
{
111+
// A TerminateInstances failure on one node must propagate as an
112+
// error yet not stop the loop: every drained node's instance is
113+
// still attempted.
114+
name: "terminate failure propagates but loop continues",
115+
objects: []runtime.Object{ec2Node("ip-1", "eu-west-3a", "i-aaa"), ec2Node("ip-2", "eu-west-3b", "i-bbb")},
116+
nodes: []string{"ip-1", "ip-2"},
117+
opts: newDrainOpts(false),
118+
stubErr: errors.New("terminate boom"),
119+
wantErr: true,
120+
wantTerminated: []string{"i-aaa", "i-bbb"},
121+
},
122+
} {
123+
t.Run(tc.name, func(t *testing.T) {
124+
client := fake.NewClientset(tc.objects...)
125+
if tc.installEvictionReactor {
126+
installPodEvictionReactor(client)
127+
}
128+
stub := &stubEC2{err: tc.stubErr}
129+
130+
err := evictStandalone(t.Context(), client, stub, tc.nodes, tc.opts)
131+
if tc.wantErr {
132+
require.Error(t, err)
133+
} else {
134+
require.NoError(t, err)
135+
}
136+
if tc.wantTerminated == nil {
137+
assert.Empty(t, stub.terminated)
138+
} else {
139+
assert.ElementsMatch(t, tc.wantTerminated, stub.terminated)
140+
}
141+
for nodeName, want := range tc.wantUnschedulable {
142+
got, getErr := client.CoreV1().Nodes().Get(t.Context(), nodeName, metav1.GetOptions{})
143+
require.NoError(t, getErr)
144+
assert.Equal(t, want, got.Spec.Unschedulable, "Spec.Unschedulable for %s", nodeName)
145+
}
146+
})
147+
}
148+
}
149+
150+
// TestEvictStandaloneCordonsAllBeforeDraining locks in the cordon-all-up-front
151+
// behavior: every node of the group is cordoned before ANY node starts
152+
// draining, so a pod evicted off one node is never rescheduled onto a sibling
153+
// node that is itself about to be drained. It asserts that at the moment ip-1's
154+
// pod is evicted, the sibling ip-2 has already been cordoned.
155+
func TestEvictStandaloneCordonsAllBeforeDraining(t *testing.T) {
156+
ec2Node := func(name, az, id string) *corev1.Node {
157+
return &corev1.Node{
158+
ObjectMeta: metav1.ObjectMeta{Name: name},
159+
Spec: corev1.NodeSpec{ProviderID: "aws:///" + az + "/" + id},
160+
}
161+
}
162+
pod1 := &corev1.Pod{
163+
ObjectMeta: metav1.ObjectMeta{Name: "p1", Namespace: "default"},
164+
Spec: corev1.PodSpec{NodeName: "ip-1"},
165+
}
166+
client := fake.NewClientset(ec2Node("ip-1", "eu-west-3a", "i-aaa"), ec2Node("ip-2", "eu-west-3b", "i-bbb"), pod1)
167+
168+
var (
169+
recorded bool
170+
node2CordonedAtFirstEviction bool
171+
)
172+
// On the first pod eviction (ip-1's drain), record whether ip-2 is already
173+
// cordoned, then delete the evicted pod so the node becomes empty and the
174+
// drain completes. Read/mutate via the tracker, not the typed client:
175+
// Fake.Invokes holds a global lock for the whole reaction, so re-entering
176+
// the typed client from inside a reactor would deadlock.
177+
client.PrependReactor("create", "pods", func(action clienttesting.Action) (bool, runtime.Object, error) {
178+
ca, ok := action.(clienttesting.CreateAction)
179+
if !ok || ca.GetSubresource() != "eviction" {
180+
return false, nil, nil
181+
}
182+
if !recorded {
183+
recorded = true
184+
if obj, err := client.Tracker().Get(corev1.SchemeGroupVersion.WithResource("nodes"), "", "ip-2"); err == nil {
185+
node2CordonedAtFirstEviction = obj.(*corev1.Node).Spec.Unschedulable
186+
}
187+
}
188+
evic := ca.GetObject().(*policyv1.Eviction)
189+
_ = client.Tracker().Delete(corev1.SchemeGroupVersion.WithResource("pods"), evic.Namespace, evic.Name)
190+
return true, ca.GetObject(), nil
191+
})
192+
193+
stub := &stubEC2{}
194+
err := evictStandalone(t.Context(), client, stub, []string{"ip-1", "ip-2"}, newDrainOpts(false))
195+
require.NoError(t, err)
196+
assert.True(t, node2CordonedAtFirstEviction, "ip-2 must be cordoned before ip-1 starts draining")
197+
assert.ElementsMatch(t, []string{"i-aaa", "i-bbb"}, stub.terminated)
198+
}

0 commit comments

Comments
 (0)