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.

Sign up for GitHub

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

Jump to bottom

fix/improve backward/forward compatibility for VM Backup and Restore #731

Merged
merged 1 commit into from
Mar 28, 2025
Merged

fix/improve backward/forward compatibility for VM Backup and Restore #731

merged 1 commit into from
Mar 28, 2025

Conversation

WebberHuang1118
Copy link
Member

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 19, 2025 •
edited
Loading

Name Link
🔨 Latest commit ad3f3f2
😎 Deploy Preview https://67e60ffbe5f9378ba5f5063f--harvester-preview.netlify.app

ihcsim
ihcsim reviewed Mar 21, 2025
View reviewed changes
Copy link
Contributor

@ihcsim ihcsim left a comment

Choose a reason for hiding this comment

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

I think we also need an upgrade note for this. For some users, this can be a big change.

In the upgrade note, I recommend we describe:

  • Affected criteria: AIUI, only 1.3 and 1.4 VMs with 3rd party storage non-root disks
  • Why we introduced this change: Backup is not a CSI standard. Hence, remote copies were never created. We need to ensure backup/restore data consistency and integrity which captures the complete state of the VMs.
  • What are the workarounds: If this is not possible with Harvester, can we recommend external tools, projects etc.?

@asettle @bk201 @innobead WDYT?

@WebberHuang1118 WebberHuang1118 changed the title fix/improve backward/forward compatibilty for VM Backup and Restore fix/improve backward/forward compatibility for VM Backup and Restore Mar 24, 2025
@WebberHuang1118 WebberHuang1118 force-pushed the issue-7738 branch 2 times, most recently from d23ed6e to fbe9b00 Compare March 24, 2025 06:34
@WebberHuang1118
Copy link
Member Author

  • What are the workarounds: If this is not possible with Harvester, can we recommend external tools, projects etc.?

@asettle @bk201 @innobead WDYT?

@ihcsim Thanks for the suggestion. IMO, this does not seem to be within the scope of this ticket. Maybe we can have another one for this?

@ihcsim
Copy link
Contributor

ihcsim commented Mar 25, 2025

@WebberHuang1118 I'm ok with doing it in another ticket. Whatever alternative approaches we recommend, it should just be reasonably brief like, "use X and refers to documentation at <href>", not an elaborated documentation.

bk201
bk201 previously approved these changes Mar 26, 2025
View reviewed changes
innobead
innobead previously approved these changes Mar 26, 2025
View reviewed changes
Copy link

@innobead innobead left a comment

Choose a reason for hiding this comment

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

LGTM

asettle
asettle suggested changes Mar 26, 2025
View reviewed changes
Copy link
Contributor

@asettle asettle left a comment

Choose a reason for hiding this comment

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

I've provided some edits to make it clearer and more concise, but otherwise, LGTM. Please apply to all the documents, not just the first one.

@asettle
Copy link
Contributor

asettle commented Mar 26, 2025

I think we also need an upgrade note for this. For some users, this can be a big change.

In the upgrade note, I recommend we describe:

  • Affected criteria: AIUI, only 1.3 and 1.4 VMs with 3rd party storage non-root disks
  • Why we introduced this change: Backup is not a CSI standard. Hence, remote copies were never created. We need to ensure backup/restore data consistency and integrity which captures the complete state of the VMs.
  • What are the workarounds: If this is not possible with Harvester, can we recommend external tools, projects etc.?

@asettle @bk201 @innobead WDYT?

100% agree.

ihcsim
ihcsim previously approved these changes Mar 26, 2025
View reviewed changes
jillian-maroket
jillian-maroket reviewed Mar 27, 2025
View reviewed changes
Copy link
Contributor

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

Review done

@WebberHuang1118 WebberHuang1118 dismissed stale reviews from ihcsim, innobead, and bk201 via 36af80e March 27, 2025 06:55
@WebberHuang1118 WebberHuang1118 force-pushed the issue-7738 branch 2 times, most recently from 61a14c9 to cc56988 Compare March 27, 2025 13:36
jillian-maroket
jillian-maroket reviewed Mar 27, 2025
View reviewed changes
Copy link
Contributor

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

I reviewed the new text.

@WebberHuang1118
Copy link
Member Author

I reviewed the new text.

Updated per suggestions, thanks.

jillian-maroket
jillian-maroket previously approved these changes Mar 28, 2025
View reviewed changes
Copy link
Contributor

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

Thanks!

@WebberHuang1118 @jillian-maroket
fix/improve backward/forward compatibilty for VM Backup and Restore
ad3f3f2 
Signed-off-by: Webber Huang <[email protected]>

Co-authored-by: Ivan Sim <[email protected]>

Co-authored-by: Alexandra Settle <[email protected]>

Co-authored-by: Jillian Maroket <[email protected]>
@WebberHuang1118 WebberHuang1118 merged commit 7aa9e9a into harvester:main Mar 28, 2025
4 checks passed
@WebberHuang1118 WebberHuang1118 mentioned this pull request Mar 28, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bk201 bk201 bk201 approved these changes

@jillian-maroket jillian-maroket jillian-maroket approved these changes

@asettle asettle Awaiting requested review from asettle

@ihcsim ihcsim Awaiting requested review from ihcsim

@innobead innobead Awaiting requested review from innobead

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@WebberHuang1118 @ihcsim @asettle @innobead @bk201 @jillian-maroket