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