diff --git a/.github/workflows/nightly-trivy-scan.yml b/.github/workflows/nightly-trivy-scan.yml index 0999b1c0f..34f3eaf0c 100644 --- a/.github/workflows/nightly-trivy-scan.yml +++ b/.github/workflows/nightly-trivy-scan.yml @@ -35,6 +35,6 @@ jobs: output: 'trivy-results.sarif' - name: Upload Trivy scan results to GitHub Security tab - uses: github/codeql-action/upload-sarif@v4.37.9 + uses: github/codeql-action/upload-sarif@v4.38.0 with: sarif_file: 'trivy-results.sarif' \ No newline at end of file diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index 9ba1b8b46..a69532faf 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -51,6 +51,6 @@ jobs: # Upload the results to GitHub's code scanning dashboard. - name: "Upload to code-scanning" - uses: github/codeql-action/upload-sarif@v4.37.9 + uses: github/codeql-action/upload-sarif@v4.38.0 with: sarif_file: results.sarif diff --git a/changelogs/unreleased/10449-PratikMane0112 b/changelogs/unreleased/10449-PratikMane0112 new file mode 100644 index 000000000..8174c2b65 --- /dev/null +++ b/changelogs/unreleased/10449-PratikMane0112 @@ -0,0 +1 @@ +Fix snapshot-location get --selector flag to actually filter VolumeSnapshotLocations by label diff --git a/changelogs/unreleased/10512-chlins b/changelogs/unreleased/10512-chlins new file mode 100644 index 000000000..b22d0c86f --- /dev/null +++ b/changelogs/unreleased/10512-chlins @@ -0,0 +1 @@ +Add in-place restore pre-flight check: PVC must be large enough for the backed-up data diff --git a/changelogs/unreleased/10520-krishhna24 b/changelogs/unreleased/10520-krishhna24 new file mode 100644 index 000000000..6f6f184de --- /dev/null +++ b/changelogs/unreleased/10520-krishhna24 @@ -0,0 +1 @@ +Fix the VolumeGroupSnapshotClass example in the VolumeGroupSnapshot docs diff --git a/changelogs/unreleased/10526-Daniel-1600 b/changelogs/unreleased/10526-Daniel-1600 new file mode 100644 index 000000000..0900b4d79 --- /dev/null +++ b/changelogs/unreleased/10526-Daniel-1600 @@ -0,0 +1 @@ +Fix schedule create dropping backup type \ No newline at end of file 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/internal/volume/volumes_information.go b/internal/volume/volumes_information.go index 3aa22f154..bc741275b 100644 --- a/internal/volume/volumes_information.go +++ b/internal/volume/volumes_information.go @@ -365,6 +365,17 @@ func newPodVolumeInfoFromPVR(pvr *velerov1api.PodVolumeRestore) *PodVolumeRestor } } +// SourceSize returns the size of the source volume recorded at backup time, or 0 if unknown. +func (v BackupVolumeInfo) SourceSize() int64 { + switch { + case v.SnapshotDataMovementInfo != nil: + return v.SnapshotDataMovementInfo.SourceSize + case v.PVBInfo != nil: + return v.PVBInfo.SourceSize + } + return 0 +} + // PVInfo is used to store some PV information modified after creation. // Those information are lost after PV recreation. type PVInfo struct { diff --git a/pkg/apis/velero/v1/labels_annotations.go b/pkg/apis/velero/v1/labels_annotations.go index 5636ecd36..0a401cd45 100644 --- a/pkg/apis/velero/v1/labels_annotations.go +++ b/pkg/apis/velero/v1/labels_annotations.go @@ -185,6 +185,13 @@ const ( // on the cluster. Using a carrier annotation avoids any dependency on the execution order // of RestoreItemActions. InplaceRestoreSelectedNodeAnnotation = "restore.velero.io/inplace-restore-selected-node" + + // InplaceRestoreSourceSizeAnnotation is a Velero-internal carrier annotation set by the + // restore engine on a PVC item before RestoreItemActions run. It carries the size of the + // source volume recorded in the backup volume info, so the PVC CSI RestoreItemAction can + // run the in-place restore capacity pre-flight check without access to the volume info. + // The annotation is always stripped by the restore engine; it never lands on the cluster. + InplaceRestoreSourceSizeAnnotation = "restore.velero.io/inplace-restore-source-size" // SkippedNoCSIPVAnnotation - Velero checks this annotation on processed PVC to // find out if the snapshot was skipped b/c the PV is not provisioned via CSI SkippedNoCSIPVAnnotation = "backup.velero.io/skipped-no-csi-pv" 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/cmd/cli/schedule/create.go b/pkg/cmd/cli/schedule/create.go index 03f5626fd..cba121f5d 100644 --- a/pkg/cmd/cli/schedule/create.go +++ b/pkg/cmd/cli/schedule/create.go @@ -163,6 +163,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { ItemOperationTimeout: metav1.Duration{Duration: o.BackupOptions.ItemOperationTimeout}, DataMover: o.BackupOptions.DataMover, SnapshotMoveData: o.BackupOptions.SnapshotMoveData.Value, + BackupType: api.BackupType(o.BackupOptions.BackupType), }, Schedule: o.Schedule, UseOwnerReferencesInBackup: &o.UseOwnerReferencesInBackup, diff --git a/pkg/cmd/cli/snapshotlocation/get.go b/pkg/cmd/cli/snapshotlocation/get.go index 79da478bf..46c4897ca 100644 --- a/pkg/cmd/cli/snapshotlocation/get.go +++ b/pkg/cmd/cli/snapshotlocation/get.go @@ -21,6 +21,7 @@ import ( "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" kbclient "sigs.k8s.io/controller-runtime/pkg/client" api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" @@ -50,7 +51,12 @@ func NewGetCommand(f client.Factory, use string) *cobra.Command { locations.Items = append(locations.Items, *location) } } else { - err = client.List(context.TODO(), locations, &kbclient.ListOptions{Namespace: f.Namespace()}) + parsedSelector, err := labels.Parse(listOptions.LabelSelector) + cmd.CheckError(err) + err = client.List(context.TODO(), locations, &kbclient.ListOptions{ + LabelSelector: parsedSelector, + Namespace: f.Namespace(), + }) cmd.CheckError(err) } _, err = output.PrintWithFormat(c, locations) diff --git a/pkg/cmd/cli/snapshotlocation/get_test.go b/pkg/cmd/cli/snapshotlocation/get_test.go new file mode 100644 index 000000000..e6ed31001 --- /dev/null +++ b/pkg/cmd/cli/snapshotlocation/get_test.go @@ -0,0 +1,104 @@ +/* +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 snapshotlocation + +import ( + "fmt" + "os" + "os/exec" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/vmware-tanzu/velero/pkg/builder" + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" + cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" + velerotest "github.com/vmware-tanzu/velero/pkg/test" + veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" +) + +func TestNewGetCommand(t *testing.T) { + vslList := []string{"vsl1", "vsl2"} + + f := &factorymocks.Factory{} + kbclient := velerotest.NewFakeControllerRuntimeClient(t) + f.On("Namespace").Return(mock.Anything) + f.On("KubebuilderClient").Return(kbclient, nil) + + // get command + c := NewGetCommand(f, "velero snapshot-location get") + assert.Equal(t, "Get snapshot locations", c.Short) + + c.Execute() + + if os.Getenv(cmdtest.CaptureFlag) == "1" { + c.SetArgs([]string{"vsl1", "vsl2"}) + c.Execute() + return + } + cmd := exec.CommandContext(t.Context(), os.Args[0], []string{"-test.run=TestNewGetCommand"}...) + cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", cmdtest.CaptureFlag)) + _, stderr, err := veleroexec.RunCommand(cmd) + + if err != nil { + assert.Contains(t, stderr, fmt.Sprintf("volumesnapshotlocations.velero.io \"%s\" not found", vslList[0])) + return + } + t.Fatalf("process ran with err %v, want snapshot location get to fail for non-existent VSL", err) +} + +func TestNewGetCommand_SelectorFiltersVSLs(t *testing.T) { + f := &factorymocks.Factory{} + client := velerotest.NewFakeControllerRuntimeClient(t) + + vslLabeled := builder.ForVolumeSnapshotLocation(cmdtest.VeleroNameSpace, "vsl-labeled"). + ObjectMeta(builder.WithLabels("env", "test")). + Result() + err := client.Create(t.Context(), vslLabeled, &kbclient.CreateOptions{}) + require.NoError(t, err) + + vslUnlabeled := builder.ForVolumeSnapshotLocation(cmdtest.VeleroNameSpace, "vsl-unlabeled"). + Result() + err = client.Create(t.Context(), vslUnlabeled, &kbclient.CreateOptions{}) + require.NoError(t, err) + + f.On("KubebuilderClient").Return(client, nil) + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + + // get command with selector + c := NewGetCommand(f, "velero snapshot-location get") + c.SetArgs([]string{"--selector", "env=test"}) + err = c.Execute() + require.NoError(t, err) + + if os.Getenv(cmdtest.CaptureFlag) == "1" { + return + } + + cmd := exec.CommandContext(t.Context(), os.Args[0], []string{"-test.run=TestNewGetCommand_SelectorFiltersVSLs"}...) + cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", cmdtest.CaptureFlag)) + stdout, _, err := veleroexec.RunCommand(cmd) + require.NoError(t, err) + + // assert that the labeled VSL is returned + assert.Contains(t, stdout, "vsl-labeled") + // assert that the unlabeled VSL is not returned + assert.NotContains(t, stdout, "vsl-unlabeled") +} diff --git a/pkg/cmd/util/flag/label_selector_test.go b/pkg/cmd/util/flag/label_selector_test.go index 9d69a2718..ad2b4c711 100644 --- a/pkg/cmd/util/flag/label_selector_test.go +++ b/pkg/cmd/util/flag/label_selector_test.go @@ -24,6 +24,28 @@ func TestSetOfLabelSelector(t *testing.T) { assert.True(t, str == "k1=v1,k2=v2" || str == "k2=v2,k2=v2") } +func TestSetOfSetBasedLabelSelector(t *testing.T) { + selector := &LabelSelector{} + require.NoError(t, selector.Set("pr-label notin (1)")) + require.NotNil(t, selector.LabelSelector) + require.Len(t, selector.LabelSelector.MatchExpressions, 1) + req := selector.LabelSelector.MatchExpressions[0] + assert.Equal(t, "pr-label", req.Key) + assert.Equal(t, metav1.LabelSelectorOpNotIn, req.Operator) + assert.Equal(t, []string{"1"}, req.Values) +} + +func TestSetOfDoesNotExistLabelSelector(t *testing.T) { + selector := &LabelSelector{} + require.NoError(t, selector.Set("!pr-label")) + require.NotNil(t, selector.LabelSelector) + require.Len(t, selector.LabelSelector.MatchExpressions, 1) + req := selector.LabelSelector.MatchExpressions[0] + assert.Equal(t, "pr-label", req.Key) + assert.Equal(t, metav1.LabelSelectorOpDoesNotExist, req.Operator) + assert.Empty(t, req.Values) +} + func TestTypeOfLabelSelector(t *testing.T) { selector := &LabelSelector{} assert.Equal(t, "labelSelector", selector.Type()) 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/podvolume/restorer.go b/pkg/podvolume/restorer.go index 488ce7331..232b6b518 100644 --- a/pkg/podvolume/restorer.go +++ b/pkg/podvolume/restorer.go @@ -189,7 +189,12 @@ func (r *restorer) RestorePodVolumes(data RestoreData, tracker *volume.RestoreVo // to write into, and they cannot write to it themselves until this // restore's PodVolumeRestores complete. if data.Restore.IsVolumeDataInplaceRestore() && pvc != nil { - if err := inplace.CheckPVCBoundToBackedUpPV(pvc, backedUpPVName(data.BackupVolumeInfos, data.SourceNamespace, pvc.Name), data.SourceNamespace); err != nil { + pvName := backedUpPVName(data.BackupVolumeInfos, data.SourceNamespace, pvc.Name) + if err := inplace.CheckPVCBoundToBackedUpPV(pvc, pvName, data.SourceNamespace); err != nil { + errs = append(errs, err) + continue + } + if err := inplace.CheckPVCCapacity(pvc, data.BackupVolumeInfos[pvName].SourceSize()); err != nil { errs = append(errs, err) continue } diff --git a/pkg/podvolume/restorer_test.go b/pkg/podvolume/restorer_test.go index 3d39d0a05..78e59fd1c 100644 --- a/pkg/podvolume/restorer_test.go +++ b/pkg/podvolume/restorer_test.go @@ -27,6 +27,7 @@ import ( "github.com/stretchr/testify/assert" appsv1api "k8s.io/api/apps/v1" corev1api "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" @@ -439,6 +440,37 @@ func TestRestorePodVolumes(t *testing.T) { }, }, }, + { + name: "in-place restore blocked when the PVC is too small for the source volume", + pvbs: []*velerov1api.PodVolumeBackup{ + createPVBObj(true, true, 1, "kopia"), + }, + inplace: true, + kubeClientObj: []runtime.Object{ + createNodeAgentDaemonset(), + func() *corev1api.PersistentVolumeClaim { + pvc := createPVCObj(1) + pvc.Status.Capacity = corev1api.ResourceList{corev1api.ResourceStorage: resource.MustParse("100Mi")} + return pvc + }(), + }, + ctlClientObj: []runtime.Object{ + createBackupRepoObj(), + }, + restoredPod: createPodObj(true, true, true, 1), + sourceNamespace: "fake-ns", + bsl: "fake-bsl", + volumeInfos: map[string]volume.BackupVolumeInfo{ + "fake-pv-1": {PVCNamespace: "fake-ns", PVCName: "fake-pvc-1", PVBInfo: &volume.PodVolumeBackupInfo{SourceSize: 200 << 20}}, + }, + runtimeScheme: scheme, + errs: []expectError{ + { + err: "in-place restore pre-flight check failed, skipping volume data restore: PVC fake-ns/fake-pvc-1 capacity 100Mi is smaller than the backed-up volume size 209715200 bytes", + prefixOnly: true, + }, + }, + }, { name: "in-place restore proceeds when the PVC is only used by the gated restored pod", pvbs: []*velerov1api.PodVolumeBackup{ 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/restore/actions/csi/pvc_action.go b/pkg/restore/actions/csi/pvc_action.go index 9a1b21955..fd28c0ed3 100644 --- a/pkg/restore/actions/csi/pvc_action.go +++ b/pkg/restore/actions/csi/pvc_action.go @@ -20,6 +20,7 @@ import ( "context" "encoding/json" "fmt" + "strconv" "time" "github.com/cockroachdb/errors" @@ -228,7 +229,7 @@ func (p *pvcRestoreItemAction) executeWithDataMove(logger *logrus.Entry, input * var dataUploadResult *velerov2alpha1.DataUploadResult dataUploadResult, err = getDataUploadResult(ctx, input.Restore, pvc, p.crClient) if err != nil { - return nil, errors.Wrapf(err, "fail get DataUploadResult for restore: %s", input.Restore.Name) + return nil, errors.Wrapf(err, "failed to get DataUploadResult for restore: %s", input.Restore.Name) } var volumeSnapshot *snapshotv1api.VolumeSnapshot @@ -238,6 +239,9 @@ func (p *pvcRestoreItemAction) executeWithDataMove(logger *logrus.Entry, input * if err := inplace.CheckPVCBoundToBackedUpPV(existingPVC, pvcFromBackup.Spec.VolumeName, pvcFromBackup.Namespace); err != nil { return nil, errors.WithStack(err) } + if err := inplace.CheckPVCCapacity(existingPVC, sourceSizeFromCarrier(pvc)); err != nil { + return nil, errors.WithStack(err) + } if err := inplace.CheckPVCNotInUse(ctx, p.crClient, existingPVC, input.Restore.UID); err != nil { return nil, errors.WithStack(err) } @@ -278,10 +282,10 @@ func (p *pvcRestoreItemAction) executeWithDataMove(logger *logrus.Entry, input * var dataDownload *velerov2alpha1.DataDownload dataDownload, err = restoreFromDataUploadResult( - context.Background(), dataUploadResult, input.Restore, backup, pvc, existingPV, newNamespace, + ctx, dataUploadResult, input.Restore, backup, pvc, existingPV, newNamespace, operationID, string(restoreType), volumeSnapshot, p.crClient) if err != nil { - logger.Errorf("Fail to restore from DataUploadResult: %s", err.Error()) + logger.Errorf("Failed to restore from DataUploadResult: %s", err.Error()) return nil, errors.WithStack(err) } logger.Infof("DataDownload %s/%s is created successfully.", @@ -338,7 +342,7 @@ func (p *pvcRestoreItemAction) Progress( p.crClient, ) if err != nil { - logger.Errorf("fail to get DataDownload: %s", err.Error()) + logger.Errorf("Failed to get DataDownload: %s", err.Error()) return progress, err } if dataDownload.Status.Phase == velerov2alpha1.DataDownloadPhaseNew || @@ -391,13 +395,13 @@ func (p *pvcRestoreItemAction) Cancel( p.crClient, ) if err != nil { - logger.Errorf("fail to get DataDownload: %s", err.Error()) + logger.Errorf("Failed to get DataDownload: %s", err.Error()) return err } err = cancelDataDownload(context.Background(), p.crClient, dataDownload) if err != nil { - logger.Errorf("fail to cancel DataDownload %s: %s", dataDownload.Name, err.Error()) + logger.Errorf("Failed to cancel DataDownload %s: %s", dataDownload.Name, err.Error()) } return err } @@ -600,7 +604,7 @@ func restoreFromDataUploadResult( ) err := crClient.Create(ctx, dataDownload) if err != nil { - return nil, errors.Wrapf(err, "fail to create DataDownload") + return nil, errors.Wrapf(err, "failed to create DataDownload") } return dataDownload, nil @@ -653,8 +657,8 @@ func (p *pvcRestoreItemAction) deleteExistingPVC(ctx context.Context, logger *lo var err error logger.Info("ExistingVolumeDataPolicy is in-place restore. Deleting the existing PVC but keep the PV...") pv := &corev1api.PersistentVolume{} - if err = p.crClient.Get(context.Background(), crclient.ObjectKey{Name: existingPVC.Spec.VolumeName}, pv); err != nil { - return nil, errors.Errorf("Fail to get PV %s: %s", existingPVC.Spec.VolumeName, err.Error()) + if err = p.crClient.Get(ctx, crclient.ObjectKey{Name: existingPVC.Spec.VolumeName}, pv); err != nil { + return nil, errors.Wrapf(err, "failed to get PV %s", existingPVC.Spec.VolumeName) } // set reclaim policy to retain @@ -728,6 +732,13 @@ func (p *pvcRestoreItemAction) createVolumeSnapshot(ctx context.Context, logger return vs, nil } +// sourceSizeFromCarrier reads the source volume size the restore engine carries on the PVC +// item from the backup volume info, or 0 if absent or malformed. +func sourceSizeFromCarrier(pvc *corev1api.PersistentVolumeClaim) int64 { + size, _ := strconv.ParseInt(pvc.Annotations[velerov1api.InplaceRestoreSourceSizeAnnotation], 10, 64) + return size +} + func NewPvcRestoreItemAction(f client.Factory) plugincommon.HandlerInitializer { return func(logger logrus.FieldLogger) (any, error) { crClient, err := f.KubebuilderClient() diff --git a/pkg/restore/actions/csi/pvc_action_test.go b/pkg/restore/actions/csi/pvc_action_test.go index 1e8850e97..2f32c0584 100644 --- a/pkg/restore/actions/csi/pvc_action_test.go +++ b/pkg/restore/actions/csi/pvc_action_test.go @@ -450,7 +450,7 @@ func TestExecute(t *testing.T) { restore: builder.ForRestore("velero", "testRestore").Backup("testBackup").Result(), pvc: builder.ForPersistentVolumeClaim("velero", "testPVC").ObjectMeta(builder.WithAnnotations(velerov1api.VolumeSnapshotLabel, "vsName", velerov1api.VolumeSnapshotRestoreSize, "10Gi", velerov1api.DataUploadNameAnnotation, "velero/")).Result(), expectedPVC: builder.ForPersistentVolumeClaim("velero", "testPVC").Result(), - expectedErr: "fail get DataUploadResult for restore: testRestore: no DataUpload result cm found with labels velero.io/pvc-namespace-name=velero.testPVC,velero.io/restore-uid=,velero.io/resource-usage=DataUpload", + expectedErr: "failed to get DataUploadResult for restore: testRestore: no DataUpload result cm found with labels velero.io/pvc-namespace-name=velero.testPVC,velero.io/restore-uid=,velero.io/resource-usage=DataUpload", }, { name: "Restore from DataUploadResult", @@ -757,6 +757,8 @@ func TestExecuteInplaceRestorePreflight(t *testing.T) { name string pod *corev1api.Pod backedUpPVName string + sourceSize string // carried on the PVC item by the restore engine + pvcCapacity string expectBlock string }{ { @@ -774,6 +776,17 @@ func TestExecuteInplaceRestorePreflight(t *testing.T) { backedUpPVName: "backupPV", expectBlock: "was bound to PV backupPV at backup time", }, + { + // Backed-up PV unknown so the same-volume skip does not apply. + name: "PVC smaller than the source volume blocks the restore", + sourceSize: "209715200", + pvcCapacity: "100Mi", + expectBlock: "capacity 100Mi is smaller than the backed-up volume size 209715200 bytes", + }, + { + name: "source size not carried skips the capacity check", + pvcCapacity: "100Mi", + }, } for _, tc := range tests { @@ -781,6 +794,9 @@ func TestExecuteInplaceRestorePreflight(t *testing.T) { existingPVC := builder.ForPersistentVolumeClaim("velero", "testPVC"). VolumeName("testPV"). Phase(corev1api.ClaimBound).Result() + if tc.pvcCapacity != "" { + existingPVC.Status.Capacity = corev1api.ResourceList{corev1api.ResourceStorage: resource.MustParse(tc.pvcCapacity)} + } existingPV := builder.ForPersistentVolume("testPV").Result() backup := builder.ForBackup("velero", "testBackup").SnapshotMoveData(true).Result() restore := builder.ForRestore("velero", "testRestore").Backup("testBackup"). @@ -811,7 +827,11 @@ func TestExecuteInplaceRestorePreflight(t *testing.T) { kubeClient: fake.NewSimpleClientset(kubeObjects...), } - pvcMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pvcFromBackup.DeepCopy()) + item := pvcFromBackup.DeepCopy() + if tc.sourceSize != "" { + item.Annotations[velerov1api.InplaceRestoreSourceSizeAnnotation] = tc.sourceSize + } + pvcMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(item) require.NoError(t, err) pvcFromBackupMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pvcFromBackup) require.NoError(t, err) @@ -884,3 +904,22 @@ func TestNewPvcRestoreItemAction(t *testing.T) { _, err1 := plugin1(logger) require.NoError(t, err1) } + +func TestDeleteExistingPVCFailure(t *testing.T) { + pvcRIA := pvcRestoreItemAction{ + log: logrus.New(), + crClient: velerotest.NewFakeControllerRuntimeClient(t), + kubeClient: fake.NewSimpleClientset(), + } + existingPVC := builder.ForPersistentVolumeClaim("ns-1", "pvc-1"). + VolumeName("non-existent-pv"). + Phase(corev1api.ClaimBound).Result() + targetPVC := builder.ForPersistentVolumeClaim("ns-1", "pvc-1").Result() + + returnedPV, err := pvcRIA.deleteExistingPVC( + t.Context(), logrus.New().WithField("test", "fail-to-get-pv"), + targetPVC, existingPVC, time.Minute) + require.Error(t, err) + assert.Nil(t, returnedPV) + assert.Contains(t, err.Error(), "failed to get PV non-existent-pv") +} diff --git a/pkg/restore/inplace/preflight.go b/pkg/restore/inplace/preflight.go index 2ad76931d..1572c67ce 100644 --- a/pkg/restore/inplace/preflight.go +++ b/pkg/restore/inplace/preflight.go @@ -26,6 +26,7 @@ import ( "github.com/cockroachdb/errors" corev1api "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/types" crclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -50,6 +51,10 @@ func CheckPVCNotInUse( pvc *corev1api.PersistentVolumeClaim, restoreUID types.UID, ) error { + if pvc == nil { + return errors.New("pvc cannot be nil") + } + podList := new(corev1api.PodList) if err := cli.List(ctx, podList, &crclient.ListOptions{Namespace: pvc.Namespace}); err != nil { return errors.Wrapf(err, "failed to check whether PVC %s/%s is in use: failed to list pods in namespace %s", pvc.Namespace, pvc.Name, pvc.Namespace) @@ -139,6 +144,9 @@ func gatedByThisRestore(pod *corev1api.Pod, restoreUID types.UID) bool { // bound to a different PV (the documented cross-namespace clone-and-restore // workflow), and when the backed-up PV name is unknown. func CheckPVCBoundToBackedUpPV(existingPVC *corev1api.PersistentVolumeClaim, backedUpPVName, sourceNamespace string) error { + if existingPVC == nil { + return errors.New("existing PVC cannot be nil") + } if existingPVC.Status.Phase != corev1api.ClaimBound { return errors.Errorf("in-place restore pre-flight check failed, skipping volume data restore: PVC %s/%s is not bound (phase %s)", existingPVC.Namespace, existingPVC.Name, existingPVC.Status.Phase) @@ -149,3 +157,26 @@ func CheckPVCBoundToBackedUpPV(existingPVC *corev1api.PersistentVolumeClaim, bac return errors.Errorf("in-place restore pre-flight check failed, skipping volume data restore: PVC %s/%s is bound to PV %s, but was bound to PV %s at backup time", existingPVC.Namespace, existingPVC.Name, existingPVC.Spec.VolumeName, backedUpPVName) } + +// CheckPVCCapacity verifies the existing PVC is large enough to hold the +// backed-up volume, failing early instead of letting the restore run out of +// space midway. sourceSize is the size of the source volume recorded at +// backup time: the device size for the block data mover, the logical size of +// the backed-up files for the file system data movers (a lower bound, since +// file system metadata is not accounted for). The check is skipped when the +// size is unknown (backups taken before it was recorded) or when the PVC's +// capacity is not reported. +func CheckPVCCapacity(existingPVC *corev1api.PersistentVolumeClaim, sourceSize int64) error { + if sourceSize <= 0 { + return nil + } + capacity, ok := existingPVC.Status.Capacity[corev1api.ResourceStorage] + if !ok || capacity.IsZero() { + return nil + } + if capacity.Cmp(*resource.NewQuantity(sourceSize, resource.BinarySI)) < 0 { + return errors.Errorf("in-place restore pre-flight check failed, skipping volume data restore: PVC %s/%s capacity %s is smaller than the backed-up volume size %d bytes", + existingPVC.Namespace, existingPVC.Name, capacity.String(), sourceSize) + } + return nil +} diff --git a/pkg/restore/inplace/preflight_test.go b/pkg/restore/inplace/preflight_test.go index 094a70273..6b2bb914d 100644 --- a/pkg/restore/inplace/preflight_test.go +++ b/pkg/restore/inplace/preflight_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1api "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -82,10 +83,16 @@ func TestCheckPVCNotInUse(t *testing.T) { tests := []struct { name string pods []*corev1api.Pod + pvc *corev1api.PersistentVolumeClaim restoreUID types.UID expectPass bool expectMessage []string + expectError string }{ + { + name: "nil PVC returns error", + expectError: "pvc cannot be nil", + }, { name: "no pods, check passes", expectPass: true, @@ -183,11 +190,19 @@ func TestCheckPVCNotInUse(t *testing.T) { } cli := velerotest.NewFakeControllerRuntimeClient(t, objs...) - pvc := &corev1api.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{Name: "pvc-1", Namespace: "default"}, + pvc := tc.pvc + if pvc == nil && tc.name != "nil PVC returns error" { + pvc = &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: "pvc-1", Namespace: "default"}, + } } err := CheckPVCNotInUse(t.Context(), cli, pvc, tc.restoreUID) + if tc.expectError != "" { + require.Error(t, err) + assert.EqualError(t, err, tc.expectError) + return + } if tc.expectPass { require.NoError(t, err) return @@ -216,6 +231,12 @@ func TestCheckPVCBoundToBackedUpPV(t *testing.T) { backedUpPVName string expectError string }{ + { + name: "nil existing PVC returns error", + existingPVC: nil, + backedUpPVName: "pv-1", + expectError: "existing PVC cannot be nil", + }, { name: "bound to the backed-up PV, check passes", existingPVC: pvc("default", "pv-1", corev1api.ClaimBound), @@ -263,3 +284,67 @@ func TestCheckPVCBoundToBackedUpPV(t *testing.T) { }) } } + +func TestCheckPVCCapacity(t *testing.T) { + pvc := func(capacity string) *corev1api.PersistentVolumeClaim { + p := &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: "pvc-1", Namespace: "default"}, + } + if capacity != "" { + p.Status.Capacity = corev1api.ResourceList{corev1api.ResourceStorage: resource.MustParse(capacity)} + } + return p + } + const mi = int64(1 << 20) + + tests := []struct { + name string + existingPVC *corev1api.PersistentVolumeClaim + sourceSize int64 + expectError string + }{ + { + name: "capacity larger than source volume, check passes", + existingPVC: pvc("100Mi"), + sourceSize: 50 * mi, + }, + { + name: "capacity equal to source volume, check passes", + existingPVC: pvc("100Mi"), + sourceSize: 100 * mi, + }, + { + name: "capacity smaller than source volume, check fails", + existingPVC: pvc("50Mi"), + sourceSize: 100 * mi, + expectError: "capacity 50Mi is smaller than the backed-up volume size 104857600 bytes", + }, + { + name: "unknown source size is skipped", + existingPVC: pvc("50Mi"), + sourceSize: 0, + }, + { + name: "missing capacity is skipped", + existingPVC: pvc(""), + sourceSize: 100 * mi, + }, + { + name: "capacity in decimal units compares by value", + existingPVC: pvc("104857600"), + sourceSize: 100 * mi, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := CheckPVCCapacity(tc.existingPVC, tc.sourceSize) + if tc.expectError == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectError) + }) + } +} diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 247c8abb3..b2ef9edf4 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -27,6 +27,7 @@ import ( "reflect" "slices" "sort" + "strconv" "strings" "sync" "time" @@ -1636,15 +1637,23 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso return warnings, errs, itemExists } - // Strip any pre-existing Velero-internal in-place restore carrier annotation coming from - // the backup metadata before RestoreItemActions run. The carrier is only trusted when it - // is set by a RestoreItemAction (the PVC CSI RIA) during this restore; a stale carrier - // baked into the backup must not be translated into the Kubernetes "selected-node" - // annotation, which could pin a newly provisioned PVC to a stale node. - if annotations := obj.GetAnnotations(); annotations != nil { - if _, present := annotations[velerov1api.InplaceRestoreSelectedNodeAnnotation]; present { - restoreLogger.Infof("Removing pre-existing %q annotation from backup metadata", velerov1api.InplaceRestoreSelectedNodeAnnotation) - delete(annotations, velerov1api.InplaceRestoreSelectedNodeAnnotation) + // Strip any pre-existing Velero-internal in-place restore carrier annotations coming from + // the backup metadata before RestoreItemActions run. A carrier is only trusted when it is + // set during this restore (by the engine below or by the PVC CSI RIA); a stale carrier + // baked into the backup must not be acted on, e.g. a stale "selected-node" could pin a + // newly provisioned PVC to a stale node. + stripInplaceRestoreCarrierAnnotations(obj) + + // Carry the source volume size from the backup volume info to the PVC CSI RIA, which has no + // access to the volume info, so it can run the in-place restore capacity pre-flight check. + if groupResource == kuberesource.PersistentVolumeClaims { + pvName, _, _ := unstructured.NestedString(obj.Object, "spec", "volumeName") + if sourceSize := ctx.backupVolumeInfoMap[pvName].SourceSize(); sourceSize > 0 { + annotations := obj.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + annotations[velerov1api.InplaceRestoreSourceSizeAnnotation] = strconv.FormatInt(sourceSize, 10) obj.SetAnnotations(annotations) } } @@ -1788,15 +1797,13 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso // while the carrier annotation passes through untouched. The carrier itself is always // stripped so it never lands on the cluster. if annotations := obj.GetAnnotations(); annotations != nil { - if selectedNode, present := annotations[velerov1api.InplaceRestoreSelectedNodeAnnotation]; present { - if selectedNode != "" { - restoreLogger.Infof("Restoring %q annotation with value %q from in-place restore carrier annotation", kube.KubeAnnSelectedNode, selectedNode) - annotations[kube.KubeAnnSelectedNode] = selectedNode - } - delete(annotations, velerov1api.InplaceRestoreSelectedNodeAnnotation) + if selectedNode := annotations[velerov1api.InplaceRestoreSelectedNodeAnnotation]; selectedNode != "" { + restoreLogger.Infof("Restoring %q annotation with value %q from in-place restore carrier annotation", kube.KubeAnnSelectedNode, selectedNode) + annotations[kube.KubeAnnSelectedNode] = selectedNode obj.SetAnnotations(annotations) } } + stripInplaceRestoreCarrierAnnotations(obj) // This comes after running item actions because we have built-in actions that restore // a PVC's associated PV (if applicable). As part of the PV being restored, the 'pvsToProvision' @@ -2513,6 +2520,22 @@ func resetMetadataAndStatus(obj *unstructured.Unstructured) (*unstructured.Unstr return obj, nil } +// inplaceRestoreCarrierAnnotations are the Velero-internal annotations used to pass data +// between the restore engine and the in-place restore RestoreItemActions. They never land on +// the cluster. +var inplaceRestoreCarrierAnnotations = []string{ + velerov1api.InplaceRestoreSelectedNodeAnnotation, + velerov1api.InplaceRestoreSourceSizeAnnotation, +} + +func stripInplaceRestoreCarrierAnnotations(obj metav1.Object) { + annotations := obj.GetAnnotations() + for _, k := range inplaceRestoreCarrierAnnotations { + delete(annotations, k) + } + obj.SetAnnotations(annotations) +} + // addRestoreLabels labels the provided object with the restore name and the // restored backup's name. func addRestoreLabels(obj metav1.Object, restoreName, backupName string) { diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index b1c1475b6..09fad7bf1 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -453,6 +453,99 @@ func TestRestoreResourceFiltering(t *testing.T) { test.PVs(): {"/pv-1"}, }, }, + { + name: "notin label selector excludes matching resources", + restore: defaultRestore().LabelSelector(&metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{ + {Key: "pr-label", Operator: metav1.LabelSelectorOpNotIn, Values: []string{"1"}}, + }}).Result(), + backup: defaultBackup().Result(), + tarball: test.NewTarWriter(t). + AddItems("pods", + builder.ForPod("ns-1", "pod-1").ObjectMeta(builder.WithLabels("pr-label", "1")).Result(), + builder.ForPod("ns-2", "pod-2").Result(), + ). + AddItems("deployments.apps", + builder.ForDeployment("ns-1", "deploy-1").Result(), + builder.ForDeployment("ns-2", "deploy-2").ObjectMeta(builder.WithLabels("pr-label", "1")).Result(), + ). + AddItems("persistentvolumes", + builder.ForPersistentVolume("pv-1").ObjectMeta(builder.WithLabels("pr-label", "1")).Result(), + builder.ForPersistentVolume("pv-2").ObjectMeta(builder.WithLabels("pr-label", "2")).Result(), + ). + Done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-2/pod-2"}, + test.Deployments(): {"ns-1/deploy-1"}, + test.PVs(): {"/pv-2"}, + }, + }, + { + name: "in label selector only restores matching resources", + restore: defaultRestore().LabelSelector(&metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{ + {Key: "pr-label", Operator: metav1.LabelSelectorOpIn, Values: []string{"1", "2"}}, + }}).Result(), + backup: defaultBackup().Result(), + tarball: test.NewTarWriter(t). + AddItems("pods", + builder.ForPod("ns-1", "pod-1").ObjectMeta(builder.WithLabels("pr-label", "1")).Result(), + builder.ForPod("ns-2", "pod-2").ObjectMeta(builder.WithLabels("pr-label", "3")).Result(), + ). + AddItems("deployments.apps", + builder.ForDeployment("ns-1", "deploy-1").Result(), + builder.ForDeployment("ns-2", "deploy-2").ObjectMeta(builder.WithLabels("pr-label", "2")).Result(), + ). + AddItems("persistentvolumes", + builder.ForPersistentVolume("pv-1").ObjectMeta(builder.WithLabels("pr-label", "2")).Result(), + builder.ForPersistentVolume("pv-2").Result(), + ). + Done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1"}, + test.Deployments(): {"ns-2/deploy-2"}, + test.PVs(): {"/pv-1"}, + }, + }, + { + name: "doesnotexist label selector only restores resources without the label key", + restore: defaultRestore().LabelSelector(&metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{ + {Key: "pr-label", Operator: metav1.LabelSelectorOpDoesNotExist}, + }}).Result(), + backup: defaultBackup().Result(), + tarball: test.NewTarWriter(t). + AddItems("pods", + builder.ForPod("ns-1", "pod-1").ObjectMeta(builder.WithLabels("pr-label", "1")).Result(), + builder.ForPod("ns-2", "pod-2").Result(), + ). + AddItems("deployments.apps", + builder.ForDeployment("ns-1", "deploy-1").Result(), + builder.ForDeployment("ns-2", "deploy-2").ObjectMeta(builder.WithLabels("pr-label", "2")).Result(), + ). + AddItems("persistentvolumes", + builder.ForPersistentVolume("pv-1").ObjectMeta(builder.WithLabels("other-label", "x")).Result(), + builder.ForPersistentVolume("pv-2").ObjectMeta(builder.WithLabels("pr-label", "1")).Result(), + ). + Done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-2/pod-2"}, + test.Deployments(): {"ns-1/deploy-1"}, + test.PVs(): {"/pv-1"}, + }, + }, { name: "OrLabelSelectors only restores matching resources", restore: defaultRestore().OrLabelSelector([]*metav1.LabelSelector{{MatchLabels: map[string]string{"a1": "b1"}}, {MatchLabels: map[string]string{"a2": "b2"}}, @@ -5160,3 +5253,84 @@ func TestHasPodVolumeBackup(t *testing.T) { }) } } + +func TestRestoreInplaceSourceSizeCarrierAnnotation(t *testing.T) { + newRequest := func(t *testing.T, h *harness, volumeInfos map[string]volume.BackupVolumeInfo) *Request { + t.Helper() + return &Request{ + Log: h.log, + Restore: defaultRestore().Result(), + Backup: defaultBackup().Result(), + BackupReader: test.NewTarWriter(t). + AddItems("persistentvolumeclaims", builder.ForPersistentVolumeClaim("ns-1", "pvc-1").VolumeName("pv-1").Result()). + Done(), + BackupVolumeInfoMap: volumeInfos, + } + } + + // captureCarrier records the source-size carrier the RIA sees on the item. + captureCarrier := func(seen *string) riav2.RestoreItemAction { + return &pluggableAction{ + executeFunc: func(input *velero.RestoreItemActionExecuteInput) (*velero.RestoreItemActionExecuteOutput, error) { + item := input.Item.(*unstructured.Unstructured) + *seen = item.GetAnnotations()[velerov1api.InplaceRestoreSourceSizeAnnotation] + return &velero.RestoreItemActionExecuteOutput{UpdatedItem: item}, nil + }, + } + } + + t.Run("source size from volume info is carried to RIAs and stripped from the cluster object", func(t *testing.T) { + h := newHarness(t) + h.AddItems(t, test.PVCs()) + var seen string + + warnings, errs := h.restorer.Restore( + newRequest(t, h, map[string]volume.BackupVolumeInfo{ + "pv-1": {PVCNamespace: "ns-1", PVCName: "pvc-1", PVBInfo: &volume.PodVolumeBackupInfo{SourceSize: 31457288}}, + }), + []riav2.RestoreItemAction{captureCarrier(&seen)}, + nil, + ) + assertEmptyResults(t, warnings, errs) + assert.Equal(t, "31457288", seen) + + got, err := h.DynamicClient.Resource(test.PVCs().GVR()).Namespace("ns-1").Get(t.Context(), "pvc-1", metav1.GetOptions{}) + require.NoError(t, err) + assert.NotContains(t, got.GetAnnotations(), velerov1api.InplaceRestoreSourceSizeAnnotation) + }) + + t.Run("no carrier when the volume info has no source size", func(t *testing.T) { + h := newHarness(t) + h.AddItems(t, test.PVCs()) + var seen string + + warnings, errs := h.restorer.Restore( + newRequest(t, h, map[string]volume.BackupVolumeInfo{ + "pv-1": {PVCNamespace: "ns-1", PVCName: "pvc-1", PVBInfo: &volume.PodVolumeBackupInfo{}}, + }), + []riav2.RestoreItemAction{captureCarrier(&seen)}, + nil, + ) + assertEmptyResults(t, warnings, errs) + assert.Empty(t, seen) + }) + + t.Run("stale carrier from the backup metadata is not trusted", func(t *testing.T) { + h := newHarness(t) + h.AddItems(t, test.PVCs()) + var seen string + + req := newRequest(t, h, nil) + req.BackupReader = test.NewTarWriter(t). + AddItems("persistentvolumeclaims", builder.ForPersistentVolumeClaim("ns-1", "pvc-1"). + ObjectMeta(builder.WithAnnotations(velerov1api.InplaceRestoreSourceSizeAnnotation, "999")).Result()). + Done() + warnings, errs := h.restorer.Restore(req, []riav2.RestoreItemAction{captureCarrier(&seen)}, nil) + assertEmptyResults(t, warnings, errs) + assert.Empty(t, seen) + + got, err := h.DynamicClient.Resource(test.PVCs().GVR()).Namespace("ns-1").Get(t.Context(), "pvc-1", metav1.GetOptions{}) + require.NoError(t, err) + assert.NotContains(t, got.GetAnnotations(), velerov1api.InplaceRestoreSourceSizeAnnotation) + }) +} 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/csi-snapshot-data-movement.md b/site/content/docs/main/csi-snapshot-data-movement.md index 9c9bc6184..1e789434d 100644 --- a/site/content/docs/main/csi-snapshot-data-movement.md +++ b/site/content/docs/main/csi-snapshot-data-movement.md @@ -373,7 +373,7 @@ spec: ...... ``` -At present, Velero doesn't allow to set `ReadOnlyRootFileSystem` parameter to data mover pods, so the root filesystem for the data mover pods are always writable. +At present, Velero doesn't allow setting the `ReadOnlyRootFileSystem` parameter on data mover pods, so the root filesystem for the data mover pods is always writable. ### Resource Consumption diff --git a/site/content/docs/main/resource-filtering.md b/site/content/docs/main/resource-filtering.md index 9f57a7f4e..94838fe99 100644 --- a/site/content/docs/main/resource-filtering.md +++ b/site/content/docs/main/resource-filtering.md @@ -103,6 +103,30 @@ Includes cluster-scoped resources. Cannot work with `--include-cluster-scoped-re velero backup create --selector " notin ()" ``` +The same selector syntax works on restore. Set-based selectors are useful for phased restores: restore labeled resources first, then everything else. + +* Restore only resources matching the label selector. + + ```bash + velero restore create --from-backup --selector = + ``` + +* Restore everything in the backup except resources matching the selector. + + ```bash + velero restore create --from-backup --selector " notin ()" + ``` + + `notin` also matches resources that don't have the `` label at all: this restores resources whose `` label has any other value, as well as resources without the `` label. + +* Restore only resources that do not have a particular label key. + + ```bash + velero restore create --from-backup --selector '!' + ``` + +Note: resources pulled in as dependencies of selected items by restore item actions (for example, a restored pod's service account or persistent volume claims) are restored even if the label selector would exclude them. + For more information read the [Kubernetes label selector documentation](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors) ### --or-selector 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 diff --git a/site/content/docs/main/volume-group-snapshots.md b/site/content/docs/main/volume-group-snapshots.md index e62a75624..9b77979ce 100644 --- a/site/content/docs/main/volume-group-snapshots.md +++ b/site/content/docs/main/volume-group-snapshots.md @@ -160,15 +160,14 @@ kubectl get volumegroupsnapshotclass -o wide **Important:** The VolumeGroupSnapshotClass must have the label `velero.io/csi-volumegroupsnapshot-class: "true"` for Velero to automatically discover and use it: ```yaml -apiVersion: groupsnapshot.storage.k8s.io/v1alpha1 +apiVersion: groupsnapshot.storage.k8s.io/v1beta2 kind: VolumeGroupSnapshotClass metadata: name: csi-vgs-class labels: velero.io/csi-volumegroupsnapshot-class: "true" -spec: - driver: ebs.csi.aws.com - deletionPolicy: Delete +driver: ebs.csi.aws.com +deletionPolicy: Delete ``` Verify your VolumeGroupSnapshotClass has the correct label: diff --git a/site/content/docs/v1.18/volume-group-snapshots.md b/site/content/docs/v1.18/volume-group-snapshots.md index e62a75624..9b77979ce 100644 --- a/site/content/docs/v1.18/volume-group-snapshots.md +++ b/site/content/docs/v1.18/volume-group-snapshots.md @@ -160,15 +160,14 @@ kubectl get volumegroupsnapshotclass -o wide **Important:** The VolumeGroupSnapshotClass must have the label `velero.io/csi-volumegroupsnapshot-class: "true"` for Velero to automatically discover and use it: ```yaml -apiVersion: groupsnapshot.storage.k8s.io/v1alpha1 +apiVersion: groupsnapshot.storage.k8s.io/v1beta2 kind: VolumeGroupSnapshotClass metadata: name: csi-vgs-class labels: velero.io/csi-volumegroupsnapshot-class: "true" -spec: - driver: ebs.csi.aws.com - deletionPolicy: Delete +driver: ebs.csi.aws.com +deletionPolicy: Delete ``` Verify your VolumeGroupSnapshotClass has the correct label: