The main reason that it does not show a ‘per variable’ message is to avoid ‘warning spam’, since we deduplicate warning messages, you only see it once per run. If we make it per variable, you are going to see one per variable per run.
1 Like
OTOH, having the option to get ‘warning spam’ is useful to the person trying to eradicate bit-rot.
2 Likes
I’m having an issue with this warning as well. I’m trying to add /usr/local/bin to the $PATH environment variable globally. This playbook gives me the deprecation warning. But if I try to change ansible_env.PATH to ansible_facts[‘env’][‘PATH’] I get an error:
“Task failed: Error processing keyword ‘environment’: object of type ‘dict’ has no attribute ‘env’”
Here is the playbook:
- name: Add /usr/local/bin to PATH
hosts: all
become: yes
environment:
PATH: "/usr/local/bin:{{ ansible_env.PATH }}"
# PATH: "/usr/local/bin:{{ ansible_facts['env']['PATH'] }}"
tasks:
- name: Print path
ansible.builtin.debug:
var: ansible_facts['env']['PATH']
- name: path
ansible.builtin.command: "echo $PATH"
register: echo_path
- name: p path
ansible.builtin.debug:
var: echo_path.stdout
After I run this playbook, $PATH has the correct value but ansible_facts[‘env’][‘PATH’] doesn’t.
I could ignore the deprecation warning, but I don’t quite understand what will happen in 2.24. Why can I access ansible_env but not ansible_facts[‘env’] in the environment: stanza?
1 Like
As I commented in the PR, I see that message once per task, not once per run. So from my POV seeing it once per variable isn’t much worse than the current spaminess.
(Also it would be pretty annoying to just find one case per run, and having to run it 100 x or so to really find all facts…)
1 Like
Interestingly, that also happens with ansible_facts.env, but not with ansible_facts.get('env'). Also it applies to all other facts. I guess it is a bug related to the use of the top-level environment directive; it also works fine with per-task environment.
@bcoca can you take a look / pass this on?
it should be only once per run, if you are seeing it per task, that is a bug
I’ll also check the env issue, but that is probably related to how jinja does access (dot notation) vs how python (the other notations)
the point of the warning was not the specific variables, but that the setting itself is changing it’s default.
I don’t think you actually can access ansible_env in the play level environment: stanza. It doesn’t error out, but it doesn’t set anything in the environment either. Behold:
---
# test.yml
- name: Ansible Fact games
hosts: localhost
gather_facts: true
environment:
XXX: "/usr/local/bin:{{ ansible_env.PATH }}"
YYY: "YYYstart:{{ ansible_facts | to_yaml }}:endYYY"
ZZZ: "ZZZstart:{{ ansible_env.PATH | default('not_yet') }}:endZZZ"
tasks:
- name: Print path
ansible.builtin.debug:
var: ansible_facts['env']
Of XXX, YYY, and ZZZ, the only ones that get printed are YYY and ZZZ, and they look like this:
YYY: |-
YYYstart:{}
:endYYY
ZZZ: ZZZstart:not_yet:endZZZ
So at the time the play level environment keyword is processed, gather_facts has not yet run, ansible_facts is defined as a dict but is empty, and other as-yet undefined variables (like ansible_env) silently cause the corresponding environment keys to not be created.
This would be better done at the task level.
Understood, but nobody cares that the default of a setting nobody has ever looked at is changing. What they care about what they need to look for to make the warning go away before errors start happening. Otherwise it may as well say “Useless purple text, but you only get one copy per run - lucky you.”
To be honest, I expect to see no more than four of these across our whole suite even if it were one per variable/fact. But I’d much rather ignore a thousand informative messages than stare pleadingly at a handful of uninformative messages that were made uninformative for the sake of deduplication.
2 Likes
environment has specific logic to prevent it from failing at play level when ansible_env or ansible_facts.env is in the play but you are trying to gather facts. Need to either add ansible_facts.get('env') and ansible_facts['env'] or make it much smarter
Replacing variable access is not the only solution, the much simpler one is setting the value explicitly in configuration. That is the main reason for this warning. Yes, moving to ansible_facts is preferable in the long run as ‘injection’ creates many issues for a very small convenience, why we also add that as an option in the warning.
If the goal is to get folks to use INJECT_FACTS_AS_VARS = false, this deprecation warning isn’t helping. Right now the only way to get rid of it without a lot of work is setting it to true explicitly, which will likely mean it will stay at that value forever (until that value itself gets deprecated - if that ever happens). If you want folks to set this to false, you need to help them find all places they have to fix - and for that more helpful deprecation messages are needed that actually point out what needs to be changed, and not only a little bit of that.
2 Likes
I’m not trying to force it, just moving to a saner default as the injection uses a lot more memory and cpu time, aside from creating possible collisions and security issues. This will incentivize new content to not rely on it.
That said, I agree, there should be simpler ways of identifying where the variables are used, your PR goes a long way for that, my fear is warning spam and we are looking at a way to address that.
As an aside, what i really want to do in the end is add better introspection in general, being able to list what vars are used where and from what origin for a play/role. But that is a much taller order.
possible fix
index 0876c0fa0d..e811968fee 100644
--- a/lib/ansible/playbook/task.py
+++ b/lib/ansible/playbook/task.py
@@ -420,7 +420,7 @@ class Task(Base, Conditional, Taggable, CollectionSearch, Notifiable, Delegatabl
return
except AnsibleUndefinedVariable as e:
error = to_native(e)
- if self.action in C._ACTION_FACT_GATHERING and 'ansible_facts.env' in error or 'ansible_env' in error:
+ if self.action in C._ACTION_FACT_GATHERING and (('ansible_facts' in error and 'env' in error) or 'ansible_env' in error):
# ignore as fact gathering is required for 'env' facts
return
raise