jaytaylor / ansible-kafka

Ansible Kafka role
https://galaxy.ansible.com/jaytaylor/kafka/
Other
67 stars 52 forks source link

kafka_server role variable not being merged correctly with kafka_server_defaults #41

Open bradsheppard opened 6 years ago

bradsheppard commented 6 years ago

In modern versions of Ansible (I'm using version 2.6.1, which this bug applies to) there is new behavior of set_fact, where set_fact no longer overrides role variables (see Ansible 22025 for a discussion of this). When passing in the role variable kafka_server via a playbook, this variable will not be merged properly with kafka_server_defaults, since the result of the merge is re-assigned back to kafka_server. This will cause problems when executing the role, as kafka_server will likely not contain much of the needed properties. As an example, try running the playbook:

- hosts: [all]
  become: yes
  roles:
    - {
      role: "ansible-kafka",
      kafka_hosts: ['xxx', 'yyy'],
      kafka_zookeeper_hosts: ['zzz'],
      kafka_version: 0.11.0.2,
      kafka_scala_serverion: 2.10,
      kafka_java_version: "openjdk-8-jre",
      kafka_server: {
        host_name: "newhostname"
      }
    }

The result will be an error of the form:

FAILED! => {"msg": "The task includes an option with an undefined variable. The error was: 'dict object' has no attribute 'port'\n\nThe error appears to have been in '/home/bradleysheppard/Dev/AnsibleKafka/ansible-kafka/tasks/kafka-cfg.yml': line 31, column 3, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n\n- name: \"Generate the kafka hosts connection string\"\n ^ here\n"}

The fix involves not reassigning the result of the merge back to kafka_server, but instead using a new variable, which I have called kafka_server_config.

jaytaylor commented 6 years ago

Wow, so first off, thanks for finding this and taking the time to figure out a solution and submit a PR for it. I know from experience that Ansible 2.x variable oddities aren't a lot of fun to chase down ;)

One question before this gets merged-

Is the kafka_server var defined in main.yml#L58 still being used?

bradsheppard commented 6 years ago

@jaytaylor No problem, and thank you for writing such a useful Ansible role :)

To address your comment, yes kafka_server will still be used as a role variable, however from an internal perspective it will only be used in tasks/kafka-cfg.yml#L3 where it is merged with kafka_server_default. The result of the merge is assigned to kafka_server_config. I then replaced all instances of kafka_server in tasks/kafka-cfg.yml and templates/usr/local/kafka/config/server.properties.j2 with kafka_server_config. From a user's perspective, there should be no breaking changes.