[go: up one dir, main page]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ItemBlock model and phase 1 (single-thread) workflow changes #8102

Merged
merged 1 commit into from
Sep 9, 2024
Merged

ItemBlock model and phase 1 (single-thread) workflow changes #8102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/8102-sseago
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@
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 @@
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 @@
},
)

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
View file
Open in desktop
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
Loading