Bug 1707225

After taking f2f with Didi we decided that the bug will be solved by him.

Eyal, sorry for not thinking about this earlier, when we introduced cinderlib integration.

Is it possible for cinderlib to use a specific db schema, not the default 'public'? This way, we can use the existing engine db, and create there another schema for cinderlib. As an example, check aaa_jdbc, the extension that manages local users (e.g. "admin@internal"), in [1].

Pros:

We get backup/restore "for free", also related stuff (sosreport, migration/upgrades, etc.)

Cons:

- Cinderlib has credentials to the engine db, so bugs in it can potentially corrupt it. I think that's a minor issue.

- We have to somehow handle upgrade of systems that used a separate database to use the schema instead.

If it's possible, it might be worth consulting Eli or Martin about other Pros/Cons I didn't think about.

[1] engine git repo, packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/aaajdbc.py

(In reply to Yedidyah Bar David from comment #2)
> Eyal, sorry for not thinking about this earlier, when we introduced
> cinderlib integration.
> 
> Is it possible for cinderlib to use a specific db schema, not the default
> 'public'? This way, we can use the existing engine db, and create there
> another schema for cinderlib. As an example, check aaa_jdbc, the extension
> that manages local users (e.g. "admin@internal"), in [1].

Sadly no, cinderlib requires his own DB, he manages it and creates there his own entities and tables.
We cannot use an existing schema, and we also cannot use the existing engine DB since we cannot assure and control on what cinderlib
creates and that it will not conflicts with the existing tables created by the engine.

> 
> Pros:
> 
> We get backup/restore "for free", also related stuff (sosreport,
> migration/upgrades, etc.)
> 
> Cons:
> 
> - Cinderlib has credentials to the engine db, so bugs in it can potentially
> corrupt it. I think that's a minor issue.
> 
> - We have to somehow handle upgrade of systems that used a separate database
> to use the schema instead.
> 
> If it's possible, it might be worth consulting Eli or Martin about other
> Pros/Cons I didn't think about.
> 
> [1] engine git repo,
> packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/aaajdbc.py

(In reply to Eyal Shenitzky from comment #3)
> (In reply to Yedidyah Bar David from comment #2)
> > Eyal, sorry for not thinking about this earlier, when we introduced
> > cinderlib integration.
> > 
> > Is it possible for cinderlib to use a specific db schema, not the default
> > 'public'? This way, we can use the existing engine db, and create there
> > another schema for cinderlib. As an example, check aaa_jdbc, the extension
> > that manages local users (e.g. "admin@internal"), in [1].
> 
> Sadly no, cinderlib requires his own DB, he manages it and creates there his
> own entities and tables.
> We cannot use an existing schema, and we also cannot use the existing engine
> DB since we cannot assure and control on what cinderlib
> creates and that it will not conflicts with the existing tables created by
> the engine.

I was referring to using its own schema inside the existing engine database.
Postgresql has a notion of 'schema', and a single database can have many.
When you create a database, you get a default schema called 'public', which
is what the engine uses. aaa-jdbc extension uses the same database, but its
own schema inside it (called 'aaa_jdbc'), with its own tables etc.

Now searched, and I do not see that cinderlib supports that, but it seems
to be possible at sqlalchemy level:

https://stackoverflow.com/questions/9298296/sqlalchemy-support-of-postgres-schemas

Also, somewhat related:

It seems to me that you pass the url to cinderlib-client.py, which includes the
password, on the command line. This is insecure. One way to solve this
is to use env vars and set them in the engine, before running the python script:

https://docs.sqlalchemy.org/en/13/dialects/postgresql.html#empty-dsn-connections-environment-variable-connections

And also, you do not url-encode the password, AFAICT. This means that a password
containing e.g. '/' will not work, IIUC.

If not handled/tracked already, please open a bug for these issues. Thanks.

Benny, Do you remember any limitation from cinderlib about using a schema inside the engine DB?

There shouldn't be a problem using cinderlib within the same db in a separate schema IMHO

For the time being I have extended the engine-backup script to include the ovirt_cinderlib database.
It might be helpful as long as migrating the cinderlib to the engine db is pending.
Is it of any interest? if yes I'd create a patch...

(In reply to Thomas Klute from comment #15)
> For the time being I have extended the engine-backup script to include the
> ovirt_cinderlib database.

That's already done here, did you look at the linked patch?

> It might be helpful as long as migrating the cinderlib to the engine db is
> pending.

I gave up on this, seeing that it took us too long to decide and that it's
quite complex technically. So I pushed a patch for engine-backup instead.
Also, some people told me in private that the price to pay (some extra potential
admin work) is low, compared to the advantages of more flexibility and perhaps
even performance (if it turns out that this db adds load, admins can move it to
a different machine).

> Is it of any interest? if yes I'd create a patch...

It could have been, but is now too late :-((. Thanks, anyway!

> 
> That's already done here, did you look at the linked patch?

oh, missed that. Thanks for the hint. 
 
> > It might be helpful as long as migrating the cinderlib to the engine db is
> > pending.
> 
> > Is it of any interest? if yes I'd create a patch...
> 
> It could have been, but is now too late :-((. Thanks, anyway!

It would have taken me some additional work to make the patch complete.
So, thanks to you - we'll use your version from now on.

Verified
Succesful to make the backup and restore option with Cinderlib DB.
The restore was made on a clean new VM with a clean engine. Every supported Cinderlib functionality works after the restoration.

Steps to reproduce:

1. Prepare an engine (4.4.0-26 Version) with Cinderlib installation for RHEL8.
2. Create a new ceph cinderlib MBD, create a new disk of ceph cinderlib MBD, and attached the disk to the new VM. Writing to the attached MBD disk.
3. Backup&restore the engine running with Cinderlib ceph SD. Documentation for engine backup and restore: https://access.redhat.com/documentation/en-us/red_hat_virtualization/4.3/html/administration_guide/chap-backups_and_migration#Restoring_a_Backup_with_the_engine-backup_Command
On the old engine run “engine-backup --scope=all --mode=backup --file=cinderlib_from_old_engine --log=log_cinderlib_from_old_engine”.
4. Copy backup files to a safe place.
5. Power-off the original engine.
6. Prepare a clean VM with the same IP address and FQDN as was set on the original engine’s VM.
We brought up a clean VM and installed an rhvm package (rhvm-4.4.0-0.29.master.el8ev.noarch) on it.
7. Copy backup files to the new engine.
8. Execute engine-restore:
engine-backup --mode=restore --file=/root/cinderlib_from_old_engine --log=/root/log_cinderlib_from_old_engine --provision-all-databases --restore-permissions
9. Running engine-setup
10.Checking that Cinderlib is with fully functional after restore and engine-setup were completed succesfully. 

Versions:
The old engine - ovirt-engine-4.4.0-0.26.master.el8ev.noarch
The new engine - ovirt-engine-4.4.0-0.29.master.el8ev.noarch
rhel-8-openstack-cinderlib (python3-cinderlib)
vdsm-4.40.7-1.el8ev.x86_64
rhceph-4-mon-for-rhel-8-rpms

Didi, Please check the doc text to make sure I got it right.

You definitely got it right, but I'd like to make a few points:

1. You _can_, if you want, specify only --scope=cinderlibdb, and then only this is backed up (and restored). If you don't do that but use --scope=all, I'd not call the backup file "cinderlib_from_old_engine", as its name implies (to me) that it's only cinderlib, whereas it's actually "all" (meaning, everything found by engine-backup, normally including files, db, dwhdb, and now cinderlibdb, if it was enabled in engine-setup).

2. Since this is doc text, and not full docs, perhaps refer to '--help'. Not sure we need an example.

3. For the example, please open a ticket to update all documentation - we have a few different versions based on different versions of oVirt/RHV and I'd like to consolidate all places to a single one, except for the backup/restore guide which should of course include several. The minimum, as of 4.3 (bug 1530031), is:

Backup:

engine-backup

Restore:

engine-backup --mode=restore --file=backupfile --provision-all-databases

Notably (at least compared to your example):

scope defaults to all (since several versions ago)

mode defaults to backup

file defaults to a file in /var/lib/ovirt-engine-backup/

log defaults to a file in /var/log/ovirt-engine-backup/

If restoring, and passing any --provision* option, we default to --restore-permissions

It's up to you whether to also include one in current doc text.

4. Minor style notes:

Before this update, there was no way to backup and restore the Cinderlib database. With this update, the engine-backup command includes the Cinderlib database.

Not sure "there was no way". Obviously there was a way - if you know the system well, and know how to use PostgreSQL tools, you can use them directly. Perhaps "there was no supported/simple way", or "engine-backup didn't handle", or something like that.

Not sure about "the command includes the database". The command handles it, the generated backups include it.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Important: RHV Manager (ovirt-engine) 4.4 security, bug fix, and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2020:3247

The needinfo request[s] on this closed bug have been removed as they have been unresolved for 120 days