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
Move Configuration Backup directory out of critical path for Rockstor RPM updates #2639
Comments
@Hooverdan96 I've linked to a potential fix/enhancement proposed on the basis of your exposition here, but in rockstor-rpmbuild. See what you think. It could well end up being a simple and robust fix to the core of this issue. And bring some redundancy along the way. |
I reviewed it and left a comment over there. |
Just briefly exploring the feasibility or amount of work needed to make the change to a different path than # Absolute filesystem path to the directory that will hold user-uploaded files.
# Example: "/home/media/media.lawrence.com/media/"
MEDIA_ROOT = os.path.join(BASE_DIR, "media")
# Absolute filesystem path where config backups are stored by default
DEFAULT_CB_DIR = os.path.join(MEDIA_ROOT, "config-backups") Build, and intentionally do NOT create the
We thus fail to create that folder. We do, however, have code in place intended to do just that in if not os.path.isdir(cb_dir):
os.mkdir(cb_dir) This is simply failing because we now have also a parent folder to
Now, we can create a config backup from the webUI and it is listed on the disk:
Note that this is the easy part... Having Django serve the files in that dir should be a bit more involved. |
Being able to download and re-upload a config backup would require adjusting the following as needed to ensure a consistency in the
Using the example above, for instance, we would need to have:
MEDIA_URL = "/media/"
MEDIA_ROOT = os.path.join(BASE_DIR, "media")
url(r"^media/(?P<path>.*)$", serve, {"document_root": settings.MEDIA_ROOT}), (note that this might need adjustments as it seems not recommended)
<a href="/media/config-backups/{{this.filename}}" title="Download the config backup to your computer"> In any case, the |
based on that it seems way easier currently to handle this in the RPMBuild with a move, delete and move back approach. And the way it's integrated into django currently, just providing a different user-defined path does not seem to be trivial (if even possible in this framework setup) as you mentioned on the |
@FroggyFlox @Hooverdan96 Thanks for the exploration on this one. |
@Hooverdan96 & @FroggyFlox in that it maintains the current location of the config back files but manages their persistence over rpm update. Note that in the rpm scriptlet approach (linked PR) I ended up using a more obvious directory name re the function implemented:
As that seemed to better fit the mechanism introduced there to address the main user experience down-fall we have as only part of this issue: that of lost config backups and a consequent disparity between db and directory contents. |
Linking back to the spin-off issue within this repo to address the part approach referenced in the last comment: |
Thanks to forum member @marciopamplona it was discovered that as a side-effect pf the RPM build configuration, the
config-backups
directory is deleted during a release update, which causes the loss of existing configuration backups, as well as an error message in the WebUI, since that directory is never recreated as part of the update.For reference:
In the RPM Build repository:
https://github.com/rockstor/rockstor-rpmbuild/blob/81bf62b3bfc14a12168b55a50d4f42aaebaf4213/rockstor.spec#L324-L329
during an update we’re deleting the static inventory, which also contains the config-backups.
The definition of the location for the config-backups happens here:
rockstor-core/src/rockstor/settings.py
Lines 96 to 97 in 4d60a2c
where MEDIA_ROOT is:
rockstor-core/src/rockstor/settings.py
Line 94 in 4d60a2c
The initial proposal is to move the directory defined to a different location, as the deletion of the
static
directory is currently required to ensure that updates of the RPM release are sticking after the update. This also seems easier than trying to make therm
command too specific to avoid the deletion of one specific sub-directory out of multiples.From the thread @FroggyFlox has suggested that a user configurable option could be introduced (or merely documented) and a future option to surface that configuration through the WebUI (but that would be for a later stage).
Finally, for reference here is the forum thread:
https://forum.rockstor.com/t/unknown-internal-error-doing-a-get-to-api-config-backup/8938
The text was updated successfully, but these errors were encountered: