From 872f903091fe60e3b1f270bea3bd62286912c759 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Mon, 14 Sep 2026 18:05:01 -0400 Subject: [PATCH] Add configurable tolerations for PodVolumeBackup and data mover pods (#9575) * Remove toleration whitelist for PodVolumeBackup and data mover pods Instead of filtering tolerations through a hardcoded allowlist (ThirdPartyTolerations), inherit all tolerations from the node-agent daemonset for PodVolumeBackup/Restore and DataUpload/Download pods, and from the Velero deployment for maintenance jobs. This enables backups and restores on nodes with custom NoExecute taints, which was previously impossible since only two specific toleration keys were whitelisted. Fixes #9476 Signed-off-by: Tiger Kaovilai Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy * Fix codespell: replace 'whitelist' with 'allowlist' in changelog Signed-off-by: Tiger Kaovilai Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy * Implement deduplication of tolerations and add unit tests for the new function Signed-off-by: Tiger Kaovilai * Merge node-agent-configmap tolerations with third-party allowlist Add a `tolerations` field to the node-agent-configmap so operators can declare hosting-pod tolerations explicitly, per blackpiglet's review feedback that tolerations shouldn't be read from the DaemonSet alone. These are merged with (and deduplicated against) DaemonSet tolerations matching the existing third-party allowlist (kubernetes.azure.com/scalesetpriority, CriticalAddonsOnly), restoring that allowlist per the follow-up suggestion to keep inheriting it alongside the new config option. The toleration dedup helper is moved from pkg/exposer to pkg/util/kube (exported as DeduplicateTolerations) so it can be shared with pkg/nodeagent without an import cycle. Signed-off-by: Tiger Kaovilai * Fix testifylint finding in TestGetTolerations golangci-lint v2.12.0 (pinned in pr-linter-check.yml) flagged the shared assert.Equal after the if/else as require-error: use require for the error assertion so each branch is self-contained, matching the pattern used elsewhere in this file. Signed-off-by: Tiger Kaovilai * Document toleration merge priority in GetTolerations Per blackpiglet's review feedback: clarify that configured tolerations take priority over allowlisted daemonset tolerations because they're appended first and DeduplicateTolerations keeps only the first occurrence of each exact (Key, Operator, Value, Effect) combination. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Tiger Kaovilai --------- Signed-off-by: Tiger Kaovilai Co-authored-by: Claude Co-authored-by: Happy --- changelogs/unreleased/9575-kaovilai | 1 + pkg/cmd/cli/nodeagent/server.go | 10 ++ pkg/controller/data_download_controller.go | 15 +- .../data_download_controller_test.go | 4 +- pkg/controller/data_upload_controller.go | 15 +- pkg/controller/data_upload_controller_test.go | 6 +- .../pod_volume_backup_controller.go | 15 +- .../pod_volume_backup_controller_test.go | 2 + .../pod_volume_restore_controller.go | 15 +- .../pod_volume_restore_controller_test.go | 2 + pkg/exposer/csi_snapshot.go | 4 +- pkg/exposer/generic_restore.go | 4 +- pkg/exposer/pod_volume.go | 4 +- pkg/nodeagent/node_agent.go | 33 ++++- pkg/nodeagent/node_agent_test.go | 138 +++++++++++------- pkg/repository/maintenance/maintenance.go | 17 +-- .../maintenance/maintenance_test.go | 136 ++++------------- pkg/types/node_agent.go | 6 + pkg/util/kube/toleration.go | 37 +++++ pkg/util/kube/toleration_test.go | 86 +++++++++++ .../node-agent-configmap.md | 42 +++++- 21 files changed, 368 insertions(+), 224 deletions(-) create mode 100644 changelogs/unreleased/9575-kaovilai create mode 100644 pkg/util/kube/toleration.go create mode 100644 pkg/util/kube/toleration_test.go diff --git a/changelogs/unreleased/9575-kaovilai b/changelogs/unreleased/9575-kaovilai new file mode 100644 index 000000000..7c7c1817c --- /dev/null +++ b/changelogs/unreleased/9575-kaovilai @@ -0,0 +1 @@ +Add configurable tolerations to the node-agent-configmap for PodVolumeBackup and data mover pods; DaemonSet tolerations matching the existing third-party allowlist (kubernetes.azure.com/scalesetpriority, CriticalAddonsOnly) continue to be inherited automatically and are merged with the configmap value diff --git a/pkg/cmd/cli/nodeagent/server.go b/pkg/cmd/cli/nodeagent/server.go index c1442aab0..973e993dd 100644 --- a/pkg/cmd/cli/nodeagent/server.go +++ b/pkg/cmd/cli/nodeagent/server.go @@ -384,6 +384,12 @@ func (s *nodeAgentServer) run() { s.logger.Infof("Using customized pod annotations %+v", podAnnotations) } + var tolerations []corev1api.Toleration + if s.dataPathConfigs != nil && len(s.dataPathConfigs.Tolerations) > 0 { + tolerations = s.dataPathConfigs.Tolerations + s.logger.Infof("Using customized tolerations %+v", tolerations) + } + if s.backupRepoConfigs != nil { s.logger.Infof("Using backup repo config %v", s.backupRepoConfigs) } else if cachePVCConfig != nil { @@ -412,6 +418,7 @@ func (s *nodeAgentServer) run() { privilegedFsBackup, podLabels, podAnnotations, + tolerations, ) if err := pvbReconciler.SetupWithManager(s.mgr); err != nil { s.logger.Fatal(err, "unable to create controller", "controller", constant.ControllerPodVolumeBackup) @@ -435,6 +442,7 @@ func (s *nodeAgentServer) run() { s.repoConfigMgr, podLabels, podAnnotations, + tolerations, ) if err := pvrReconciler.SetupWithManager(s.mgr); err != nil { s.logger.WithError(err).Fatal("Unable to create the pod volume restore controller") @@ -459,6 +467,7 @@ func (s *nodeAgentServer) run() { podLabels, podAnnotations, csiSnapshotMetadataServiceConfigs, + tolerations, ) if err := dataUploadReconciler.SetupWithManager(s.mgr); err != nil { s.logger.WithError(err).Fatal("Unable to create the data upload controller") @@ -490,6 +499,7 @@ func (s *nodeAgentServer) run() { podLabels, podAnnotations, csiSnapshotMetadataServiceConfigs, + tolerations, ) if err := dataDownloadReconciler.SetupWithManager(s.mgr); err != nil { diff --git a/pkg/controller/data_download_controller.go b/pkg/controller/data_download_controller.go index 3507edfe5..513a3eefc 100644 --- a/pkg/controller/data_download_controller.go +++ b/pkg/controller/data_download_controller.go @@ -81,6 +81,7 @@ type DataDownloadReconciler struct { podLabels map[string]string podAnnotations map[string]string snapshotMetadataServiceConfigs *velerotypes.CSISnapshotMetadataService + tolerations []corev1api.Toleration } func NewDataDownloadReconciler( @@ -103,6 +104,7 @@ func NewDataDownloadReconciler( podLabels map[string]string, podAnnotations map[string]string, snapshotMetadataServiceConfigs *velerotypes.CSISnapshotMetadataService, + tolerations []corev1api.Toleration, ) *DataDownloadReconciler { return &DataDownloadReconciler{ client: client, @@ -126,6 +128,7 @@ func NewDataDownloadReconciler( podLabels: podLabels, podAnnotations: podAnnotations, snapshotMetadataServiceConfigs: snapshotMetadataServiceConfigs, + tolerations: tolerations, } } @@ -940,15 +943,9 @@ func (r *DataDownloadReconciler) setupExposeParam(dd *velerov2alpha1api.DataDown } } - hostingPodTolerations := []corev1api.Toleration{} - for _, k := range util.ThirdPartyTolerations { - if v, err := nodeagent.GetToleration(context.Background(), r.kubeClient, dd.Namespace, k, nodeOS); err != nil { - if err != nodeagent.ErrNodeAgentTolerationNotFound { - log.WithError(err).Warnf("Failed to check node-agent toleration, skip adding host pod toleration %s", k) - } - } else { - hostingPodTolerations = append(hostingPodTolerations, *v) - } + hostingPodTolerations, err := nodeagent.GetTolerations(context.Background(), r.kubeClient, dd.Namespace, nodeOS, r.tolerations) + if err != nil { + log.WithError(err).Warn("Failed to get node-agent daemonset tolerations, hosting pod will only get configured tolerations") } var cacheVolume *exposer.CacheConfigs diff --git a/pkg/controller/data_download_controller_test.go b/pkg/controller/data_download_controller_test.go index c9026eafa..7fcbb8978 100644 --- a/pkg/controller/data_download_controller_test.go +++ b/pkg/controller/data_download_controller_test.go @@ -151,6 +151,7 @@ func initDataDownloadReconcilerWithError(t *testing.T, objects []any, needError nil, // podLabels nil, // podAnnotations nil, // snapshotMetadataServiceConfigs + nil, // tolerations ), nil } @@ -1464,7 +1465,8 @@ func TestDataDownloadSetupExposeParam(t *testing.T) { nil, // repoConfigMgr (unused when cacheVolumeConfigs is nil) tt.args.customLabels, tt.args.customAnnotations, - nil, + nil, // snapshotMetadataServiceConfigs + nil, // tolerations ) // Act diff --git a/pkg/controller/data_upload_controller.go b/pkg/controller/data_upload_controller.go index 84504153f..5752eb0a4 100644 --- a/pkg/controller/data_upload_controller.go +++ b/pkg/controller/data_upload_controller.go @@ -86,6 +86,7 @@ type DataUploadReconciler struct { podLabels map[string]string podAnnotations map[string]string snapshotMetadataServiceConfigs *velerotypes.CSISnapshotMetadataService + tolerations []corev1api.Toleration } func NewDataUploadReconciler( @@ -107,6 +108,7 @@ func NewDataUploadReconciler( podLabels map[string]string, podAnnotations map[string]string, snapshotMetadataServiceConfigs *velerotypes.CSISnapshotMetadataService, + tolerations []corev1api.Toleration, ) *DataUploadReconciler { return &DataUploadReconciler{ client: client, @@ -134,6 +136,7 @@ func NewDataUploadReconciler( podLabels: podLabels, podAnnotations: podAnnotations, snapshotMetadataServiceConfigs: snapshotMetadataServiceConfigs, + tolerations: tolerations, } } @@ -1010,15 +1013,9 @@ func (r *DataUploadReconciler) setupExposeParam(du *velerov2alpha1api.DataUpload } } - hostingPodTolerations := []corev1api.Toleration{} - for _, k := range util.ThirdPartyTolerations { - if v, err := nodeagent.GetToleration(context.Background(), r.kubeClient, du.Namespace, k, nodeOS); err != nil { - if err != nodeagent.ErrNodeAgentTolerationNotFound { - log.WithError(err).Warnf("Failed to check node-agent toleration, skip adding host pod toleration %s", k) - } - } else { - hostingPodTolerations = append(hostingPodTolerations, *v) - } + hostingPodTolerations, err := nodeagent.GetTolerations(context.Background(), r.kubeClient, du.Namespace, nodeOS, r.tolerations) + if err != nil { + log.WithError(err).Warn("Failed to get node-agent daemonset tolerations, hosting pod will only get configured tolerations") } return &exposer.CSISnapshotExposeParam{ diff --git a/pkg/controller/data_upload_controller_test.go b/pkg/controller/data_upload_controller_test.go index d50ceb749..5b4fc7780 100644 --- a/pkg/controller/data_upload_controller_test.go +++ b/pkg/controller/data_upload_controller_test.go @@ -270,7 +270,8 @@ func initDataUploaderReconcilerWithError(needError ...error) (*DataUploadReconci "", // dataMovePriorityClass nil, // podLabels nil, // podAnnotations - nil, + nil, // snapshotMetadataServiceConfigs + nil, // tolerations ), nil } @@ -1565,7 +1566,8 @@ func TestDataUploadSetupExposeParam(t *testing.T) { "upload-priority", tt.args.customLabels, tt.args.customAnnotations, - nil, + nil, // snapshotMetadataServiceConfigs + nil, // tolerations ) // Act diff --git a/pkg/controller/pod_volume_backup_controller.go b/pkg/controller/pod_volume_backup_controller.go index 399f4a3b8..2bf0b1fd2 100644 --- a/pkg/controller/pod_volume_backup_controller.go +++ b/pkg/controller/pod_volume_backup_controller.go @@ -74,6 +74,7 @@ func NewPodVolumeBackupReconciler( privileged bool, podLabels map[string]string, podAnnotations map[string]string, + tolerations []corev1api.Toleration, ) *PodVolumeBackupReconciler { return &PodVolumeBackupReconciler{ client: client, @@ -93,6 +94,7 @@ func NewPodVolumeBackupReconciler( privileged: privileged, podLabels: podLabels, podAnnotations: podAnnotations, + tolerations: tolerations, } } @@ -116,6 +118,7 @@ type PodVolumeBackupReconciler struct { privileged bool podLabels map[string]string podAnnotations map[string]string + tolerations []corev1api.Toleration } // +kubebuilder:rbac:groups=velero.io,resources=podvolumebackups,verbs=get;list;watch;create;update;patch;delete @@ -867,15 +870,9 @@ func (r *PodVolumeBackupReconciler) setupExposeParam(pvb *velerov1api.PodVolumeB } } - hostingPodTolerations := []corev1api.Toleration{} - for _, k := range util.ThirdPartyTolerations { - if v, err := nodeagent.GetToleration(context.Background(), r.kubeClient, pvb.Namespace, k, nodeOS); err != nil { - if err != nodeagent.ErrNodeAgentTolerationNotFound { - log.WithError(err).Warnf("Failed to check node-agent toleration, skip adding host pod toleration %s", k) - } - } else { - hostingPodTolerations = append(hostingPodTolerations, *v) - } + hostingPodTolerations, err := nodeagent.GetTolerations(context.Background(), r.kubeClient, pvb.Namespace, nodeOS, r.tolerations) + if err != nil { + log.WithError(err).Warn("Failed to get node-agent daemonset tolerations, hosting pod will only get configured tolerations") } return exposer.PodVolumeExposeParam{ diff --git a/pkg/controller/pod_volume_backup_controller_test.go b/pkg/controller/pod_volume_backup_controller_test.go index e74a1c269..9c8240951 100644 --- a/pkg/controller/pod_volume_backup_controller_test.go +++ b/pkg/controller/pod_volume_backup_controller_test.go @@ -157,6 +157,7 @@ func initPVBReconcilerWithError(needError ...error) (*PodVolumeBackupReconciler, false, // privileged nil, // podLabels nil, // podAnnotations + nil, // tolerations ), nil } @@ -1317,6 +1318,7 @@ func TestPodVolumeBackupSetupExposeParam(t *testing.T) { true, tt.args.customLabels, tt.args.customAnnotations, + nil, ) // Act diff --git a/pkg/controller/pod_volume_restore_controller.go b/pkg/controller/pod_volume_restore_controller.go index 81540aa15..7da739d2f 100644 --- a/pkg/controller/pod_volume_restore_controller.go +++ b/pkg/controller/pod_volume_restore_controller.go @@ -75,6 +75,7 @@ func NewPodVolumeRestoreReconciler( repoConfigMgr repository.ConfigManager, podLabels map[string]string, podAnnotations map[string]string, + tolerations []corev1api.Toleration, ) *PodVolumeRestoreReconciler { return &PodVolumeRestoreReconciler{ client: client, @@ -96,6 +97,7 @@ func NewPodVolumeRestoreReconciler( repoConfigMgr: repoConfigMgr, podLabels: podLabels, podAnnotations: podAnnotations, + tolerations: tolerations, } } @@ -120,6 +122,7 @@ type PodVolumeRestoreReconciler struct { repoConfigMgr repository.ConfigManager podLabels map[string]string podAnnotations map[string]string + tolerations []corev1api.Toleration } // +kubebuilder:rbac:groups=velero.io,resources=podvolumerestores,verbs=get;list;watch;create;update;patch;delete @@ -968,15 +971,9 @@ func (r *PodVolumeRestoreReconciler) setupExposeParam(pvr *velerov1api.PodVolume } } - hostingPodTolerations := []corev1api.Toleration{} - for _, k := range util.ThirdPartyTolerations { - if v, err := nodeagent.GetToleration(context.Background(), r.kubeClient, pvr.Namespace, k, nodeOS); err != nil { - if err != nodeagent.ErrNodeAgentTolerationNotFound { - log.WithError(err).Warnf("Failed to check node-agent toleration, skip adding host pod toleration %s", k) - } - } else { - hostingPodTolerations = append(hostingPodTolerations, *v) - } + hostingPodTolerations, err := nodeagent.GetTolerations(context.Background(), r.kubeClient, pvr.Namespace, nodeOS, r.tolerations) + if err != nil { + log.WithError(err).Warn("Failed to get node-agent daemonset tolerations, hosting pod will only get configured tolerations") } var cacheVolume *exposer.CacheConfigs diff --git a/pkg/controller/pod_volume_restore_controller_test.go b/pkg/controller/pod_volume_restore_controller_test.go index 6bd68e7e1..03b2841d1 100644 --- a/pkg/controller/pod_volume_restore_controller_test.go +++ b/pkg/controller/pod_volume_restore_controller_test.go @@ -751,6 +751,7 @@ func initPodVolumeRestoreReconcilerWithError(objects []runtime.Object, cliObj [] nil, nil, // podLabels nil, // podAnnotations + nil, // tolerations ), nil } @@ -1335,6 +1336,7 @@ func TestPodVolumeRestoreSetupExposeParam(t *testing.T) { nil, // repoConfigMgr (unused when cacheVolumeConfigs is nil) tt.args.customLabels, tt.args.customAnnotations, + nil, ) // Act diff --git a/pkg/exposer/csi_snapshot.go b/pkg/exposer/csi_snapshot.go index ea67d192d..5e51d0e99 100644 --- a/pkg/exposer/csi_snapshot.go +++ b/pkg/exposer/csi_snapshot.go @@ -785,7 +785,7 @@ func (e *csiSnapshotExposer) createBackupPod( Operator: metav1.LabelSelectorOpIn, }) - toleration = append(toleration, []corev1api.Toleration{ + toleration = kube.DeduplicateTolerations(append(toleration, []corev1api.Toleration{ { Key: "os", Operator: "Equal", @@ -798,7 +798,7 @@ func (e *csiSnapshotExposer) createBackupPod( Effect: "NoExecute", Value: "windows", }, - }...) + }...)) } else { userID := int64(0) securityCtx = &corev1api.PodSecurityContext{ diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index f144a03ae..bc22b1849 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -842,7 +842,7 @@ func (e *genericRestoreExposer) createRestorePod( Operator: metav1.LabelSelectorOpIn, }) - toleration = append(toleration, []corev1api.Toleration{ + toleration = kube.DeduplicateTolerations(append(toleration, []corev1api.Toleration{ { Key: "os", Operator: "Equal", @@ -855,7 +855,7 @@ func (e *genericRestoreExposer) createRestorePod( Effect: "NoExecute", Value: "windows", }, - }...) + }...)) } else { userID := int64(0) securityCtx = &corev1api.PodSecurityContext{ diff --git a/pkg/exposer/pod_volume.go b/pkg/exposer/pod_volume.go index 5d6de1831..4402396cb 100644 --- a/pkg/exposer/pod_volume.go +++ b/pkg/exposer/pod_volume.go @@ -456,7 +456,7 @@ func (e *podVolumeExposer) createHostingPod( Operator: metav1.LabelSelectorOpIn, }) - toleration = append(toleration, []corev1api.Toleration{ + toleration = kube.DeduplicateTolerations(append(toleration, []corev1api.Toleration{ { Key: "os", Operator: "Equal", @@ -469,7 +469,7 @@ func (e *podVolumeExposer) createHostingPod( Effect: "NoExecute", Value: "windows", }, - }...) + }...)) } else { userID := int64(0) securityCtx = &corev1api.PodSecurityContext{ diff --git a/pkg/nodeagent/node_agent.go b/pkg/nodeagent/node_agent.go index 6dcdbf89b..4d5040db7 100644 --- a/pkg/nodeagent/node_agent.go +++ b/pkg/nodeagent/node_agent.go @@ -31,6 +31,7 @@ import ( ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" velerotypes "github.com/vmware-tanzu/velero/pkg/types" + "github.com/vmware-tanzu/velero/pkg/util" "github.com/vmware-tanzu/velero/pkg/util/kube" ) @@ -55,7 +56,6 @@ var ( ErrDaemonSetNotFound = errors.New("daemonset not found") ErrNodeAgentLabelNotFound = errors.New("node-agent label not found") ErrNodeAgentAnnotationNotFound = errors.New("node-agent annotation not found") - ErrNodeAgentTolerationNotFound = errors.New("node-agent toleration not found") ) func IsRunningOnLinux(ctx context.Context, kubeClient kubernetes.Interface, namespace string) error { @@ -249,7 +249,14 @@ func GetAnnotationValue(ctx context.Context, kubeClient kubernetes.Interface, na return val, nil } -func GetToleration(ctx context.Context, kubeClient kubernetes.Interface, namespace string, key string, osType string) (*corev1api.Toleration, error) { +// GetTolerations returns the tolerations that should be applied to a node-agent-driven +// hosting pod: the explicitly configured tolerations (typically sourced from the +// node-agent-configmap), plus any toleration on the node-agent daemonset (linux or +// windows, based on osType) whose key is in util.ThirdPartyTolerations. The combined +// list is deduplicated by kube.DeduplicateTolerations. On a daemonset lookup error, +// configuredTolerations is still returned alongside the error so callers don't lose +// explicitly configured tolerations to a transient lookup failure. +func GetTolerations(ctx context.Context, kubeClient kubernetes.Interface, namespace string, osType string, configuredTolerations []corev1api.Toleration) ([]corev1api.Toleration, error) { dsName := daemonSet if osType == kube.NodeOSWindows { dsName = daemonsetWindows @@ -257,16 +264,28 @@ func GetToleration(ctx context.Context, kubeClient kubernetes.Interface, namespa ds, err := kubeClient.AppsV1().DaemonSets(namespace).Get(ctx, dsName, metav1.GetOptions{}) if err != nil { - return nil, errors.Wrapf(err, "error getting %s daemonset", dsName) + return configuredTolerations, errors.Wrapf(err, "error getting %s daemonset", dsName) } - for i, t := range ds.Spec.Template.Spec.Tolerations { - if t.Key == key { - return &ds.Spec.Template.Spec.Tolerations[i], nil + // configuredTolerations is appended first so it wins: DeduplicateTolerations + // keeps only the first occurrence of each exact (Key, Operator, Value, + // Effect) combination, so an allowlisted daemonset toleration identical to + // one already set in the configmap is dropped as a duplicate rather than + // overriding it. A daemonset toleration that only shares a Key (but + // differs in Operator/Value/Effect) isn't a duplicate and is kept + // alongside the configured one, not replaced by it. + merged := make([]corev1api.Toleration, 0, len(configuredTolerations)+len(ds.Spec.Template.Spec.Tolerations)) + merged = append(merged, configuredTolerations...) + for _, t := range ds.Spec.Template.Spec.Tolerations { + for _, allowed := range util.ThirdPartyTolerations { + if t.Key == allowed { + merged = append(merged, t) + break + } } } - return nil, ErrNodeAgentTolerationNotFound + return kube.DeduplicateTolerations(merged), nil } func GetHostPodPath(ctx context.Context, kubeClient kubernetes.Interface, namespace string, osType string) (string, error) { diff --git a/pkg/nodeagent/node_agent_test.go b/pkg/nodeagent/node_agent_test.go index 4a406dd39..61efaac90 100644 --- a/pkg/nodeagent/node_agent_test.go +++ b/pkg/nodeagent/node_agent_test.go @@ -884,7 +884,7 @@ func TestGetAnnotationValue(t *testing.T) { } } -func TestGetToleration(t *testing.T) { +func TestGetTolerations(t *testing.T) { daemonSet := &appsv1api.DaemonSet{ ObjectMeta: metav1.ObjectMeta{ Namespace: "fake-ns", @@ -895,7 +895,7 @@ func TestGetToleration(t *testing.T) { }, } - daemonSetWithOtherToleration := &appsv1api.DaemonSet{ + daemonSetWithTolerations := &appsv1api.DaemonSet{ ObjectMeta: metav1.ObjectMeta{ Namespace: "fake-ns", Name: "node-agent", @@ -908,7 +908,14 @@ func TestGetToleration(t *testing.T) { Spec: corev1api.PodSpec{ Tolerations: []corev1api.Toleration{ { - Key: "other-toleration-key", + Key: "custom-taint", + Value: "true", + }, + { + Key: "kubernetes.azure.com/scalesetpriority", + Operator: "Equal", + Value: "spot", + Effect: "NoSchedule", }, }, }, @@ -916,79 +923,110 @@ func TestGetToleration(t *testing.T) { }, } - daemonSetWithToleration := &appsv1api.DaemonSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "fake-ns", - Name: "node-agent", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "DaemonSet", - }, - Spec: appsv1api.DaemonSetSpec{ - Template: corev1api.PodTemplateSpec{ - Spec: corev1api.PodSpec{ - Tolerations: []corev1api.Toleration{ - { - Key: "fake-toleration", - Value: "true", - }, - }, - }, - }, - }, + configuredToleration := corev1api.Toleration{ + Key: "dedicated", + Operator: "Equal", + Value: "backup", + Effect: "NoSchedule", } tests := []struct { - name string - kubeClientObj []runtime.Object - namespace string - expectedValue corev1api.Toleration - expectErr string + name string + kubeClientObj []runtime.Object + namespace string + configuredTolerations []corev1api.Toleration + expectedValues []corev1api.Toleration + expectErr string }{ - // { - // name: "ds get error", - // namespace: "fake-ns", - // expectErr: "error getting node-agent daemonset: daemonsets.apps \"node-agent\" not found", - // }, { - name: "no toleration", - namespace: "fake-ns", - kubeClientObj: []runtime.Object{ - daemonSet, - }, - expectErr: ErrNodeAgentTolerationNotFound.Error(), + name: "no tolerations", + namespace: "fake-ns", + kubeClientObj: []runtime.Object{daemonSet}, + expectedValues: []corev1api.Toleration{}, }, { - name: "no expecting toleration", + name: "only non-allowlisted daemonset tolerations are dropped", namespace: "fake-ns", kubeClientObj: []runtime.Object{ - daemonSetWithOtherToleration, + daemonSetWithTolerations, + }, + expectedValues: []corev1api.Toleration{ + { + Key: "kubernetes.azure.com/scalesetpriority", + Operator: "Equal", + Value: "spot", + Effect: "NoSchedule", + }, }, - expectErr: ErrNodeAgentTolerationNotFound.Error(), }, { - name: "expecting toleration", + name: "configured tolerations only", + namespace: "fake-ns", + kubeClientObj: []runtime.Object{daemonSet}, + configuredTolerations: []corev1api.Toleration{configuredToleration}, + expectedValues: []corev1api.Toleration{configuredToleration}, + }, + { + name: "configured and allowlisted daemonset tolerations are merged", namespace: "fake-ns", kubeClientObj: []runtime.Object{ - daemonSetWithToleration, + daemonSetWithTolerations, }, - expectedValue: corev1api.Toleration{ - Key: "fake-toleration", - Value: "true", + configuredTolerations: []corev1api.Toleration{configuredToleration}, + expectedValues: []corev1api.Toleration{ + configuredToleration, + { + Key: "kubernetes.azure.com/scalesetpriority", + Operator: "Equal", + Value: "spot", + Effect: "NoSchedule", + }, }, }, + { + name: "duplicate between configured and daemonset tolerations is deduplicated", + namespace: "fake-ns", + kubeClientObj: []runtime.Object{ + daemonSetWithTolerations, + }, + configuredTolerations: []corev1api.Toleration{ + { + Key: "kubernetes.azure.com/scalesetpriority", + Operator: "Equal", + Value: "spot", + Effect: "NoSchedule", + }, + }, + expectedValues: []corev1api.Toleration{ + { + Key: "kubernetes.azure.com/scalesetpriority", + Operator: "Equal", + Value: "spot", + Effect: "NoSchedule", + }, + }, + }, + { + name: "daemonset get error still returns configured tolerations", + namespace: "fake-ns", + kubeClientObj: []runtime.Object{}, + configuredTolerations: []corev1api.Toleration{configuredToleration}, + expectedValues: []corev1api.Toleration{configuredToleration}, + expectErr: "error getting node-agent daemonset: daemonsets.apps \"node-agent\" not found", + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { fakeKubeClient := fake.NewSimpleClientset(test.kubeClientObj...) - value, err := GetToleration(t.Context(), fakeKubeClient, test.namespace, "fake-toleration", kube.NodeOSLinux) + values, err := GetTolerations(t.Context(), fakeKubeClient, test.namespace, kube.NodeOSLinux, test.configuredTolerations) if test.expectErr == "" { require.NoError(t, err) - assert.Equal(t, test.expectedValue, *value) + assert.Equal(t, test.expectedValues, values) } else { - assert.EqualError(t, err, test.expectErr) + require.EqualError(t, err, test.expectErr) + assert.Equal(t, test.expectedValues, values) } }) } diff --git a/pkg/repository/maintenance/maintenance.go b/pkg/repository/maintenance/maintenance.go index 86525d54f..9fa8ba054 100644 --- a/pkg/repository/maintenance/maintenance.go +++ b/pkg/repository/maintenance/maintenance.go @@ -482,9 +482,8 @@ func StartNewJob( } // buildTolerationsForMaintenanceJob builds the tolerations for maintenance jobs. -// It includes the required Windows toleration for backward compatibility and filters -// tolerations from the Velero deployment to only include those with keys that are -// in the ThirdPartyTolerations allowlist, following the same pattern as labels and annotations. +// It includes the required Windows toleration for backward compatibility and +// inherits all tolerations from the Velero deployment. func buildTolerationsForMaintenanceJob(deployment *appsv1api.Deployment) []corev1api.Toleration { // Start with the Windows toleration for backward compatibility windowsToleration := corev1api.Toleration{ @@ -495,17 +494,9 @@ func buildTolerationsForMaintenanceJob(deployment *appsv1api.Deployment) []corev } result := []corev1api.Toleration{windowsToleration} - // Filter tolerations from the Velero deployment to only include allowed ones - // Only tolerations that exist on the deployment AND have keys in the allowlist are inherited + // Inherit all tolerations from the Velero deployment deploymentTolerations := veleroutil.GetTolerationsFromVeleroServer(deployment) - for _, k := range util.ThirdPartyTolerations { - for _, toleration := range deploymentTolerations { - if toleration.Key == k { - result = append(result, toleration) - break // Only add the first matching toleration for each allowed key - } - } - } + result = append(result, deploymentTolerations...) return result } diff --git a/pkg/repository/maintenance/maintenance_test.go b/pkg/repository/maintenance/maintenance_test.go index ee34241ce..e69dadb33 100644 --- a/pkg/repository/maintenance/maintenance_test.go +++ b/pkg/repository/maintenance/maintenance_test.go @@ -1954,7 +1954,7 @@ func TestBuildTolerationsForMaintenanceJob(t *testing.T) { }, }, { - name: "non-allowed toleration should not be inherited", + name: "all tolerations should be inherited", deploymentTolerations: []corev1api.Toleration{ { Key: "vng-ondemand", @@ -1962,88 +1962,36 @@ func TestBuildTolerationsForMaintenanceJob(t *testing.T) { Effect: "NoSchedule", Value: "amd64", }, - }, - expectedTolerations: []corev1api.Toleration{ - windowsToleration, - }, - }, - { - name: "allowed toleration should be inherited", - deploymentTolerations: []corev1api.Toleration{ { - Key: "kubernetes.azure.com/scalesetpriority", - Operator: "Equal", - Effect: "NoSchedule", - Value: "spot", - }, - }, - expectedTolerations: []corev1api.Toleration{ - windowsToleration, - { - Key: "kubernetes.azure.com/scalesetpriority", - Operator: "Equal", - Effect: "NoSchedule", - Value: "spot", - }, - }, - }, - { - name: "mixed allowed and non-allowed tolerations should only inherit allowed", - deploymentTolerations: []corev1api.Toleration{ - { - Key: "vng-ondemand", // not in allowlist - Operator: "Equal", - Effect: "NoSchedule", - Value: "amd64", - }, - { - Key: "CriticalAddonsOnly", // in allowlist + Key: "CriticalAddonsOnly", Operator: "Exists", Effect: "NoSchedule", }, { - Key: "custom-key", // not in allowlist + Key: "custom-key", Operator: "Equal", - Effect: "NoSchedule", + Effect: "NoExecute", Value: "custom-value", }, }, expectedTolerations: []corev1api.Toleration{ windowsToleration, { - Key: "CriticalAddonsOnly", - Operator: "Exists", - Effect: "NoSchedule", - }, - }, - }, - { - name: "multiple allowed tolerations should all be inherited", - deploymentTolerations: []corev1api.Toleration{ - { - Key: "kubernetes.azure.com/scalesetpriority", + Key: "vng-ondemand", Operator: "Equal", Effect: "NoSchedule", - Value: "spot", + Value: "amd64", }, { Key: "CriticalAddonsOnly", Operator: "Exists", Effect: "NoSchedule", }, - }, - expectedTolerations: []corev1api.Toleration{ - windowsToleration, { - Key: "kubernetes.azure.com/scalesetpriority", + Key: "custom-key", Operator: "Equal", - Effect: "NoSchedule", - Value: "spot", - }, - { - Key: "CriticalAddonsOnly", - Operator: "Exists", - Effect: "NoSchedule", + Effect: "NoExecute", + Value: "custom-value", }, }, }, @@ -2069,36 +2017,6 @@ func TestBuildTolerationsForMaintenanceJob(t *testing.T) { } func TestBuildJobWithTolerationsInheritance(t *testing.T) { - // Define allowed tolerations that would be set on Velero deployment - allowedTolerations := []corev1api.Toleration{ - { - Key: "kubernetes.azure.com/scalesetpriority", - Operator: "Equal", - Effect: "NoSchedule", - Value: "spot", - }, - { - Key: "CriticalAddonsOnly", - Operator: "Exists", - Effect: "NoSchedule", - }, - } - - // Mixed tolerations (allowed and non-allowed) - mixedTolerations := []corev1api.Toleration{ - { - Key: "vng-ondemand", // not in allowlist - Operator: "Equal", - Effect: "NoSchedule", - Value: "amd64", - }, - { - Key: "CriticalAddonsOnly", // in allowlist - Operator: "Exists", - Effect: "NoSchedule", - }, - } - // Windows toleration that should always be present windowsToleration := corev1api.Toleration{ Key: "os", @@ -2120,8 +2038,21 @@ func TestBuildJobWithTolerationsInheritance(t *testing.T) { }, }, { - name: "allowed tolerations should be inherited along with Windows toleration", - deploymentTolerations: allowedTolerations, + name: "all tolerations should be inherited along with Windows toleration", + deploymentTolerations: []corev1api.Toleration{ + { + Key: "kubernetes.azure.com/scalesetpriority", + Operator: "Equal", + Effect: "NoSchedule", + Value: "spot", + }, + { + Key: "custom-taint", + Operator: "Equal", + Effect: "NoExecute", + Value: "dedicated", + }, + }, expectedTolerations: []corev1api.Toleration{ windowsToleration, { @@ -2131,21 +2062,10 @@ func TestBuildJobWithTolerationsInheritance(t *testing.T) { Value: "spot", }, { - Key: "CriticalAddonsOnly", - Operator: "Exists", - Effect: "NoSchedule", - }, - }, - }, - { - name: "mixed tolerations should only inherit allowed ones", - deploymentTolerations: mixedTolerations, - expectedTolerations: []corev1api.Toleration{ - windowsToleration, - { - Key: "CriticalAddonsOnly", - Operator: "Exists", - Effect: "NoSchedule", + Key: "custom-taint", + Operator: "Equal", + Effect: "NoExecute", + Value: "dedicated", }, }, }, diff --git a/pkg/types/node_agent.go b/pkg/types/node_agent.go index 16e50b283..a372d17e9 100644 --- a/pkg/types/node_agent.go +++ b/pkg/types/node_agent.go @@ -17,6 +17,7 @@ limitations under the License. package types import ( + corev1api "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/vmware-tanzu/velero/pkg/util/kube" @@ -142,4 +143,9 @@ type NodeAgentConfigs struct { // CSISnapshotMetadataServiceConfigs is the config for CSI snapshot metadata service CSISnapshotMetadataServiceConfigs *CSISnapshotMetadataService `json:"csiSnapshotMetadataServiceConfigs,omitempty"` + + // Tolerations are tolerations to be added to pods created by node-agent, i.e., data mover pods. + // These are merged with (and deduplicated against) any node-agent DaemonSet tolerations + // whose key is in util.ThirdPartyTolerations. + Tolerations []corev1api.Toleration `json:"tolerations,omitempty"` } diff --git a/pkg/util/kube/toleration.go b/pkg/util/kube/toleration.go new file mode 100644 index 000000000..97b43e8d8 --- /dev/null +++ b/pkg/util/kube/toleration.go @@ -0,0 +1,37 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + corev1api "k8s.io/api/core/v1" +) + +// DeduplicateTolerations removes duplicate tolerations from the slice. +// A toleration is considered a duplicate if another toleration with the same +// Key, Operator, Value, and Effect already exists in the slice. +func DeduplicateTolerations(tolerations []corev1api.Toleration) []corev1api.Toleration { + seen := make(map[string]struct{}) + result := make([]corev1api.Toleration, 0, len(tolerations)) + for _, t := range tolerations { + key := t.Key + "|" + string(t.Operator) + "|" + t.Value + "|" + string(t.Effect) + if _, exists := seen[key]; !exists { + seen[key] = struct{}{} + result = append(result, t) + } + } + return result +} diff --git a/pkg/util/kube/toleration_test.go b/pkg/util/kube/toleration_test.go new file mode 100644 index 000000000..1370af00c --- /dev/null +++ b/pkg/util/kube/toleration_test.go @@ -0,0 +1,86 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1api "k8s.io/api/core/v1" +) + +func TestDeduplicateTolerations(t *testing.T) { + tests := []struct { + name string + input []corev1api.Toleration + expected []corev1api.Toleration + }{ + { + name: "nil input", + input: nil, + expected: []corev1api.Toleration{}, + }, + { + name: "empty input", + input: []corev1api.Toleration{}, + expected: []corev1api.Toleration{}, + }, + { + name: "no duplicates", + input: []corev1api.Toleration{ + {Key: "os", Operator: "Equal", Value: "windows", Effect: "NoSchedule"}, + {Key: "os", Operator: "Equal", Value: "windows", Effect: "NoExecute"}, + }, + expected: []corev1api.Toleration{ + {Key: "os", Operator: "Equal", Value: "windows", Effect: "NoSchedule"}, + {Key: "os", Operator: "Equal", Value: "windows", Effect: "NoExecute"}, + }, + }, + { + name: "duplicates removed", + input: []corev1api.Toleration{ + {Key: "os", Operator: "Equal", Value: "windows", Effect: "NoSchedule"}, + {Key: "os", Operator: "Equal", Value: "windows", Effect: "NoExecute"}, + {Key: "os", Operator: "Equal", Value: "windows", Effect: "NoSchedule"}, + {Key: "os", Operator: "Equal", Value: "windows", Effect: "NoExecute"}, + }, + expected: []corev1api.Toleration{ + {Key: "os", Operator: "Equal", Value: "windows", Effect: "NoSchedule"}, + {Key: "os", Operator: "Equal", Value: "windows", Effect: "NoExecute"}, + }, + }, + { + name: "preserves order of first occurrence", + input: []corev1api.Toleration{ + {Key: "custom-taint", Operator: "Equal", Value: "true", Effect: "NoExecute"}, + {Key: "os", Operator: "Equal", Value: "windows", Effect: "NoSchedule"}, + {Key: "os", Operator: "Equal", Value: "windows", Effect: "NoSchedule"}, + }, + expected: []corev1api.Toleration{ + {Key: "custom-taint", Operator: "Equal", Value: "true", Effect: "NoExecute"}, + {Key: "os", Operator: "Equal", Value: "windows", Effect: "NoSchedule"}, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := DeduplicateTolerations(test.input) + assert.Equal(t, test.expected, result) + }) + } +} diff --git a/site/content/docs/main/supported-configmaps/node-agent-configmap.md b/site/content/docs/main/supported-configmaps/node-agent-configmap.md index f8f8f2c5c..bea0e0a71 100644 --- a/site/content/docs/main/supported-configmaps/node-agent-configmap.md +++ b/site/content/docs/main/supported-configmaps/node-agent-configmap.md @@ -498,6 +498,37 @@ The configurations work for DataUpload, DataDownload, PodVolumeBackup, and PodVo - **Explicit Configuration Required**: If you need both custom annotations and in-tree third-party annotations, explicitly include the in-tree annotations in the `podAnnotations` configuration - **In-tree Annotations**: The default in-tree annotations include support for AWS IAM roles +### Tolerations Configuration (`tolerations`) + +Add customized tolerations for data mover pods to allow scheduling on nodes with custom taints. + +Unlike `podLabels`/`podAnnotations`, `tolerations` does **not** replace Velero's [in-tree third-party toleration allowlist](https://github.com/vmware-tanzu/velero/blob/main/pkg/util/third_party.go). Any toleration on the node-agent DaemonSet whose key is in that allowlist (currently `kubernetes.azure.com/scalesetpriority` and `CriticalAddonsOnly`) is always merged in alongside the tolerations configured here, with duplicates removed. + +The configurations work for DataUpload, DataDownload, PodVolumeBackup, and PodVolumeRestore pods. This does not affect repository maintenance jobs, which inherit tolerations from the Velero Deployment directly. + +#### Configuration Example +```json +{ + "tolerations": [ + { + "key": "dedicated", + "operator": "Equal", + "value": "backup", + "effect": "NoSchedule" + } + ] +} +``` + +#### Use Cases +- **Dedicated Backup Node Pools**: Nodes tainted to only run backup/restore workloads +- **Spot/Preemptible Node Pools**: Additional spot-instance taints beyond the built-in Azure allowlist +- **Custom Maintenance Taints**: Nodes with `NoExecute` taints applied during maintenance windows + +#### Important Notes +- **Merge, Not Replace**: `tolerations` is merged with (not a replacement for) the in-tree third-party allowlisted tolerations inherited from the node-agent DaemonSet +- **Deduplication**: Identical tolerations (same key, operator, value, and effect) from either source are only applied once + ## Complete Configuration Example Here's a comprehensive example showing how all configuration sections work together: @@ -579,7 +610,15 @@ Here's a comprehensive example showing how all configuration sections work toget "vault.hashicorp.com/agent-inject": "true", "prometheus.io/scrape": "true", "custom.company.com/environment": "production" - } + }, + "tolerations": [ + { + "key": "dedicated", + "operator": "Equal", + "value": "backup", + "effect": "NoSchedule" + } + ] } ``` @@ -596,6 +635,7 @@ This configuration: - Enable cache PVC for file system restore - The cache threshold is 1GB and use dedicated StorageClass - Use customized labels and annotations data mover pods +- Tolerate the `dedicated=backup:NoSchedule` taint, merged with any allowlisted DaemonSet tolerations ## Troubleshooting