[BUG]Restore will get stuck if the restore error is not captured

[shuo-wu]

Describe the bug
Right now some errors caused by replica restore are not captured by the longhorn manager and not recorded in engine restore status. Then there is no way to handle the error and continue the restore.

To Reproduce
Steps to reproduce the behavior:

1. Set backupstore for Longhorn
2. Create a regular Longhorn volume with the workload, then write data to the volume.
3. Create a backup for the volume, then launch a DR volume based on the backup.
4. Wait for the DR volume init restore complete. Then create a non-empty directory named volume-delta-<last restored backup name>.img for one replica of the DR volume. This directory will fail the following incremental restoration for the replica. (e.g., mkdir -p <replica data path>/volume-delta-backup-c3edca9817c14f1a.img/dir)
5. Write data to the regular volume then create a backup. Then wait for the DR volume incremental restoration result.
6. Check restoreStatus and lastRestoredBackup in the DR volume engine status. There is no restoreStatus and engine.status.lastRestoredBackup is not updated.
7. Create 2 more backups for the regular volume. The DR volume won't continue to restore volume.

Expected behavior

1. The error should be recorded in restoreStatus and engine.status.lastRestoredBackup should be updated.
2. The replica that fails to restore the data will be marked as failed

Environment:

• Longhorn version: v1.0.0
• Kubernetes version:
• Node OS type and version:

[shuo-wu]

Integration test plan 1:
Test if the DR volume will become Faulted when the backup data is messed up.

1. Set the backupstore.
2. Create then attach a volume.
3. Directly create a backup for the volume when there is no data in the volume.
4. Create a DR volume from the backup.
5. Write some data to the volume and create the 2nd backup.
6. Delete some data blocks of the backup once the DR volume starts incremental restoration. ( Do not just delete the blocks that are created by the 1st backup.)
7. Wait for the restore volume Faulted.
   --> Check if volume.conditions[restore].status == False && volume.conditions[restore].reason == "RestoreFailure".
   --> Check if volume.ready == false
   This test is similar to the 1st test in [BUG] State transitions for a failure in backup restore #1260

Integration test 2:
Test if the replica fails the incremental restore will be marked as ERROR.

1. Set a random the backupstore.
2. Create a regular volume and a backup (with some data).
3. Create a DR volume from the backup.
4. Create the 2nd backup for the regular volume
5. Wait for the DR volume incremental restore complete.
6. Pick up a replica of the DR volume, Create a non-empty directory named volume-delta-<last restored backup>.img in the replica directory. This operation will lead to replica incremental restore failure later.
   7 Create the 3rd backup for the regular volume
7. Wait for the DR volume incremental restore complete. And check:
   --> Check if the volume is Degraded state and the replica in step5 is ERROR.
   --> Check if volume.conditions[restore].status == True && volume.conditions[restore].reason == "RestoreInProgress".
8. See if the DR volume still works fine. (e.g. Trigger more incremental restore, trigger expansion)
9. Activate the DR volume and check the data.

[shuo-wu]

The PR for issue #1260 cannot completely fix this issue. The 1st test can be passed but the 2nd one will still fail after the fix. Since there are some errors during the incremental restore not being recorded in the restore status. Then the longhorn manager cannot mark the related replica as failed.
The errors are not recorded in the sync agent server restore status:

1. Parameters check failure before restore status initialization.
2. Delta file check and cleanup error before starting the incremental restoration.
3. File coalesce error in the post inc restore function.
4. Replica reload error in the post inc restore function.
   ...
   Besides, there is no related integration test covers those corner cases.

And the regular restore implementation has the similar issue, too.

If we want to cover those errors in the restore status, we need to check which error may be fatal for the replica (longhorn manager needs to fail the replica according to the restore status error msg) and which error won't affect the replica (longhorn manager doesn't need to fail the replica and simply retrying restore may be enough.). It means we need to refactor the whole restore, including incremental restore. I don't think we have time to do that in this release.

Actually I can fix the issue that triggered by the reproduce step, but I don't think that's what we want.

[yasker]

Sounds like we need to move this out of 1.0.0.

Have we fixed #1328 along with #1260 ?

[yasker]

@shuo-wu Can you assess the impact of missing the fix for this issue? E.g. in which case the user will have trouble, what's the workaround, etc.

[shuo-wu]

No, #1328 will be fixed along with other issues later.
The fix for #1260 would only handle parts of the restore failure cases. We will record all found cases and track the fixes in #1337.

[yasker]

Code merged, need the pre-merge checklist.

[shuo-wu]

Pre-merged Checklist

• Does the PR include the explanation for the fix or the feature?
  It's in the PR.

• Is the backend code merged (Manager, Engine, Instance Manager, BackupStore, etc)?
  The PR is at:
  Check and cleanup the file before restore backupstore#43
  Restore Rafactoring longhorn-engine#501
  Capture and handle the restore cmd error longhorn-manager#609

• Is the reproduce steps/test steps documented?
  See the reproducing step.

• Which areas/issues this PR might have potential impacts on?
  Area: Manager & Engine
  Issues: Restore failure handling

• If the fix introduces the code for backward compatibility Has a separate issue been filed with the label release/obsolete-compatibility?
  The compatibility issue is filed at

• If labeled: area/ui Has the UI issue filed or ready to be merged?
  The UI issue/PR is at

• if labeled: require/doc Has the necessary document PR submitted or merged?
  The Doc issue/PR is at

• If labeled: require/automation-e2e Has the end-to-end test plan been merged? Have QAs agreed on the automation test case?
  The automation skeleton PR is at Add test test_dr_volume_with_restore_command_error() longhorn-tests#357
  The automation test case PR is at Add test test_dr_volume_with_restore_command_error() longhorn-tests#357

• if labeled: require/automation-engine Has the engine integration test been merged?
  The engine automation PR is at Restore Rafactoring longhorn-engine#501

• if labeled: require/manual-test-plan Has the manual test plan been documented?
  The updated manual test plan is at

[khushboo-rancher]

Tried reproducing issue from #1188 (comment)

1. On V1.0.0, The DR volume got stuck in Restore in progress forever. - It can't be activated.
   Volume name - volume-restore-stuck-test-1
   logs:
   time="2020-07-09T23:46:24Z" level=error msg="BUG: engine volume-restore-stuck-test-1-e-197c85d1: different lastRestored values even though engine is not restoring"
   longhorn-support-bundle_e27c1f86-2285-474c-8127-9c41cc352773_2020-07-09T23-36-05Z.zip

2. On master 97a869e, DR volume got degraded but healthy. Also, checked the data after activation, data is intact.
   Volume name - volume-restore-stuck-test-2
   logs
   longhorn-support-bundle_516cbf10-706f-4852-860b-729747916e3d_2020-07-09T23-36-20Z.zip

In above set up I don't see restoreStatus and lastRestoredBackup empty.
@shuo-wu Is this expected?

[shuo-wu]

1. The error log in the v1.0.0 test means the status stored in the engine process is inconsistent with the engine object status. Since the restore error cannot be handled correctly in v1.0.0, the whole volume will mess up after the test then many kinds of weird issues may get triggered.
2. There will be a big refactor for the restore feature in the next release. This kind of error will be fixed after the reactor.

In general, I don't think we need to worry about the error you triggered in v1.0.0 test now.

[khushboo-rancher]

Verified on v1.0.1-rc1

Validation- Passed

Followed the steps from #1188 (comment) DR volume remains healthy.
Verified the restored data.