Bug 1658649

In the original file, there was:

[glusterfs]
gluster_node_1.example.com glusterfs_devices='[ "/gluster_node_1/gluster_mount/" ]'
gluster_node_2.example.com glusterfs_devices='[ "/gluster_node_2/gluster_mount/" ]'
gluster_node_3.example.com glusterfs_devices='[ "/gluster_node_3/gluster_mount/" ]'

Should this still be included or should I take it out?

This is as it currently stands under # Node:

# Node
[masters]
master.example.com

[etcd]
master.example.com

[glusterfs]
gluster_node_1.example.com glusterfs_devices='[ "/gluster_node_1/gluster_mount/" ]'
gluster_node_2.example.com glusterfs_devices='[ "/gluster_node_2/gluster_mount/" ]'
gluster_node_3.example.com glusterfs_devices='[ "/gluster_node_3/gluster_mount/" ]'

[nodes]
master.example.com openshift_node_group_name='node-config-master-infra-crio'
node1.example.com openshift_node_group_name='node-config-compute-crio'
node2.example.com openshift_node_group_name='node-config-compute-crio'
----

In addition to my question about [glusterfs], should [etcd] be included?

yes please keep them,
i didnt mention them because i wanted to highlight the changes

Sending the updated version now - please review and I will put it into the repo! Thank you :)



[[example-inventory-file]]
=== {product-title} example inventory file

The file below is an example Ansible inventory file for an {product-title} 
cluster suitable for {ProductName}. In this scenario, the master node is also 
an infra node, and there are two separate compute nodes. It uses a 
three-node OpenShift Container Storage `[glusterfs]` cluster for storage 
backend.

----
[OSEv3:children]
masters
nodes
etcd
glusterfs

[OSEv3:vars]
openshift_deployment_type=openshift-enterprise
ansible_ssh_user=root
openshift_clock_enabled=true
ansible_service_broker_registry_whitelist=['.*-apb$']
ansible_service_broker_local_registry_whitelist=['.*-apb$']

# Enable admission controller webhooks
openshift_master_admission_plugin_config={"ValidatingAdmissionWebhook":{"configuration":{"kind": "DefaultAdmissionConfig","apiVersion": "v1","disable": false}},"MutatingAdmissionWebhook":{"configuration":{"kind": "DefaultAdmissionConfig","apiVersion": "v1","disable": false}}}

# CRI-O
openshift_use_crio=true

# Provide your credentials to consume the redhat.io registry
oreg_auth_user=$rhnuser
oreg_auth_password='$rhnpassword'

# Node
[masters]
master.example.com

[etcd]
master.example.com

[glusterfs]
gluster_node_1.example.com glusterfs_devices='[ "/gluster_node_1/gluster_mount/" ]'
gluster_node_2.example.com glusterfs_devices='[ "/gluster_node_2/gluster_mount/" ]'
gluster_node_3.example.com glusterfs_devices='[ "/gluster_node_3/gluster_mount/" ]'

[nodes]
master.example.com openshift_node_group_name='node-config-master-infra-crio'
node1.example.com openshift_node_group_name='node-config-compute-crio'
node2.example.com openshift_node_group_name='node-config-compute-crio'
----

comments inline
@Lukas, would you mind also taking a look to make sure i didnt miss something?

(In reply to Pan Ousley from comment #4)
> Sending the updated version now - please review and I will put it into the
> repo! Thank you :)
> 
> 
> 
> [[example-inventory-file]]
> === {product-title} example inventory file
> 
> The file below is an example Ansible inventory file for an {product-title} 
> cluster suitable for {ProductName}. In this scenario, the master node is
> also 
> an infra node, and there are two separate compute nodes. It uses a 
> three-node OpenShift Container Storage `[glusterfs]` cluster for storage 
> backend.
> 
> ----
> [OSEv3:children]
> masters
> nodes
> etcd
> glusterfs
> 
> [OSEv3:vars]
> openshift_deployment_type=openshift-enterprise
> ansible_ssh_user=root
> openshift_clock_enabled=true

isnt it the default value?

> ansible_service_broker_registry_whitelist=['.*-apb$']
> ansible_service_broker_local_registry_whitelist=['.*-apb$']
> 
> # Enable admission controller webhooks
> openshift_master_admission_plugin_config={"ValidatingAdmissionWebhook":
> {"configuration":{"kind": "DefaultAdmissionConfig","apiVersion":
> "v1","disable": false}},"MutatingAdmissionWebhook":{"configuration":{"kind":
> "DefaultAdmissionConfig","apiVersion": "v1","disable": false}}}
> 
> # CRI-O
> openshift_use_crio=true
> 
> # Provide your credentials to consume the redhat.io registry
> oreg_auth_user=$rhnuser
> oreg_auth_password='$rhnpassword'

missing oreg_url?

> 
> # Node
i would either remove this comment or rename to '# host groups'


> [masters]
> master.example.com
> 
> [etcd]
> master.example.com
> 
> [glusterfs]
> gluster_node_1.example.com glusterfs_devices='[
> "/gluster_node_1/gluster_mount/" ]'
> gluster_node_2.example.com glusterfs_devices='[
> "/gluster_node_2/gluster_mount/" ]'
> gluster_node_3.example.com glusterfs_devices='[
> "/gluster_node_3/gluster_mount/" ]'
> 
> [nodes]
> master.example.com openshift_node_group_name='node-config-master-infra-crio'
> node1.example.com openshift_node_group_name='node-config-compute-crio'
> node2.example.com openshift_node_group_name='node-config-compute-crio'
> ----

(In reply to Nelly Credi from comment #5)
> 
> comments inline
> @Lukas, would you mind also taking a look to make sure i didnt miss
> something?
> 
> (In reply to Pan Ousley from comment #4)
> > Sending the updated version now - please review and I will put it into the
> > repo! Thank you :)
> > 
> > 
> > 
> > [[example-inventory-file]]
> > === {product-title} example inventory file
> > 
> > The file below is an example Ansible inventory file for an {product-title} 
> > cluster suitable for {ProductName}. In this scenario, the master node is
> > also 
> > an infra node, and there are two separate compute nodes. It uses a 
> > three-node OpenShift Container Storage `[glusterfs]` cluster for storage 
> > backend.
> > 
> > ----
> > [OSEv3:children]
> > masters
> > nodes
> > etcd
> > glusterfs
> > 
> > [OSEv3:vars]
> > openshift_deployment_type=openshift-enterprise
> > ansible_ssh_user=root
> > openshift_clock_enabled=true
> 
> isnt it the default value?


Yes, it is. Removing openshift_clock_enabled=true


> 
> > ansible_service_broker_registry_whitelist=['.*-apb$']
> > ansible_service_broker_local_registry_whitelist=['.*-apb$']
> > 
> > # Enable admission controller webhooks
> > openshift_master_admission_plugin_config={"ValidatingAdmissionWebhook":
> > {"configuration":{"kind": "DefaultAdmissionConfig","apiVersion":
> > "v1","disable": false}},"MutatingAdmissionWebhook":{"configuration":{"kind":
> > "DefaultAdmissionConfig","apiVersion": "v1","disable": false}}}
> > 
> > # CRI-O
> > openshift_use_crio=true
> > 
> > # Provide your credentials to consume the redhat.io registry
> > oreg_auth_user=$rhnuser
> > oreg_auth_password='$rhnpassword'
> 
> missing oreg_url?


oreg_url is only used when the customer is not using the default registry at registry.redhat.io -- should we include it?


> 
> > 
> > # Node
> i would either remove this comment or rename to '# host groups'


Renamed to "# Host groups"


> 
> 
> > [masters]
> > master.example.com
> > 
> > [etcd]
> > master.example.com
> > 
> > [glusterfs]
> > gluster_node_1.example.com glusterfs_devices='[
> > "/gluster_node_1/gluster_mount/" ]'
> > gluster_node_2.example.com glusterfs_devices='[
> > "/gluster_node_2/gluster_mount/" ]'
> > gluster_node_3.example.com glusterfs_devices='[
> > "/gluster_node_3/gluster_mount/" ]'
> > 
> > [nodes]
> > master.example.com openshift_node_group_name='node-config-master-infra-crio'
> > node1.example.com openshift_node_group_name='node-config-compute-crio'
> > node2.example.com openshift_node_group_name='node-config-compute-crio'
> > ----

The latest version is in the repo and can be viewed here: https://cnv_setup--ocpdocs.netlify.com/openshift-enterprise/latest/cnv_install/cnv_install.html#example-inventory-file

Lukas/Nelly: I will make additional changes if necessary, just let me know! Thanks!

I was waiting to hear back from Federico/Steve, but I decided to make the change in the interest of time. If it needs to be changed again, that's okay too. 

Here is the commit: https://github.com/openshift/openshift-docs/pull/12518/commits/fdbcb88fd0547059dc568d1c9394a1cbd8b02102
When the preview updates it will be viewable here: https://cnv_setup--ocpdocs.netlify.com/openshift-enterprise/latest/cnv_install/cnv_install.html#example-inventory-file

Nelly, can you please review the change? Thanks!

LGTM

OK for the gluster nodes on master+compute.

But few other comments:

1. am I the only one seeing a special character at the beginning of the line "[OSEv3:children]"?
2. node2 was used in the past as infra node but I see that it's currently marked as: "node-config-compute-crio", should it be "node-config-infra-crio" ?
3. the glusterfs_devices are in fact devices (not mounts) so the current "/gluster_node_1/gluster_mount/" is confusing, it should be something along the lines of openshift-ansible examples (e.g. /dev/sdb)
4. we should add a disclaimer that this inventory is provided just for PoC. For real clusters (I would have said "production", but we're TP anyway) it should be taken just for guidelines of what's required (e.g. Webhooks, etc.)

1. just retype this line
2. not an issue, master is also infra
3. we decided to remove gluster from the inventory
4. pan to put something

moving back to assigned

(In reply to Nelly Credi from comment #12)
> 1. just retype this line

Done, please check to make sure the special character is gone. I don't see it, though.

> 2. not an issue, master is also infra

Ack.

> 3. we decided to remove gluster from the inventory

Removed the [glusterfs] stanza. Nelly, am I also supposed to remove "glusterfs" from underneath [OSEv3:children]?

> 4. pan to put something

I rewrote the text at the top of the inventory file to emphasize that this is an example, and not a file meant to be used as-is. Re: not using the inventory file in production, the Tech Preview warnings that are already in the document should suffice. If we disagree on this point I can probably put an extra warning in, but let me know what you think of the current text. Thanks!

> 
> moving back to assigned

Moving to Modified.

(In reply to Pan Ousley from comment #13)
> (In reply to Nelly Credi from comment #12)
> > 1. just retype this line
> 
> Done, please check to make sure the special character is gone. I don't see
> it, though.

looks good now

> 
> > 2. not an issue, master is also infra
> 
> Ack.
> 
> > 3. we decided to remove gluster from the inventory
> 
> Removed the [glusterfs] stanza. Nelly, am I also supposed to remove
> "glusterfs" from underneath [OSEv3:children]?


yes

> 
> > 4. pan to put something
> 
> I rewrote the text at the top of the inventory file to emphasize that this
> is an example, and not a file meant to be used as-is. Re: not using the
> inventory file in production, the Tech Preview warnings that are already in
> the document should suffice. If we disagree on this point I can probably put
> an extra warning in, but let me know what you think of the current text.
> Thanks!
> 
> > 
> > moving back to assigned
> 
> Moving to Modified.

(In reply to Nelly Credi from comment #14)
> (In reply to Pan Ousley from comment #13)
> > (In reply to Nelly Credi from comment #12)
> > > 1. just retype this line
> > 
> > Done, please check to make sure the special character is gone. I don't see
> > it, though.
> 
> looks good now
> 
> > 
> > > 2. not an issue, master is also infra
> > 
> > Ack.
> > 
> > > 3. we decided to remove gluster from the inventory
> > 
> > Removed the [glusterfs] stanza. Nelly, am I also supposed to remove
> > "glusterfs" from underneath [OSEv3:children]?
> 
> 
> yes
> 


Removed it in this commit: https://github.com/openshift/openshift-docs/pull/12518/commits/b579aa95bf9b0583696652f140c393c2cb3debe1
View the current version here: https://cnv_setup--ocpdocs.netlify.com/openshift-enterprise/latest/cnv_install/cnv_install.html#example-inventory-file

@Federico, please let me know if the revised text above the inventory file is sufficient or if you think another warning is needed (see my comment 13). Thanks!

Thanks!

(In reply to Pan Ousley from comment #15)
> (In reply to Nelly Credi from comment #14)
> > (In reply to Pan Ousley from comment #13)
> > > (In reply to Nelly Credi from comment #12)
> > > > 1. just retype this line
> > > 
> > > Done, please check to make sure the special character is gone. I don't see
> > > it, though.
> > 
> > looks good now
> > 
> > > 
> > > > 2. not an issue, master is also infra
> > > 
> > > Ack.
> > > 
> > > > 3. we decided to remove gluster from the inventory
> > > 
> > > Removed the [glusterfs] stanza. Nelly, am I also supposed to remove
> > > "glusterfs" from underneath [OSEv3:children]?
> > 
> > 
> > yes
> > 
> 
> 
> Removed it in this commit:
> https://github.com/openshift/openshift-docs/pull/12518/commits/
> b579aa95bf9b0583696652f140c393c2cb3debe1

Thanks for pointing out diffs! This is excellent way to track what has changed since last review!


> View the current version here:
> https://cnv_setup--ocpdocs.netlify.com/openshift-enterprise/latest/
> cnv_install/cnv_install.html#example-inventory-file
> 
> @Federico, please let me know if the revised text above the inventory file
> is sufficient or if you think another warning is needed (see my comment 13).
> Thanks!

LGTM!
I think we can move to ON_QA.

(In reply to Federico Simoncelli from comment #16)
> (In reply to Pan Ousley from comment #15)
> > (In reply to Nelly Credi from comment #14)
> > > (In reply to Pan Ousley from comment #13)
> > > > (In reply to Nelly Credi from comment #12)
> > > > > 1. just retype this line
> > > > 
> > > > Done, please check to make sure the special character is gone. I don't see
> > > > it, though.
> > > 
> > > looks good now
> > > 
> > > > 
> > > > > 2. not an issue, master is also infra
> > > > 
> > > > Ack.
> > > > 
> > > > > 3. we decided to remove gluster from the inventory
> > > > 
> > > > Removed the [glusterfs] stanza. Nelly, am I also supposed to remove
> > > > "glusterfs" from underneath [OSEv3:children]?
> > > 
> > > 
> > > yes
> > > 
> > 
> > 
> > Removed it in this commit:
> > https://github.com/openshift/openshift-docs/pull/12518/commits/
> > b579aa95bf9b0583696652f140c393c2cb3debe1
> 
> Thanks for pointing out diffs! This is excellent way to track what has
> changed since last review!

You're welcome, I'm glad it helps!

> 
> 
> > View the current version here:
> > https://cnv_setup--ocpdocs.netlify.com/openshift-enterprise/latest/
> > cnv_install/cnv_install.html#example-inventory-file
> > 
> > @Federico, please let me know if the revised text above the inventory file
> > is sufficient or if you think another warning is needed (see my comment 13).
> > Thanks!
> 
> LGTM!
> I think we can move to ON_QA.


Awesome! Moving to ON_QA now.

Checked. Looks good. Thanks.