[clusteragent/admission/mutate] Mark injected volumes as safe to evic…

…t for the k8s cluster autoscaler (#28993)

pkg/clusteragent/admission/mutate/agent_sidecar/agent_sidecar_test.go

@@ -17,6 +17,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	mutatecommon "github.com/DataDog/datadog-agent/pkg/clusteragent/admission/mutate/common"
 	configmock "github.com/DataDog/datadog-agent/pkg/config/mock"
 	apicommon "github.com/DataDog/datadog-agent/pkg/util/kubernetes/apiserver/common"
 	"github.com/DataDog/datadog-agent/pkg/util/pointer"
@@ -180,6 +181,9 @@ func TestInjectAgentSidecar(t *testing.T) {
 				return &corev1.Pod{
 					ObjectMeta: metav1.ObjectMeta{
 						Name: "pod-name",
+						Annotations: map[string]string{
+							mutatecommon.K8sAutoscalerSafeToEvictVolumesAnnotation: "ddsockets",
+						},
 					},
 					Spec: corev1.PodSpec{
 						ShareProcessNamespace: pointer.Ptr(true),
@@ -297,6 +301,9 @@ func TestInjectAgentSidecar(t *testing.T) {
 				return &corev1.Pod{
 					ObjectMeta: metav1.ObjectMeta{
 						Name: "pod-name",
+						Annotations: map[string]string{
+							mutatecommon.K8sAutoscalerSafeToEvictVolumesAnnotation: "ddsockets",
+						},
 					},
 					Spec: corev1.PodSpec{
 						ShareProcessNamespace: pointer.Ptr(true),

pkg/clusteragent/admission/mutate/agent_sidecar/providers.go

@@ -92,6 +92,10 @@ func applyFargateOverrides(pod *corev1.Pod) (bool, error) {
 
 	volume, volumeMount := socketsVolume()
 	injected := common.InjectVolume(pod, volume, volumeMount)
+	if injected {
+		common.MarkVolumeAsSafeToEvictForAutoscaler(pod, volume.Name)
+	}
+
 	mutated = mutated || injected
 
 	// ShareProcessNamespace is required for the process collection feature

pkg/clusteragent/admission/mutate/agent_sidecar/providers_test.go

@@ -14,7 +14,9 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	mutatecommon "github.com/DataDog/datadog-agent/pkg/clusteragent/admission/mutate/common"
 	configmock "github.com/DataDog/datadog-agent/pkg/config/mock"
 	"github.com/DataDog/datadog-agent/pkg/util/pointer"
 )
@@ -99,6 +101,11 @@ func TestApplyProviderOverrides(t *testing.T) {
 				},
 			},
 			expectedPodAfterOverride: &corev1.Pod{
+				ObjectMeta: v1.ObjectMeta{
+					Annotations: map[string]string{
+						mutatecommon.K8sAutoscalerSafeToEvictVolumesAnnotation: "ddsockets",
+					},
+				},
 				Spec: corev1.PodSpec{
 					ShareProcessNamespace: pointer.Ptr(true),
 					Containers: []corev1.Container{
@@ -203,6 +210,11 @@ func TestApplyProviderOverrides(t *testing.T) {
 				},
 			},
 			expectedPodAfterOverride: &corev1.Pod{
+				ObjectMeta: v1.ObjectMeta{
+					Annotations: map[string]string{
+						mutatecommon.K8sAutoscalerSafeToEvictVolumesAnnotation: "ddsockets",
+					},
+				},
 				Spec: corev1.PodSpec{
 					ShareProcessNamespace: pointer.Ptr(true),
 					Containers: []corev1.Container{

pkg/clusteragent/admission/mutate/autoinstrumentation/auto_instrumentation_test.go

@@ -262,6 +262,10 @@ func TestInjectAutoInstruConfigV2(t *testing.T) {
 			require.Equal(t, etcVolume.Name, tt.pod.Spec.Volumes[1].Name,
 				"expected datadog-etc volume to be injected")
 
+			volumesMarkedAsSafeToEvict := strings.Split(tt.pod.Annotations[common.K8sAutoscalerSafeToEvictVolumesAnnotation], ",")
+			require.Contains(t, volumesMarkedAsSafeToEvict, volumeName, "expected volume %s to be marked as safe to evict", volumeName)
+			require.Contains(t, volumesMarkedAsSafeToEvict, etcVolume.Name, "expected volume %s to be marked as safe to evict", etcVolume.Name)
+
 			require.Equal(t, len(tt.libInfo.libs)+1, len(tt.pod.Spec.InitContainers),
 				"expected there to be one more than the number of libs to inject for init containers")
 

pkg/clusteragent/admission/mutate/autoinstrumentation/mutators.go

@@ -101,6 +101,8 @@ func (v volume) mount(mount corev1.VolumeMount) volumeMount {
 
 // mutatePod implements podMutator for volume.
 func (v volume) mutatePod(pod *corev1.Pod) error {
+	common.MarkVolumeAsSafeToEvictForAutoscaler(pod, v.Name)
+
 	vol := v.Volume
 	for idx, i := range pod.Spec.Volumes {
 		if i.Name == v.Volume.Name {

pkg/clusteragent/admission/mutate/common/common.go

@@ -11,7 +11,9 @@ package common
 import (
 	"encoding/json"
 	"fmt"
+	"slices"
 	"strconv"
+	"strings"
 
 	"github.com/wI2L/jsondiff"
 	corev1 "k8s.io/api/core/v1"
@@ -22,6 +24,10 @@ import (
 	"github.com/DataDog/datadog-agent/pkg/util/log"
 )
 
+// K8sAutoscalerSafeToEvictVolumesAnnotation is the annotation used by the
+// Kubernetes cluster-autoscaler to mark a volume as safe to evict
+const K8sAutoscalerSafeToEvictVolumesAnnotation = "cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes"
+
 // MutationFunc is a function that mutates a pod
 type MutationFunc func(pod *corev1.Pod, ns string, cl dynamic.Interface) (bool, error)
 
@@ -181,3 +187,29 @@ func ContainerRegistry(specificConfigOpt string) string {
 
 	return config.Datadog().GetString("admission_controller.container_registry")
 }
+
+// MarkVolumeAsSafeToEvictForAutoscaler adds the Kubernetes cluster-autoscaler
+// annotation to the given pod, marking the specified local volume as safe to
+// evict. This annotation allows the cluster-autoscaler to evict pods with the
+// local volume mounted, enabling the node to scale down if necessary.
+// This function will not add the volume to the annotation if it is already
+// there.
+// Ref: https://github.com/kubernetes/autoscaler/blob/cluster-autoscaler-release-1.31/cluster-autoscaler/FAQ.md#what-types-of-pods-can-prevent-ca-from-removing-a-node
+func MarkVolumeAsSafeToEvictForAutoscaler(pod *corev1.Pod, volumeNameToAdd string) {
+	if pod.Annotations == nil {
+		pod.Annotations = make(map[string]string)
+	}
+
+	currentVolumes := pod.Annotations[K8sAutoscalerSafeToEvictVolumesAnnotation]
+	var volumeList []string
+	if currentVolumes != "" {
+		volumeList = strings.Split(currentVolumes, ",")
+	}
+
+	if slices.Contains(volumeList, volumeNameToAdd) {
+		return // Volume already in the list, no need to add
+	}
+
+	volumeList = append(volumeList, volumeNameToAdd)
+	pod.Annotations[K8sAutoscalerSafeToEvictVolumesAnnotation] = strings.Join(volumeList, ",")
+}

pkg/clusteragent/admission/mutate/common/common_test.go

@@ -218,3 +218,50 @@ func Test_injectVolume(t *testing.T) {
 		})
 	}
 }
+
+func TestMarkVolumeAsSafeToEvictForAutoscaler(t *testing.T) {
+	tests := []struct {
+		name                                  string
+		currentSafeToEvictAnnotationValue     string
+		volumeToAdd                           string
+		expectedNewSafeToEvictAnnotationValue string
+	}{
+		{
+			name:                                  "the annotation is not set",
+			currentSafeToEvictAnnotationValue:     "",
+			volumeToAdd:                           "datadog",
+			expectedNewSafeToEvictAnnotationValue: "datadog",
+		},
+		{
+			name:                                  "the annotation is already set",
+			currentSafeToEvictAnnotationValue:     "someVolume1,someVolume2",
+			volumeToAdd:                           "datadog",
+			expectedNewSafeToEvictAnnotationValue: "someVolume1,someVolume2,datadog",
+		},
+		{
+			name:                                  "the annotation is already set and the volume is already in the list",
+			currentSafeToEvictAnnotationValue:     "someVolume1,someVolume2",
+			volumeToAdd:                           "someVolume2",
+			expectedNewSafeToEvictAnnotationValue: "someVolume1,someVolume2",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(_ *testing.T) {
+			annotations := map[string]string{}
+			if test.currentSafeToEvictAnnotationValue != "" {
+				annotations["cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes"] = test.currentSafeToEvictAnnotationValue
+			}
+			pod := FakePodWithAnnotations(annotations)
+
+			MarkVolumeAsSafeToEvictForAutoscaler(pod, test.volumeToAdd)
+
+			assert.Equal(
+				t,
+				test.expectedNewSafeToEvictAnnotationValue,
+				pod.Annotations["cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes"],
+			)
+		})
+	}
+
+}

pkg/clusteragent/admission/mutate/config/config.go

@@ -186,6 +186,10 @@ func (w *Webhook) inject(pod *corev1.Pod, _ string, _ dynamic.Interface) (bool,
 	case socket:
 		volume, volumeMount := buildVolume(DatadogVolumeName, config.Datadog().GetString("admission_controller.inject_config.socket_path"), true)
 		injectedVol := common.InjectVolume(pod, volume, volumeMount)
+		if injectedVol {
+			common.MarkVolumeAsSafeToEvictForAutoscaler(pod, DatadogVolumeName)
+		}
+
 		injectedEnv := common.InjectEnv(pod, traceURLSocketEnvVar)
 		injectedEnv = common.InjectEnv(pod, dogstatsdURLSocketEnvVar) || injectedEnv
 		injectedConfig = injectedEnv || injectedVol

pkg/clusteragent/admission/mutate/config/config_test.go

@@ -315,6 +315,7 @@ func TestInjectSocket(t *testing.T) {
 	assert.Equal(t, pod.Spec.Volumes[0].Name, "datadog")
 	assert.Equal(t, pod.Spec.Volumes[0].VolumeSource.HostPath.Path, "/var/run/datadog")
 	assert.Equal(t, *pod.Spec.Volumes[0].VolumeSource.HostPath.Type, corev1.HostPathDirectoryOrCreate)
+	assert.Equal(t, "datadog", pod.Annotations[mutatecommon.K8sAutoscalerSafeToEvictVolumesAnnotation])
 }
 
 func TestInjectSocketWithConflictingVolumeAndInitContainer(t *testing.T) {

pkg/clusteragent/admission/mutate/cwsinstrumentation/cws_instrumentation.go

@@ -714,6 +714,8 @@ func injectCWSVolume(pod *corev1.Pod) {
 		Name:         cwsVolumeName,
 		VolumeSource: volumeSource,
 	})
+
+	common.MarkVolumeAsSafeToEvictForAutoscaler(pod, cwsVolumeName)
 }
 
 func injectCWSVolumeMount(container *corev1.Container) {

pkg/clusteragent/admission/mutate/cwsinstrumentation/cws_instrumentation_test.go

@@ -875,6 +875,7 @@ func Test_injectCWSPodInstrumentation(t *testing.T) {
 						require.NotNil(t, annotations, "failed to annotate pod")
 						if annotations != nil {
 							require.Equal(t, cwsInstrumentationPodAnotationReady, annotations[cwsInstrumentationPodAnotationStatus], "CWS instrumentation annotation is missing")
+							require.Equal(t, cwsVolumeName, annotations[common.K8sAutoscalerSafeToEvictVolumesAnnotation], "CWS instrumentation volume should be marked as safe to evict")
 						}
 					} else {
 						testNoInjectedCWSVolume(t, tt.args.pod)

releasenotes-dca/notes/configure-volumes-for-k8s-autoscaler-d96fa7357c4be699.yaml

@@ -0,0 +1,12 @@
+# Each section from every release note are combined when the
+# CHANGELOG.rst is rendered. So the text needs to be worded so that
+# it does not depend on any information only available in another
+# section. This may mean repeating some details, but each section
+# must be readable independently of the other.
+#
+# Each section note must be formatted as reStructuredText.
+---
+fixes:
+  - |
+    Fixed an issue that prevented the Kubernetes autoscaler from evicting pods
+    injected by the Admission Controller.

test/new-e2e/tests/containers/k8s_test.go

@@ -983,8 +983,13 @@ func (suite *k8sSuite) testAdmissionControllerPod(namespace string, name string,
 		}
 	}
 
+	volumesMarkedAsSafeToEvict := strings.Split(
+		pod.Annotations["cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes"], ",",
+	)
+
 	if suite.Contains(hostPathVolumes, "datadog") {
 		suite.Equal("/var/run/datadog", hostPathVolumes["datadog"].Path)
+		suite.Contains(volumesMarkedAsSafeToEvict, "datadog")
 	}
 
 	volumeMounts := make(map[string][]string)
@@ -1006,7 +1011,14 @@ func (suite *k8sSuite) testAdmissionControllerPod(namespace string, name string,
 			}
 		}
 
-		suite.Contains(emptyDirVolumes, "datadog-auto-instrumentation")
+		if suite.Contains(emptyDirVolumes, "datadog-auto-instrumentation") {
+			suite.Contains(volumesMarkedAsSafeToEvict, "datadog-auto-instrumentation")
+		}
+
+		if suite.Contains(emptyDirVolumes, "datadog-auto-instrumentation-etc") {
+			suite.Contains(volumesMarkedAsSafeToEvict, "datadog-auto-instrumentation-etc")
+		}
+
 		if suite.Contains(volumeMounts, "datadog-auto-instrumentation") {
 			suite.ElementsMatch([]string{
 				"/opt/datadog-packages/datadog-apm-inject",