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

Limit PVC block mode logic to non-Windows platform #6986

Merged

Conversation

blackpiglet
Copy link
Contributor

@blackpiglet blackpiglet commented Oct 20, 2023

Release v1.12.rc.1 failed with the Windows binary build. The error message is:

# github.com/vmware-tanzu/velero/pkg/uploader/kopia
pkg/uploader/kopia/block_backup.go:41:31: undefined: syscall.Stat_t
pkg/uploader/kopia/block_restore.go:94:31: undefined: syscall.Stat_t

The reason is that PVC block mode backup and restore introduced some OS-specific system calls.
Those calls are not available for Windows, so add both non-Windows version and Windows version code and return an error for block mode on the Windows platform.

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #(issue)

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

@blackpiglet blackpiglet added the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Oct 20, 2023
@blackpiglet blackpiglet force-pushed the support_windows_build branch from 168c18a to 49a5c50 Compare October 20, 2023 07:57
@blackpiglet blackpiglet force-pushed the support_windows_build branch 2 times, most recently from 0f179f3 to 2586f1e Compare October 20, 2023 08:01
@blackpiglet blackpiglet removed the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Oct 20, 2023
@blackpiglet blackpiglet marked this pull request as ready for review October 20, 2023 08:04
@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

Merging #6986 (3034cdb) into release-1.12 (353ff55) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##           release-1.12    #6986      +/-   ##
================================================
- Coverage         60.47%   60.44%   -0.04%     
================================================
  Files               247      247              
  Lines             26398    26398              
================================================
- Hits              15965    15957       -8     
- Misses             9312     9319       +7     
- Partials           1121     1122       +1     
Files Coverage Δ
pkg/uploader/kopia/block_backup.go 30.00% <ø> (ø)
pkg/uploader/kopia/block_restore.go 21.27% <ø> (ø)

... and 1 file with indirect coverage changes

@blackpiglet
Copy link
Contributor Author

@dzaninovic
Hi David, please also help to take a look at this PR.

@blackpiglet blackpiglet changed the title Make Windows build skip BlockMode code. Add both non-Windows version and Windows version code for PVC block mode logic Oct 20, 2023
Lyndon-Li
Lyndon-Li previously approved these changes Oct 20, 2023
PVC block mode backup and restore introduced some OS specific
system calls. Those calls are not available for Windows, so
add both non Windows version and Windows version code, and
return error for block mode on the Windows platform.

Signed-off-by: Xun Jiang <[email protected]>
@Lyndon-Li Lyndon-Li merged commit 905cd43 into vmware-tanzu:release-1.12 Oct 20, 2023
23 of 24 checks passed
@dzaninovic
Copy link
Contributor

@blackpiglet, fix looks good to me.

@anshulahuja98
Copy link
Collaborator

@Lyndon-Li should we add this to public docs?

@blackpiglet
Copy link
Contributor Author

@anshulahuja98
What information would you like to add?
IMO, this change is not visible to users, so there seems no need to mention it in the document.

@Lyndon-Li
Copy link
Contributor

I think it is about adding a limitation statement for Windows, is that what you mean? @anshulahuja98
I see the doc indeed needs to change as it is still saying "block mode is not supported". Let me correct it.

@anshulahuja98
Copy link
Collaborator

I think it is about adding a limitation statement for Windows, is that what you mean? @anshulahuja98
I see the doc indeed needs to change as it is still saying "block mode is not supported". Let me correct it.

Yes it was for windows limitation.

@blackpiglet
Copy link
Contributor Author

@Lyndon-Li @anshulahuja98
I see. I will create a PR to record that in the documents.

@anshulahuja98
Copy link
Collaborator

Even the changelog / pr title of current pr is misleading

@blackpiglet blackpiglet changed the title Add both non-Windows version and Windows version code for PVC block mode logic Limit PVC block mode logic for only non Windows platform Oct 25, 2023
@blackpiglet blackpiglet changed the title Limit PVC block mode logic for only non Windows platform Limit PVC block mode logic for non Windows platform Oct 25, 2023
@blackpiglet
Copy link
Contributor Author

@anshulahuja98 @Lyndon-Li
PR is created to record the platform limitation: #7013.

@anshulahuja98 anshulahuja98 changed the title Limit PVC block mode logic for non Windows platform Limit PVC block mode logic to non-Windows platform Oct 25, 2023
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.

5 participants