introducing ovirt-engine-setup role #123

tareqalayan · 2017-10-26T13:44:53Z

This role will enable you to deploy ovirt-engine with answere file.

machacekondra · 2017-10-27T08:10:49Z

roles/ovirt-engine-setup/README.md

+
+| Name                            | Default value         |  Description                                              |
+|---------------------------------|-----------------------|-----------------------------------------------------------|
+| ovirt_engine_type               | ovirt-engine          | Type of product 'ovirt-engine|rhevm'  |


the | after ovirt-engine is interpreted as end of column

machacekondra · 2017-10-27T08:13:26Z

roles/ovirt-engine-setup/tasks/main.yml

+  delay: 10
+  until: health_page|success
+
+- name: clean tmp files


I would make sure this is always removed to not store any password in plain in case of failure.

i will change it to block and add the always section

machacekondra · 2017-10-27T08:13:47Z

roles/ovirt-engine-setup/tasks/main.yml

+  template:
+    src: answerfile_{{ ovirt_engine_version }}_basic.txt.j2
+    dest: /tmp/answerfile.txt
+    mode: 0644


Any reason this is world readable if there are stored passwords in plain text?

no idea.
but the engine setup should read this file. so it should be plain text and readable. ...
do u suggest to make it 0600 ?

yes, please make it 0600 and set no_log to true, as I think it's printed to stdout as well

What is the credentials of the user running this Ansible script? You do NOT put files with predicted name under /tmp (especially if you are root!).

machacekondra · 2017-10-27T08:21:39Z

roles/ovirt-engine-setup/templates/answerfile_3.6_basic.txt.j2

+OVESETUP_CONFIG/applicationMode=str:both
+OVESETUP_CONFIG/remoteEngineSetupStyle=none:None
+OVESETUP_CONFIG/sanWipeAfterDelete=bool:False
+OVESETUP_CONFIG/storageIsLocal=bool:False


Do we need to have all of the options listed here? Can we just have those which can be changed and run with accept-defaults ? Or is there any issue with that?

i think its there because it is needed..

I just wonder what will happen, if there will be introduced new option in zstream, it can stop to work, so I think the accept defaults is better way, but I don't have deep knowledge of this. Just asking.

I tend to agree with accept default for oVirt side, as the playbook accomodates to it autoamtically.
I tend to disagree with accept default for oVirt QA side, as we would need to track changes manually., as we want to know what are we testing and new options we might miss new options.

I think you should create one common file and use include jinja statement, better than copy-pasting. And change only things which are relevant for the verison, this is unreadeble.

Hmm, I think it's better to have one answer file per version as variable might be renamed between versions and no backward compatibility is ensured, so having only 1 answer could complicate things significantly.

But I think, we should provide a way how to use --accept-defaults and override only variables that user selects (for example disable OVN, but leave the rest on defaults). But this can be done in separate RFE

I created a basic answer file and already tested on 4.1 and 4.2 and I don't see issues. I think it's good idea and definitely makes it much easier to read.

'--accept-defaults' already behaves like that - it obeys all var/val in the answer file, and "accepts the default" only if not supplied.

machacekondra · 2017-10-27T08:22:05Z

cc @mwperina

machacekondra · 2017-10-27T08:54:09Z

roles/ovirt-engine-setup/README.md

+    ovirt_engine_type: 'ovirt-engine'
+    ovirt_engine_version: '4.1'
+    ovirt_engine_organization: 'of.ovirt.engine.com'
+    ovirt_engine_admin_password: 'secret'


In the example please follow the best practise with password using ansible vault

Can you please use the vault?

machacekondra · 2017-10-27T08:55:42Z

roles/ovirt-engine-setup/README.md

+- hosts: engine
+  vars:
+    ovirt_engine_type: 'ovirt-engine'
+    ovirt_engine_version: '4.1'


I just wonder why installer need to have version variable, any reason to not autodetect it?

it is just to load the answer file.
I would like to assume that whoever runs this actually knows what he is doing and what version is he installing.

Since we decided to install engine packages in this role as well, we don't really have way to know engine version.
So, I guess this variable will be needed.

machacekondra · 2017-10-27T08:56:08Z

roles/ovirt-engine-setup/README.md

+    ovirt_engine_type: 'ovirt-engine'
+    ovirt_engine_version: '4.1'
+    ovirt_rpm_repo: 'http://resources.ovirt.org/pub/yum-repo/ovirt-release41.rpm'
+    ovirt_engine_organization: 'dwhmanualenginetest.ovirt.org'


organization is twice here

machacekondra · 2017-10-27T08:56:59Z

roles/ovirt-engine-setup/defaults/main.yml

+ovirt_engine_dwh: True
+ovirt_engine_type: 'ovirt-engine'
+ovirt_engine_version: '4.1'
+ovirt_engine_admin_password: '123456'


please remove

machacekondra · 2017-10-27T08:57:23Z

roles/ovirt-engine-setup/defaults/main.yml

+ovirt_engine_admin_password: '123456'
+
+ovirt_provider_ovn_username: 'admin@internal'
+ovirt_provider_ovn_password: '{{ ovirt_engine_admin_password }}'


this as well

machacekondra · 2017-10-27T08:57:35Z

roles/ovirt-engine-setup/defaults/main.yml

+ovirt_engine_db_port: 5432
+ovirt_engine_db_name: 'engine'
+ovirt_engine_db_user: 'engine'
+ovirt_engine_db_password: 'AqbXg4dpkbcVRZwPbY8WOR'


dont put db password to asnwer files, we want them to be generated by engine

please remove

StLuke · 2017-11-01T13:03:54Z

roles/ovirt-engine-setup/meta/main.yml

@@ -0,0 +1,23 @@
+---
+galaxy_info:
+  author: Tareq Alayan


I would add Petr Kubica - original developer and maintainer of these

StLuke · 2017-11-01T13:05:16Z

roles/ovirt-engine-setup/meta/main.yml

+  description: Role to run engine-setup.
+  company: Red Hat, Inc.
+
+  license: Apache License 2.0


Original playbooks were published under GPL, ovirt is published under Apache License 2.0, we might want to check what is the best approach on licensing here.

what should it be?

This role will enable you to deploy ovirt-engine with answere file.

ovirt-infra · 2017-11-20T09:36:53Z

Some tests failed

machacekondra · 2017-11-20T14:24:49Z

roles/ovirt-engine-setup/README.md

+
+| Name                            | Default value         |  Description                                              |
+|---------------------------------|-----------------------|-----------------------------------------------------------|
+| ovirt_engine_type               | ovirt-engine          | Type of product 'ovirt-engine' or 'rhevm'  |


should the product be 'rhvm' instead of 'rhevm'

should the product be 'rhv' instead of 'rhvm'

rhvm is fine, as this is the downstream package name for 4.2 (ie yum install rhvm)

I think this parameter can be removed. we know the version, we can create the matrix for package name per version and hide it from user.

Yes, but how do you know if user wants to install oVirt or RHV?

Hmm, you 're right. I 'll keep it

machacekondra · 2017-11-20T14:25:04Z

roles/ovirt-engine-setup/README.md

+| Name                            | Default value         |  Description                                              |
+|---------------------------------|-----------------------|-----------------------------------------------------------|
+| ovirt_engine_type               | ovirt-engine          | Type of product 'ovirt-engine' or 'rhevm'  |
+| ovirt_engine_version            | UNDEF                 | Allowed version: [3.6, 4.0, 4.1] |


4.2 as well

machacekondra · 2017-11-20T14:26:07Z

roles/ovirt-engine-setup/README.md

+| ovirt_engine_dwh                | UNDEF                 | Bool value for installing local DWH. |
+| ovirt_engine_answer_file_path   | UNDEF                 | /path/to/custom/answerfile. |
+| ovirt_engine_db_host            | localhost             | IP or hostname of PostgreSQL server for engine database. |
+| ovirt_engine_db_port            | 5432                  |  Server listening port. |


Database server port.

Port of a PostgreSQL server for Engine database

machacekondra · 2017-11-20T14:26:21Z

roles/ovirt-engine-setup/README.md

+| ovirt_engine_type               | ovirt-engine          | Type of product 'ovirt-engine' or 'rhevm'  |
+| ovirt_engine_version            | UNDEF                 | Allowed version: [3.6, 4.0, 4.1] |
+| ovirt_engine_dwh                | UNDEF                 | Bool value for installing local DWH. |
+| ovirt_engine_answer_file_path   | UNDEF                 | /path/to/custom/answerfile. |


Path to custom answerfile.

Path to custom answer file for engine-setup

machacekondra · 2017-11-20T14:27:04Z

roles/ovirt-engine-setup/README.md

+| ovirt_engine_answer_file_path   | UNDEF                 | /path/to/custom/answerfile. |
+| ovirt_engine_db_host            | localhost             | IP or hostname of PostgreSQL server for engine database. |
+| ovirt_engine_db_port            | 5432                  |  Server listening port. |
+| ovirt_engine_db_name            | engine                | DB name for ovirt-engine. |


