Skip to content

Migrate to New Middleware style #2662 #2663

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 22, 2023
Merged

Migrate to New Middleware style #2662 #2663

merged 1 commit into from
Aug 22, 2023

Conversation

@FroggyFlox
Copy link
Member

@FroggyFlox FroggyFlox commented Aug 20, 2023

Fixes #2662
@phillxnet, @Hooverdan96: ready for review.

In Django 1.10, the old MIDDLEWARE_CLASSES style was deprecated and we should now use MIDDLEWARE setting.

This Pull Request (PR) includes this move and takes advantage of the new MiddlewareMixin to help provide compatibility for our custom middleware.

With this commit in place, I no-longer see the DJango Warning described in #2662.

All unit tests still pass:

buildvm155:/opt/rockstor # cd src/rockstor/ && poetry run django-admin test ; cd -
Creating test database for alias 'default'...
Creating test database for alias 'smart_manager'...
System check identified no issues (0 silenced).
......................................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 262 tests in 15.693s

OK
Destroying test database for alias 'default'...
Destroying test database for alias 'smart_manager'...

IMPORTANT
I have not yet tested that our custom middleware still works, unfortunately. I would need to find a way to trigger an exception and verify we still have the creation of the logs tarball.

@FroggyFlox
Migrate to New Middleware style rockstor#2662
e17feca 
In Django 1.10, the old `MIDDLEWARE_CLASSES` style was deprecated and we
should now use `MIDDLEWARE` setting.

This commit includes this move and takes advantage of the new
MiddlewareMixin to help provide compatibility for our custom middleware.
@FroggyFlox FroggyFlox linked an issue Aug 20, 2023 that may be closed by this pull request
(t) Update Middlewares style #2662
Closed
@phillxnet phillxnet added the needs review Ideally by prior rockstor-core contributor label Aug 21, 2023
@FroggyFlox
Copy link
Member Author

FroggyFlox commented Aug 21, 2023

IMPORTANT
I have not yet tested that our custom middleware still works, unfortunately. I would need to find a way to trigger an exception and verify we still have the creation of the logs tarball.

I finally could find a way to trigger this middleware. I had to try a lot of different "breaking" things but I believe I have it narrowed down to the following:

  1. add a new disk to the VM
  2. create a new pool
  3. create a share in this pool
  4. Turn OFF the VM
  5. Remove the disk added in step 1
  6. Turn the VM back ON. As expected, Rockstor complains when trying to mount the pool:
[21/Aug/2023 17:01:37] ERROR [storageadmin.views.command:90] Skipping Pool (ro_pool) mount as there are no attached devices. Moving on.
[21/Aug/2023 17:01:37] INFO [system.pkg_mgmt:118] No "rockstor*x86_64" package found: source install?
[21/Aug/2023 17:01:37] DEBUG [fs.btrfs:1005] Skipping excluded subvol: name=(@).
[21/Aug/2023 17:01:37] DEBUG [fs.btrfs:1005] Skipping excluded subvol: name=(.snapshots).
[21/Aug/2023 17:01:37] DEBUG [fs.btrfs:1005] Skipping excluded subvol: name=(.snapshots/1/snapshot).
[21/Aug/2023 17:01:37] DEBUG [fs.btrfs:1005] Skipping excluded subvol: name=(opt).
[21/Aug/2023 17:01:37] DEBUG [fs.btrfs:1005] Skipping excluded subvol: name=(root).
[21/Aug/2023 17:01:37] DEBUG [fs.btrfs:1005] Skipping excluded subvol: name=(srv).
[21/Aug/2023 17:01:37] DEBUG [fs.btrfs:1005] Skipping excluded subvol: name=(tmp).
[21/Aug/2023 17:01:37] DEBUG [fs.btrfs:1005] Skipping excluded subvol: name=(var).
[21/Aug/2023 17:01:37] DEBUG [fs.btrfs:1005] Skipping excluded subvol: name=(usr/local).
[21/Aug/2023 17:01:37] DEBUG [fs.btrfs:1005] Skipping excluded subvol: name=(boot/grub2/i386-pc).
[21/Aug/2023 17:01:37] DEBUG [fs.btrfs:1005] Skipping excluded subvol: name=(boot/grub2/x86_64-efi).
[21/Aug/2023 17:01:37] DEBUG [storageadmin.views.share_helpers:111] ---- Share name = home.
[21/Aug/2023 17:01:37] DEBUG [storageadmin.views.share_helpers:113] Updating pre-existing same pool db share entry.
[21/Aug/2023 17:01:37] DEBUG [system.osi:225] Running command: /usr/sbin/btrfs subvolume list /mnt2/ROOT
[21/Aug/2023 17:01:37] DEBUG [storageadmin.views.command:235] Bootstrap operations completed
[21/Aug/2023 17:01:37] DEBUG [system.pkg_mgmt:391] No changelog found: trying yum update for info.
[21/Aug/2023 17:01:37] ERROR [storageadmin.views.command:90] Skipping Pool (ro_pool) mount as there are no attached devices. Moving on.
[21/Aug/2023 17:01:37] INFO [fs.btrfs:771] Mount by label (/dev/disk/by-label/ro_pool) failed.
[21/Aug/2023 17:01:37] ERROR [storageadmin.middleware:34] Exception occurred while processing a request. Path: /api/commands/refresh-share-state method: POST
[21/Aug/2023 17:01:37] ERROR [storageadmin.middleware:35] Failed to mount Pool(ro_pool) due to an unknown reason. Command used ['/usr/bin/mount', '/dev/disk/by-label/ro_pool', '/mnt2/ro_pool']
Traceback (most recent call last):
  File "/opt/rockstor/.venv/lib/python3.6/site-packages/django/core/handlers/base.py", line 185, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/opt/rockstor/.venv/lib/python3.6/site-packages/django/views/decorators/csrf.py", line 58, in wrapped_view
    return view_func(*args, **kwargs)
  File "/opt/rockstor/.venv/lib/python3.6/site-packages/django/views/generic/base.py", line 68, in view
    return self.dispatch(request, *args, **kwargs)
  File "/opt/rockstor/.venv/lib/python3.6/site-packages/rest_framework/views.py", line 495, in dispatch
    response = self.handle_exception(exc)
  File "/opt/rockstor/.venv/lib/python3.6/site-packages/rest_framework/views.py", line 455, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/opt/rockstor/.venv/lib/python3.6/site-packages/rest_framework/views.py", line 466, in raise_uncaught_exception
    raise exc
  File "/opt/rockstor/.venv/lib/python3.6/site-packages/rest_framework/views.py", line 492, in dispatch
    response = handler(request, *args, **kwargs)
  File "/usr/lib64/python3.6/contextlib.py", line 52, in inner
    return func(*args, **kwds)
  File "/opt/rockstor/src/rockstor/storageadmin/views/command.py", line 395, in post
    import_shares(p, request)
  File "/opt/rockstor/src/rockstor/storageadmin/views/share_helpers.py", line 96, in import_shares
    shares_in_pool = shares_info(pool)
  File "/opt/rockstor/src/rockstor/fs/btrfs.py", line 971, in shares_info
    pool_mnt_pt = mount_root(pool)
  File "/opt/rockstor/src/rockstor/fs/btrfs.py", line 795, in mount_root
    "Command used {}".format(pool.name, mnt_cmd)
Exception: Failed to mount Pool(ro_pool) due to an unknown reason. Command used ['/usr/bin/mount', '/dev/disk/by-label/ro_pool', '/mnt2/ro_pool']

We can thus see that our custom middleware is indeed at play:

[21/Aug/2023 17:01:37] ERROR [storageadmin.middleware:34] Exception occurred while processing a request. Path: /api/commands/refresh-share-state method: POST
[21/Aug/2023 17:01:37] ERROR [storageadmin.middleware:35] Failed to mount Pool(ro_pool) due to an unknown reason. Command used ['/usr/bin/mount', '/dev/disk/by-label/ro_pool', '/mnt2/ro_pool']

As a reminder, this middleware also creates a tarball of rockstor's logs at the following path: src/rockstor/logs/error.tgz. In our case, we can also verify that this tarball has successfully been created:

buildvm155:/opt/rockstor # ls -lah src/rockstor/logs/
total 56K
drwxr-xr-x 1 root root  62 Aug 20 15:37 .
drwxr-xr-x 1 root root 296 Aug 19 11:40 ..
-rw-r--r-- 1 root root 54K Aug 21 17:03 error.tgz
-rw-r--r-- 1 root root   0 Jun 11  2022 Logs-Manager-Directory

All thus seems to behave as it should!

@phillxnet phillxnet removed the needs review Ideally by prior rockstor-core contributor label Aug 22, 2023
@phillxnet
Copy link
Member

phillxnet commented Aug 22, 2023

@FroggyFlox Thanks for doing the exception testing on this one. Much appreciated.
Merging now directly given we now have full testing of the change's efficacy.

@phillxnet phillxnet merged commit c0f7db9 into rockstor:testing Aug 22, 2023
@phillxnet
Copy link
Member

phillxnet commented Aug 22, 2023

@FroggyFlox Thanks for seeing to all these stragglers. All pushing us towards a smoother transition to our next major Django version.

@phillxnet phillxnet mentioned this pull request Aug 22, 2023
Closed
@FroggyFlox FroggyFlox deleted the 2662-Update-Middlewares-styles branch October 20, 2023 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

(t) Update Middlewares style

2 participants

@FroggyFlox @phillxnet