Data mover backup for Windows nodes #8555
Conversation
Lyndon-Li
force-pushed
the
data-mover-backup-for-windows-nodes
branch
from
7a3bde1 to
8a4cf01
Compare
Lyndon-Li
force-pushed
the
data-mover-backup-for-windows-nodes
branch
from
8a4cf01 to
101899e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8555 +/- ##
==========================================
- Coverage 59.17% 59.15% -0.02%
==========================================
Files 370 370
Lines 39487 39568 +81
==========================================
+ Hits 23366 23408 +42
- Misses 14647 14680 +33
- Partials 1474 1480 +6 ☔ View full report in Codecov by Sentry. |
Lyndon-Li
force-pushed
the
data-mover-backup-for-windows-nodes
branch
from
101899e to
af2d1ce
Compare
Signed-off-by: Lyndon-Li <[email protected]>
Lyndon-Li
force-pushed
the
data-mover-backup-for-windows-nodes
branch
from
af2d1ce to
f5d13ae
Compare
| @@ -182,7 +182,7 @@ func (r *DataDownloadReconciler) Reconcile(ctx context.Context, req ctrl.Request | |||
|
|
|||
| hostingPodLabels := map[string]string{velerov1api.DataDownloadLabel: dd.Name} | |||
| for _, k := range util.ThirdPartyLabels { | |||
| if v, err := nodeagent.GetLabelValue(ctx, r.kubeClient, dd.Namespace, k); err != nil { | |||
| if v, err := nodeagent.GetLabelValue(ctx, r.kubeClient, dd.Namespace, k, kube.NodeOSLinux); err != nil { | |||
There was a problem hiding this comment.
By far, the ThirdPartyLabels only has one value: "azure.workload.identity/use".
First, I'm not sure whether this label is only applied to Linux.
Second, I'm not sure whether Linux-specific is true for the future adding labels.
There was a problem hiding this comment.
This PR is for data mover backup only, we will have another PR for restore.
So we hardcode it to kube.NodeOSLinux for dataDownload for now.
There was a problem hiding this comment.
As for now, we don't want to support linux/Windows differential labels for the ThirdPartyLabels mechanism, we don't see that requirement.
In future, it is required, we come back to see how to support it.
| accessMode := exposer.AccessModeFileSystem | ||
| if pvc.Spec.VolumeMode != nil && *pvc.Spec.VolumeMode == corev1.PersistentVolumeBlock { | ||
| accessMode = exposer.AccessModeBlock | ||
| } | ||
|
|
||
| hostingPodLabels := map[string]string{velerov1api.DataUploadLabel: du.Name} | ||
| for _, k := range util.ThirdPartyLabels { | ||
| if v, err := nodeagent.GetLabelValue(context.Background(), r.kubeClient, du.Namespace, k); err != nil { | ||
| if v, err := nodeagent.GetLabelValue(context.Background(), r.kubeClient, du.Namespace, k, nodeOS); err != nil { |
There was a problem hiding this comment.
Also have the same comments here.
There was a problem hiding this comment.
| @@ -182,7 +182,7 @@ func (r *DataDownloadReconciler) Reconcile(ctx context.Context, req ctrl.Request | |||
|
|
|||
| hostingPodLabels := map[string]string{velerov1api.DataDownloadLabel: dd.Name} | |||
| for _, k := range util.ThirdPartyLabels { | |||
| if v, err := nodeagent.GetLabelValue(ctx, r.kubeClient, dd.Namespace, k); err != nil { | |||
| if v, err := nodeagent.GetLabelValue(ctx, r.kubeClient, dd.Namespace, k, kube.NodeOSLinux); err != nil { | |||
There was a problem hiding this comment.
Why the OS is hardcoded as Linux here while reading OS from the PVC attaching node in data upload controller?
There was a problem hiding this comment.
See #8555 (comment)
| @@ -354,7 +354,7 @@ func (e *genericRestoreExposer) createRestorePod(ctx context.Context, ownerObjec | |||
| containerName := string(ownerObject.UID) | |||
| volumeName := string(ownerObject.UID) | |||
|
|
|||
| podInfo, err := getInheritedPodInfo(ctx, e.kubeClient, ownerObject.Namespace) | |||
| podInfo, err := getInheritedPodInfo(ctx, e.kubeClient, ownerObject.Namespace, kube.NodeOSLinux) | |||
Fix issue #8418, support data mover backup for Windows nodes