Skip to content

Commit 3b75c43

Browse files
authored
Merge branch 'main' into fix/skip-volume-policy-pv-restore
2 parents 26eb929 + baf2491 commit 3b75c43

21 files changed

Lines changed: 591 additions & 106 deletions
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix issue #9626, let go for uninitialized repo under readonly mode
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix issue #9659, in the case that PVB/PVR/DU/DD is cancelled before the data path is really started, call EndEvent to prevent data mover pod from crashing because of delay event distribution
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix issue #9666, fix node-agent node detection in multiple instances scenario
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
# Add custom volume policy action
2+
3+
## Abstract
4+
5+
Currently, velero supports 3 different volume policy actions:
6+
snapshot, fs-backup, and skip, which tell Velero how to handle backing
7+
up PVC contents. Any other policy action is not allowed. This prevents
8+
third party BackupItemAction (BIA) plugins which might want to perform
9+
different actions on PVC via defined volume policies.
10+
11+
## Background
12+
13+
An external BIA plugin that wants to back up volumes via some custom
14+
means (i.e. not CSI snapshots or fs-backup with kopia) is not able to
15+
make use of the existing volume policy API. While the plugin could use
16+
something like PVC annotations instead, this won't integrate with
17+
existing volume policies, which is desirable in case the user wants to
18+
specify some PVCs to use the custom plugin while leaving others using
19+
CSI snapshots or fs-backup.
20+
21+
## Goals
22+
23+
- Add a fourth valid volume policy action "custom"
24+
- Make use of the existing action parameters field to distinguish between multiple custom actions.
25+
26+
## Non Goals
27+
28+
- Implementing custom action logic in velero repo
29+
30+
## High-Level Design
31+
32+
A new VolumeActionType with the value "custom" will be added to
33+
`internal/resourcepolicies`. When the action is "custom", velero will
34+
not perform a snapshot or use fs-backup on the PVC. If there is no
35+
registered plugin which implements the desired custom action, then it
36+
will be equivalent to the "skip" action. Since there could be
37+
different plugins that implement custom actions, when making the API
38+
call (defined below) the plugin should also pass in a partial action
39+
parameters map containing the portion of the map that identifies the
40+
custom plugin as belonging to a particular external
41+
implementation. For example, there might be a custom BIA that's
42+
looking for a `custom` volume policy action with the parameter
43+
`myCustomAction=true`. The volume policy action would be defined like
44+
this:
45+
46+
```yaml
47+
action:
48+
type: custom
49+
parameters:
50+
myCustomAction: true
51+
```
52+
53+
In `internal/volumehelper/volume_policy_helper.go` a new interface
54+
method will be added, similar to `ShouldPerformSnapshot` but it takes
55+
a partial parameter map as an additional input, since for custom
56+
actions to match, the action type must be `custom`, but also there may
57+
be some parameter that needs to match (to distinguish between
58+
different custom actions). We also want a way for the plugin to get
59+
the parameter map for the action. This should probably just return the
60+
map rather than the Action struct is under `internal`.
61+
62+
```go
63+
type VolumeHelper interface {
64+
ShouldPerformSnapshot(obj runtime.Unstructured, groupResource schema.GroupResource) (bool, error)
65+
ShouldPerformFSBackup(volume corev1api.Volume, pod corev1api.Pod) (bool, error)
66+
ShouldPerformCustomAction(obj runtime.Unstructured, groupResource schema.GroupResource, map[string]any) (bool, error)
67+
GetActionParameters(obj runtime.Unstructured, groupResource schema.GroupResource) (map[string]any, error)
68+
}
69+
```
70+
71+
In addition, since the VolumeHelper interface is expected to be called by external plugins, the interface (but not the implementation) should be moved from `internal/volumehelper` to `pkg/util/volumehelper`.
72+
73+
In `pkg/plugin/utils/volumehelper/volume_policy_helper.go`, a new helper func will be added which delegates to the internal volumehelper.NewVolumeHelperImplWithNamespaces
74+
75+
```go
76+
func NewVolumeHelper(
77+
volumePolicy *resourcepolicies.Policies,
78+
snapshotVolumes *bool,
79+
logger logrus.FieldLogger,
80+
client crclient.Client,
81+
defaultVolumesToFSBackup bool,
82+
backupExcludePVC bool,
83+
namespaces []string,
84+
) (VolumeHelper, error) {
85+
```
86+
87+
88+
## Alternative Considered
89+
90+
An alternate approach was to create a new server arg to allow
91+
user-defined parameters. That was rejected in favor of this approach,
92+
as the explicitly-supported "custom" option integrates more easily
93+
into a supportable plugin-callable API.

hack/build-image/Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ ENV GOPROXY=${GOPROXY}
2222

2323
# kubebuilder test bundle is separated from kubebuilder. Need to setup it for CI test.
2424
# Using setup-envtest to download envtest binaries
25-
RUN go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest && \
25+
RUN go install sigs.k8s.io/controller-runtime/tools/setup-envtest@v0.0.0-20260305094418-8122a6266696 && \
2626
mkdir -p /usr/local/kubebuilder/bin && \
2727
ENVTEST_ASSETS_DIR=$(setup-envtest use 1.33.0 --bin-dir /usr/local/kubebuilder/bin -p path) && \
2828
cp -r ${ENVTEST_ASSETS_DIR}/* /usr/local/kubebuilder/bin/

pkg/datamover/backup_micro_service.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,7 @@ func (r *BackupMicroService) cancelDataUpload(du *velerov2alpha1api.DataUpload)
310310
fsBackup := r.dataPathMgr.GetAsyncBR(du.Name)
311311
if fsBackup == nil {
312312
r.OnDataUploadCancelled(r.ctx, du.GetNamespace(), du.GetName())
313+
r.eventRecorder.EndingEvent(du, false, datapath.EventReasonStopped, "Data path for %s exited without start", du.Name)
313314
} else {
314315
fsBackup.Cancel()
315316
}

pkg/datamover/backup_micro_service_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,8 @@ func TestCancelDataUpload(t *testing.T) {
259259
}{
260260
{
261261
name: "no fs backup",
262-
expectedEventReason: datapath.EventReasonCancelled,
263-
expectedEventMsg: "Data path for data upload fake-data-upload canceled",
262+
expectedEventReason: datapath.EventReasonStopped,
263+
expectedEventMsg: "Data path for fake-data-upload exited without start",
264264
expectedErr: datapath.ErrCancelled,
265265
},
266266
}

pkg/datamover/restore_micro_service.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,7 @@ func (r *RestoreMicroService) cancelDataDownload(dd *velerov2alpha1api.DataDownl
288288
fsBackup := r.dataPathMgr.GetAsyncBR(dd.Name)
289289
if fsBackup == nil {
290290
r.OnDataDownloadCancelled(r.ctx, dd.GetNamespace(), dd.GetName())
291+
r.eventRecorder.EndingEvent(dd, false, datapath.EventReasonStopped, "Data path for %s exited without start", dd.Name)
291292
} else {
292293
fsBackup.Cancel()
293294
}

pkg/datamover/restore_micro_service_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,8 @@ func TestCancelDataDownload(t *testing.T) {
203203
}{
204204
{
205205
name: "no fs restore",
206-
expectedEventReason: datapath.EventReasonCancelled,
207-
expectedEventMsg: "Data path for data download fake-data-download canceled",
206+
expectedEventReason: datapath.EventReasonStopped,
207+
expectedEventMsg: "Data path for fake-data-download exited without start",
208208
expectedErr: datapath.ErrCancelled,
209209
},
210210
}

pkg/nodeagent/node_agent.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,10 @@ func isRunningInNode(ctx context.Context, namespace string, nodeName string, crC
9797
}
9898

9999
if crClient != nil {
100-
err = crClient.List(ctx, pods, &ctrlclient.ListOptions{LabelSelector: parsedSelector})
100+
err = crClient.List(ctx, pods, &ctrlclient.ListOptions{
101+
LabelSelector: parsedSelector,
102+
Namespace: namespace,
103+
})
101104
} else {
102105
pods, err = kubeClient.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{LabelSelector: parsedSelector.String()})
103106
}

0 commit comments

Comments
 (0)