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

kafka_server role variable not being merged correctly with kafka_server_defaults #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bradsheppard
Copy link

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.

… not merged correctly with kafka_server_defaults due to a new behaviour of set_facts on modern Ansible versions. The set_facts behaviour no longer overrides role variables.
@jaytaylor
Copy link
Owner

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants