Note: This bug is displayed in read-only format because the product is no longer active in Red Hat Bugzilla.

Bug 1212752

Summary: [engine-backup] restore fails if using change-{,dwh}-db-credentials and previous user exists
Product: [Retired] oVirt Reporter: Jiri Belka <jbelka>
Component: ovirt-engine-installerAssignee: Yedidyah Bar David <didi>
Status: CLOSED CURRENTRELEASE QA Contact: Lukas Svaty <lsvaty>
Severity: low Docs Contact:
Priority: medium    
Version: 3.6CC: bugs, didi, ecohen, emesika, gklein, jbelka, lsurette, lsvaty, rbalakri, yeylon
Target Milestone: ---   
Target Release: 3.6.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: integration
Fixed In Version: ovirt-3.6.0-alpha1.2 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-11-04 11:33:44 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1212526    
Attachments:
Description Flags
restore log none

Description Jiri Belka 2015-04-17 09:48:01 UTC 
Created attachment 1015505 [details]
restore log

Description of problem:

engine-backup --mode=restore --file=/root/rhevm_backup.tar --scope=all --log=/root/rhevm_restore.log --db-host=127.0.0.1 --db-user=newengine --db-password=123456 --db-name=newengine --dwh-db-host=127.0.0.1 --dwh-db-name=newdwh --dwh-db-user=newdwh --dwh-db-password=123456 --change-db-credentials --change-dwh-db-credentials

dbs and user roles created as described in engine-backup --help!

...
+ PGPASSFILE=/tmp/engine-backup.4lQeLthWwq/.pgpass
+ psql -w -U newengine -h 127.0.0.1 -p 5432 -d newengine
+ file=/tmp/engine-backup.4lQeLthWwq/db/engine_backup.db
+ '[' -f /tmp/engine-backup.4lQeLthWwq/db/engine_backup.db ']'
+ '[' -f /tmp/engine-backup.4lQeLthWwq/db/engine_backup.db.bz2 ']'
+ bzcat
+ cat /tmp/engine-backup.4lQeLthWwq/psql-restore-log
++ cat
++ egrep -v '^$|^#'
++ sed 's/|$//'
++ tr '\012' '|'
+ local 'IGNORED_ERRORS=language "plpgsql" already exists|must be owner of language plpgsql|must be owner of extension plpgsql|permission denied for language c|function public.uuid_generate_v1\(\) does not exist$
function public.uuid_generate_v1mc\(\) does not exist|function public.uuid_generate_v3\(uuid, text\) does not exist|function public.uuid_generate_v4\(\) does not exist|function public.uuid_generate_v5\(uuid, text
\) does not exist|function public.uuid_nil\(\) does not exist|function public.uuid_ns_dns\(\) does not exist|function public.uuid_ns_oid\(\) does not exist|function public.uuid_ns_url\(\) does not exist|function 
public.uuid_ns_x500\(\) does not exist|role "engine" does not exist'
++ grep 'ERROR: ' /tmp/engine-backup.4lQeLthWwq/psql-restore-log
++ grep -Ev 'language "plpgsql" already exists|must be owner of language plpgsql|must be owner of extension plpgsql|permission denied for language c|function public.uuid_generate_v1\(\) does not exist|function pu
blic.uuid_generate_v1mc\(\) does not exist|function public.uuid_generate_v3\(uuid, text\) does not exist|function public.uuid_generate_v4\(\) does not exist|function public.uuid_generate_v5\(uuid, text\) does not
 exist|function public.uuid_nil\(\) does not exist|function public.uuid_ns_dns\(\) does not exist|function public.uuid_ns_oid\(\) does not exist|function public.uuid_ns_url\(\) does not exist|function public.uuid
_ns_x500\(\) does not exist|role "engine" does not exist'
...

i'm not db man but it seems 'engine' is hardcoded in db backup file:

# bzcat ./db/engine_backup.db.bz2 | egrep '^[A-Z]+.*\bengine\b' | head
ALTER PROCEDURAL LANGUAGE plpgsql OWNER TO engine;
ALTER TYPE public.all_storage_usage_rs OWNER TO engine;
ALTER TYPE public.all_vds_group_usage_rs OWNER TO engine;
ALTER TYPE public.async_tasks_info_rs OWNER TO engine;
ALTER TYPE public.authzentryinfotype OWNER TO engine;
ALTER TYPE public.booleanresulttype OWNER TO engine;
ALTER TYPE public.disks_basic_rs OWNER TO engine;
ALTER TYPE public.get_all_commands_rs OWNER TO engine;
ALTER TYPE public.getallfromsnapshotsbyvmid_rs OWNER TO engine;
ALTER TYPE public.getallfromvm_pools_rs OWNER TO engine;

...
2015-04-16 16:23:07 29709: restoreDB: backupfile /tmp/engine-backup.aQ0vIfRkSW/db/engine_backup.db user engine host localhost port 5432 database engine orig_user engine
SET
SET
SET
SET
SET
SET
ERROR:  language "plpgsql" already exists
ALTER LANGUAGE
SET
...

Version-Release number of selected component (if applicable):
ovirt-engine-tools-3.6.0-0.0.master.20150412172306.git55ba764.el6.noarch

How reproducible:
100%

Steps to Reproduce:
1. make backup
2. create dbs and user roles as engine-backup --help instructs but use your own db names and user names
3. engine-backup --mode=restore --file=/root/rhevm_backup.tar --scope=all --log=/root/rhevm_restore.log --db-host=127.0.0.1 --db-user=newengine --db-password=123456 --db-name=newengine --dwh-db-host=127.0.0.1 --dwh-db-name=newdwh --dwh-db-user=newdwh --dwh-db-password=123456 --change-db-credentials --change-dwh-db-credentials # CHANGE TO FIT YOUR VALUES!

Actual results:
failure

Expected results:
should work

Additional info:
Comment 1 Yedidyah Bar David 2015-04-19 06:49:00 UTC 
(In reply to Jiri Belka from comment #0)
> 
> i'm not db man but it seems 'engine' is hardcoded in db backup file:

Indeed, and we do not try to change that.

Instead we merely ignore the errors this causes, and rely on the default to work - that is, that new objects created by restore will be owned by the user used to restore. See also bug 1121961.

> ERROR:  language "plpgsql" already exists

That's not the error causing the failure, it's already ignored.

But it seems that the one added for bug 1121961 was not enough -

role "$orig_user" does not exist

But this isn't the error you have in the attached log, which is also not filtered yet:

ERROR:  must be member of role "engine"

It seems to me to be emitted when the user 'engine' does exist. Now reproduced this accordingly.

So a workaround is to make sure the previous user does not exist - either start from a clean empty pg setup or manually 'drop user engine' before restoring.

Jiri - please verify workaround.

The fix will merely be adding the above error to the list of ignored errors.
Comment 2 Yedidyah Bar David 2015-04-19 06:50:15 UTC 
Eli - what about, instead, passing '--no-owner' to pg_dump? This will solve all these kinds of problems at once, and I can't see any problem with it.
Comment 3 Yedidyah Bar David 2015-04-19 06:51:37 UTC 
BTW, I am pretty certain that this affects 3.3+.
Comment 4 Eli Mesika 2015-04-19 15:05:51 UTC 
(In reply to Yedidyah Bar David from comment #2)
> Eli - what about, instead, passing '--no-owner' to pg_dump? This will solve
> all these kinds of problems at once, and I can't see any problem with it.

Don't we want all all objects to be owned by user 'engine' after restore ???
Comment 5 Yedidyah Bar David 2015-04-20 07:36:57 UTC 
(In reply to Eli Mesika from comment #4)
> (In reply to Yedidyah Bar David from comment #2)
> > Eli - what about, instead, passing '--no-owner' to pg_dump? This will solve
> > all these kinds of problems at once, and I can't see any problem with it.
> 
> Don't we want all all objects to be owned by user 'engine' after restore ???

I think we want them to be owned by the user doing the restore, which with --change-db-credentials might be different. No?

I'll detail a bit: We anyway do not use (at least generally) a privileged account, so granting/revoking will fail anyway. This means that:

On machine A:
1. install and setup engine with (e.g.) pg user 'engine'
2. create various pg users/roles/etc and grant them various permissions on various engine-owned objects
3. engine-backup --mode=backup --file=f1

On machine B:
4. Install engine
5. Create a pg db/user newengine (or whatever)
6. engine-backup --mode=restore --file=f1 --change-db-credentials --db-user=newengine --db-name=newengine

Will:
A. Not be able to restore on machine B the custom grants set on machine A, even if we wanted to (because it's using a non-privileged user)
B. Is not even supposed to do that, because (imo?) that's out-of-scope for this tool.

If we accept the above (do we?), then:
C. We do not even need to keep the grants in the backup file
D. We can rely on pg's default behavior, to give permissions on created objects to the user creating them (on restore, in current case).

Thus passing '--no-owner' to pg_dump is both enough in terms of what we do want to achieve, and will save us from having to filter/deal with errors during restore about failing to grant permissions.
Comment 6 Jiri Belka 2015-04-20 07:47:02 UTC 
IMHO the tool is not very user friendly, imo change-{,dwh}-db-credentials should be dropped from options and if explicitly defined db-name etc... it should also mean to use explicitly define credentials. We know defaults and I suppose defaults are saved in the backup, thus change-{,dwh}-db-credentials is redundant.

Also --help should show (default) for db-name etc...
Comment 7 Eli Mesika 2015-04-20 07:59:37 UTC 
(In reply to Yedidyah Bar David from comment #5)
OK, then I agree with this approach 

I assume that we are not using a hard-coded 'engine' user anywhere
Comment 8 Yedidyah Bar David 2015-04-20 08:03:21 UTC 
(In reply to Eli Mesika from comment #7)
> (In reply to Yedidyah Bar David from comment #5)
> OK, then I agree with this approach 
> 
> I assume that we are not using a hard-coded 'engine' user anywhere

We do not, and if we do, that's a bug. See e.g. bug 1184113.
Comment 9 Yedidyah Bar David 2015-04-20 08:24:39 UTC 
(In reply to Jiri Belka from comment #6)
> IMHO the tool is not very user friendly, imo change-{,dwh}-db-credentials
> should be dropped from options and if explicitly defined db-name etc... it
> should also mean to use explicitly define credentials.

You are welcome to open a bug, you know :-)

Not sure I agree, but of course what you say makes sense.

> We know defaults and
> I suppose defaults are saved in the backup,

What's saved in the backup is of course not the defaults, but what's actually used by the engine etc.

The defaults affect only engine-setup.

> thus
> change-{,dwh}-db-credentials is redundant.
> 
> Also --help should show (default) for db-name etc...

Why?

If at all, you might want an options --show-credentials that will show what's actually saved in the backup file. We'll have to handle this, especially the passwords, a bit carefully - perhaps e.g. --write-credentials=file (which will be created with suitable perms etc) or something like that. You are welcome to open a bug for that. Not sure it's that important. Better, imo, is getting rid of this shell script and rewrite using the setup framework, see bug 1064503. But I can't see this currently getting into 3.6, although I did start working on this.
Comment 11 Sandro Bonazzola 2015-11-04 11:33:44 UTC 
oVirt 3.6.0 has been released on November 4th, 2015 and should fix this issue.
If problems still persist, please open a new BZ and reference this one.