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

Typecast variables are not preserved #12071

Closed
patrickheeney opened this issue Aug 24, 2015 · 15 comments
Closed

Typecast variables are not preserved #12071

patrickheeney opened this issue Aug 24, 2015 · 15 comments
Labels
affects_1.9 This issue/PR affects Ansible v1.9 bug This issue/PR relates to a bug. c:template/templar needs_info This issue requires further information. Please answer any outstanding questions.

Comments

@patrickheeney
Copy link
Contributor

patrickheeney commented Aug 24, 2015

ISSUE TYPE

Bug Report

COMPONENT NAME

core

ANSIBLE VERSION
ansible 1.9.2
CONFIGURATION

N/A

OS / ENVIRONMENT

N/A

SUMMARY

I have a role like below, that typecasts a variable to an int. However this appears to be converted back to a string by jinaja2. Ansible then throws this error "msg: value of payment_term must be one of: 1,12,24, got: 1" because it was not a valid int.

STEPS TO REPRODUCE

At the moment this is breaking the linode module or any module that relies on an int with choices.

- name: setup linode
  linode:
    name: '{{ item.name }}'
    state: '{{ item.state | default("present") }}'
    #doesn't seem to work
    #payment_term: '{{ item.payment | int | default(1) }}'
    payment_term: '{{ item.payment | int }}'
    api_key: 'test'
  with_items: 
    - name: 'test'
      payment: 1

In the linode example, it seems to be caused by https://github.com/ansible/ansible-modules-core/blob/devel/cloud/linode/linode.py#L452 expecting one of the int choices, but jinaja is giving it a string.
This may or may not be related to #5463

EXPECTED RESULTS

Linode should not error.

ACTUAL RESULTS

Errors.

@AbsReyalp
Copy link

I have the same problem creating VM passing gb size as a parameter.
msg: size_gb must be of type <type 'int'>
Tried also to force to int: size_gb: "{{ size | int }}" but same problem.
Hoping someone could figure out a solution.

@jimi-c jimi-c removed the P3 label Dec 7, 2015
@bcoca bcoca added this to the stable-1.9 milestone Feb 4, 2016
@bcoca bcoca removed this from the stable-1.9 milestone Feb 12, 2016
@bcoca
Copy link
Member

bcoca commented Feb 12, 2016

this should be fixed already in current development as typing is stricter there, can you please test?

@patrickheeney
Copy link
Contributor Author

@bcoca I don't have a test playbook handy yet that wouldn't attempt to spin up a linode to test this. It may be worthwhile to post in #5463 as it seems to be related and more people are involved with better test playbooks.

@bcoca
Copy link
Member

bcoca commented Feb 24, 2016

That is a closed issue, the problem is similar but not the same. Also we don't normally see comments on these (we filter them out) so posting to the mailing list is normally a better way to get our attention.

2.0 is much better at type discovery, if that is the problem here, if it is specifically related to the linode module, this is not the correct repo for the issue. The spec seems correct in 2.0:

payment_term = dict(type='int', default=1, choices=[1, 12, 24])

@patrickheeney
Copy link
Contributor Author

@bcoca I am not a frequent contributor, so it is good to know about the comments and mailing list. The problem is not specific to the linode module, but the linode module was how I discovered it.

I don't have a way to test this at the moment, but: payment_term: '{{ item.payment | int }}' led to "msg: value of payment_term must be one of: 1,12,24, got: 1". I assume because payment_term is not actually converted to a int so it is not a valid choice. I assume the issue still exists unless {{ | int }} would be included in the type discovery changes.

The problem seems very similar to that other issue in my opinion, especially considering all the comments say they are still experiencing it (they may wish to know the comments are not monitored as well).

@patrickheeney
Copy link
Contributor Author

@bcoca I found my old playbook but it is not compatible with ansible 2.0 so I will not be able to test this for some time. FYI I did post this to the mailing list https://groups.google.com/forum/#!topic/ansible-project/4uQ7otaU8P0 but it never got any attention either.

@bcoca
Copy link
Member

bcoca commented Feb 24, 2016

i was going to respond to the post, but you had already opened this issue.

@ryanwalls
Copy link
Contributor

This is still a problem for me in v2.0.1.0-1

@ansibot ansibot added the affects_1.9 This issue/PR affects Ansible v1.9 label Sep 8, 2016
@alikins
Copy link
Contributor

alikins commented Jan 5, 2017

(Mentioning #17992 here to keep track of related issues)

@ansibot ansibot added needs_info This issue requires further information. Please answer any outstanding questions. needs_template This issue/PR has an incomplete description. Please fill in the proposed template correctly. and removed needs_info This issue requires further information. Please answer any outstanding questions. needs_template This issue/PR has an incomplete description. Please fill in the proposed template correctly. labels Apr 4, 2017
@jctanner
Copy link
Contributor

jctanner commented Apr 4, 2017

I agree that #17992 is related, but this issue deserves more attention. There's literally no way to make this work right now for a user except by changing the module arg parser or having the module do post arg processing. The second case doesn't help all the other modules though.

@abadger
Copy link
Contributor

abadger commented Apr 4, 2017

This needs to be handled module side (because k=v parsing will always yield strings). However, I thought it already was: https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/basic.py#L1668

@abadger
Copy link
Contributor

abadger commented Apr 4, 2017

From the error message it looks like it's the combination of type=int and choices in particular that's provoking this error.

@abadger
Copy link
Contributor

abadger commented Apr 4, 2017

I cannot reproduce this in devel or with a checkout of 2.0.1.0:

$ ansible --version                                       *[bb6cade]  (12:38:51)
ansible 2.0.1.0 (detached HEAD bb6cadefa2) last updated 2017/04/04 12:26:03 (GMT -700)
  lib/ansible/modules/core: (detached HEAD 262e2a3302) last updated 2017/04/04 12:26:08 (GMT -700)
  lib/ansible/modules/extras: (detached HEAD e0be11da08) last updated 2017/04/04 12:26:08 (GMT -700)
  config file = 
  configured module search path = Default w/o overrides
$ ANSIBLE_NOCOWS=1 ansible-playbook test.yml -v           *[bb6cade]  (12:38:58)
No config file found; using defaults
 [WARNING]: provided hosts list is empty, only localhost is available


PLAY ***************************************************************************

TASK [setup linode] ************************************************************
failed: [localhost] => (item={u'name': u'test', u'payment': 1}) => {"failed": true, "item": {"name": "test", "payment": 1}, "msg": "Authentication failed"}

NO MORE HOSTS LEFT *************************************************************
        to retry, use: --limit @test.retry

PLAY RECAP *********************************************************************
localhost                  : ok=0    changed=0    unreachable=0    failed=1 
$ cat test.yml                                            *[bb6cade]  (12:39:53)
---
- hosts: localhost
  gather_facts: False
  tasks:
  - name: setup linode
    linode:
      name: '{{ item.name }}'
      state: '{{ item.state | default("present") }}'
      #doesn't seem to work
      #payment_term: '{{ item.payment | int | default(1) }}'
      payment_term: '{{ item.payment | int }}'
      api_key: 'test'
    with_items: 
      - name: 'test'
        payment: 1

and looking at the code in basic.py it seems like the module-side conversion from string to int happens before the check that the value is in choices. So we'll need more information to be able to reproduce this on 2.x (preferably a current release or devel) otherwise it may be that the bug reported against 1.9.x has been fixed in 2.x and the report against v2.0.1.0-1 is something different.

needs_info

@ansibot ansibot added the needs_info This issue requires further information. Please answer any outstanding questions. label Apr 4, 2017
@abadger
Copy link
Contributor

abadger commented Apr 5, 2017

One further point of interest: I can confirm that the code does the wrong thing on 1.9.x but was changed in 2.0.0. In 1.9 we have check_argument_values() being called before check_argument_types(): https://github.com/ansible/ansible/blob/stable-1.9/lib/ansible/module_utils/basic.py#L392
check_argument_values() is what processes choices: https://github.com/ansible/ansible/blob/stable-1.9/lib/ansible/module_utils/basic.py#L978 Therefore, we end up examining the arguments as strings instead of as integers.

In 2.0.0, check_argument_types happens before check_argument_values(): https://github.com/ansible/ansible/blob/v2.0.0.0-1/lib/ansible/module_utils/basic.py#L572 which means that conversion from strings to integers happens before choices is processed.

I'm going to close this bug as both experimentation and code examination shows that it is now fixed. @ryanwalls , it seems like you've encountered a different bug. If you're still interested in it after all this time, we'll need a new bug report with the particulars of reproducing in your circumstances as it will have a different root cause.

@abadger abadger closed this as completed Apr 5, 2017
@dagwieers
Copy link
Member

There is a light at the end of the tunnel. We made a change to Jinja2 so we don't see all variable types changed into strings. See: pallets/jinja#708

@ansibot ansibot removed the bug_report label Mar 6, 2018
@ansibot ansibot added the bug This issue/PR relates to a bug. label Mar 6, 2018
@ansible ansible locked and limited conversation to collaborators Apr 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_1.9 This issue/PR affects Ansible v1.9 bug This issue/PR relates to a bug. c:template/templar needs_info This issue requires further information. Please answer any outstanding questions.
Projects
None yet
Development

No branches or pull requests

10 participants