Skip to content

Commit b372610

Browse files
authored
Merge branch 'main' into secret-perms
2 parents b0d51fe + b7ffcf6 commit b372610

26 files changed

+930
-268
lines changed

Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ endif
6565
BUILDER_IMAGE := $(REGISTRY)/build-image:$(BUILDER_IMAGE_TAG)
6666
BUILDER_IMAGE_CACHED := $(shell docker images -q ${BUILDER_IMAGE} 2>/dev/null )
6767

68-
HUGO_IMAGE := hugo-builder
68+
HUGO_IMAGE := ghcr.io/gohugoio/hugo
6969

7070
# Which architecture to build - see $(ALL_ARCH) for options.
7171
# if the 'local' rule is being run, detect the ARCH from 'go env'
@@ -451,7 +451,7 @@ release:
451451
serve-docs: build-image-hugo
452452
docker run \
453453
--rm \
454-
-v "$$(pwd)/site:/srv/hugo" \
454+
-v "$$(pwd)/site:/project" \
455455
-it -p 1313:1313 \
456456
$(HUGO_IMAGE) \
457457
server --bind=0.0.0.0 --enableGitInfo=false
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
The backup and restore VGDP affinity enhancement implementation.

changelogs/unreleased/9048-sseago

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Allow for proper tracking of multiple hooks per container

design/vgdp-affinity-enhancement.md

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ The implemented [VGDP LoadAffinity design][3] already defined the a structure `L
1212

1313
There are still some limitations of this design:
1414
* The affinity setting is global. Say there are two StorageClasses and the underlying storage can only provision volumes to part of the cluster nodes. The supported nodes don't have intersection. Then the affinity will definitely not work in some cases.
15-
* The old design only take the first element of the []*LoadAffinity array. By this way, it cannot support the or logic between Affinity selectors.
1615
* The old design focuses on the backupPod affinity, but the restorePod also needs the affinity setting.
1716

1817
As a result, create this design to address the limitations.
@@ -34,7 +33,6 @@ This design still uses the ConfigMap specified by `velero node-agent` CLI's para
3433
Upon the implemented [VGDP LoadAffinity design][3] introduced `[]*LoadAffinity` structure, this design add a new field `StorageClass`. This field is optional.
3534
* If the `LoadAffinity` element's `StorageClass` doesn't have value, it means this element is applied to global, just as the old design.
3635
* If the `LoadAffinity` element's `StorageClass` has value, it means this element is applied to the VGDP instances' PVCs use the specified StorageClass.
37-
* To support the or logic between LoadAffinity elements, this design allows multiple instances of `LoadAffinity` whose `StorageClass` field have the same value.
3836
* The `LoadAffinity` element whose `StorageClass` has value has higher priority than the `LoadAffinity` element whose `StorageClass` doesn't have value.
3937

4038

@@ -93,14 +91,8 @@ flowchart TD
9391
9492
O -->|No loadAffinity configured| R[No affinity constraints<br/>Schedule on any available node<br/>🌐 DEFAULT]
9593
96-
N --> S{Multiple rules in array?}
97-
S -->|Yes| T[Apply all rules as OR conditions<br/>Pod scheduled on nodes matching ANY rule]
98-
S -->|No| U[Apply single rule<br/>Pod scheduled on nodes matching this rule]
99-
100-
O --> S
101-
102-
T --> V[Validate node-agent availability<br/>⚠️ Ensure node-agent pods exist on target nodes]
103-
U --> V
94+
O --> V[Validate node-agent availability<br/>⚠️ Ensure node-agent pods exist on target nodes]
95+
N --> V
10496
10597
V --> W{Node-agent available on selected nodes?}
10698
W -->|Yes| X[✅ VGDP Pod scheduled successfully]
@@ -126,40 +118,6 @@ flowchart TD
126118

127119
### Examples
128120

129-
#### Multiple LoadAffinities
130-
131-
``` json
132-
{
133-
"loadAffinity": [
134-
{
135-
"nodeSelector": {
136-
"matchLabels": {
137-
"beta.kubernetes.io/instance-type": "Standard_B4ms"
138-
}
139-
}
140-
},
141-
{
142-
"nodeSelector": {
143-
"matchExpressions": [
144-
{
145-
"key": "topology.kubernetes.io/zone",
146-
"operator": "In",
147-
"values": [
148-
"us-central1-a"
149-
]
150-
}
151-
]
152-
}
153-
}
154-
]
155-
}
156-
```
157-
158-
This sample demonstrates how to use multiple affinities in `loadAffinity`. That can support more complicated scenarios, e.g. need to filter nodes satisfied either of two conditions, instead of satisfied both of two conditions.
159-
160-
In this example, the VGDP pods will be assigned to nodes, which instance type is `Standard_B4ms` or which zone is `us-central1-a`.
161-
162-
163121
#### LoadAffinity interacts with LoadAffinityPerStorageClass
164122

165123
``` json

