Skip to content
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.

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

roles: hosted_engine_setup: restore - remove host also based on name #567

Merged
merged 4 commits into from
Aug 10, 2022

Conversation

arachmani
Copy link
Member

In restore, remove the host from DB based on the name
in addition to the UUID.
Using multiple hosts with the same name is not allowed anyway.

Bug-Url: https://bugzilla.redhat.com/2003515
Signed-off-by: Asaf Rachmani arachman@redhat.com

Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@didib
Copy link
Member

didib commented Aug 3, 2022

OK for me, but perhaps better warn the user if there are any inconsistencies - at least if we removed more than one host (e.g. one matching uuid and one matching name).

@arachmani
Copy link
Member Author

OK for me, but perhaps better warn the user if there are any inconsistencies - at least if we removed more than one host (e.g. one matching uuid and one matching name).

Where do you think we need to warn the user?
We already mentioned it in the documentation:

If you deploy on a new host, you must assign a unique name to the host. Reusing the name of an existing host included in the backup can cause conflicts in the new environment.

https://access.redhat.com/documentation/en-us/red_hat_virtualization/4.4/html/administration_guide/chap-backups_and_migration#Backing_up_and_Restoring_a_Self-hosted_Engine

@didib
Copy link
Member

didib commented Aug 3, 2022

Where do you think we need to warn the user?

That's a good question. I wrote a bit about it in the bug. Ideally IMO we should prompt and ask the user what to do, e.g. similarly to what we do when 'Add host' fails. I realize this is a large change, so not requiring it right now. I also see it's not easy to emit warnings, ansible/ansible#67260, but perhaps we can still do something, not sure.

In restore, remove the host from DB based on the name
in addition to the UUID.
Using multiple hosts with the same name is not allowed anyway.

Bug-Url: https://bugzilla.redhat.com/2003515
Signed-off-by: Asaf Rachmani <arachman@redhat.com>
Signed-off-by: Asaf Rachmani <arachman@redhat.com>
Signed-off-by: Asaf Rachmani <arachman@redhat.com>
@arachmani
Copy link
Member Author

Verified locally:

2022-08-10 08:57:58,709+0000 DEBUG var changed: host "localhost" var "db_remove_he_host" type "<class 'dict'>" value: "{
    "changed": true,
    "cmd": [
        "/usr/share/ovirt-engine/dbscripts/engine-psql.sh",
        "-c",
        "SELECT deletevds(vds_id) FROM (SELECT vds_id FROM vds WHERE upper(vds_name)=upper('h452u.asrachmani.com')) t"
    ],
    "delta": "0:00:01.201757",
    "end": "2022-08-10 08:57:58.238446",
    "failed": false,
    "msg": "",
    "rc": 0,
    "start": "2022-08-10 08:57:57.036689",
    "stderr": "",
    "stderr_lines": [],
    "stdout": " deletevds \n-----------\n \n(1 row)",
    "stdout_lines": [
        " deletevds ",
        "-----------",
        " ",
        "(1 row)"
    ]
}"

@arachmani arachmani merged commit f1e26a9 into oVirt:master Aug 10, 2022
@mnecas mnecas mentioned this pull request Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants