Merge https://github.com/vmware-tanzu/velero:main (66ac235) into oadp-dev

changelogs/unreleased/9581-shubham-pampattiwar

@@ -0,0 +1 @@
+Fix DBR stuck when CSI snapshot no longer exists in cloud provider

internal/delete/actions/csi/volumesnapshotcontent_action.go

@@ -137,6 +137,10 @@ func (p *volumeSnapshotContentDeleteItemAction) Execute(
 			return checkVSCReadiness(ctx, &snapCont, p.crClient)
 		},
 	); err != nil {
+		// Clean up the VSC we created since it can't become ready
+		if deleteErr := p.crClient.Delete(context.TODO(), &snapCont); deleteErr != nil && !apierrors.IsNotFound(deleteErr) {
+			p.log.WithError(deleteErr).Errorf("Failed to clean up VolumeSnapshotContent %s", snapCont.Name)
+		}
 		return errors.Wrapf(err, "fail to wait VolumeSnapshotContent %s becomes ready.", snapCont.Name)
 	}
 
@@ -167,6 +171,13 @@ var checkVSCReadiness = func(
 		return true, nil
 	}
 
+	// Fail fast on permanent CSI driver errors (e.g., InvalidSnapshot.NotFound)
+	if tmpVSC.Status != nil && tmpVSC.Status.Error != nil && tmpVSC.Status.Error.Message != nil {
+		return false, errors.Errorf(
+			"VolumeSnapshotContent %s has error: %s", vsc.Name, *tmpVSC.Status.Error.Message,
+		)
+	}
+
 	return false, nil
 }
 

internal/delete/actions/csi/volumesnapshotcontent_action_test.go

@@ -94,6 +94,19 @@ func TestVSCExecute(t *testing.T) {
 				return false, errors.Errorf("test error case")
 			},
 		},
+		{
+			name:      "Error case with CSI error, dangling VSC should be cleaned up",
+			vsc:       builder.ForVolumeSnapshotContent("bar").ObjectMeta(builder.WithLabelsMap(map[string]string{velerov1api.BackupNameLabel: "backup"})).Status(&snapshotv1api.VolumeSnapshotContentStatus{SnapshotHandle: &snapshotHandleStr}).Result(),
+			backup:    builder.ForBackup("velero", "backup").ObjectMeta(builder.WithAnnotationsMap(map[string]string{velerov1api.ResourceTimeoutAnnotation: "5s"})).Result(),
+			expectErr: true,
+			function: func(
+				ctx context.Context,
+				vsc *snapshotv1api.VolumeSnapshotContent,
+				client crclient.Client,
+			) (bool, error) {
+				return false, errors.Errorf("VolumeSnapshotContent %s has error: InvalidSnapshot.NotFound", vsc.Name)
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -190,6 +203,24 @@ func TestCheckVSCReadiness(t *testing.T) {
 			expectErr: false,
 			ready:     false,
 		},
+		{
+			name: "VSC has error from CSI driver",
+			vsc: &snapshotv1api.VolumeSnapshotContent{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "vsc-1",
+					Namespace: "velero",
+				},
+				Status: &snapshotv1api.VolumeSnapshotContentStatus{
+					ReadyToUse: boolPtr(false),
+					Error: &snapshotv1api.VolumeSnapshotError{
+						Message: stringPtr("InvalidSnapshot.NotFound: The snapshot 'snap-0abc123' does not exist."),
+					},
+				},
+			},
+			createVSC: true,
+			expectErr: true,
+			ready:     false,
+		},
 	}
 
 	for _, test := range tests {
@@ -207,3 +238,11 @@ func TestCheckVSCReadiness(t *testing.T) {
 		})
 	}
 }
+
+func boolPtr(b bool) *bool {
+	return &b
+}
+
+func stringPtr(s string) *string {
+	return &s
+}

site/content/docs/main/api-types/schedule.md

@@ -63,6 +63,10 @@ spec:
     # CSI VolumeSnapshot status turns to ReadyToUse during creation, before
     # returning error as timeout. The default value is 10 minute.
     csiSnapshotTimeout: 10m
+    # ItemOperationTimeout specifies the time used to wait for
+    # asynchronous BackupItemAction operations
+    # The default value is 4 hour.
+    itemOperationTimeout: 4h
     # resourcePolicy specifies the referenced resource policies that backup should follow
     # optional
     resourcePolicy:

site/layouts/docs/docs.html

