oVirt / ovirt-engine

The oVirt Engine virtualization manager
Other
519 stars 271 forks source link

ovirt-engine-setup database checks: being member of the database owner is also ok #842

Closed betanummeric closed 1 year ago

betanummeric commented 1 year ago

fixes issue #841

This PR makes the database check succeed also when some object is not owned by the user, but the user is member of some role which owns the object.

The pg_catalog.pg_has_role function is available since postgres 8.1 (released in 2005). The argument 'usage' means that the user does not need to explicitly switch to the owner role using SET ROLE, which may be necessary with the alternative 'member'.

didib commented 1 year ago

Makes sense. Did you verify backup/restore?

@emesika might want to review as well.

betanummeric commented 1 year ago

@didib Backup and restore are tested successfully.

michalskrivanek commented 1 year ago

lgtm, let's run ost

michalskrivanek commented 1 year ago

/ost

didib commented 1 year ago

Sorry for being annoying. Did you test also the backup/restore done by engine-setup itself? It backs up the database at some point, and clears and restores it if there was a failure and it needs to rollback. To cause it to fail, you can simply press ^C at a strategic point - after backup, before finishing the transaction - or something like this:

dnf install otopi-debug-plugins
# install and setup an engine with your patch and with a DB configured as you want
OTOPI_FORCE_FAIL_STAGE=STAGE_MISC OTOPI_FORCE_FAIL_PRIORITY=PRIORITY_LOW engine-setup
betanummeric commented 1 year ago

These commands are tested:

engine-setup --reconfigure-components
engine-backup --mode=backup --scope=all --file=ovirt-engine-backup-test.backup --log=ovirt-engine-backup.log
engine-backup --mode=restore --scope=db --file=ovirt-engine-backup-test.backup --log=ovirt-engine-restore.log --restore-permissions

Is this sufficient?

didib commented 1 year ago

These commands are tested:

engine-setup --reconfigure-components
engine-backup --mode=backup --scope=all --file=ovirt-engine-backup-test.backup --log=ovirt-engine-backup.log
engine-backup --mode=restore --scope=db --file=ovirt-engine-backup-test.backup --log=ovirt-engine-restore.log --restore-permissions

Is this sufficient?

It is, for testing engine-backup. But engine-setup has its own backup/restore code, which does not use engine-backup.

didib commented 1 year ago

To clarify: engine-setup has to backup, and potentially restore, only on upgrades - so that if the upgrade fails, you get back to your original state, before trying to upgrade. So you should first do a clean setup, then upgrade (can be to same version), and fail it in the middle of upgrade - after it backed up and before it committed.

betanummeric commented 1 year ago

I can now also confirm a successful restore upon a failed upgrade (interrupted with ctrl+c). The engine worked after that.

[ INFO  ] Cleaning async tasks and compensations
[ INFO  ] Unlocking existing entities
[ INFO  ] Checking the Engine database consistency
[ INFO  ] Stage: Transaction setup
[ INFO  ] Stopping engine service
[ INFO  ] Stopping ovirt-fence-kdump-listener service
[ INFO  ] Stopping dwh service
[ INFO  ] Stopping vmconsole-proxy service
[ INFO  ] Stopping websocket-proxy service
[ INFO  ] Stopping service: grafana-server
[ INFO  ] Stage: Misc configuration (early)
[ INFO  ] Stage: Package installation
[ INFO  ] Stage: Misc configuration
[ INFO  ] Running vacuum full on the engine schema
[ INFO  ] Running vacuum full elapsed 0:00:02.563703
[ INFO  ] Upgrading CA
[ INFO  ] Configuring WebSocket Proxy
[ INFO  ] Backing up ***:engine to '/var/lib/ovirt-engine/backups/engine-20230504075229.1edo8krs.dump'.                                                                                                                  
[ INFO  ] Creating/refreshing Engine database schema
^C[ ERROR ] Failed to execute stage 'Misc configuration': SIG2
[ INFO  ] Rolling back database schema
[ INFO  ] Clearing Engine database engine
[ INFO  ] Restoring Engine database engine
[ INFO  ] Restoring file '/var/lib/ovirt-engine/backups/engine-20230504075229.1edo8krs.dump' to ***:engine.                                                                                                              
[ INFO  ] Stage: Clean up
          Log file is located at /var/log/ovirt-engine/setup/ovirt-engine-setup-20230504075206-ozuwpw.log
[ INFO  ] Generating answer file '/var/lib/ovirt-engine/setup/answers/20230504075244-setup.conf'
[ INFO  ] Stage: Pre-termination
[ INFO  ] Stage: Termination
[ ERROR ] Execution of setup failed
didib commented 1 year ago

LGTM, merging. Thanks!

betanummeric commented 1 year ago

thanks for merging :)