ansible-collections / community.kubernetes

Kubernetes Collection for Ansible
https://galaxy.ansible.com/community/kubernetes
GNU General Public License v3.0
265 stars 104 forks source link

k8s module quotes integer variables #290

Closed andiolsi closed 4 years ago

andiolsi commented 4 years ago
SUMMARY

The k8s module incorrectly quotes entries such as port and replicas when they are read from a dict using jinja2 templating

ISSUE TYPE
COMPONENT NAME

k8s

ANSIBLE VERSION
CONFIGURATION
ansible 2.9.11
  config file = /Users/myuser/.ansible.cfg
  configured module search path = ['/Users/myuser/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/Cellar/ansible/2.9.11/libexec/lib/python3.8/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.8.5 (default, Jul 21 2020, 10:48:26) [Clang 11.0.3 (clang-1103.0.32.62)]
OS / ENVIRONMENT

Mac OS X 10.15.6

STEPS TO REPRODUCE

Define variables

dict_with_service_info:
  - name: serviceX
     service-name: service-X
     replicas: 1
  - name: serviceY
     service-name: service-Y
     replicas: 2

create playbook

---
- hosts: k8smaster
  tasks:
  - name: Create service object
    k8s:
      state: present
      definition:
        apiVersion: v1
        kind: Service
        metadata:
          name: "{{item.service_name}}"
          namespace: default
          labels:
            run: "{{item.service_name}}"
        spec:
          selector:
            run: "{{item.service_name}}"
          ports:
          - protocol: TCP
            targetPort: 8080
            port: 8080
    loop: "{{dict_with_service_info | flatten(1)}}"
  - name: Deploy PODs
    vars:
      name: sv_cache
      service_name: service-cache
      replicas: 2
    k8s:
      state: present
      definition:
        apiVersion: apps/v1
        kind: Deployment
        metadata:
          name: "{{item.service_name}}"
          namespace: default
        spec:
          selector:
            matchLabels:
              run: "{{item.service_name}}"
          replicas: "{{item.replicas}}"
          template:
            metadata:
              namespace: default
              labels:
                run: "{{item.service_name}}"
            spec:
              containers:
                - name: "{{item.service_name}}"
                  image: "nginx:latest"
                  imagePullPolicy: Always
                  ports:
                    - containerPort: 8080
    loop: "{{dict_with_service_info | flatten(1)}}"
EXPECTED RESULTS
TASK [some-services : Deploy PODs] **********************************************************************************************************
changed: [k8smaster]
ACTUAL RESULTS

replicas gets quoted

ASK [some-services : Deploy PODs] **********************************************************************************************************
failed: [k8smaster] (item={'

....
v1.DeploymentSpec.Replicas: readUint32: unexpected character: \\ufffd, error found in #10 byte of ...|eplicas\\\":\\\"1\\\"

The same will be the case if you do:

replicas: "{{variable | to_json | from_json | int}}"

Whereas the following works:

replicas: "{{variable}}"
tima commented 4 years ago

@andiolsi Have you tried this: replicas: "{{ item.replicas | int }}"?

andiolsi commented 4 years ago

yes, absolutely, even replicas: "{{variable | to_json | from_json | int}}"does have the same issue

andiolsi commented 4 years ago

i was told that each modules code should have something like 'replicas': dict(type='int' .... to sanitize json creation.

it actually should even make a string "8080" into a unquoted integer when creating the request to kuberentes.

the k8s module does not do this

andiolsi commented 4 years ago

Found something else. If you set an environment variable:

spec:
  containers:
    - name: foobar
       env:
         - name: TEST_ENVIRONMENT_VARIABLE_LISTENING_PORT_FOR_PROGRAM_IN_POD
            value: 8080

You get:

 ReadString: expects \\\" or n, but found 8, error found in #10 byte of ...|,\\\"value\\\":8080}],\\\"ima|..., bigger context ...|alue\\\":\\\"olsitrack\\\"},{\\\"name\\\":\\\"LISTEN_PORT\\\",\\\"value\\\":8080}],\\\"image\\\":\\\"registry.olsitec.de:20443/olsitrack|...\",\"field\":\"patch\"}]},\"code\":422}\n", "reason": "Unprocessable Entity", "status": 422

In this case it is non fatal as basically every linux environment variable is a string and every application that wants to treat it like an integer has to read and typecast it.

The k8s module code should auto-quote all "value" items that are passed in "env:"

This becomes even worse since most people will likely use the port both for the environment variable and as the targetPort and containerPort. The latter two would always produce an error if you would have the variable be a string and the former would error if it was an integer.

This so clearly is not expected behaviour. Unfortunately my Python skills are too basic to patch it myself.

I am a little astonished, that noone has stumbled across this. It would seem I am the first person to use ansible to deploy items to a kubernetes cluster in a more abstractly configured way. Or at least I am the first one to report it. (Or someone else did and I could not find that here.)

If you are handling multiple services/pods of the same function (microservices) then it es rather likely that you would have stored the amount of replicas in a variable. It seems very likely that this variable is inside a dictionary.

tima commented 4 years ago

@andiolsi

I've had to make some changes to what you've provided here to get something that runs and I can now reproduce the problem.

To your point RE: 'replicas': dict(type='int' ..... This is true if Ansible is managing a module parameter. In the k8s module things like definition is treated as YAML and everything else is essentially a pass thru to the underlying client and the K8s API.

https://docs.ansible.com/ansible/latest/collections/community/kubernetes/k8s_module.html#parameter-resource_definition https://github.com/ansible-collections/community.kubernetes/blob/aad4696f205cc21a3d264083fa16a0eb24d75ee5/plugins/module_utils/common.py#L119

I am a little astonished, that noone has stumbled across this. It would seem I am the first person to use ansible to deploy items to a kubernetes cluster in a more abstractly configured way. Or at least I am the first one to report it. (Or someone else did and I could not find that here.)

Lots of people are using Ansible and this Kubernetes content (including myself) to deploy clusters without issue. There is something you are specifically doing that is causing the int to get treated/reverted to a string. So I'm not as astonished. I'm trying to keep up with you to nail down where that is happening. Seems to be something about the int filter from what I'm seeing.

Still seeing what I can find before I turn this over to an engineer.

tima commented 4 years ago

It seems that this issue has been seen before in the kubernetes client library that the openshift client depends upon.

http://mail-archives.apache.org/mod_mbox/airflow-commits/201907.mbox/%3CJIRA.13232744.1557522669000.43644.1564007280393@Atlassian.JIRA%3E

Passing this along to some one more knowledgeable for further review.

tima commented 4 years ago

One detail I left out -- I was able to reproduce this message without using a dict.

tima commented 4 years ago

@andiolsi:

I got ahold of @fabianvf offline to take a look at this. It is related to Jinja treating everything as a string. He said you "got to use a template to work around that, since we don't have a schema we can't tell Ansible how to coerce those types and it seems to really like making them strings."

I move your definition into an external template file (removed the wrapping quotes on all of the variables after my cut-and-paste) and let the k8s read that in with the new template param. No error.

He noted another option is to turn on the Jinja Native Types option (requires Jinja 2.10+) in your ansible config.

https://docs.ansible.com/ansible/latest/reference_appendices/config.html#default-jinja2-native https://gist.github.com/mkrizek/cbcd450ec5dbd4c78a37c73667159cb3

I prepended ANSIBLE_JINJA2_NATIVE=1 to my ansible-playbook command to run your play and it worked with the inline definition and | int filter on the replicas.

Either of these approaches will solve the issue you are experiencing.

I'm marking this one wontfix.

goetzc commented 3 years ago

I also stumbled upon this "int vs string" issue, exactly with the same replicas key while using a loop.

Might be worth adding this workaround to the README, as this ticket is not easy to find and looks to be a more and more common.

andiolsi commented 3 years ago

Just some feedback to future readers: For a while I used ANSIBLE_JINJA2_NATIVE=1 , as it does the trick.

Recently I went completely around it. I am now generating the kubernetes manifest files and then kubectl applying them. This has the added benefit of giving you the option to debug the manifest itself too.

goetzc commented 3 years ago

@andiolsi Interesting. Thanks for the tip!