Arksine / moonraker

Web API Server for Klipper
https://moonraker.readthedocs.io
GNU General Public License v3.0
1.02k stars 392 forks source link

Login fails after upgrade #864

Closed ginkel closed 1 month ago

ginkel commented 1 month ago

What happened

I am using the mkuf/moonraker:latest Docker image to run moonraker inside Docker. Yesterday this image has been updated to a recent moonraker version (based on commit 73df63d). Part of that update seems to have been the migration of the users table to sqlite.

Since applying that update I can no longer login to moonraker via fluidd. I suspect this is caused by the sqlite migration.

When attempting to login, the logs just state:

2024-05-27 21:11:21,813 [application.py:log_request()] - 400 POST /access/login (10.147.10.18) [No User] 5.62ms

My password contains some special characters, not sure if that's a possible cause.

The image causing trouble is based on commit c958120. The last known good image is based on commit 73df63d. Reverting to the last-known good version and restoring a backup of the moonraker-db directory solves the issue.

/cc @mkuf

Client

Fluidd

Browser

Chrome, Firefox

How to reproduce

Upgrade from 73df63d to c958120.

Additional information

-------------------- Log Start | Mon May 27 21:03:33 2024 --------------------
platform: Linux-5.15.0-1055-raspi-aarch64-with-glibc2.36
data_path: /opt/printer_data
is_default_data_path: False
config_file: /opt/printer_data/config/moonraker.conf
startup_warnings: []
verbose: False
debug: False
asyncio_debug: False
is_backup_config: False
is_python_package: False
instance_uuid: 42ff1cd3e3aa4952a0c0213a5486a284
unix_socket_path: /opt/printer_data/comms/moonraker.sock
software_version: ?
log_file: /opt/printer_data/logs/moonraker.log
python_version: 3.12.3 (main, May 14 2024, 05:51:03) [GCC 12.2.0]
launch_args: /opt/venv/bin/python moonraker/moonraker/moonraker.py -d /opt/printer_data
msgspec_enabled: False
uvloop_enabled: False
2024-05-27 21:03:33,400 [confighelper.py:read_file()] - Configuration File '/opt/printer_data/config/moonraker.con
f' parsed, total size: 606 B
2024-05-27 21:03:33,400 [server.py:add_log_rollover_item()] - 
Arksine commented 1 month ago

Please attach the full moonraker.log.

FWIW, I don't think its related to special characters. The DB doesn't store the password, it stores a salt and a hash. It may be due to a refactoring I did to make make the transition to sqlite easier. It would be helpful to get the traceback, this can be done by enabling verbose logging. In a standard installation this can be done by adding -v to the MOONRAKER_ARGS environment variable in printer_data/systemd/moonraker.env, however this likely won't work in a container. I'm unsure if similar functionality is available, or what the best way is to add an argument for Moonraker when running in this container.

Alternatively the traceback is sent to the browser. If you open the dev console (usually by right clicking on the page and selecting inspect), navigate to the network tab, then attempt to login. You should see a red login on the left. Click on it, then click on the Response to the right and copy everything in the window.

ginkel commented 1 month ago

Thanks for the speedy reply! Please find attached the logs for 2024-05-26 and 2024-05-27. The end of the latter is based on the last-known good version mentioned above as I wanted to run a print job, so don't be surprised if there are no errors at the end.

logs.tar.gz

Arksine commented 1 month ago

Thanks, I see the problem. I removed the lmdb dependency as its no longer necessary for new installations, therefore lmdb is not included in the container build. Since that is missing, Moonraker cannot open the lmdb database and convert it to sqlite. In a normal installation lmdb will already be installed before the upgrade, so it isn't an issue.

Commit 3b1c21a72db53a05f1e127119cd74e6e83406f03 should resolve this issue by attempting to install lmdb if its missing. The container will need to be rebuilt to include this commit. You will need to remove printer_data/database/moonraker-sql.db prior to staring the updated container to force a conversion. FWIW, it shouldn't be necessary to restore the lmdb files, Moonraker does not remove them after conversion in the event that something goes wrong during conversion, such as what happened in your situation.

For a container, the lmdb installation wont persist, but that should be fine. It will only be needed during the initial conversion.

Edit: The patch will only work if pip is installed. If pip isn't installed in the container then the dockerfile needs to be modified to install lmdb itself.

Arksine commented 1 month ago

Also, some recommendations based on observations from your log:

1) Set the provider option in [machine] to none, unless supervisorctl is installed (in which case the provider should be supervisord. 2) Dont include an - when attempting to add trusted clients as you would in yaml. Indentation is all that is necessary.

ginkel commented 1 month ago

I updated my installation to 4fe89d5571e644c8222c15859f3b14d0e118fbd8, which unfortunately broke the login again. Log attached.

Thanks also for the recommendations. Will have a look...

log-4fe89d5.tar.gz

Arksine commented 1 month ago

Unfortunately the container failed to build the wheel. I suspect that the solution will be to update the dockerfile to install lmdb itself.

mkuf commented 1 month ago

The runtime image does not contain any build tools, so no packages can be installed. This is also in an effort to have reproducible images.

I'm currently unavailable, but I probably can spend some time this weekend to get the necessary package into the moonraker image.

Set the provider option in [machine] to none, unless supervisorctl is installed (in which case the provider should be supervisord.

Iirc setting the provider to none disables also the machine endpoints of moonraker, making it impossible to shut down the host via the API. That's why I opted to add some systemd utilities into the container and configure the provider like this.

mkuf commented 1 month ago

@ginkel Tag v0.8.0-377-g4fe89d5 has been rebuilt to include lmdb

ginkel commented 1 month ago

Works like a charm, thanks a lot for the speedy fix! :smiley: