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

Samba exports incompletely restored from configuration backup (missing share-level custom config) #2905

Open
Hooverdan96 opened this issue Sep 25, 2024 · 11 comments

Comments

@Hooverdan96
Copy link
Member

Hooverdan96 commented Sep 25, 2024

As mentioned in #2847, samba shares seem to not be consistently restored. This issue is a continuance of that observation. Thanks to user simon-77 on the community forum who observed that their samba exports were restored, however without share-level custom configuration (moving from 4.6.1-0 and restoring onto a 5.0.14-0 environment:)

https://forum.rockstor.com/t/missing-repository-rockstor-stable-on-15-6/9643/16 and more specifically

https://forum.rockstor.com/t/missing-repository-rockstor-stable-on-15-6/9643/21

Additionally, NFS exports were restored as read-only, despite the read-write setting.

Here are the examples extracted from the configuration backup json file (from Rockstor version 4.6.1)
for Samba:

{"model": "storageadmin.sambashare", "pk": 1, "fields": {"share": 39, "path": "/mnt2/scanner", "comment": "SMB share for scanner", "browsable": "yes", "read_only": "no", "guest_ok": "no", "shadow_copy": false, "time_machine": false, "snapshot_prefix": null}}, {"model": "storageadmin.sambashare", "pk": 2, "fields": {"share": 30, "path": "/mnt2/old-photos", "comment": "Samba-Export", "browsable": "yes", "read_only": "no", "guest_ok": "no", "shadow_copy": false, "time_machine": false, "snapshot_prefix": null}}, {"model": "storageadmin.sambacustomconfig", "pk": 1, "fields": {"smb_share": 1, "custom_config": "valid users = @scanner"}}, {"model": "storageadmin.sambacustomconfig", "pk": 3, "fields": {"smb_share": 1, "custom_config": "directory mask = 0775"}}, {"model": "storageadmin.sambacustomconfig", "pk": 4, "fields": {"smb_share": 1, "custom_config": "force group = scanner"}}, {"model": "storageadmin.sambacustomconfig", "pk": 6, "fields": {"smb_share": 2, "custom_config": "valid users = @family"}}, {"model": "storageadmin.sambacustomconfig", "pk": 10, "fields": {"smb_share": 2, "custom_config": "force user = admin"}}, {"model": "storageadmin.sambacustomconfig", "pk": 11, "fields": {"smb_share": 2, "custom_config": "force group = users"}}, {"model": "storageadmin.sambacustomconfig", "pk": 12, "fields": {"smb_share": 1, "custom_config": "create mask = 0664"}}, {"model": "storageadmin.sambacustomconfig", "pk": 15, "fields": {"smb_share": 2, "custom_config": "directory mask = 0755"}}, {"model": "storageadmin.sambacustomconfig", "pk": 16, "fields": {"smb_share": 2, "custom_config": "create mask = 0644"}}, 

and the corresponding WebUI view:
image

for NFS:

{"model": "storageadmin.nfsexportgroup", "pk": 2, "fields": {"host_str": "10.71.128.0/24", "editable": "rw", "syncable": "async", "mount_security": "insecure", "nohide": false, "enabled": true, "admin_host": null}},

so, in both cases, it seems the information is exported, but not considered during the import.

@phillxnet phillxnet added this to the 5.1.X-X Stable release milestone Sep 28, 2024
@phillxnet
Copy link
Member

@Hooverdan96 I've now added this to our current milestone, but we may have to prioritise the SMB custom options. We can then address the NFS restore under another issue. Not looked into this just yet but of course, as you suggest, it may be that one fix works for both and we can do the custom options fail to restore whole-sale.

@FroggyFlox
Copy link
Member

@phillxnet, @Hooverdan96,

A brief note here on the Samba custom options part:
The samba custom options model (storageadmin.sambacustomconfig) is indeed not restored during a config backup restore. It's not a bug per se but more a feature that is not implemented yet. We should definitely implement it, I agree, but we could discuss if it is eligible for being part of the next stable if we are indeed in "feature freeze and big fixes only" as we're very late in our current testing cycle. As always, it all depends on how involved it would be to implement.

@Hooverdan96
Copy link
Member Author

@FroggyFlox does this also apply to the rw flag that's was observed as not being restored? I assume, it's a different area where that setting is stored compared to the samba custom config.

@FroggyFlox
Copy link
Member

@FroggyFlox does this also apply to the rw flag that's was observed as not being restored?

The part related to the NFS restore might be a bug; I haven't looked at it yet and I would need to refresh myself on it. Maybe we should split the NFS issue into a separate one as it is unrelated to the Samba restore process.

@Hooverdan96
Copy link
Member Author

Will do.

@Hooverdan96
Copy link
Member Author

I have created issue #2912 to break out the nfs piece. I will adjust the title of this issue and remove the NFS reference.

@Hooverdan96 Hooverdan96 changed the title Samba/NFS export incompletely restored from configuration backup Samba exports incompletely restored from configuration backup (missing share-level custom config) Oct 3, 2024
@phillxnet
Copy link
Member

Re @FroggyFlox 's #2905 (comment)

It's not a bug per se but more a feature that is not implemented yet. We should definitely implement it, I agree, but we could discuss if it is eligible for being part of the next stable if we are indeed in "feature freeze and big fixes only" as we're very late in our current testing cycle.

I'll remove this, at least for now, from the current 5.1.X-X Stable release Milestone, and replace it with the apparently far simpler NFS issue @Hooverdan96 has just spun-out from this one.

@phillxnet phillxnet removed this from the 5.1.X-X Stable release milestone Oct 3, 2024
@Hooverdan96
Copy link
Member Author

If this is not addressed for the next stable release, it might be worthwhile to mention that piece missing, maybe during the announcement of the stable release (when it happens)? Since the intent with the next stable release is to also entice people to move from both from existing 3.x on CentOS or 4.6 to the new and shiny 5.x stable, some might opt to reinstall the latest version vs. running an upgrade, for which the config backup is then a key component? Or am I overthinking that?

@simon-77
Copy link

simon-77 commented Oct 3, 2024

Yeah, I think a note should be definitely there.
This was also how I noticed it - I switched from 4.6 built on Leap 15.4 to 5.0.14 built on Leap 15.6 via a fresh installation.

@simon-77
Copy link

simon-77 commented Oct 3, 2024

Regarding the NFS rw-flag:
I assume (without having looked into the code) that this is a bug, as all other settings where restored correctly (including the IP address).

@phillxnet
Copy link
Member

phillxnet commented Oct 4, 2024

@Hooverdan96 & @simon-77
Agreed. I have created the following doc entry to address this short-fall for the time being:

rockstor/rockstor-doc#487

Our concern here is that it can be deceptively complex to restore from our DB dump type save. So as @FroggyFlox has highlighted, we need to keep code changes to a minimum: and likely the majority of folks config is now restored successfully. Amd we are not stopping development here. We are only transitioning to a Stable release. And I think we are nearly ready for that. But given SMB's prominance over NFS, I am uncertain of having prioritised that. But SMB does restore (bar the custom settings): and with only a cursory look I'm surmising that the NFS incomplete restore is more a bug rather than a missing reverse engineering of our DB dump to fit-in with current system state.

@simon-77 A complexity we have in restore is that we have to account for DB having id as canonical: with intra-relations based on this. I.e. the first export is id1. But on a restore that is inconsistent if folks have already created an export. So we have to reverse match stuff sometimes. Anyway all in the code and it may be simple enough. And we have made gradual improvements as we have developed in facilitating more & more restore resolution. But we must tread carefully on such a critical system.

Essentially the docs and their clarity are our allies here. We can only add so-much Web-UI before it becomes the docs. Contextual links to docs for explanations/limitations is an ongoing effort: most notably in the services section: which in turn required a doc revamp to ensure we at least had doc entries to accommodate those links. Thanks to @FroggyFlox for that more recent effort.

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

No branches or pull requests

4 participants