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

role: hosted_engine_setup: Use ovirt_host module #275

Merged
merged 1 commit into from
May 27, 2021

Conversation

arachmani
Copy link
Member

Use ovirt_host module to discover iSCSI
instead of using REST API.

Requires a change in ovirt-hosted-engine-setup and cockpit-ovirt

Bug-Url: https://bugzilla.redhat.com/1922748
Signed-off-by: Asaf Rachmani [email protected]

Use `ovirt_host` module to discover iSCSI
instead of using REST API.

Requires a change in ovirt-hosted-engine-setup and cockpit-ovirt

Bug-Url: https://bugzilla.redhat.com/1922748
Signed-off-by: Asaf Rachmani <[email protected]>
@arachmani
Copy link
Member Author

Requires the following changes:
ovirt-hosted-engine-setup - https://gerrit.ovirt.org/#/c/ovirt-hosted-engine-setup/+/114826/
cockpit-ovirt - https://gerrit.ovirt.org/#/c/cockpit-ovirt/+/114827/

@arachmani
Copy link
Member Author

Verified using hosted-engine --deploy command

Copy link
Member

@sandrobonazzola sandrobonazzola left a comment

Choose a reason for hiding this comment

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

LGTM

@arachmani arachmani merged commit f6a8428 into oVirt:master May 27, 2021
gerrit-ovirt-org pushed a commit to oVirt/ovirt-hosted-engine-setup that referenced this pull request May 27, 2021
The change [1] in hosted_engine_setup role impacts the structure of
the otopi_iscsi_targets variable

[1] oVirt/ovirt-ansible-collection#275

Bug-Url: https://bugzilla.redhat.com/1922748
Signed-off-by: Asaf Rachmani <[email protected]>
Change-Id: Ieaa7a787e2815ed26e94c33d3109ea3ad176065e
Comment on lines +28 to +29
username: "{{ he_iscsi_discover_username }}"
password: "{{ he_iscsi_discover_password }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

if these parameters are null maybe it will be better to omit them?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's possible.

Content-Type: application/json
Accept: application/json
Authorization: "Basic {{ ('admin@internal' + ':' + he_admin_password ) | b64encode }}"
- name: iSCSI discover
Copy link
Contributor

Choose a reason for hiding this comment

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

with this change, I think you need to remove lines # 3-11

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, I'll do remove these lines.

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