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.

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

Uploaders windows support #8569

Merged
merged 3 commits into from
Jan 3, 2025

Conversation

Lyndon-Li
Copy link
Contributor

fs uploader and block uploader support Windows nodes:

  • For fs uploader, skip the Windows system folders in volume's root
  • For block uploader, choose linux node only since Windows doesn't support block mode volumes yet

Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
Copy link

codecov bot commented Jan 2, 2025

Codecov Report

Attention: Patch coverage is 75.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 59.18%. Comparing base (03d0bd9) to head (cb22dfc).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
pkg/controller/data_upload_controller.go 0.00% 2 Missing and 1 partial ⚠️
pkg/uploader/kopia/snapshot.go 0.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8569   +/-   ##
=======================================
  Coverage   59.17%   59.18%           
=======================================
  Files         370      370           
  Lines       39568    39591   +23     
=======================================
+ Hits        23416    23433   +17     
- Misses      14673    14677    +4     
- Partials     1479     1481    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
@Lyndon-Li Lyndon-Li force-pushed the uploaders-windows-support branch from c025bfd to cb22dfc Compare January 2, 2025 05:25
@Lyndon-Li Lyndon-Li marked this pull request as ready for review January 2, 2025 05:50
@github-actions github-actions bot requested review from kaovilai and sseago January 2, 2025 05:50
@@ -434,6 +434,11 @@ func GetPVCAttachingNodeOS(pvc *corev1api.PersistentVolumeClaim, nodeClient core
var nodeOS string
var scFsType string

if pvc.Spec.VolumeMode != nil && *pvc.Spec.VolumeMode == corev1api.PersistentVolumeBlock {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we only check the volume mode when we cannot determine the OS via the annotation just like checking the file system type if scFsType == "ntfs"?

Copy link
Contributor Author

@Lyndon-Li Lyndon-Li Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At present, Windows nodes doesn't support blockMode volumes. Or in another word, pods running in Windows nodes won't be with blockMode volumes.

Therefore, until Kubernetes supports it in a future release and Velero also supports blockMode on Windows nodes, we should always use linux nodes for blockMode volumes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will happen when Kubernetes supports the Window block mode volumes in future? Although the PVC is attached to a Windows node, the "Linux" is returned as the OS, and the backup pod will run on the Linux node to handle the Windows block mode volumes, is this the expected behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blockMode volumes are technically be backupable from either linux node or Windows node.
Running blockMode volumes backups on Windows nodes require further efforts.

Therefore, once Windows block mode volumes are supported, on the one hand, we can run all the backups on linux nodes with no efforts; one the other hand, as the ultimate solution, we will support Windows blockMode volumes backup.

@Lyndon-Li Lyndon-Li requested a review from ywk253100 January 2, 2025 06:43
@Lyndon-Li Lyndon-Li merged commit 6860dab into vmware-tanzu:main Jan 3, 2025
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants