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

Add Windows support #657

Closed
wants to merge 2 commits into from
Closed

Add Windows support #657

wants to merge 2 commits into from

Conversation

ljeromets
Copy link
Contributor

@ljeromets ljeromets commented Feb 3, 2020

Pull Request (PR) description

Hi,
This PR is based on the work done by @wardhus here #592

This Pull Request (PR) fixes the following issues: #175

This PR should give the possibility to install zabbix-agent on Windows with chocolatey.

manifests/agent.pp Outdated Show resolved Hide resolved
manifests/agent.pp Outdated Show resolved Hide resolved
manifests/agent.pp Outdated Show resolved Hide resolved
manifests/agent.pp Outdated Show resolved Hide resolved
manifests/repo.pp Outdated Show resolved Hide resolved
manifests/repo.pp Outdated Show resolved Hide resolved
@bastelfreak bastelfreak added enhancement New feature or request needs-work not ready to merge just yet tests-fail labels Feb 4, 2020
@ljeromets
Copy link
Contributor Author

Hi @bastelfreak ,

I've added some units tests for windows but not sure they are really used.

The default value for zabbix_version is set to '3.4' but this version doesn't exists into chocolatey repo. ( https://chocolatey.org/packages/zabbix-agent#versionhistory)
Should we just add this limitation as a comment or add another condition into the params.pp?

@bastelfreak
Copy link
Member

@ljeromets thanks for all the updates. I propose that you add a selector block to the params.pp that sets $zabbix_version based on the kernel:

diff --git a/manifests/params.pp b/manifests/params.pp
index 501eff7..11c9eaa 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -119,6 +119,11 @@ class zabbix::params {
   }
 
   # Zabbix overall params. Is used by all components.
+  $zabbix_version = downcase($facts['kernel']) ? {
+    'windows' => '4.4.5',
+    default   => '3.4',
+  }
+
   $zabbix_package_state                     = 'present'
   $zabbix_proxy                             = 'localhost'
   $zabbix_proxy_ip                          = '127.0.0.1'
@@ -127,7 +132,6 @@ class zabbix::params {
   $zabbix_template_dir                      = '/etc/zabbix/imported_templates'
   $zabbix_timezone                          = 'Europe/Amsterdam'
   $zabbix_url                               = 'localhost'
-  $zabbix_version                           = '3.4'
   $zabbix_web                               = 'localhost'
   $zabbix_web_ip                            = '127.0.0.1'
   $manage_database                          = true

@bastelfreak
Copy link
Member

Also while running the tests I noticed the following error message multiple times:

One or more the json documents could not be parsed. Run jgrep -v for to display documents

You can also see that in the Travis Ci output:
https://travis-ci.org/voxpupuli/puppet-zabbix/jobs/647484182?utm_medium=notification&utm_source=github_status

I assume some of the windows facts are broken :( I will have a closer look later at facterdb

@ljeromets
Copy link
Contributor Author

ljeromets commented Feb 8, 2020

@bastelfreak I may have a idea about the json parsing issue.

PS C:\Users\Administrator> puppet facts | findstr operatingsystemrelease
    "operatingsystemrelease": "2012 R2",

I have update the metadata.json but I have now the message below:

No facts were found in the FacterDB for Facter v3.14.0, using v3.8.0 instead

metadata.json Outdated Show resolved Hide resolved
@bastelfreak bastelfreak added the needs-work not ready to merge just yet label Feb 8, 2020
@vox-pupuli-tasks
Copy link

Dear @ljeromets, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

1 similar comment
@vox-pupuli-tasks
Copy link

Dear @ljeromets, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@ljeromets
Copy link
Contributor Author

I would like help on the unit test ^^

manifests/agent.pp Outdated Show resolved Hide resolved
@bastelfreak
Copy link
Member

@ljeromets thanks for the awesome work! I made two little inline comments. Can you please update the codebase and afterwards squash the commits down to one or a few? Afterwards we can merge and release this!

@vox-pupuli-tasks
Copy link

Dear @ljeromets, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@ljeromets
Copy link
Contributor Author

Hum, I don't know why the tests have failed.
I did another commit to trigger the checks, if they work I will clean back the git history.

@vox-pupuli-tasks
Copy link

Dear @ljeromets, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

1 similar comment
@vox-pupuli-tasks
Copy link

Dear @ljeromets, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@ljeromets
Copy link
Contributor Author

Hello @bastelfreak ,
I will close this PR and open a fresh one, it is easier for me because I had too much issues with the git history.
But before that, can you help on the failling test?

  Warning: PostgresqlValidator.attempt_connection: Sleeping for 1 seconds
  Error: Unable to connect to PostgreSQL server! (:5432)
  Error: /Stage[main]/Postgresql::Server::Service/Postgresql_conn_validator[validate_service_is_running]/ensure: change from 'absent' to 'present' failed: Unable to connect to PostgreSQL server! (:5432)

source => https://travis-ci.org/voxpupuli/puppet-zabbix/jobs/653843178#L3867

We don't use it directly in this module.  It *is* a dependency of at
least one of our dependencies, so it needs to remain in .fixtures.yml.

Fixes voxpupuli#658
@bastelfreak
Copy link
Member

Thanks for the work! We merged it in #675

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-work not ready to merge just yet tests-fail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants