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

Change default of TRANSFORM_INVALID_GROUP_CHARS from 'never' to 'ignore' #70908

Closed
geerlingguy opened this issue Jul 27, 2020 · 7 comments
Closed
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. cloud inventory Inventory category openstack python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@geerlingguy
Copy link
Contributor

geerlingguy commented Jul 27, 2020

SUMMARY

After the long and drawn out issue ansible/ansible-documentation#89 was mostly concluded by un-deprecating dashes in group names, I noticed that my Ansible installations are still spitting out warnings by default when using inventory plugins that build group names based on things like AWS regions.

[WARNING]: Invalid characters were found in group names but not replaced, use -vvvv to see details

I propose that the default for TRANSFORM_INVALID_GROUP_CHARS be changed from 'never' to 'ignore'.

It's also unclear to me what value there is in having a 'never' option, since it just does nothing of substance... and then spits out a warning? It seems like a scary message indicating something could break if I ignore it, but nothing does break, so I wonder what's the point of displaying the message?

ISSUE TYPE
  • Bug Report
COMPONENT NAME

lib/ansible/inventory/group.py

ANSIBLE VERSION
ansible 2.9.10
  config file = /Users/jgeerling/Development/GitHub/ansible-for-devops/lamp-infrastructure/ansible.cfg
  configured module search path = ['/Users/jgeerling/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.7/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.7.7 (default, Apr  9 2020, 17:13:06) [Clang 11.0.3 (clang-1103.0.32.29)]
CONFIGURATION
ANSIBLE_NOCOWS(/Users/jgeerling/Development/GitHub/ansible-for-devops/lamp-infrastructure/ansible.cfg) = True
DEFAULT_ROLES_PATH(/Users/jgeerling/Development/GitHub/ansible-for-devops/lamp-infrastructure/ansible.cfg) = ['/Users/jg
HOST_KEY_CHECKING(/Users/jgeerling/Development/GitHub/ansible-for-devops/lamp-infrastructure/ansible.cfg) = False
RETRY_FILES_ENABLED(/Users/jgeerling/Development/GitHub/ansible-for-devops/lamp-infrastructure/ansible.cfg) = False
OS / ENVIRONMENT

macOS 10.15.5.

STEPS TO REPRODUCE

inventory.ini contents:

[my-hosts]
127.0.0.1 ansible_connection=local

main.yml contents:

- hosts: my-hosts
  tasks:
    - meta: noop

Run the playbook with:

EXPECTED RESULTS

No warnings appear.

ACTUAL RESULTS
$ ansible-playbook -i inventory.ini main.yml 
[WARNING]: Invalid characters were found in group names but not replaced, use -vvvv to see details

PLAY [my-hosts] *******************************************
...

When I run with -vvvv it says:

Not replacing invalid character(s) "{'-'}" in group name (my-hosts)

But I'm not sure how that's actionable or why it matters enough to throw a warning.

@ansibot
Copy link
Contributor

ansibot commented Jul 27, 2020

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. cloud inventory Inventory category needs_triage Needs a first human triage before being processed. openstack python3 support:community This issue/PR relates to code supported by the Ansible community. labels Jul 27, 2020
@geerlingguy
Copy link
Contributor Author

geerlingguy commented Jul 27, 2020

!component =lib/ansible/inventory/group.py

@ansibot ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed support:community This issue/PR relates to code supported by the Ansible community. labels Jul 27, 2020
@jamescassell
Copy link
Contributor

I support this change.

@ansibot
Copy link
Contributor

ansibot commented Jul 27, 2020

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Jul 27, 2020

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Jul 28, 2020
@sivel
Copy link
Member

sivel commented Jul 28, 2020

This horse has been beat to something that no longer looks like a horse. We've discussed this again in our triage meeting, and we believe that the current configuration addresses both sides of this concern. However neither side is necessarily 100% happy with the outcome. I believe we have concluded the zero-sum game here.

We warn to let users know they may have issues with invalid chars, but we don't implicitly modify those entries any longer.

@bcoca
Copy link
Member

bcoca commented Jul 28, 2020

closing as per above.

@bcoca bcoca closed this as completed Jul 28, 2020
@ansible ansible locked and limited conversation to collaborators Aug 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. cloud inventory Inventory category openstack python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

No branches or pull requests

6 participants