@@ -27,16 +27,6 @@ <h1>Documentation</h1>
           <div class="col-md-3 toc">
             {{ .Render "versions" }}
             <br/>
-            <div id="docsearch">
-              <!-- <form class="d-flex align-items-center">
-                <span class="algolia-autocomplete" style="position: relative; display: inline-block; direction: ltr;">
-                  <input type="search" class="form-control docsearch" id="search-input" placeholder="Search..."
-                          aria-label="Search for..." autocomplete="off" spellcheck="false" role="combobox"
-                          aria-autocomplete="list" aria-expanded="false" aria-owns="algolia-autocomplete-listbox-0"
-                          dir="auto" style="position: relative; vertical-align: top;">
-                </span>
-            </form> -->
-          </div>
             {{ .Render "nav" }}
           </div>
           <div class="col-md-8">
@@ -58,16 +48,6 @@ <h1>Documentation</h1>
     {{ .Render "footer" }}
   </div>
 </div>
-<script src="https://cdn.jsdelivr.net/npm/@docsearch/js@3"></script>
-<script type="text/javascript"> docsearch({
-    appId: '9ASKQJ1HR3',
-    apiKey: '170ba79bfa16cebfdf10726ae4771d7e',
-    indexName: 'velero_new',
-    container: '#docsearch',
-    searchParameters: {
-      facetFilters: ["version:{{ .CurrentSection.Params.version }}"]},
-  });
-</script>
 </body>
 
 </html>

site/layouts/partials/head-docs.html

@@ -8,6 +8,4 @@
   {{ $styles := resources.Get "styles.scss" | toCSS $options | resources.Fingerprint }}
   <link rel="stylesheet" href="{{ $styles.RelPermalink }}" integrity="{{ $styles.Data.Integrity }}">
   {{/* TODO  {% seo %}*/}}
-  <link rel="preconnect" href="https://9ASKQJ1HR3-dsn.algolia.net" crossorigin />
-  <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@docsearch/css@3" />
 </head>

test/e2e/basic/restore_exec_hooks.go

@@ -0,0 +1,150 @@
+/*
+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 basic
+
+import (
+	"fmt"
+	"path"
+	"strings"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	"github.com/vmware-tanzu/velero/test/e2e/test"
+	. "github.com/vmware-tanzu/velero/test/e2e/test"
+	"github.com/vmware-tanzu/velero/test/util/common"
+	. "github.com/vmware-tanzu/velero/test/util/k8s"
+)
+
+// RestoreExecHooks tests that a pod with multiple restore exec hooks does not hang
+// at the Finalizing phase during restore (Issue #9359 / PR #9366).
+type RestoreExecHooks struct {
+	TestCase
+	podName string
+}
+
+var RestoreExecHooksTest func() = test.TestFunc(&RestoreExecHooks{})
+
+func (r *RestoreExecHooks) Init() error {
+	Expect(r.TestCase.Init()).To(Succeed())
+	r.CaseBaseName = "restore-exec-hooks-" + r.UUIDgen
+	r.BackupName = "backup-" + r.CaseBaseName
+	r.RestoreName = "restore-" + r.CaseBaseName
+	r.podName = "pod-multiple-hooks"
+	r.NamespacesTotal = 1
+	r.NSIncluded = &[]string{}
+
+	for nsNum := 0; nsNum < r.NamespacesTotal; nsNum++ {
+		createNSName := fmt.Sprintf("%s-%00000d", r.CaseBaseName, nsNum)
+		*r.NSIncluded = append(*r.NSIncluded, createNSName)
+	}
+
+	r.TestMsg = &test.TestMSG{
+		Desc:      "Restore pod with multiple restore exec hooks",
+		Text:      "Should successfully backup and restore without hanging at Finalizing phase",
+		FailedMSG: "Failed to successfully backup and restore pod with multiple hooks",
+	}
+
+	r.BackupArgs = []string{
+		"create", "--namespace", r.VeleroCfg.VeleroNamespace, "backup", r.BackupName,
+		"--include-namespaces", strings.Join(*r.NSIncluded, ","),
+		"--default-volumes-to-fs-backup", "--wait",
+	}
+
+	r.RestoreArgs = []string{
+		"create", "--namespace", r.VeleroCfg.VeleroNamespace, "restore", r.RestoreName,
+		"--from-backup", r.BackupName, "--wait",
+	}
+
+	return nil
+}
+
+func (r *RestoreExecHooks) CreateResources() error {
+	for nsNum := 0; nsNum < r.NamespacesTotal; nsNum++ {
+		createNSName := fmt.Sprintf("%s-%00000d", r.CaseBaseName, nsNum)
+
+		By(fmt.Sprintf("Creating namespace %s", createNSName), func() {
+			Expect(CreateNamespace(r.Ctx, r.Client, createNSName)).
+				To(Succeed(), fmt.Sprintf("Failed to create namespace %s", createNSName))
+		})
+
+		// Prepare images and commands adaptively for the target OS
+		imageAddress := LinuxTestImage
+		initCommand := `["/bin/sh", "-c", "echo init-hook-done"]`
+		execCommand1 := `["/bin/sh", "-c", "echo hook1"]`
+		execCommand2 := `["/bin/sh", "-c", "echo hook2"]`
+
+		if r.VeleroCfg.WorkerOS == common.WorkerOSLinux && r.VeleroCfg.ImageRegistryProxy != "" {
+			imageAddress = path.Join(r.VeleroCfg.ImageRegistryProxy, LinuxTestImage)
+		} else if r.VeleroCfg.WorkerOS == common.WorkerOSWindows {
+			imageAddress = WindowTestImage
+			initCommand = `["cmd", "/c", "echo init-hook-done"]`
+			execCommand1 = `["cmd", "/c", "echo hook1"]`
+			execCommand2 = `["cmd", "/c", "echo hook2"]`
+		}
+
+		// Inject mixing InitContainer hook and multiple Exec post-restore hooks.
+		// This guarantees that the loop index 'i' mismatched 'hook.hookIndex' (Issue #9359),
+		// ensuring the bug is properly reproduced and the fix is verified.
+		ann := map[string]string{
+			// Inject InitContainer Restore Hook
+			"init.hook.restore.velero.io/container-image": imageAddress,
+			"init.hook.restore.velero.io/container-name":  "test-init-hook",
+			"init.hook.restore.velero.io/command":         initCommand,
+
+			// Inject multiple Exec Restore Hooks
+			"post.hook.restore.velero.io/test1.command":   execCommand1,
+			"post.hook.restore.velero.io/test1.container": r.podName,
+			"post.hook.restore.velero.io/test2.command":   execCommand2,
+			"post.hook.restore.velero.io/test2.container": r.podName,
+		}
+
+		By(fmt.Sprintf("Creating pod %s with multiple restore hooks in namespace %s", r.podName, createNSName), func() {
+			_, err := CreatePod(
+				r.Client,
+				createNSName,
+				r.podName,
+				"",         // No storage class needed
+				"",         // No PVC needed
+				[]string{}, // No volumes
+				nil,
+				ann,
+				r.VeleroCfg.ImageRegistryProxy,
+				r.VeleroCfg.WorkerOS,
+			)
+			Expect(err).To(Succeed(), fmt.Sprintf("Failed to create pod with hooks in namespace %s", createNSName))
+		})
+
+		By(fmt.Sprintf("Waiting for pod %s to be ready", r.podName), func() {
+			err := WaitForPods(r.Ctx, r.Client, createNSName, []string{r.podName})
+			Expect(err).To(Succeed(), fmt.Sprintf("Failed to wait for pod %s in namespace %s", r.podName, createNSName))
+		})
+	}
+	return nil
+}
+
+func (r *RestoreExecHooks) Verify() error {
+	for nsNum := 0; nsNum < r.NamespacesTotal; nsNum++ {
+		createNSName := fmt.Sprintf("%s-%00000d", r.CaseBaseName, nsNum)
+
+		By(fmt.Sprintf("Verifying pod %s in namespace %s after restore", r.podName, createNSName), func() {
+			err := WaitForPods(r.Ctx, r.Client, createNSName, []string{r.podName})
+			Expect(err).To(Succeed(), fmt.Sprintf("Failed to verify pod %s in namespace %s after restore", r.podName, createNSName))
+		})
+	}
+	return nil
+}

test/e2e/e2e_suite_test.go

@@ -440,6 +440,12 @@ var _ = Describe(
 	StorageClasssChangingTest,
 )
 
+var _ = Describe(
+	"Restore phase does not block at Finalizing when a container has multiple exec hooks",
+	Label("Basic", "Hooks"),
+	RestoreExecHooksTest,
+)
+
 var _ = Describe(
 	"Backup/restore of 2500 namespaces",
 	Label("Scale", "LongTime"),