internal/hook/hook_tracker.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ type hookKey struct {
4646
// Container indicates the container hooks use.
4747
// For hooks specified in the backup/restore spec, the container might be the same under different hookName.
4848
container string
49+
// hookIndex contains the slice index for the specific hook, in order to track multiple hooks
50+
// for the same container
51+
hookIndex int
4952
}
5053

5154
// hookStatus records the execution status of a specific hook.
@@ -83,7 +86,7 @@ func NewHookTracker() *HookTracker {
8386
// Add adds a hook to the hook tracker
8487
// Add must precede the Record for each individual hook.
8588
// In other words, a hook must be added to the tracker before its execution result is recorded.
86-
func (ht *HookTracker) Add(podNamespace, podName, container, source, hookName string, hookPhase HookPhase) {
89+
func (ht *HookTracker) Add(podNamespace, podName, container, source, hookName string, hookPhase HookPhase, hookIndex int) {
8790
ht.lock.Lock()
8891
defer ht.lock.Unlock()
8992

@@ -94,6 +97,7 @@ func (ht *HookTracker) Add(podNamespace, podName, container, source, hookName st
9497
container: container,
9598
hookPhase: hookPhase,
9699
hookName: hookName,
100+
hookIndex: hookIndex,
97101
}
98102

99103
if _, ok := ht.tracker[key]; !ok {
@@ -108,7 +112,7 @@ func (ht *HookTracker) Add(podNamespace, podName, container, source, hookName st
108112
// Record records the hook's execution status
109113
// Add must precede the Record for each individual hook.
110114
// In other words, a hook must be added to the tracker before its execution result is recorded.
111-
func (ht *HookTracker) Record(podNamespace, podName, container, source, hookName string, hookPhase HookPhase, hookFailed bool, hookErr error) error {
115+
func (ht *HookTracker) Record(podNamespace, podName, container, source, hookName string, hookPhase HookPhase, hookIndex int, hookFailed bool, hookErr error) error {
112116
ht.lock.Lock()
113117
defer ht.lock.Unlock()
114118

@@ -119,6 +123,7 @@ func (ht *HookTracker) Record(podNamespace, podName, container, source, hookName
119123
container: container,
120124
hookPhase: hookPhase,
121125
hookName: hookName,
126+
hookIndex: hookIndex,
122127
}
123128

124129
if _, ok := ht.tracker[key]; !ok {
@@ -179,24 +184,24 @@ func NewMultiHookTracker() *MultiHookTracker {
179184
}
180185

181186
// Add adds a backup/restore hook to the tracker
182-
func (mht *MultiHookTracker) Add(name, podNamespace, podName, container, source, hookName string, hookPhase HookPhase) {
187+
func (mht *MultiHookTracker) Add(name, podNamespace, podName, container, source, hookName string, hookPhase HookPhase, hookIndex int) {
183188
mht.lock.Lock()
184189
defer mht.lock.Unlock()
185190

186191
if _, ok := mht.trackers[name]; !ok {
187192
mht.trackers[name] = NewHookTracker()
188193
}
189-
mht.trackers[name].Add(podNamespace, podName, container, source, hookName, hookPhase)
194+
mht.trackers[name].Add(podNamespace, podName, container, source, hookName, hookPhase, hookIndex)
190195
}
191196

192197
// Record records a backup/restore hook execution status
193-
func (mht *MultiHookTracker) Record(name, podNamespace, podName, container, source, hookName string, hookPhase HookPhase, hookFailed bool, hookErr error) error {
198+
func (mht *MultiHookTracker) Record(name, podNamespace, podName, container, source, hookName string, hookPhase HookPhase, hookIndex int, hookFailed bool, hookErr error) error {
194199
mht.lock.RLock()
195200
defer mht.lock.RUnlock()
196201

197202
var err error
198203
if _, ok := mht.trackers[name]; ok {
199-
err = mht.trackers[name].Record(podNamespace, podName, container, source, hookName, hookPhase, hookFailed, hookErr)
204+
err = mht.trackers[name].Record(podNamespace, podName, container, source, hookName, hookPhase, hookIndex, hookFailed, hookErr)
200205
} else {
201206
err = fmt.Errorf("the backup/restore not exist in hook tracker, backup/restore name: %s", name)
202207
}

internal/hook/hook_tracker_test.go

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func TestNewHookTracker(t *testing.T) {
3434
func TestHookTracker_Add(t *testing.T) {
3535
tracker := NewHookTracker()
3636

37-
tracker.Add("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "")
37+
tracker.Add("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0)
3838

3939
key := hookKey{
4040
podNamespace: "ns1",
@@ -51,8 +51,8 @@ func TestHookTracker_Add(t *testing.T) {
5151

5252
func TestHookTracker_Record(t *testing.T) {
5353
tracker := NewHookTracker()
54-
tracker.Add("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "")
55-
err := tracker.Record("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
54+
tracker.Add("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0)
55+
err := tracker.Record("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0, true, fmt.Errorf("err"))
5656

5757
key := hookKey{
5858
podNamespace: "ns1",
@@ -68,40 +68,41 @@ func TestHookTracker_Record(t *testing.T) {
6868
assert.True(t, info.hookExecuted)
6969
require.NoError(t, err)
7070

71-
err = tracker.Record("ns2", "pod2", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
71+
err = tracker.Record("ns2", "pod2", "container1", HookSourceAnnotation, "h1", "", 0, true, fmt.Errorf("err"))
7272
require.Error(t, err)
7373

74-
err = tracker.Record("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", false, nil)
74+
err = tracker.Record("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0, false, nil)
7575
require.NoError(t, err)
7676
assert.True(t, info.hookFailed)
7777
}
7878

7979
func TestHookTracker_Stat(t *testing.T) {
8080
tracker := NewHookTracker()
8181

82-
tracker.Add("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "")
83-
tracker.Add("ns2", "pod2", "container1", HookSourceAnnotation, "h2", "")
84-
tracker.Record("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
82+
tracker.Add("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0)
83+
tracker.Add("ns2", "pod2", "container1", HookSourceAnnotation, "h2", "", 0)
84+
tracker.Add("ns2", "pod2", "container1", HookSourceAnnotation, "h2", "", 1)
85+
tracker.Record("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0, true, fmt.Errorf("err"))
8586

8687
attempted, failed := tracker.Stat()
87-
assert.Equal(t, 2, attempted)
88+
assert.Equal(t, 3, attempted)
8889
assert.Equal(t, 1, failed)
8990
}
9091

9192
func TestHookTracker_IsComplete(t *testing.T) {
9293
tracker := NewHookTracker()
93-
tracker.Add("ns1", "pod1", "container1", HookSourceAnnotation, "h1", PhasePre)
94-
tracker.Record("ns1", "pod1", "container1", HookSourceAnnotation, "h1", PhasePre, true, fmt.Errorf("err"))
94+
tracker.Add("ns1", "pod1", "container1", HookSourceAnnotation, "h1", PhasePre, 0)
95+
tracker.Record("ns1", "pod1", "container1", HookSourceAnnotation, "h1", PhasePre, 0, true, fmt.Errorf("err"))
9596
assert.True(t, tracker.IsComplete())
9697

97-
tracker.Add("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "")
98+
tracker.Add("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0)
9899
assert.False(t, tracker.IsComplete())
99100
}
100101

101102
func TestHookTracker_HookErrs(t *testing.T) {
102103
tracker := NewHookTracker()
103-
tracker.Add("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "")
104-
tracker.Record("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
104+
tracker.Add("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0)
105+
tracker.Record("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0, true, fmt.Errorf("err"))
105106

106107
hookErrs := tracker.HookErrs()
107108
assert.Len(t, hookErrs, 1)
@@ -110,7 +111,7 @@ func TestHookTracker_HookErrs(t *testing.T) {
110111
func TestMultiHookTracker_Add(t *testing.T) {
111112
mht := NewMultiHookTracker()
112113

113-
mht.Add("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "")
114+
mht.Add("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0)
114115

115116
key := hookKey{
116117
podNamespace: "ns1",
@@ -119,6 +120,7 @@ func TestMultiHookTracker_Add(t *testing.T) {
119120
hookPhase: "",
120121
hookSource: HookSourceAnnotation,
121122
hookName: "h1",
123+
hookIndex: 0,
122124
}
123125

124126
_, ok := mht.trackers["restore1"].tracker[key]
@@ -127,8 +129,8 @@ func TestMultiHookTracker_Add(t *testing.T) {
127129

128130
func TestMultiHookTracker_Record(t *testing.T) {
129131
mht := NewMultiHookTracker()
130-
mht.Add("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "")
131-
err := mht.Record("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
132+
mht.Add("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0)
133+
err := mht.Record("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0, true, fmt.Errorf("err"))
132134

133135
key := hookKey{
134136
podNamespace: "ns1",
@@ -137,36 +139,39 @@ func TestMultiHookTracker_Record(t *testing.T) {
137139
hookPhase: "",
138140
hookSource: HookSourceAnnotation,
139141
hookName: "h1",
142+
hookIndex: 0,
140143
}
141144

142145
info := mht.trackers["restore1"].tracker[key]
143146
assert.True(t, info.hookFailed)
144147
assert.True(t, info.hookExecuted)
145148
require.NoError(t, err)
146149

147-
err = mht.Record("restore1", "ns2", "pod2", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
150+
err = mht.Record("restore1", "ns2", "pod2", "container1", HookSourceAnnotation, "h1", "", 0, true, fmt.Errorf("err"))
148151
require.Error(t, err)
149152

150-
err = mht.Record("restore2", "ns2", "pod2", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
153+
err = mht.Record("restore2", "ns2", "pod2", "container1", HookSourceAnnotation, "h1", "", 0, true, fmt.Errorf("err"))
151154
assert.Error(t, err)
152155
}
153156

154157
func TestMultiHookTracker_Stat(t *testing.T) {
155158
mht := NewMultiHookTracker()
156159

157-
mht.Add("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "")
158-
mht.Add("restore1", "ns2", "pod2", "container1", HookSourceAnnotation, "h2", "")
159-
mht.Record("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
160-
mht.Record("restore1", "ns2", "pod2", "container1", HookSourceAnnotation, "h2", "", false, nil)
160+
mht.Add("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0)
161+
mht.Add("restore1", "ns2", "pod2", "container1", HookSourceAnnotation, "h2", "", 0)
162+
mht.Add("restore1", "ns2", "pod2", "container1", HookSourceAnnotation, "h2", "", 1)
163+
mht.Record("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0, true, fmt.Errorf("err"))
164+
mht.Record("restore1", "ns2", "pod2", "container1", HookSourceAnnotation, "h2", "", 0, false, nil)
165+
mht.Record("restore1", "ns2", "pod2", "container1", HookSourceAnnotation, "h2", "", 1, false, nil)
161166

162167
attempted, failed := mht.Stat("restore1")
163-
assert.Equal(t, 2, attempted)
168+
assert.Equal(t, 3, attempted)
164169
assert.Equal(t, 1, failed)
165170
}
166171

167172
func TestMultiHookTracker_Delete(t *testing.T) {
168173
mht := NewMultiHookTracker()
169-
mht.Add("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "")
174+
mht.Add("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0)
170175
mht.Delete("restore1")
171176

172177
_, ok := mht.trackers["restore1"]
@@ -175,20 +180,20 @@ func TestMultiHookTracker_Delete(t *testing.T) {
175180

176181
func TestMultiHookTracker_IsComplete(t *testing.T) {
177182
mht := NewMultiHookTracker()
178-
mht.Add("backup1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", PhasePre)
179-
mht.Record("backup1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", PhasePre, true, fmt.Errorf("err"))
183+
mht.Add("backup1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", PhasePre, 0)
184+
mht.Record("backup1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", PhasePre, 0, true, fmt.Errorf("err"))
180185
assert.True(t, mht.IsComplete("backup1"))
181186

182-
mht.Add("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "")
187+
mht.Add("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0)
183188
assert.False(t, mht.IsComplete("restore1"))
184189

185190
assert.True(t, mht.IsComplete("restore2"))
186191
}
187192

188193
func TestMultiHookTracker_HookErrs(t *testing.T) {
189194
mht := NewMultiHookTracker()
190-
mht.Add("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "")
191-
mht.Record("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
195+
mht.Add("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0)
196+
mht.Record("restore1", "ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", 0, true, fmt.Errorf("err"))
192197

193198
hookErrs := mht.HookErrs("restore1")
194199
assert.Len(t, hookErrs, 1)

0 commit comments

Comments
 (0)