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

[Config restore] Failure to restore pool scrub task when pool ID has changed #2508

Closed
FroggyFlox opened this issue Mar 3, 2023 · 4 comments · Fixed by #2513
Closed

[Config restore] Failure to restore pool scrub task when pool ID has changed #2508

FroggyFlox opened this issue Mar 3, 2023 · 4 comments · Fixed by #2513
Assignees

Comments

@FroggyFlox
Copy link
Member

FroggyFlox commented Mar 3, 2023

This issue is identical to #2355 but concerns the scheduled tasks of type pool scrub.

Reproducer

  1. Create a VM with 1 OS disk + 2 data disks (Data1 and Data2).
  2. Create the following pools:
    • rock-pool on Data1
    • rock-pool2 on Data2
  3. Create two scheduled tasks:
    • pool scrub on rock-pool: rockpool_scrub
    • pool scrub on rockpool2: rockpool2_scrub
  4. Create a config backup
  5. Delete rock-pool2 and wipe the disk
  6. Create a new pool named rock-pool2 using disk Data2. The pool named rock-pool2 thus now has a different pool ID than before.
  7. Restore config backup taken in step 4

Error:

[03/Mar/2023 15:11:40] INFO [storageadmin.views.config_backup:223] Started restoring scheduled tasks.
[03/Mar/2023 15:11:41] INFO [storageadmin.views.config_backup:57] Successfully created resource: https://localhost/api/sm/tasks. Payload: {'task_type': u'scrub', 'name': u'rockpool_scrub', 'crontabwindow': u'*-*-*-*-*-*', 'enabled': False, 'crontab': u'42 3 * * 5', 'meta': {u'pool_name': u'rock-pool', u'pool': u'2'}}
[03/Mar/2023 15:11:41] ERROR [storageadmin.views.config_backup:63] Exception occurred while creating resource: https://localhost/api/sm/tasks. Payload: {'task_type': u'scrub', 'name': u'rockpool2_scrub', 'crontabwindow': u'*-*-*-*-*-*', 'enabled': False, 'crontab': u'42 3 * * 5', 'meta': {u'pool_name': u'rock-pool2', u'pool': u'3'}}. Exception: 500 Server Error: Internal Server Error for url: https://localhost/api/sm/tasks. Moving on.
[03/Mar/2023 15:11:41] INFO [storageadmin.views.config_backup:227] Finished restoring scheduled tasks.

While the first scrub task was restored successfully, the second one failed:

ERROR [storageadmin.views.config_backup:63] Exception occurred while creating resource: https://localhost/api/sm/tasks. Payload: {'task_type': u'scrub', 'name': u'rockpool2_scrub', 'crontabwindow': u'*-*-*-*-*-*', 'enabled': False, 'crontab': u'42 3 * * 5', 'meta': {u'pool_name': u'rock-pool2', u'pool': u'3'}}. 

Contrary to tasks of type snapshot, it seems we have both the pool ID and the pool name in the json_meta. Based on git history, this is on purpose and for display of the pool name in UI elements (#1760):

# Add pool_name to task meta dictionary
pool = Pool.objects.get(id=meta["pool"])
meta["pool_name"] = pool.name

The fix for this issue should be similar to #2507: validate the pool ID in the config backup to ensure it reflects the correct pool in the target system.

FroggyFlox added a commit to FroggyFlox/rockstor-core that referenced this issue Mar 3, 2023
We currently use the pool ID for a scrub scheduled task in a config
backup file as is. If this pool now has a different ID in the target
system (the one where the config backup file is being restored), this
will lead to a failure in restoring the pool scrub task.

This commit thus implements a simple conversion logic in which we fetch
the pool ID for the given pool name from the target system's database.
@FroggyFlox
Copy link
Member Author

A simple conversion logic was added in cffa735 as proof of concept. With this commit applied, pool scrub tasks are restored without errors.

FroggyFlox added a commit to FroggyFlox/rockstor-core that referenced this issue Mar 3, 2023
FroggyFlox added a commit to FroggyFlox/rockstor-core that referenced this issue Mar 3, 2023
We currently use the pool ID for a scrub scheduled task in a config
backup file as is. If this pool now has a different ID in the target
system (the one where the config backup file is being restored), this
will lead to a failure in restoring the pool scrub task.

This commit thus implements a simple conversion logic in which we fetch
the pool ID for the given pool name from the target system's database.

Update unit tests accordingly.
@FroggyFlox FroggyFlox added this to the First Stable Poetry build milestone Mar 4, 2023
@FroggyFlox FroggyFlox self-assigned this Mar 4, 2023
@phillxnet
Copy link
Member

@FroggyFlox Nice. I did wonder about this while testing your latest pull request that is now merged.

Fancy popping in a pr with your referenced fix for this one.

As always much appreciated. I'll try and get our publishing back-end ready for our next rpm release give the recent repo changes, as I think this would be a good addition along with what I've just merged of yours.

@FroggyFlox
Copy link
Member Author

Thanks @phillxnet !

I'll prepare the PR as soon as I have some time. It should (hopefully) all be ready to submit.

@FroggyFlox FroggyFlox changed the title [Config restore] Pool scrub tasks can be restored against the wrong pool [Config restore] Failure to restore pool scrub task when pool ID has changed Mar 6, 2023
phillxnet added a commit that referenced this issue Mar 7, 2023
…rrect_pool

Convert pool ID in backup file to the one in the database #2508
@FroggyFlox
Copy link
Member Author

Closing as Fixed by #2513

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 a pull request may close this issue.

2 participants