Database name of ovirt-engine.

Not sure but I think of is better than for here.

Name of a database for Engine

machacekondra · 2017-11-20T14:27:27Z

roles/ovirt-engine-setup/README.md

+| ovirt_engine_db_host            | localhost             | IP or hostname of PostgreSQL server for engine database. |
+| ovirt_engine_db_port            | 5432                  |  Server listening port. |
+| ovirt_engine_db_name            | engine                | DB name for ovirt-engine. |
+| ovirt_engine_db_user            | engine                |  DB user which can access ovirt-engine DB. |


s/DB/Database

Name of a user to access Engine database

machacekondra · 2017-11-20T14:32:44Z

roles/ovirt-engine-setup/defaults/main.yml

+ovirt_engine_db_port: 5432
+ovirt_engine_db_name: 'engine'
+ovirt_engine_db_user: 'engine'
+ovirt_engine_db_password: 'AqbXg4dpkbcVRZwPbY8WOR'


please remove

machacekondra · 2017-11-20T14:35:01Z

roles/ovirt-engine-setup/tasks/main.yml

+    template:
+      src: answerfile_{{ ovirt_engine_version }}_basic.txt.j2
+      dest: /tmp/answerfile.txt
+      mode: 0644


I think there still could be password no? So again 0600 please

machacekondra · 2017-11-20T14:39:01Z

roles/ovirt-engine-setup/tasks/main.yml

+      - skip_ansible_lint
+
+  - name: run engine-setup with answerfile
+    shell: 'engine-setup --config-append=/tmp/answerfile.txt'


What is the reason to use shell, instead of command? It is a best practise to use command unless shell is a must to use.

Not sure it's a valid reason, but you might want to pass certain shell env vars, e.g. 'OTOPI_DEBUG=1'. Not sure ansible's shell modules allows to do this without changing the play either.

machacekondra · 2017-11-20T14:40:31Z

roles/ovirt-engine-setup/tasks/main.yml

+  - name: restart of ovirt-engine service
+    service:
+      name: ovirt-engine
+      state: restarted


Hmm, if you restart it here, then the started state before is useless, no?

Why do we restart ovirt-engine there? The service is (re)started during engine-setup, no?

machacekondra · 2017-11-20T14:45:10Z

roles/ovirt-engine-setup/templates/answerfile_3.6_basic.txt.j2

+OVESETUP_CONFIG/applicationMode=str:both
+OVESETUP_CONFIG/remoteEngineSetupStyle=none:None
+OVESETUP_CONFIG/sanWipeAfterDelete=bool:False
+OVESETUP_CONFIG/storageIsLocal=bool:False


I think you should create one common file and use include jinja statement, better than copy-pasting. And change only things which are relevant for the verison, this is unreadeble.

KKoukiou · 2017-12-08T13:01:21Z

@machacekondra I 'll adjust all comments and create PR for the separate repository https://github.com/oVirt/ovirt-ansible-engine-setup.
I think we can close this one here.

mwperina · 2017-12-08T12:38:39Z

roles/ovirt-engine-setup/README.md

+
+| Name                            | Default value         |  Description                                              |
+|---------------------------------|-----------------------|-----------------------------------------------------------|
+| ovirt_engine_type               | ovirt-engine          | Type of product 'ovirt-engine' or 'rhevm'  |


rhvm is fine, as this is the downstream package name for 4.2 (ie yum install rhvm)

mwperina · 2017-12-08T12:40:34Z

roles/ovirt-engine-setup/README.md

+| ovirt_engine_answer_file_path   | UNDEF                 | /path/to/custom/answerfile. |
+| ovirt_engine_db_host            | localhost             | IP or hostname of PostgreSQL server for engine database. |
+| ovirt_engine_db_port            | 5432                  |  Server listening port. |
+| ovirt_engine_db_name            | engine                | DB name for ovirt-engine. |


Name of a database for Engine

mwperina · 2017-12-08T12:42:06Z

roles/ovirt-engine-setup/README.md

+| ovirt_engine_dwh                | UNDEF                 | Bool value for installing local DWH. |
+| ovirt_engine_answer_file_path   | UNDEF                 | /path/to/custom/answerfile. |
+| ovirt_engine_db_host            | localhost             | IP or hostname of PostgreSQL server for engine database. |
+| ovirt_engine_db_port            | 5432                  |  Server listening port. |


