Skip to content

[config_drive] Fix conditional to handle NoneType properly #3683

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.

Sign up for GitHub

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

Closed
holser wants to merge 1 commit into from
Closed

[config_drive] Fix conditional to handle NoneType properly #3683

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions roles/config_drive/tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -61,9 +60,7 @@

- name: Generate network-data
register: _net_data_change
when:
- cifmw_config_drive_networkconfig is not none
Copy link
Contributor

@hjensas hjensas Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@holser holser Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@hjensas hjensas Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 @@
  • 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 to cifmw_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 none returns False → task is skipped ✅

New code (from the patch):

when:
  - cifmw_config_drive_userdata | default({}) | length > 0
  • When the variable is null, default({}) does nothing (returns None)
  • Then None | length > 0 tries to get the length of None → ERROR ❌

The Fix to Make default Work

If you wanted to use default, you'd need:

when:
  - cifmw_config_drive_userdata | default({}, true) | length > 0

The 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.

- cifmw_config_drive_networkconfig | length > 0
when: cifmw_config_drive_networkconfig | default({}) | length > 0
ansible.builtin.template:
backup: true
src: "network-config.j2"
Expand Down