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

refactor: remove mount logic in harvester (backport #4910) #5854

Merged
merged 1 commit into from
May 27, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented May 23, 2024

Solution:
LH introduces VolumeAttachment CRD after v1.5. It can automatically attach/detach volume when there is a request from LH API / controller / CSI. We can just leverage LH mechanism and remove our implementation.

Related Issue:
#4907, #5853, #5841

Test plan:

Setup

  1. Create a 3-node harvester from this branch.
  2. Setup backup-target.

Case 1: snapshot can work on a stopped VM.

  1. Create a VM.
  2. After the VM is ready, stop the VM.
  3. Check VM volumes are detached.
  4. Take a snapshot on the VM. The snapshot can be ready.

Case 2: restore a snapshot from detached volumes can work.

  1. Follow Case 1.
  2. Make sure VM volumes are detached.
  3. Restore the snapshot to a new VM. The new VM can be ready.
  4. Restore the snapshot to replace the old VM. The old VM can be ready.

Case 3: backup can work on a stopped VM.

  1. Create a VM.
  2. After the VM is ready, stop the VM.
  3. Check VM volumes are detached.
  4. Take a backup on the VM. The backup can be ready.

Case 4: race condition doesn't break VMBackup.

  1. Create a VM.
  2. After the VM is ready, stop the VM.
  3. Check VM volumes are detached.
  4. Take multiple backup on the VM in a short time. All backup can be ready.

Case 5: restore a backup from detached volumes can work.

  1. Follow Case 3.
  2. Make sure VM volumes are detached.
  3. Restore the backup to a new VM. The new VM can be ready.
  4. Restore the backup to replace the old VM. The old VM can be ready.

This is an automatic backport of pull request #4910 done by [Mergify](https://mergify.com).

Copy link
Author

mergify bot commented May 23, 2024

Cherry-pick of e25e153 has failed:

On branch mergify/bp/v1.2/pr-4910
Your branch is up to date with 'origin/v1.2'.

You are currently cherry-picking commit e25e153c.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   pkg/api/volume/handler.go
	modified:   pkg/api/volumesnapshot/handler.go
	modified:   pkg/controller/master/backup/backup.go
	modified:   pkg/controller/master/backup/backup_status.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   pkg/controller/master/backup/restore.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Signed-off-by: PoAn Yang <poan.yang@suse.com>
(cherry picked from commit e25e153)
Copy link
Member

@bk201 bk201 left a comment

Choose a reason for hiding this comment

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

LGTM, verify with test plans in the description.

Copy link
Member

@WebberHuang1118 WebberHuang1118 left a comment

Choose a reason for hiding this comment

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

Verify with #5841, and it works without issue in my env.

LGTM, thanks.

@WebberHuang1118 WebberHuang1118 merged commit 7fac003 into v1.2 May 27, 2024
4 checks passed
@mergify mergify bot deleted the mergify/bp/v1.2/pr-4910 branch May 27, 2024 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants