Backup/Restore references incorrect shares for Scheduled Tasks

[mattyvau]

Creating a backup from rockstor 3.9 and restoring in 4.1 incorrectly restores scheduled tasks by pointing them to the wrong shares. Example as per the screenshot below, a task for a photos share now points to a music share.

As discussed in forum thread 8249, this likely stems from the backup referencing shares numerically rather than by name. Relevant extract from backup JSON below.

{
		"fields": {
			"task_type": "snapshot",
			"name": "photos-weekly-snapshot",
			"enabled": true,
			"crontabwindow": "*-*-*-*-*-*",
			"crontab": "15 8 * * 5",
			"json_meta": "{\"share\": \"19\", \"max_count\": \"5\", \"writable\": false, \"visible\": false, \"prefix\": \"photos-weekly\"}"
		},
		"model": "smart_manager.taskdefinition",
		"pk": 5
	},

[phillxnet]

@mattyvau Thanks for opening this issue. I'm pretty sure this has failed since we changed our shares api towards the end of 3.9, so it's good we have an issue open on this now.

[FroggyFlox]

@phillxnet, I just had a better look at this issue.

It actually seems to be related to the fact that we are indeed looking for a share ID in the API call to create a scheduled task:

rockstor-core/src/rockstor/smart_manager/views/task_scheduler.py

Lines 55 to 61 in ac664dc

	if "share" in meta:
	if not meta["share"].isdigit():
	raise Exception(
	"Non-digit share element ({}) in meta {}".format(
	meta["share"], meta
	)
	)

Of course, these share IDs can and most likely will vary in a re-install scenario so that is why it will most likely be problematic in such situations. It should be fine when restoring a config backup on the same system used as during the creation of the config backup.

While we could maybe alter what info we originally gather and expect for this API call, this seems rather cumbersome and heavy-handed.
We could alternatively, add some share validation routine in the restore_scheduled_tasks() process to fetch the share.name from the config backup itself, and then get the new share.id matching this name on the new system. We already do something similar during the restore_rockons() process for similar reasons; this means we might be able to re-use some of that code for it, thereby lowering the workload.

I personally would vote for the second approach.

[phillxnet]

While we could maybe alter what info we originally gather and expect for this API call, this seems rather cumbersome and heavy-handed.

Agreed, plus it changes the format of our config save file which is something it would be good to avoid. Given folks will have some going back in time. Plus we then have to account for older and newer formats etc. So if the info is in there and we have to do just a little one-off work to retrieve the names that should do it. Complicated by the changing of share names, but we don't support that yet anyway. And when we do we may have to advertise this side-effect unless we can address it at that time.

We could alternatively, add some share validation routine in the restore_scheduled_tasks() process to fetch the share.name from the config backup itself, and then get the new share.id matching this name on the new system.

Agreed; and as you say, we (you as it goes) wrote similar code to do this back when the Rock-ons restore capability was added.

I personally would vote for the second approach.

Agreed also.

[FroggyFlox]

After a brief look, it seems that Pool-related scheduled tasks (Scrub) would suffer from the same issue. It may be better to deal with that in its own separate issue, though.

[FroggyFlox]

Following #2365 , we also have a failure to restore any scheduled tasks as the API endpoint has changed (an extra trailing slash needs to be removed). This simple fix will be included in the coming PR.

[FroggyFlox]

I now have a simple share ID conversion logic that replaces the corresponding json_meta in the config backup:

"json_meta": "{\"writable\": true, \"visible\": true, \"prefix\": \"snap_daily_ts01\", \"share\": \"77\", \"max_count\": \"4\"}",

with the update share ID (was 77 in the config backup file, now is 3 on the target system):
(below is an excerpt from a successful config restore)

'meta': {u'writable': True, u'visible': True, u'prefix': u'snap_daily_ts01', u'share': u'3', u'max_count': u'4'}

We now need to write a few unit tests to cover the scheduled tasks restore as that isn't currently covered.

[FroggyFlox]

Closing as fixed by #2507.