Skip to content

Commit

Permalink
Merge pull request #6918 from Ripolin/main
Browse files Browse the repository at this point in the history 
Add WaitForReady flag to check container readiness state before exec a hook
  • Loading branch information
@blackpiglet
blackpiglet committed Oct 15, 2023
2 parents ed441de + e5af7f5 commit b4fb2d9
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 77 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/6918-Ripolin
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Optional check if targeted container is ready before executing a hook
28 changes: 23 additions & 5 deletions internal/hook/item_hook_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package hook
import (
"encoding/json"
"fmt"
"strconv"
"strings"
"time"

Expand All @@ -37,6 +38,7 @@ import (
"github.com/vmware-tanzu/velero/pkg/kuberesource"
"github.com/vmware-tanzu/velero/pkg/podexec"
"github.com/vmware-tanzu/velero/pkg/restorehelper"
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
"github.com/vmware-tanzu/velero/pkg/util/collections"
"github.com/vmware-tanzu/velero/pkg/util/kube"
)
Expand All @@ -61,6 +63,7 @@ const (
podRestoreHookOnErrorAnnotationKey = "post.hook.restore.velero.io/on-error"
podRestoreHookTimeoutAnnotationKey = "post.hook.restore.velero.io/exec-timeout"
podRestoreHookWaitTimeoutAnnotationKey = "post.hook.restore.velero.io/wait-timeout"
podRestoreHookWaitForReadyAnnotationKey = "post.hook.restore.velero.io/wait-for-ready"
podRestoreHookInitContainerImageAnnotationKey = "init.hook.restore.velero.io/container-image"
podRestoreHookInitContainerNameAnnotationKey = "init.hook.restore.velero.io/container-name"
podRestoreHookInitContainerCommandAnnotationKey = "init.hook.restore.velero.io/command"
Expand Down Expand Up @@ -477,12 +480,23 @@ func getPodExecRestoreHookFromAnnotations(annotations map[string]string, log log
}
}

waitForReadyString := annotations[podRestoreHookWaitForReadyAnnotationKey]
waitForReady := boolptr.False()
if waitForReadyString != "" {
var err error
*waitForReady, err = strconv.ParseBool(waitForReadyString)
if err != nil {
log.Warn(errors.Wrapf(err, "Unable to parse wait for ready %s, ignoring", waitForReadyString))
}
}

return &velerov1api.ExecRestoreHook{
Container: container,
Command: parseStringToCommand(commandValue),
OnError: onError,
ExecTimeout: metav1.Duration{Duration: execTimeout},
WaitTimeout: metav1.Duration{Duration: waitTimeout},
Container: container,
Command: parseStringToCommand(commandValue),
OnError: onError,
ExecTimeout: metav1.Duration{Duration: execTimeout},
WaitTimeout: metav1.Duration{Duration: waitTimeout},
WaitForReady: waitForReady,
}
}

Expand Down Expand Up @@ -542,6 +556,10 @@ func GroupRestoreExecHooks(
Hook: *rh.Exec,
HookSource: "backupSpec",
}
// default to false if attr WaitForReady not set
if named.Hook.WaitForReady == nil {
named.Hook.WaitForReady = boolptr.False()
}
// default to first container in pod if unset, without mutating resource restore hook
if named.Hook.Container == "" {
named.Hook.Container = pod.Spec.Containers[0].Name
Expand Down
154 changes: 87 additions & 67 deletions internal/hook/item_hook_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/vmware-tanzu/velero/pkg/builder"
"github.com/vmware-tanzu/velero/pkg/kuberesource"
velerotest "github.com/vmware-tanzu/velero/pkg/test"
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
"github.com/vmware-tanzu/velero/pkg/util/collections"
)

Expand Down Expand Up @@ -724,7 +725,8 @@ func TestGetPodExecRestoreHookFromAnnotations(t *testing.T) {
podRestoreHookCommandAnnotationKey: "/usr/bin/foo",
},
expected: &velerov1api.ExecRestoreHook{
Command: []string{"/usr/bin/foo"},
Command: []string{"/usr/bin/foo"},
WaitForReady: boolptr.False(),
},
},
{
Expand All @@ -733,7 +735,8 @@ func TestGetPodExecRestoreHookFromAnnotations(t *testing.T) {
podRestoreHookCommandAnnotationKey: `["a","b","c"]`,
},
expected: &velerov1api.ExecRestoreHook{
Command: []string{"a", "b", "c"},
Command: []string{"a", "b", "c"},
WaitForReady: boolptr.False(),
},
},
{
Expand All @@ -743,8 +746,9 @@ func TestGetPodExecRestoreHookFromAnnotations(t *testing.T) {
podRestoreHookOnErrorAnnotationKey: string(velerov1api.HookErrorModeContinue),
},
expected: &velerov1api.ExecRestoreHook{
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeContinue,
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeContinue,
WaitForReady: boolptr.False(),
},
},
{
Expand All @@ -754,8 +758,9 @@ func TestGetPodExecRestoreHookFromAnnotations(t *testing.T) {
podRestoreHookOnErrorAnnotationKey: string(velerov1api.HookErrorModeFail),
},
expected: &velerov1api.ExecRestoreHook{
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeFail,
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeFail,
WaitForReady: boolptr.False(),
},
},
{
Expand All @@ -766,9 +771,10 @@ func TestGetPodExecRestoreHookFromAnnotations(t *testing.T) {
podRestoreHookWaitTimeoutAnnotationKey: "1h",
},
expected: &velerov1api.ExecRestoreHook{
Command: []string{"/usr/bin/foo"},
ExecTimeout: metav1.Duration{Duration: 45 * time.Second},
WaitTimeout: metav1.Duration{Duration: time.Hour},
Command: []string{"/usr/bin/foo"},
ExecTimeout: metav1.Duration{Duration: 45 * time.Second},
WaitTimeout: metav1.Duration{Duration: time.Hour},
WaitForReady: boolptr.False(),
},
},
{
Expand All @@ -778,8 +784,9 @@ func TestGetPodExecRestoreHookFromAnnotations(t *testing.T) {
podRestoreHookContainerAnnotationKey: "my-app",
},
expected: &velerov1api.ExecRestoreHook{
Command: []string{"/usr/bin/foo"},
Container: "my-app",
Command: []string{"/usr/bin/foo"},
Container: "my-app",
WaitForReady: boolptr.False(),
},
},
{
Expand All @@ -790,9 +797,10 @@ func TestGetPodExecRestoreHookFromAnnotations(t *testing.T) {
podRestoreHookTimeoutAnnotationKey: "none",
},
expected: &velerov1api.ExecRestoreHook{
Command: []string{"/usr/bin/foo"},
Container: "my-app",
ExecTimeout: metav1.Duration{Duration: 0},
Command: []string{"/usr/bin/foo"},
Container: "my-app",
ExecTimeout: metav1.Duration{Duration: 0},
WaitForReady: boolptr.False(),
},
},
{
Expand All @@ -803,9 +811,10 @@ func TestGetPodExecRestoreHookFromAnnotations(t *testing.T) {
podRestoreHookWaitTimeoutAnnotationKey: "none",
},
expected: &velerov1api.ExecRestoreHook{
Command: []string{"/usr/bin/foo"},
Container: "my-app",
ExecTimeout: metav1.Duration{Duration: 0},
Command: []string{"/usr/bin/foo"},
Container: "my-app",
ExecTimeout: metav1.Duration{Duration: 0},
WaitForReady: boolptr.False(),
},
},
}
Expand Down Expand Up @@ -842,6 +851,7 @@ func TestGroupRestoreExecHooks(t *testing.T) {
podRestoreHookOnErrorAnnotationKey, string(velerov1api.HookErrorModeContinue),
podRestoreHookTimeoutAnnotationKey, "1s",
podRestoreHookWaitTimeoutAnnotationKey, "1m",
podRestoreHookWaitForReadyAnnotationKey, "true",
)).
Containers(&corev1api.Container{
Name: "container1",
Expand All @@ -853,11 +863,12 @@ func TestGroupRestoreExecHooks(t *testing.T) {
HookName: "<from-annotation>",
HookSource: "annotation",
Hook: velerov1api.ExecRestoreHook{
Container: "container1",
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second},
WaitTimeout: metav1.Duration{Duration: time.Minute},
Container: "container1",
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second},
WaitTimeout: metav1.Duration{Duration: time.Minute},
WaitForReady: boolptr.True(),
},
},
},
Expand All @@ -883,11 +894,12 @@ func TestGroupRestoreExecHooks(t *testing.T) {
HookName: "<from-annotation>",
HookSource: "annotation",
Hook: velerov1api.ExecRestoreHook{
Container: "container1",
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second},
WaitTimeout: metav1.Duration{Duration: time.Minute},
Container: "container1",
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second},
WaitTimeout: metav1.Duration{Duration: time.Minute},
WaitForReady: boolptr.False(),
},
},
},
Expand Down Expand Up @@ -923,11 +935,12 @@ func TestGroupRestoreExecHooks(t *testing.T) {
HookName: "hook1",
HookSource: "backupSpec",
Hook: velerov1api.ExecRestoreHook{
Container: "container1",
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second},
WaitTimeout: metav1.Duration{Duration: time.Minute},
Container: "container1",
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second},
WaitTimeout: metav1.Duration{Duration: time.Minute},
WaitForReady: boolptr.False(),
},
},
},
Expand Down Expand Up @@ -962,11 +975,12 @@ func TestGroupRestoreExecHooks(t *testing.T) {
HookName: "hook1",
HookSource: "backupSpec",
Hook: velerov1api.ExecRestoreHook{
Container: "container1",
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second},
WaitTimeout: metav1.Duration{Duration: time.Minute},
Container: "container1",
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second},
WaitTimeout: metav1.Duration{Duration: time.Minute},
WaitForReady: boolptr.False(),
},
},
},
Expand Down Expand Up @@ -1009,11 +1023,12 @@ func TestGroupRestoreExecHooks(t *testing.T) {
HookName: "<from-annotation>",
HookSource: "annotation",
Hook: velerov1api.ExecRestoreHook{
Container: "container1",
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second},
WaitTimeout: metav1.Duration{Duration: time.Minute},
Container: "container1",
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second},
WaitTimeout: metav1.Duration{Duration: time.Minute},
WaitForReady: boolptr.False(),
},
},
},
Expand Down Expand Up @@ -1105,11 +1120,12 @@ func TestGroupRestoreExecHooks(t *testing.T) {
RestoreHooks: []velerov1api.RestoreResourceHook{
{
Exec: &velerov1api.ExecRestoreHook{
Container: "container1",
Command: []string{"/usr/bin/aaa"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second * 4},
WaitTimeout: metav1.Duration{Duration: time.Minute * 4},
Container: "container1",
Command: []string{"/usr/bin/aaa"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second * 4},
WaitTimeout: metav1.Duration{Duration: time.Minute * 4},
WaitForReady: boolptr.True(),
},
},
},
Expand All @@ -1126,33 +1142,36 @@ func TestGroupRestoreExecHooks(t *testing.T) {
HookName: "hook1",
HookSource: "backupSpec",
Hook: velerov1api.ExecRestoreHook{
Container: "container1",
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeFail,
ExecTimeout: metav1.Duration{Duration: time.Second},
WaitTimeout: metav1.Duration{Duration: time.Minute},
Container: "container1",
Command: []string{"/usr/bin/foo"},
OnError: velerov1api.HookErrorModeFail,
ExecTimeout: metav1.Duration{Duration: time.Second},
WaitTimeout: metav1.Duration{Duration: time.Minute},
WaitForReady: boolptr.False(),
},
},
{
HookName: "hook1",
HookSource: "backupSpec",
Hook: velerov1api.ExecRestoreHook{
Container: "container1",
Command: []string{"/usr/bin/bar"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second * 2},
WaitTimeout: metav1.Duration{Duration: time.Minute * 2},
Container: "container1",
Command: []string{"/usr/bin/bar"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second * 2},
WaitTimeout: metav1.Duration{Duration: time.Minute * 2},
WaitForReady: boolptr.False(),
},
},
{
HookName: "hook2",
HookSource: "backupSpec",
Hook: velerov1api.ExecRestoreHook{
Container: "container1",
Command: []string{"/usr/bin/aaa"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second * 4},
WaitTimeout: metav1.Duration{Duration: time.Minute * 4},
Container: "container1",
Command: []string{"/usr/bin/aaa"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second * 4},
WaitTimeout: metav1.Duration{Duration: time.Minute * 4},
WaitForReady: boolptr.True(),
},
},
},
Expand All @@ -1161,11 +1180,12 @@ func TestGroupRestoreExecHooks(t *testing.T) {
HookName: "hook1",
HookSource: "backupSpec",
Hook: velerov1api.ExecRestoreHook{
Container: "container2",
Command: []string{"/usr/bin/baz"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second * 3},
WaitTimeout: metav1.Duration{Duration: time.Second * 3},
Container: "container2",
Command: []string{"/usr/bin/baz"},
OnError: velerov1api.HookErrorModeContinue,
ExecTimeout: metav1.Duration{Duration: time.Second * 3},
WaitTimeout: metav1.Duration{Duration: time.Second * 3},
WaitForReady: boolptr.False(),
},
},
},
Expand Down
Loading

0 comments on commit b4fb2d9

Please sign in to comment.