Skip to content

Commit

Permalink
ItemBlock model and phase 1 (single-thread) workflow changes
Browse files Browse the repository at this point in the history
Signed-off-by: Scott Seago <[email protected]>
  • Loading branch information
sseago committed Sep 3, 2024
1 parent 8ae667e commit 9d6f4d2
Show file tree
Hide file tree
Showing 18 changed files with 1,446 additions and 213 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/8102-sseago
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ItemBlock model and phase 1 (single-thread) workflow changes
20 changes: 14 additions & 6 deletions design/backup-performance-improvements.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,19 +109,27 @@ This mainly applies to plugins that operate on pods which reference resources wh

### Changes to processing item list from the Item Collector

#### New structs ItemBlock and ItemBlockItem
#### New structs BackupItemBlock, ItemBlock, and ItemBlockItem
```go
type ItemBlock struct {
log logrus.FieldLogger
package backup

type BackupItemBlock struct {
itemblock.ItemBlock
// This is a reference to the shared itemBackupper for the backup
itemBackupper *itemBackupper
}

package itemblock

type ItemBlock struct {
Log logrus.FieldLogger
Items []ItemBlockItem
}

type ItemBlockItem struct {
gr schema.GroupResource
item *unstructured.Unstructured
preferredGVR schema.GroupVersionResource
Gr schema.GroupResource
Item *unstructured.Unstructured
PreferredGVR schema.GroupVersionResource
}
```

