Skip to content

Commit 306e23a

Browse files
L3n41cL3n41c
authored
[CASCL-1386] (4/11) Add evict-legacy-nodes preflight warnings (#3163)
* [CASCL-1386] Add evict-legacy-nodes command skeleton 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 evict-legacy-nodes execution plan building 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 evict-legacy-nodes plan display and confirmation 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 evict-legacy-nodes preflight warnings 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 evict-legacy-nodes preflight warnings 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 0206a27 commit 306e23a

3 files changed

Lines changed: 188 additions & 3 deletions

File tree

‎cmd/kubectl-datadog/autoscaling/cluster/evict/preflight.go‎

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,73 @@ package evict
22

33
import (
44
"context"
5+
"fmt"
6+
"log"
57

8+
"github.com/fatih/color"
9+
"github.com/samber/lo"
10+
"k8s.io/apimachinery/pkg/api/meta"
611
"k8s.io/cli-runtime/pkg/genericclioptions"
712
"sigs.k8s.io/controller-runtime/pkg/client"
13+
karpv1 "sigs.k8s.io/karpenter/pkg/apis/v1"
814
)
915

10-
func runPreflightWarnings(ctx context.Context, streams genericclioptions.IOStreams, ctrlClient client.Client, targets []Target) {
11-
panic("TODO: runPreflightWarnings — implemented in PR https://github.com/DataDog/datadog-operator/pull/3163")
16+
// ddNodePoolCreatedLabel is the label every Datadog autoscaling product (this
17+
// CLI and the cluster agent) puts on the Karpenter NodePools it creates. Used
18+
// here to identify "Datadog-side" NodePools when comparing weights against
19+
// user-side ones.
20+
const ddNodePoolCreatedLabel = "autoscaling.datadoghq.com/created"
21+
22+
// warnKarpenterWeightConflicts compares the spec.weight of every user-managed
23+
// Karpenter NodePool in the cluster against the max spec.weight among
24+
// Datadog-managed NodePools. When a user NodePool has a weight >= the Datadog
25+
// max, freshly evicted pods may be re-scheduled onto a new node from a user
26+
// NodePool, defeating the migration.
27+
//
28+
// The check spans all NodePools, not just the eviction targets: Karpenter
29+
// arbitrates provisioning across every NodePool by weight, so a high-weight
30+
// user NodePool can capture evicted pods even when it is not itself a target
31+
// (e.g. the run targets an ASG, or the user NodePool currently has no nodes
32+
// and is therefore absent from the eviction plan).
33+
//
34+
// This is a non-blocking pre-flight warning: it flags a situation that may
35+
// surprise the operator after the fact but does not prevent eviction. Pure
36+
// best-effort — failures here are logged but do not abort the run.
37+
func warnKarpenterWeightConflicts(ctx context.Context, streams genericclioptions.IOStreams, ctrlClient client.Client) {
38+
list := &karpv1.NodePoolList{}
39+
if err := ctrlClient.List(ctx, list); err != nil {
40+
if meta.IsNoMatchError(err) {
41+
return
42+
}
43+
log.Printf("Warning: failed to list NodePools for pre-flight weight check: %v", err)
44+
return
45+
}
46+
47+
ddMaxWeight := int32(-1)
48+
for i := range list.Items {
49+
np := &list.Items[i]
50+
if np.Labels[ddNodePoolCreatedLabel] != "true" {
51+
continue
52+
}
53+
if w := lo.FromPtr(np.Spec.Weight); w > ddMaxWeight {
54+
ddMaxWeight = w
55+
}
56+
}
57+
58+
if ddMaxWeight < 0 {
59+
return
60+
}
61+
62+
for i := range list.Items {
63+
np := &list.Items[i]
64+
if np.Labels[ddNodePoolCreatedLabel] == "true" {
65+
continue
66+
}
67+
if w := lo.FromPtr(np.Spec.Weight); w >= ddMaxWeight {
68+
fmt.Fprintln(streams.ErrOut, color.YellowString(
69+
"⚠ user NodePool %q has spec.weight=%d ≥ Datadog NodePool max weight=%d; evicted pods may be re-scheduled onto a freshly provisioned node from a user NodePool.",
70+
np.Name, w, ddMaxWeight,
71+
))
72+
}
73+
}
1274
}

‎cmd/kubectl-datadog/autoscaling/cluster/evict/preflight_test.go‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package evict
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
"k8s.io/apimachinery/pkg/runtime"
10+
"k8s.io/apimachinery/pkg/runtime/schema"
11+
"k8s.io/cli-runtime/pkg/genericclioptions"
12+
"k8s.io/client-go/kubernetes/scheme"
13+
"k8s.io/utils/ptr"
14+
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
15+
ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
16+
karpv1 "sigs.k8s.io/karpenter/pkg/apis/v1"
17+
)
18+
19+
func newKarpenterScheme(t *testing.T) *runtime.Scheme {
20+
t.Helper()
21+
sch := runtime.NewScheme()
22+
require.NoError(t, scheme.AddToScheme(sch))
23+
gv := schema.GroupVersion{Group: "karpenter.sh", Version: "v1"}
24+
sch.AddKnownTypes(gv, &karpv1.NodePool{}, &karpv1.NodePoolList{})
25+
metav1.AddToGroupVersion(sch, gv)
26+
return sch
27+
}
28+
29+
func mkNodePool(name string, weight *int32, datadogManaged bool) *karpv1.NodePool {
30+
labels := map[string]string{}
31+
if datadogManaged {
32+
labels[ddNodePoolCreatedLabel] = "true"
33+
}
34+
return &karpv1.NodePool{
35+
ObjectMeta: metav1.ObjectMeta{Name: name, Labels: labels},
36+
Spec: karpv1.NodePoolSpec{Weight: weight},
37+
}
38+
}
39+
40+
func TestWarnKarpenterWeightConflicts(t *testing.T) {
41+
// The check is cluster-wide: it never looks at the eviction targets, only at
42+
// the NodePools present in the cluster, because Karpenter arbitrates
43+
// provisioning across every NodePool by weight.
44+
for _, tc := range []struct {
45+
name string
46+
nodePools []ctrlclient.Object
47+
// wantContains lists substrings that MUST appear in the stderr output.
48+
wantContains []string
49+
// wantWarnEmpty asserts the stderr output is empty (no warning fired).
50+
wantWarnEmpty bool
51+
}{
52+
{
53+
// No Datadog NodePool ⇒ nothing to migrate onto ⇒ no warning, even
54+
// though a user NodePool exists.
55+
name: "no Datadog NodePool ⇒ no warning",
56+
nodePools: []ctrlclient.Object{
57+
mkNodePool("user-np", ptr.To(int32(50)), false),
58+
},
59+
wantWarnEmpty: true,
60+
},
61+
{
62+
// User NP weight < Datadog NP weight ⇒ no conflict, no warning.
63+
name: "user weight below Datadog weight",
64+
nodePools: []ctrlclient.Object{
65+
mkNodePool("dd-np", ptr.To(int32(100)), true),
66+
mkNodePool("user-np", ptr.To(int32(50)), false),
67+
},
68+
wantWarnEmpty: true,
69+
},
70+
{
71+
// User NP weight > Datadog NP weight ⇒ warns (`>=` check). The user
72+
// NodePool is not an eviction target here, demonstrating the
73+
// cluster-wide scope.
74+
name: "user weight above Datadog weight warns",
75+
nodePools: []ctrlclient.Object{
76+
mkNodePool("dd-np", ptr.To(int32(10)), true),
77+
mkNodePool("user-np", ptr.To(int32(50)), false),
78+
},
79+
wantContains: []string{"user-np", "weight=50"},
80+
},
81+
{
82+
// Tie at the max weight 100: mirrors the cluster-agent edge case
83+
// where a target NodePool already at weight 100 yields an equal
84+
// (not higher) Datadog replica weight. The `>=` check catches it.
85+
name: "equal weight at max 100 warns",
86+
nodePools: []ctrlclient.Object{
87+
mkNodePool("dd-np", ptr.To(int32(100)), true),
88+
mkNodePool("user-np", ptr.To(int32(100)), false),
89+
},
90+
wantContains: []string{"user-np", "weight=100"},
91+
},
92+
{
93+
// Both nil ⇒ both default to 0 ⇒ equal-weight conflict warns.
94+
name: "nil weights both default to 0 (equal-weight conflict)",
95+
nodePools: []ctrlclient.Object{
96+
mkNodePool("dd-np", nil, true),
97+
mkNodePool("user-np", nil, false),
98+
},
99+
wantContains: []string{"user-np"},
100+
},
101+
} {
102+
t.Run(tc.name, func(t *testing.T) {
103+
require.True(t, tc.wantWarnEmpty || len(tc.wantContains) > 0,
104+
"test case must assert on the output (set wantWarnEmpty or wantContains)")
105+
106+
cli := ctrlfake.NewClientBuilder().
107+
WithScheme(newKarpenterScheme(t)).
108+
WithObjects(tc.nodePools...).
109+
Build()
110+
streams, _, _, errBuf := genericclioptions.NewTestIOStreams()
111+
112+
warnKarpenterWeightConflicts(t.Context(), streams, cli)
113+
114+
out := errBuf.String()
115+
if tc.wantWarnEmpty {
116+
assert.Empty(t, out)
117+
}
118+
for _, s := range tc.wantContains {
119+
assert.Contains(t, out, s)
120+
}
121+
})
122+
}
123+
}

‎cmd/kubectl-datadog/autoscaling/cluster/evict/run.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func Run(ctx context.Context, streams genericclioptions.IOStreams, configFlags *
104104
return nil
105105
}
106106

107-
runPreflightWarnings(ctx, streams, cli.K8sClient, targets)
107+
warnKarpenterWeightConflicts(ctx, streams, cli.K8sClient)
108108
printPlan(streams, info, targets, !opts.SkipCA, opts.EnsurePDBs)
109109

110110
if !opts.Yes && !opts.DryRun {

0 commit comments

Comments
 (0)