immutable playbook #82

mbukatov · 2018-03-04T15:09:55Z

This change default site.yml.example playbook file to be usable without changes. All ansible variables are now suggested to be placed in the inventory file (or variable file).

This is work in progress, waiting for feedback.

tendrl-bug-id: #74

mbukatov · 2018-03-04T15:11:19Z

@julienlim could you check if the new readme file makes sense to you?

https://github.com/mbukatov/tendrl-ansible/blob/031aa50450f3ee484e0218da54733292ee5f0559/README.md

nthomas-redhat · 2018-03-05T07:58:49Z

site.yml.sample

-# Run yum update.
-#
-
- hosts: all


Is there a specific reason why we are doing this?

Previously, we expected that admin will edit the main playbook file so that he will notice what's going on there, commenting out or tweaking stuff to fit his needs. But this approach didn't work. So now, we no longer expect admin to look into the file and tweak it - which is what this pull request is about, and removal of "run yum update" task was done here as it's not obvious we are doing it in the playbook and some people may not want to run full system update during Tendrl installation.

nthomas-redhat · 2018-03-05T08:09:18Z

README.md

-
-## Setup with Vagrant using virtualbox provider
-
-TODO


I think its a good idea to talk about vaiables like:
tendrl_copr_repo
etcd auth related(etcd_tls_client_auth, etcd_cert_file etc)
The respective README files now, but atleast linking it to this file will be really helpful

I could mention these variables in a section where I talk about optional variables. Right now it states:

Add optional ansible variables into the invenory file.

Based on Tendrl documentation and description in README files of
tendrl-ansible roles, specify values for variables you like to tweak.

This is important because some features tendrl-ansible can help you
with are disabled by default as they require additional user input.

This includes etcd tls client authentication and tendrl notifier
configuration for snmp or smtp. Other features such as firewalld setup
for Tendrl can be disabled if needed.

Would your suggestion be addressed by mentioning the variables ( tendrl_copr_repo, etcd_tls_client_auth, ...) in this section?

@nthomas-redhat Does 752319f address your suggestion?

mbukatov · 2018-03-05T12:57:37Z

Based from @fbalak , I'm going to add a section related to expand cluster.
Update: added in commit 14228ae

dahorak · 2018-03-05T13:18:24Z

README.md

+* `tendrl-server`: installation of **Tendrl Server** machine (where Tendrl api,
+ web and etcd are running)
+* `tendrl-storage-node`: installation of **Tendrl Storage Node** machines
+ (Gluster servers, which you would like to monitor by Tendrl)


When installed form rpm, the roles contain prefix tendrl-ansible., is it ok to reference them here in README file without the prefix? (If I didn't miss something, this README is not updated during package creation.)

For the entry about "tendrl-storage-node: installation of Tendrl Storage Node machines (Gluster servers, which you would like to monitor by Tendrl)..."

I might suggest some mention that the tendrl-storage node is the tendrl agent (aka node agent) as it's really an agent, and the "tendrl-storage-node" is may be confusing to the user.

@dahorak yeah, good point - I need to unify the role names for both package and source repo
update: hopefully unified in 828f55b

dahorak · 2018-03-05T13:25:20Z

README.md

+as well.
+
+And last but not least, both `tendrl-server` and `tendrl-storage-node` contains
+many variables which one can use to tweak the installation. See README files of


To be clear, I would add ...both tendrl-server and tendrl-storage-node roles contains...

Fixed in 818ac41

dahorak · 2018-03-05T13:26:46Z

README.md

+
+## What installation steps from Tendrl installation documentation are not part of tendrl-ansible?
+
+This is should be clear from [Tendrl installation


Probably spare is... in the This is should be clear... sentence.

fixed in a3091aa

dahorak · 2018-03-05T13:28:52Z

README.md

+* Installation and configuration of GlusterFS on the machines, see
+ [gdeploy](https://gdeploy.readthedocs.io/en/latest/) for automation of this
+ task.
+* Setup of dedicated disk for etcd and graphite data directories.


If it would be possible, it would be nice to have link to the particular requirement here.

I would rather do as few such links as possible. The guide is a moving target and linking into it may break soon. The whole guide is linked in the beginning of the section.

dahorak

I would suggest just to fix few small typos and similar issues.

dahorak · 2018-03-05T13:36:59Z

README.md

+ * `etcd_fqdn` configure tendrl components to be able to connect to etcd
+ instance
+ * `graphite_fqdn` configures tendrl components (this value doesn't
+ reconfigure graphite itself!) to be able to connect to graphite instance


I would put the note in brackets at the end of the description:

* `graphite_fqdn` configures tendrl components to be able to connect to graphite instance (this value doesn't reconfigure graphite itself!)

fixed in b4dfed1

dahorak · 2018-03-05T13:40:09Z

README.md

+ for Tendrl can be disabled if needed.
+
+5) If you use tendrl-ansible from rpm package, copy `site.yml` playbook into
+ local directory (where you already store the inventory file):


I think, that working directory would be slightly better than local directory.

ok, fixed in a2646c0

dahorak · 2018-03-05T13:44:05Z

README.md

+ $ ansible-playbook -i inventory_file site.yml
+ ```
+
+ Assuming we have deployed ssh keys on the machines and have a cluster


The note about ssh keys is already mentioned in the step 6, so I'm not sure if it is required here?
And cluster already installed means Gluster cluster?

Repetition in this context is not a bad thing, but good point about gluster, fix: c48bc68

dahorak · 2018-03-05T13:47:27Z

README.md

+ Assuming we have deployed ssh keys on the machines and have a cluster
+ already installed and running there.
+
+9) Log in to your tendrl server at ``http://ip.of.tendrl.server`` with


I would use http://tendrl.example.com instead of http://ip.of.tendrl.server (maybe with some note describing what it is - pointing back to the example inventory file). I think it is not necessary to use IP address (instead of hostname).

good point, fixed in: 1c9d195

julienlim · 2018-03-05T15:21:25Z

README.md

+ * `etcd_fqdn` configure tendrl components to be able to connect to etcd
+ instance
+ * `graphite_fqdn` configures tendrl components (this value doesn't
+ reconfigure graphite itself!) to be able to connect to graphite instance


[all:vars]

This is awesome to now have this in the inventory file as opposed to editing them in the site.yml file!

julienlim

@mbukatov I've reviewed and made some comments directly.

I've also another question/comment:

(1) Will you provide some guidance how someone can run the playbook as non-root?

I like the changes I read about, i.e. adding all:vars into the inventory file to make it easier to use the playbooks, support for production vs. non-production environments, instructions to run the precheck.yml, etc.

Nice work!

julienlim · 2018-03-05T15:24:28Z

README.md

+ for Tendrl can be disabled if needed.
+
+5) If you use tendrl-ansible from rpm package, copy `site.yml` playbook into
+ local directory (where you already store the inventory file):


"4) Add optional ansible variables into the invenory file."

Can you provide specific examples for the optional ansible variables into the invenory (typo... should be inventory) file" i.e. tls client authentication and tendrl notified for SNMP or SMTP, firewalld?

You mean something like this?

This includes etcd tls client authentication (etcd_tls_client_auth and other variables), tendrl notifier configuration for snmp or smtp (tendrl_notifier_email_id and other variables), and other tweaks (eg. tendrl_copr_repo variable of tendrl-copr role).

@julienlim Does 752319f address your suggestion?

@mbukatov That works. Thanks.

Addressing feedback from Nishanth and Ju.

Expand cluster section added based on suggestion from fbalak. The procedure described in this commit is based on ongoing specification for cluster expansion: Tendrl/specifications#254 That said, the expand procedure is not fully finished at the time of writing this commit, see also: Tendrl/commons#849

mbukatov · 2018-03-06T14:55:03Z

@fbalak @r0h4n I have added a brief description of expand into the readme file, see commit 14228ae

fbalak · 2018-03-06T16:09:36Z

README.md

+ machines you just added.
+
+4) Now, you should be able to see new servers in Tendrl web ui (see Tendrl
+ documentation for details).


I would add a mention about a notification that should be present after expand is finished: https://github.com/Tendrl/specifications/pull/254/files#diff-8e71bd954ebddb4a0ef4fd3c413d9f04R90

I will add there more details, but when we have this resolved Tendrl/commons#849

This is needed to have the same role name for both rpm packaged tendrl-ansible and the version straight from the git repository.

mbukatov · 2018-03-07T18:32:47Z

(1) Will you provide some guidance how someone can run the playbook as non-root?

@julienlim I have added an example for this in 5f53a29

mbukatov added 4 commits March 1, 2018 23:16

drop system update from site playbook

12d5cc9

remove all defaults and docs from main playbook

464edec

rename main playbook

72a1ca8

README unification, extension and cleanup

031aa50

mbukatov self-assigned this Mar 4, 2018

mbukatov requested review from r0h4n, dahorak, julienlim and nthomas-redhat March 4, 2018 15:09

mbukatov mentioned this pull request Mar 4, 2018

proposal: change default site.yml.example file to be usable without editing it #74

Closed

update ansible req. in spec file

ce30f4d

nthomas-redhat requested changes Mar 5, 2018

View reviewed changes

mbukatov added this to the Milestone 3 (2018) milestone Mar 5, 2018

dahorak reviewed Mar 5, 2018

View reviewed changes

julienlim reviewed Mar 5, 2018

View reviewed changes

readme: add examples of variables

752319f

Addressing feedback from Nishanth and Ju.

mbukatov mentioned this pull request Mar 6, 2018

Added specification for expansion of cluster in tendrl post detection Tendrl/specifications#254

Open

mbukatov added 7 commits March 6, 2018 16:10

fix writing style

818ac41

fyx typo

a3091aa

fix writing style

b4dfed1

fix writing style

a2646c0

little clarification

c48bc68

clarification

1c9d195

fixing typos

c775d4b

fbalak reviewed Mar 6, 2018

View reviewed changes

mbukatov added 2 commits March 6, 2018 18:10

add prefix to all tendrl-ansbile roles

828f55b

This is needed to have the same role name for both rpm packaged tendrl-ansible and the version straight from the git repository.

explain how to use ansible as non-root remote user

5f53a29

mention grafana password lookup file

70b726c

mbukatov changed the title ~~wip: immutable playbook~~ immutable playbook Mar 7, 2018

polishing examples

216220f

mbukatov merged commit 216220f into Tendrl:master Mar 8, 2018

mbukatov deleted the issue_74_immutable_playbook branch March 8, 2018 00:09


		## What installation steps from Tendrl installation documentation are not part of tendrl-ansible?

		This is should be clear from [Tendrl installation

immutable playbook #82

immutable playbook #82

Conversation

mbukatov commented Mar 4, 2018

mbukatov commented Mar 4, 2018 • 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

mbukatov commented Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbukatov Mar 6, 2018 • 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

mbukatov Mar 6, 2018 • edited Loading

Choose a reason for hiding this comment

dahorak left a 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

julienlim Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

julienlim left a 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

mbukatov commented Mar 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbukatov commented Mar 7, 2018

mbukatov commented Mar 4, 2018 •

edited

Loading

mbukatov commented Mar 5, 2018 •

edited

Loading

mbukatov Mar 6, 2018 •

edited

Loading

mbukatov Mar 6, 2018 •

edited

Loading

julienlim Mar 5, 2018 •

edited

Loading