Skip to content

Commit 37b4085

Browse files
committed
[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 7dc54d8 commit 37b4085

2 files changed

Lines changed: 233 additions & 1 deletion

File tree

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

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,49 @@ 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+
22+
for _, node := range cordoned {
23+
nodeName := node.Name
24+
id, hasInstanceID := commonaws.ExtractEC2InstanceID(node)
25+
if !hasInstanceID {
26+
log.Printf("Warning: node %s has unexpected providerID %q; cannot terminate the underlying instance", nodeName, node.Spec.ProviderID)
27+
}
28+
if err := drainNode(ctx, clientset, nodeName, drainOpts); err != nil {
29+
errs = append(errs, fmt.Errorf("drain node %s: %w", nodeName, err))
30+
continue // do NOT terminate this instance: workloads are still on it
31+
}
32+
// The node drained cleanly: terminate its instance right away so its
33+
// capacity is freed without waiting for the rest of the group.
34+
if !hasInstanceID {
35+
continue
36+
}
37+
if drainOpts.DryRun {
38+
log.Printf("[dry-run] would terminate standalone instance %s", id)
39+
continue
40+
}
41+
if _, err := ec2API.TerminateInstances(ctx, &ec2.TerminateInstancesInput{
42+
InstanceIds: []string{id},
43+
}); err != nil {
44+
errs = append(errs, fmt.Errorf("terminate instance %s: %w", id, err))
45+
} else {
46+
log.Printf("Terminated standalone EC2 instance %s.", id)
47+
}
48+
}
49+
return errors.Join(errs...)
1650
}
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)