[config_drive] Fix conditional to handle NoneType properly#3683
[config_drive] Fix conditional to handle NoneType properly#3683holser wants to merge 1 commit intoopenstack-k8s-operators:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/efbd14b3673b443b9866353ec72e1d11 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 37m 58s |
| - name: Generate network-data | ||
| register: _net_data_change | ||
| when: | ||
| - cifmw_config_drive_networkconfig is not none |
There was a problem hiding this comment.
This is not needed.
The varialbe is defined in the role default, see: https://github.com/openstack-k8s-operators/ci-framework/blob/main/roles/config_drive/defaults/main.yml#L31-L32 - so it will be None or some value the user provided. The is not none test is safe here.
There was a problem hiding this comment.
Same with default({}) it's a safe mechanism if someone will define it as
cifmw_config_drive_networkconfig:
However, this change is required for Ansible 2.17 as cifmw_config_drive_networkconfig should be defined before checking length
There was a problem hiding this comment.
Is'nt cifmw_config_drive_networkconfig: == cifmw_config_drive_networkconfig: null
So the is not none will skip the task in that case?
If you define:
cifmw_config_drive_networkconfig;
And then do "cifmw_config_drive_networkconfig | default({})" - what is the result? I think it would be cifmw_config_drive_networkconfig == (null|none) - it wont enter the | default({} because cifmw_config_drive_networkconfig is defined as null|none value?
I think | default here is "dead code" it cannot happen?
--- I asked Cursor/Claude AI:
Ansible conditionals and patch necessity
Exported on 2/24/2026 at 15:15:16 GMT+1 from Cursor (2.5.20)
User
I want you to take a look at commit 9742844
It is suopposed to fix something related to Ansible 2.16+ that require conditionals to return a boolean value.
I think the patch is not needed.
My reasoning.
This is not needed.
The varialbe is defined in the role default, see: https://github.com/openstack-k8s-operators/ci-framework/blob/main/roles/config_drive/defaults/main.yml#L31-L32 - so it will be None or some value the user provided. The is not none test is safe here. @roles/config_drive/defaults/main.yml:31-32
The change made to the conditions: @roles/config_drive/tasks/main.yml:54 @roles/config_drive/tasks/main.yml:63
--- a/roles/config_drive/tasks/main.yml
+++ b/roles/config_drive/tasks/main.yml
@@ -51,8 +51,7 @@
- name: Generate user-data
register: _user_data_change
when: -
- cifmw_config_drive_userdata is not none
-
- cifmw_config_drive_userdata | length > 0
-
- cifmw_config_drive_userdata | default({}) | length > 0
ansible.builtin.template:
backup: true
src: "user-data.j2"
@@ -61,9 +60,7 @@
- cifmw_config_drive_userdata | default({}) | length > 0
- name: Generate network-data
register: _net_data_change - when:
-
- cifmw_config_drive_networkconfig is not none
-
- cifmw_config_drive_networkconfig | length > 0
- when: cifmw_config_drive_networkconfig | default({}) | length > 0
ansible.builtin.template:
backup: true
src: "network-config.j2"
Is'nt cifmw_config_drive_networkconfig: == cifmw_config_drive_networkconfig: null
So the is not none will skip the task in that case?
If you define:
cifmw_config_drive_networkconfig;
And then do "cifmw_config_drive_networkconfig | default({})" - what is the result? I think it would be cifmw_config_drive_networkconfig == (null|none) - it wont enter the | default({} because cifmw_config_drive_networkconfig is defined as null|none value?
I think | default here is "dead code" it cannot happen?
Cursor
I'll help you analyze this commit and the Ansible variable handling behavior. Let me first look at the commit and the relevant files.
You are absolutely correct! Your analysis is spot-on. Let me explain why:
The Critical Difference
In YAML/Ansible:
cifmw_config_drive_userdata:(no value) is equivalent tocifmw_config_drive_userdata: null- This means the variable IS defined, but its value is
None/null
How Jinja2 default Filter Works
The default filter only provides a fallback value when a variable is UNDEFINED, not when it's defined as None:
# Variable is UNDEFINED
{{ undefined_var | default({}) }} # Returns {}
# Variable is DEFINED as None
{{ none_var | default({}) }} # Returns None (NOT {})Why the Original Code Was Correct
Original code:
when:
- cifmw_config_drive_userdata is not none
- cifmw_config_drive_userdata | length > 0- When the variable is
null(as defined in defaults),is not nonereturnsFalse→ task is skipped ✅
New code (from the patch):
when:
- cifmw_config_drive_userdata | default({}) | length > 0- When the variable is
null,default({})does nothing (returnsNone) - Then
None | length > 0tries to get the length ofNone→ ERROR ❌
The Fix to Make default Work
If you wanted to use default, you'd need:
when:
- cifmw_config_drive_userdata | default({}, true) | length > 0The true parameter makes the default filter treat None/null values as undefined.
Or, remove these variables from the defaults file entirely so they're truly undefined.
Conclusion
Your reasoning is correct. The patch in commit 974284426086b6b863c25e02b6cb5e832ec87173 is incorrect and would actually break the functionality. The | default({}) is indeed "dead code" as you suspected - it cannot trigger because the variables are defined (as null) in the defaults file.
The original is not none check was the correct approach for this scenario.
holser
force-pushed
the
ansible-core-2.17.14_config_drive
branch
from
4d2c13b to
79c9f07
Compare
Replace explicit 'is not none' check with default filter for None safety. It sets minimal dict as default for cifmw_config_drive_network as stated in README.md. Ansible 2.16+ requires conditionals to return boolean values, not NoneType. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Sergii Golovatiuk <sgolovat@redhat.com>
holser
force-pushed
the
ansible-core-2.17.14_config_drive
branch
from
79c9f07 to
9742844
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/280c0bfcf89447b48502ade1e8d1ce54 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 27m 45s |
|
I have removed this patch and tested to create OCP and EDPM nodes "Create and attach ISO" task was executed successfully without this patch using ansible 2.17.14. Closing this |
2 participants
[config_drive] Fix conditional to handle NoneType properly
Replace explicit 'is not none' check with default filter for None
safety. It sets minimal dict as default for cifmw_config_drive_network
as stated in README.md. Ansible 2.16+ requires conditionals to return
boolean values, not NoneType.