Port of a PostgreSQL server for Engine database

mwperina · 2017-12-08T12:43:22Z

roles/ovirt-engine-setup/README.md

+| ovirt_engine_db_host            | localhost             | IP or hostname of PostgreSQL server for engine database. |
+| ovirt_engine_db_port            | 5432                  |  Server listening port. |
+| ovirt_engine_db_name            | engine                | DB name for ovirt-engine. |
+| ovirt_engine_db_user            | engine                |  DB user which can access ovirt-engine DB. |


Name of a user to access Engine database

mwperina · 2017-12-08T12:45:00Z

roles/ovirt-engine-setup/README.md

+| ovirt_engine_db_port            | 5432                  |  Server listening port. |
+| ovirt_engine_db_name            | engine                | DB name for ovirt-engine. |
+| ovirt_engine_db_user            | engine                |  DB user which can access ovirt-engine DB. |
+| ovirt_engine_db_password        | UNDEF                 | password for user of ovirt-engine DB. |


Password of a user to access Engine database

mwperina · 2017-12-08T13:12:34Z

roles/ovirt-engine-setup/tasks/main.yml

+      group: root
+    when: ovirt_engine_answer_file_path is defined
+
+  - name: update setup packages


Update setup packages

mwperina · 2017-12-08T13:15:14Z

roles/ovirt-engine-setup/tasks/main.yml

+  - name: restart of ovirt-engine service
+    service:
+      name: ovirt-engine
+      state: restarted


Why do we restart ovirt-engine there? The service is (re)started during engine-setup, no?

mwperina · 2017-12-08T13:17:47Z

roles/ovirt-engine-setup/tasks/main.yml

+      name: ovirt-engine
+      state: restarted
+
+  - name: check health status of page


Check if Engine is running

mwperina · 2017-12-08T13:23:45Z

roles/ovirt-engine-setup/templates/answerfile_3.6_basic.txt.j2

+OVESETUP_CONFIG/applicationMode=str:both
+OVESETUP_CONFIG/remoteEngineSetupStyle=none:None
+OVESETUP_CONFIG/sanWipeAfterDelete=bool:False
+OVESETUP_CONFIG/storageIsLocal=bool:False


Hmm, I think it's better to have one answer file per version as variable might be renamed between versions and no backward compatibility is ensured, so having only 1 answer could complicate things significantly.

But I think, we should provide a way how to use --accept-defaults and override only variables that user selects (for example disable OVN, but leave the rest on defaults). But this can be done in separate RFE

mwperina · 2017-12-08T13:25:53Z

roles/ovirt-engine-setup/templates/answerfile_4.1_upgrade.txt.j2

@@ -0,0 +1,72 @@
+# action=setup


Why do we need a special templates for new installation and upgrades?

Because for upgrading you have some extra questions when running the engine setup.

OK, makes sense

mwperina · 2017-12-08T13:27:35Z

Sandro/Didi could you please also take a look?
@sandrobonazzola @didib

KKoukiou · 2017-12-08T13:52:00Z

@mwperina I will check your comments from here and adjust but please let's continue the review once I repost in the new repository as I said above.
https://github.com/oVirt/ovirt-ansible-engine-setup.
And I think we can close this PR here.

didib · 2017-12-10T15:13:07Z

Didn't review the code, only replied to some comments. Feel free to add me as a reviewer on the new patch once it's ready.

machacekondra · 2017-12-11T09:24:52Z

Closing this PR, as it was moved to : oVirt/ovirt-ansible-engine-setup#1

machacekondra reviewed Oct 27, 2017

View reviewed changes

StLuke suggested changes Nov 1, 2017

View reviewed changes

introducing ovirt-engine-setup role

20f0f15

This role will enable you to deploy ovirt-engine with answere file.

tareqalayan force-pushed the ovirt-deployment-role branch from bcd443e to 20f0f15 Compare November 20, 2017 09:26

machacekondra reviewed Nov 20, 2017

View reviewed changes

mwperina suggested changes Dec 8, 2017

View reviewed changes

machacekondra closed this Dec 11, 2017

introducing ovirt-engine-setup role #123

introducing ovirt-engine-setup role #123

Conversation

tareqalayan commented Oct 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machacekondra Oct 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machacekondra commented Oct 27, 2017

machacekondra Oct 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ovirt-infra commented Nov 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KKoukiou commented Dec 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machacekondra Oct 31, 2017 •

edited

Loading

machacekondra Oct 27, 2017 •

edited

Loading