Expand Down
10 changes: 5 additions & 5 deletions internal/hook/hook_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type hookKey struct {
// For hooks specified in pod annotation, this field is the pod where hooks are annotated.
podName string
// HookPhase is only for backup hooks, for restore hooks, this field is empty.
hookPhase hookPhase
hookPhase HookPhase
// HookName is only for hooks specified in the backup/restore spec.
// For hooks specified in pod annotation, this field is empty or "<from-annotation>".
hookName string
Expand Down Expand Up @@ -83,7 +83,7 @@ func NewHookTracker() *HookTracker {
// Add adds a hook to the hook tracker
// Add must precede the Record for each individual hook.
// In other words, a hook must be added to the tracker before its execution result is recorded.
func (ht *HookTracker) Add(podNamespace, podName, container, source, hookName string, hookPhase hookPhase) {
func (ht *HookTracker) Add(podNamespace, podName, container, source, hookName string, hookPhase HookPhase) {
ht.lock.Lock()
defer ht.lock.Unlock()

Expand All @@ -108,7 +108,7 @@ func (ht *HookTracker) Add(podNamespace, podName, container, source, hookName st
// Record records the hook's execution status
// Add must precede the Record for each individual hook.
// In other words, a hook must be added to the tracker before its execution result is recorded.
func (ht *HookTracker) Record(podNamespace, podName, container, source, hookName string, hookPhase hookPhase, hookFailed bool, hookErr error) error {
func (ht *HookTracker) Record(podNamespace, podName, container, source, hookName string, hookPhase HookPhase, hookFailed bool, hookErr error) error {
ht.lock.Lock()
defer ht.lock.Unlock()

Expand Down Expand Up @@ -179,7 +179,7 @@ func NewMultiHookTracker() *MultiHookTracker {
}

// Add adds a backup/restore hook to the tracker
func (mht *MultiHookTracker) Add(name, podNamespace, podName, container, source, hookName string, hookPhase hookPhase) {
func (mht *MultiHookTracker) Add(name, podNamespace, podName, container, source, hookName string, hookPhase HookPhase) {
mht.lock.Lock()
defer mht.lock.Unlock()

Expand All @@ -190,7 +190,7 @@ func (mht *MultiHookTracker) Add(name, podNamespace, podName, container, source,
}

// Record records a backup/restore hook execution status
func (mht *MultiHookTracker) Record(name, podNamespace, podName, container, source, hookName string, hookPhase hookPhase, hookFailed bool, hookErr error) error {
func (mht *MultiHookTracker) Record(name, podNamespace, podName, container, source, hookName string, hookPhase HookPhase, hookFailed bool, hookErr error) error {
mht.lock.RLock()
defer mht.lock.RUnlock()

Expand Down
22 changes: 11 additions & 11 deletions internal/hook/item_hook_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ import (
"github.com/vmware-tanzu/velero/pkg/util/kube"
)

type hookPhase string
type HookPhase string

const (
PhasePre hookPhase = "pre"
PhasePost hookPhase = "post"
PhasePre HookPhase = "pre"
PhasePost HookPhase = "post"
)

const (
Expand Down Expand Up @@ -81,7 +81,7 @@ type ItemHookHandler interface {
groupResource schema.GroupResource,
obj runtime.Unstructured,
resourceHooks []ResourceHook,
phase hookPhase,
phase HookPhase,
hookTracker *HookTracker,
) error
}
Expand Down Expand Up @@ -200,7 +200,7 @@ func (h *DefaultItemHookHandler) HandleHooks(
groupResource schema.GroupResource,
obj runtime.Unstructured,
resourceHooks []ResourceHook,
phase hookPhase,
phase HookPhase,
hookTracker *HookTracker,
) error {
// We only support hooks on pods right now
Expand Down Expand Up @@ -312,27 +312,27 @@ func (h *NoOpItemHookHandler) HandleHooks(
groupResource schema.GroupResource,
obj runtime.Unstructured,
resourceHooks []ResourceHook,
phase hookPhase,
phase HookPhase,
hookTracker *HookTracker,
) error {
return nil
}

func phasedKey(phase hookPhase, key string) string {
func phasedKey(phase HookPhase, key string) string {
if phase != "" {
return fmt.Sprintf("%v.%v", phase, key)
}
return key
}

func getHookAnnotation(annotations map[string]string, key string, phase hookPhase) string {
func getHookAnnotation(annotations map[string]string, key string, phase HookPhase) string {
return annotations[phasedKey(phase, key)]
}

// getPodExecHookFromAnnotations returns an ExecHook based on the annotations, as long as the
// 'command' annotation is present. If it is absent, this returns nil.
// If there is an error in parsing a supplied timeout, it is logged.
func getPodExecHookFromAnnotations(annotations map[string]string, phase hookPhase, log logrus.FieldLogger) *velerov1api.ExecHook {
func getPodExecHookFromAnnotations(annotations map[string]string, phase HookPhase, log logrus.FieldLogger) *velerov1api.ExecHook {
commandValue := getHookAnnotation(annotations, podBackupHookCommandAnnotationKey, phase)
if commandValue == "" {
return nil
Expand Down Expand Up @@ -561,7 +561,7 @@ func GroupRestoreExecHooks(
if hookFromAnnotation.Container == "" {
hookFromAnnotation.Container = pod.Spec.Containers[0].Name
}
hookTrack.Add(restoreName, metadata.GetNamespace(), metadata.GetName(), hookFromAnnotation.Container, HookSourceAnnotation, "<from-annotation>", hookPhase(""))
hookTrack.Add(restoreName, metadata.GetNamespace(), metadata.GetName(), hookFromAnnotation.Container, HookSourceAnnotation, "<from-annotation>", HookPhase(""))
byContainer[hookFromAnnotation.Container] = []PodExecRestoreHook{
{
HookName: "<from-annotation>",
Expand Down Expand Up @@ -596,7 +596,7 @@ func GroupRestoreExecHooks(
if named.Hook.Container == "" {
named.Hook.Container = pod.Spec.Containers[0].Name
}
hookTrack.Add(restoreName, metadata.GetNamespace(), metadata.GetName(), named.Hook.Container, HookSourceSpec, rrh.Name, hookPhase(""))
hookTrack.Add(restoreName, metadata.GetNamespace(), metadata.GetName(), named.Hook.Container, HookSourceSpec, rrh.Name, HookPhase(""))
byContainer[named.Hook.Container] = append(byContainer[named.Hook.Container], named)
}
}
Expand Down
6 changes: 3 additions & 3 deletions internal/hook/item_hook_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func TestHandleHooksSkips(t *testing.T) {
func TestHandleHooks(t *testing.T) {
tests := []struct {
name string
phase hookPhase
phase HookPhase
groupResource string
item runtime.Unstructured
hooks []ResourceHook
Expand Down Expand Up @@ -500,7 +500,7 @@ func TestHandleHooks(t *testing.T) {
}

func TestGetPodExecHookFromAnnotations(t *testing.T) {
phases := []hookPhase{"", PhasePre, PhasePost}
phases := []HookPhase{"", PhasePre, PhasePost}
for _, phase := range phases {
tests := []struct {
name string
Expand Down Expand Up @@ -1999,7 +1999,7 @@ func TestBackupHookTracker(t *testing.T) {
}
test1 := []struct {
name string
phase hookPhase
phase HookPhase
groupResource string
pods []podWithHook
hookTracker *HookTracker
Expand Down
6 changes: 3 additions & 3 deletions internal/hook/wait_exec_hook_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func (e *DefaultWaitExecHookHandler) HandleHooks(
hookLog.Error(err)
errors = append(errors, err)

errTracker := multiHookTracker.Record(restoreName, newPod.Namespace, newPod.Name, hook.Hook.Container, hook.HookSource, hook.HookName, hookPhase(""), true, err)
errTracker := multiHookTracker.Record(restoreName, newPod.Namespace, newPod.Name, hook.Hook.Container, hook.HookSource, hook.HookName, HookPhase(""), true, err)

Check warning on line 172 in internal/hook/wait_exec_hook_handler.go

View check run for this annotation

Codecov / codecov/patch

internal/hook/wait_exec_hook_handler.go#L172

Added line #L172 was not covered by tests
if errTracker != nil {
hookLog.WithError(errTracker).Warn("Error recording the hook in hook tracker")
}
Expand All @@ -195,7 +195,7 @@ func (e *DefaultWaitExecHookHandler) HandleHooks(
hookFailed = true
}

errTracker := multiHookTracker.Record(restoreName, newPod.Namespace, newPod.Name, hook.Hook.Container, hook.HookSource, hook.HookName, hookPhase(""), hookFailed, hookErr)
errTracker := multiHookTracker.Record(restoreName, newPod.Namespace, newPod.Name, hook.Hook.Container, hook.HookSource, hook.HookName, HookPhase(""), hookFailed, hookErr)
if errTracker != nil {
hookLog.WithError(errTracker).Warn("Error recording the hook in hook tracker")
}
Expand Down Expand Up @@ -247,7 +247,7 @@ func (e *DefaultWaitExecHookHandler) HandleHooks(
},
)

errTracker := multiHookTracker.Record(restoreName, pod.Namespace, pod.Name, hook.Hook.Container, hook.HookSource, hook.HookName, hookPhase(""), true, err)
errTracker := multiHookTracker.Record(restoreName, pod.Namespace, pod.Name, hook.Hook.Container, hook.HookSource, hook.HookName, HookPhase(""), true, err)
if errTracker != nil {
hookLog.WithError(errTracker).Warn("Error recording the hook in hook tracker")
}
Expand Down
10 changes: 5 additions & 5 deletions internal/hook/wait_exec_hook_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1012,17 +1012,17 @@ func TestRestoreHookTrackerUpdate(t *testing.T) {
}

hookTracker1 := NewMultiHookTracker()
hookTracker1.Add("restore1", "default", "my-pod", "container1", HookSourceAnnotation, "<from-annotation>", hookPhase(""))
hookTracker1.Add("restore1", "default", "my-pod", "container1", HookSourceAnnotation, "<from-annotation>", HookPhase(""))

hookTracker2 := NewMultiHookTracker()
hookTracker2.Add("restore1", "default", "my-pod", "container1", HookSourceSpec, "my-hook-1", hookPhase(""))
hookTracker2.Add("restore1", "default", "my-pod", "container1", HookSourceSpec, "my-hook-1", HookPhase(""))

hookTracker3 := NewMultiHookTracker()
hookTracker3.Add("restore1", "default", "my-pod", "container1", HookSourceSpec, "my-hook-1", hookPhase(""))
hookTracker3.Add("restore1", "default", "my-pod", "container2", HookSourceSpec, "my-hook-2", hookPhase(""))
hookTracker3.Add("restore1", "default", "my-pod", "container1", HookSourceSpec, "my-hook-1", HookPhase(""))
hookTracker3.Add("restore1", "default", "my-pod", "container2", HookSourceSpec, "my-hook-2", HookPhase(""))

hookTracker4 := NewMultiHookTracker()
hookTracker4.Add("restore1", "default", "my-pod", "container1", HookSourceSpec, "my-hook-1", hookPhase(""))
hookTracker4.Add("restore1", "default", "my-pod", "container1", HookSourceSpec, "my-hook-1", HookPhase(""))

tests1 := []struct {
name string
Expand Down
Loading

0 comments on commit 9d6f4d2

Please sign in to comment.