adding ssh_port variable to inventory file

[thisdwhitley]

This is an attempt to address https://github.com/ansible/lightbulb/issues/292

Instead of setting ssh_port in lightbulb/tools/aws_lab_setup/inventory/global_vars/all.yml it can be assigned in the inventory.ini file.

It is used in more than one role so defining it at the inventory level seems appropriate.

[tima]

Those provisioner roles are just horrid. :(

I can see how adding it to inventory would work though I'd argue that "right thing" to do is set ssh_port: 22 in the common role's defaults (because that is more portable) and get rid of all the inline default handling ({{ ssh_port | default('22') }}) seen throughout the tasks.

[thisdwhitley]

The inline default handling ({{ ssh_port | default('22') }}) was actually added in PR https://github.com/ansible/lightbulb/pull/295 after I created issue https://github.com/ansible/lightbulb/issues/292 and this PR https://github.com/ansible/lightbulb/pull/293

So this PR my no longer be relevant.

---

FWIW pt.1: I am not sure I understand the use case where a non-standard ssh port is pertinent (this could simply be because I'm only familiar with MY use case) FWIW pt.2: I agree that, if it is indeed necessary, setting it in a default where it can be overridden easily, such as in the common role is ideal

[liquidat]

I do not really understand the initial concern: with global_vars/all.yml it already was defined in the inventory. The group vars are part of the inventory, their whole purpose is to not clutter the inventory.ini with variable entries. Or am I missing something here?

Anyway, since the ssh port is an information which depends on the actual machine we are talking to, this value should be defined in the inventory, as close to the machines as possible. Having said that, @dswhitley is right that setting alternative ssh ports is not that common these days, so I agree with @tima to set defaults in the roles - and only overwrite them when necessary.

So, @dswhitley , if you don't mind, how about we update this PR to remove the ssh ports entry in the group_vars as soon as the roles have all defaults set?

[thisdwhitley]

Sorry for the confusion. The initial concern was that variables set in global_vars/all.yml were not being honored. I'm not sure I understand why, but I think I tracked it down to changes introduced in https://github.com/ansible/lightbulb/commit/ec59cbaa96ace8135ed59f4936676e8ccf03aa0e

Since opening my issue and this PR, PR https://github.com/ansible/lightbulb/pull/295 addressed this in an alternative way.

I also suspect that all of this will become irrelevant when issue https://github.com/ansible/lightbulb/issues/196 is addressed. I see alternate provisioners in almost every fork (I have my own now too) so this particular PR can actually be closed (sorry to have let it linger).