Fix zabbix_agent role on Windows #1290

emrocha · 2024-06-12T22:15:46Z

SUMMARY

Role zabbix_agent is not working on Windows.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

Role zabbix_agent

ADDITIONAL INFORMATION

Single quotes to avoid double backslashes.
Remove default variable zabbix_agent_win_include because zabbix-agent2 may be used.
Fix agent config template name to agent.conf.j2
Default values for zabbix_win_package and zabbix_win_download_link
Default value for zabbix_agent_controlsocket
zabbix_agent_tlspskidentity_check and zabbix_agent_tlspskcheck must be defined in a separately task. There were two tasks that could define these variables. If the second task is skipped, as it has register keyword, the variable is reset.
Define a value for _win_include

pyrodie18

Thank you for the contribution. A few comments but looks good.

pyrodie18 · 2024-06-15T15:18:52Z

roles/zabbix_agent/tasks/psk_identity.yml

@@ -3,17 +3,21 @@
    - name: AutoPSK | Check for existing TLS PSK identity | Windows
      ansible.windows.win_stat:
        path: "{{ zabbix_agent_tlspskidentity_file }}"
-      register: zabbix_agent_tlspskidentity_check
+      register: zabbix_agent_tlspskidentity_check_windows


I don't see how zabbix_agent_tlspskidentity_check could be set twice since the two choices for the ansible_os_family variable are mutually exclusive.

In the original code, there is two tasks that use the "register" keyword for the same variable. This two task are, indeed, mutual exclusive. But Ansible also set the variable if the task is skipped.

If a task fails or is skipped, Ansible still registers a variable with a failure or skipped status, unless the task is skipped based on tags.
https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_variables.html

pyrodie18 · 2024-06-15T15:19:07Z

roles/zabbix_agent/tasks/psk_secret.yml

@@ -12,17 +12,21 @@
    - name: AutoPSK | Check for existing TLS PSK file | Windows
      ansible.windows.win_stat:
        path: "{{ zabbix_agent_tlspskfile }}"
-      register: zabbix_agent_tlspskcheck
+      register: zabbix_agent_tlspskcheck_windows


I don't see how zabbix_agent_tlspskcheck could be set twice since the two choices for the ansible_os_family variable are mutually exclusive.

In the original code, there is two tasks that use the "register" keyword for the same variable. This two task are, indeed, mutual exclusive. But Ansible also set the variable if the task is skipped.

If a task fails or is skipped, Ansible still registers a variable with a failure or skipped status, unless the task is skipped based on tags.

https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_variables.html

pyrodie18 · 2024-06-15T15:20:32Z

roles/zabbix_agent/vars/agent2_vars.yml

@@ -3,7 +3,8 @@ _logfile: /var/log/zabbix/zabbix_agent2.log
 _include: /etc/zabbix/zabbix_agent2.d
 _tls_subject: "{{ zabbix_agent_tlsservercertsubject | default(omit) }}" # FIXME this is not correct and should be removed with 2.0.0, here only to prevent regression
 _win_package: zabbix_agent2-{{ zabbix_version_long }}-windows-amd64-openssl-static.zip
-_win_download_link: "{{ zabbix_win_download_url }}/{{ zabbix_version_long | regex_search('^\\d+\\.\\d+') }}/{{ zabbix_version_long }}/{{ zabbix2_win_package }}"
-_win_logfile: "{{ zabbix_win_install_dir }}\\zabbix_agent2.log"
+_win_download_link: "{{ zabbix_win_download_url }}/{{ zabbix_version_long | regex_search('^\\d+\\.\\d+') }}/{{ zabbix_version_long }}/{{ zabbix2_win_package | default('_win_package') }}"


See the latest changes. Support for zabbix2_win_package has been removed with the release of 3.0.0

What do you suggest as an alternative?

The code below uses the variable _win_download_link when the variables zabbix_agent_win_download_link and zabbix_agent2_win_download_link are not set.

community.zabbix/roles/zabbix_agent/tasks/Windows.yml

Lines 1 to 4 in 45ed88b

---

- name: "Windows | Set some variables"

ansible.builtin.set_fact:

zabbix_agent_win_download_link: "{{ zabbix_agent_win_download_link is defined | ternary(zabbix_agent_win_download_link, zabbix_agent2_win_download_link) | default(_win_download_link) }}"

On the other hand, the code below uses the variable zabbix2_win_package that is not set by default.

community.zabbix/roles/zabbix_agent/vars/agent2_vars.yml

Line 6 in 45ed88b

_win_download_link: "{{ zabbix_win_download_url }}/{{ zabbix_version_long | regex_search('^\\d+\\.\\d+') }}/{{ zabbix_version_long }}/{{ zabbix2_win_package }}"

Therefore, the script will fail if none of these variables are set when the Role is applied.

Maybe I misunderstood your comment. Do you just want to unify the parameter for Zabbix_agent and Zabbix_agent2?

pyrodie18 · 2024-06-15T15:21:44Z

roles/zabbix_agent/tasks/Windows_conf.yml

+
+- name: "Set Include Path Info"
+  ansible.builtin.set_fact:
+    zabbix_agent_include: "{{ zabbix_agent_win_include is defined | ternary(zabbix_agent_win_include, zabbix_agent2_win_include) | default(_win_include) }}"


See the latest changes. Support for zabbix_agent2_win_include has been removed with the release of 3.0.0

In template of zabbix_agent.conf, the parameter Include= will be set because the variable zabbix_agent_include is defined in the main.yml script.

community.zabbix/roles/zabbix_agent/tasks/main.yml

Lines 27 to 29 in 45ed88b

- name: Set Variables

ansible.builtin.set_fact:

zabbix_agent_include: "{{ zabbix_agent_include is defined | ternary(zabbix_agent_include, _include) }}"

community.zabbix/roles/zabbix_agent/templates/agent.conf.j2

Line 69 in 45ed88b

{{ (zabbix_agent_include is defined and zabbix_agent_include is not none) | ternary('', '# ') }}Include={{ zabbix_agent_include | default('') }}

So, for Windows, if the variable zabbix_agent_include was not explicited defined, the parameter Include will be set to /etc/zabbix/zabbix_agentd.d because of the code below.

community.zabbix/roles/zabbix_agent/vars/agent_vars.yml

Line 3 in 45ed88b

_include: /etc/zabbix/zabbix_agentd.d

Maybe I misunderstood your comment. Do you just want to unify the parameter for Zabbix_agent and Zabbix_agent2?

Yep! There are no more zabbix_agent2 vars. They are all combined into zabbix_agent vars

pyrodie18 · 2024-06-15T15:22:29Z

roles/zabbix_agent/tasks/Windows.yml

@@ -191,8 +192,8 @@

 - name: "Windows | Set installation settings (agent 2)"
  ansible.builtin.set_fact:
-    zabbix_win_package: "{{ zabbix2_win_package }}"
-    zabbix_win_download_link: "{{ zabbix2_win_download_link }}"
+    zabbix_win_package: "{{ zabbix2_win_package | default('_win_package') }}"


See the latest changes. Support for zabbix2_win_package has been removed with the release of 3.0.0

pyrodie18 · 2024-06-15T15:22:40Z

roles/zabbix_agent/tasks/Windows.yml

-    zabbix_win_package: "{{ zabbix2_win_package }}"
-    zabbix_win_download_link: "{{ zabbix2_win_download_link }}"
+    zabbix_win_package: "{{ zabbix2_win_package | default('_win_package') }}"
+    zabbix_win_download_link: "{{ zabbix2_win_download_link | default('_win_download_link') }}"


See the latest changes. Support for zabbix2_win_download_link has been removed with the release of 3.0.0

david-sieg · 2024-06-27T12:19:17Z

Are there any news here?

the-iot · 2024-06-27T12:37:27Z

the agent role is fixed by commit b0f05b4 but the version 3.0.1 is not released.

see #1301 (comment)

emrocha · 2024-06-28T15:01:42Z

I was a little busy this week, but I will take a look soo n.

emrocha · 2024-07-01T16:29:37Z

the agent role is fixed by commit b0f05b4 but the version 3.0.1 is not released.

see #1301 (comment)

The code below in zabbix_agent role still include the role zabbix_repo for Windows hosts.

community.zabbix/roles/zabbix_agent/tasks/main.yml

Lines 43 to 52 in 28c4921

    
           - name: Install Repository 
        
             ansible.builtin.include_role: 
        
               name: community.zabbix.zabbix_repo 
        
             vars: 
        
               zabbix_repo_version: "{{ zabbix_agent_version }}" 
        
               zabbix_repo_package: "{{ zabbix_agent_package }}" 
        
               zabbix_repo_apt_priority: "{{ zabbix_agent_apt_priority | default (omit) }}" 
        
             when: 
        
               - zabbix_manage_repo | default(true) 
        
               - not (zabbix_agent_docker | bool) or ansible_os_family == "Windows"

In my view, the correct logic in the "when" statement shoud be:

 when:
    - zabbix_manage_repo | default(true)
    - not (zabbix_agent_docker | bool)
    - ansible_os_family != "Windows"

In the role zabbix_repo, the code below tries to load the Windows.yml file that do not exists in role zabbix_repo.

community.zabbix/roles/zabbix_repo/tasks/main.yml

Lines 7 to 8 in 28c4921

    
           - name: Install the correct repository 
        
             ansible.builtin.include_tasks: "{{ ansible_os_family }}.yml"

For this reason, the script still does not work for Windows Hosts.

david-sieg · 2024-07-01T16:53:14Z

Yes mate, you're right. Can u open a PR?

emrocha · 2024-07-01T17:36:10Z

@david-sieg
I made the correction in the "when" statement for the "Install Repository" task now.
But a keep it in this pull request that include others changes. Is the ok for you?

@pyrodie18

I remove some "zabbix2_*" variables. But not all. The code below is necessary because the script tries to remove old versions and need to test both zabbix agents (1 and 2).

community.zabbix/roles/zabbix_agent/tasks/Windows.yml

Lines 28 to 35 in 28c4921

    
           - name: "Windows | Set variables specific to Zabbix" 
        
             ansible.builtin.set_fact: 
        
               zabbix_win_svc_name: Zabbix Agent 
        
               zabbix_win_exe_path: '{{ zabbix_win_install_dir }}\bin\zabbix_agentd.exe' 
        
               zabbix_win_config_name: "zabbix_agentd.conf" 
        
               zabbix2_win_svc_name: Zabbix Agent 2 
        
               zabbix2_win_exe_path: '{{ zabbix_win_install_dir }}\bin\zabbix_agent2.exe' 
        
               zabbix2_win_config_name: "zabbix_agent2.conf"

david-sieg · 2024-07-01T17:52:41Z

That‘s ok for me, thx!

the-iot · 2024-07-02T21:48:52Z

please add merge into main and release 3.0.2 - momentary we cant install zabbix agent on windows with release 3.x

pyrodie18 · 2024-07-03T04:29:22Z

OK, this looks good to me. The only thing still needed is change fragment(s).

emrocha · 2024-07-03T20:25:51Z

OK, this looks good to me. The only thing still needed is change fragment(s).

I didn't understand. What do you mean by "fragments"?

Do I need to change anything else?

BGmot · 2024-07-04T02:04:44Z

OK, this looks good to me. The only thing still needed is change fragment(s).

I didn't understand. What do you mean by "fragments"?

Do I need to change anything else?

You need to create a file describing your changes here https://github.com/ansible-collections/community.zabbix/tree/main/changelogs/fragments

Here you can read instructions about how to create changelog fragments https://docs.ansible.com/ansible/latest/community/development_process.html#creating-a-changelog-fragment

the-iot · 2024-07-04T14:39:00Z

ok...merge is done.

please add tag for the new version 3.0.2 and publish to galaxy

pyrodie18 requested changes Jun 15, 2024

View reviewed changes

emrocha closed this Jul 1, 2024

emrocha force-pushed the fix-windows-role branch from 077f8d9 to 78658d6 Compare July 1, 2024 17:18

emrocha reopened this Jul 1, 2024

Fix role zabbix_agent for Windows hosts

6fb2f2d

pyrodie18 force-pushed the fix-windows-role branch from a895a99 to 6fb2f2d Compare July 3, 2024 04:22

pyrodie18 merged commit d756b78 into ansible-collections:main Jul 4, 2024
102 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix zabbix_agent role on Windows #1290

Fix zabbix_agent role on Windows #1290

emrocha commented Jun 12, 2024

pyrodie18 left a comment

pyrodie18 Jun 15, 2024

emrocha Jun 17, 2024

pyrodie18 Jun 15, 2024

emrocha Jun 17, 2024

pyrodie18 Jun 15, 2024

emrocha Jun 17, 2024

emrocha Jun 17, 2024

pyrodie18 Jun 15, 2024

emrocha Jun 17, 2024

emrocha Jun 17, 2024

pyrodie18 Jun 21, 2024

pyrodie18 Jun 15, 2024

pyrodie18 Jun 15, 2024

david-sieg commented Jun 27, 2024

the-iot commented Jun 27, 2024 •

edited

Loading

emrocha commented Jun 28, 2024

emrocha commented Jul 1, 2024

david-sieg commented Jul 1, 2024

emrocha commented Jul 1, 2024

david-sieg commented Jul 1, 2024

the-iot commented Jul 2, 2024

pyrodie18 commented Jul 3, 2024

emrocha commented Jul 3, 2024

BGmot commented Jul 4, 2024

the-iot commented Jul 4, 2024

	---
	- name: "Windows \| Set some variables"
	ansible.builtin.set_fact:
	zabbix_agent_win_download_link: "{{ zabbix_agent_win_download_link is defined \| ternary(zabbix_agent_win_download_link, zabbix_agent2_win_download_link) \| default(_win_download_link) }}"

	- name: Set Variables
	ansible.builtin.set_fact:
	zabbix_agent_include: "{{ zabbix_agent_include is defined \| ternary(zabbix_agent_include, _include) }}"

Fix zabbix_agent role on Windows #1290

Fix zabbix_agent role on Windows #1290

Conversation

emrocha commented Jun 12, 2024

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

pyrodie18 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-sieg commented Jun 27, 2024

the-iot commented Jun 27, 2024 • edited Loading

emrocha commented Jun 28, 2024

emrocha commented Jul 1, 2024

david-sieg commented Jul 1, 2024

emrocha commented Jul 1, 2024

david-sieg commented Jul 1, 2024

the-iot commented Jul 2, 2024

pyrodie18 commented Jul 3, 2024

emrocha commented Jul 3, 2024

BGmot commented Jul 4, 2024

the-iot commented Jul 4, 2024

the-iot commented Jun 27, 2024 •

edited

Loading