voxpupuli / puppet-zabbix

Puppet module for creating and maintaining zabbix components with puppet.
https://forge.puppet.com/puppet/zabbix
Apache License 2.0
80 stars 228 forks source link

Added details to host network interface to configure SNMP hosts #785

Closed oraziobattaglia closed 3 years ago

oraziobattaglia commented 3 years ago

Pull Request (PR) description

Following the pull request #697 we add an hash of interface options to configure some details about SNMP. We want to use to add SNMP host with community and other details.

Example

  zabbix_host { 'SNMPServer':
    ipaddress     => '192.168.1.1',
    use_ip        => true,
    port          => 161,
    groups        => ['SNMP servers'],
    group_create  => true,
    templates     => ['Template Module ICMP Ping'],
    macros        => [],
    proxy         => '',
    interfacetype => 2,
    interfacedetails => {"version" => 2, "bulk" => 0, "community" => "public"},
  }

Using interfacetype = 2 without details we have the error

{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid params.","data":"Incorrect arguments passed to function."},"id":42907}

This Pull Request (PR) fixes the following issues

No issues opened, follow pull request #697

root-expert commented 3 years ago

Hello @oraziobattaglia and thanks for you contribution! Can you add some acceptance tests for this? You can add another host with interfacetype set to 2 and the interfacedetails here

oraziobattaglia commented 3 years ago

Hello @root-expert thanks to review my request. I added an acceptance tests as you suggested. Thank you

root-expert commented 3 years ago

Hello @root-expert thanks to review my request. I added an acceptance tests as you suggested. Thank you

Thank you, can you also check that the host is created correctly by querying the API like this ? Don't forget to query your own parameters and remove the ones you don't use (if any)

oraziobattaglia commented 3 years ago

Ok @root-expert added context test3.example.com, not sure about the syntax to check hash values of interfacedetails! Thank you

root-expert commented 3 years ago

I took a look at zabbix Host API. Seems version 4.0 are supposed to fail because details field is not available (only bulk) so you need to add conditional for that.

I don't know why 5.0/5.2 and 5.4 fail though :( Take a look at https://github.com/voxpupuli/puppet-zabbix/blob/master/.github/CONTRIBUTING.md#the-test-matrix on how to run Rubocop/linter locally.

You can also run acceptance tests locally if you want, more details on the document.

oraziobattaglia commented 3 years ago

Thank you @root-expert yes we are going in your direction. Now we start to test locally so we can reduce commits and test here. Maybe my last commit continue to not work so you can avoid to start the pipeline. Thank you

oraziobattaglia commented 3 years ago

@root-expert I added your suggestions and tried to squash, thank you

smortex commented 3 years ago

@oraziobattaglia after squashing you should have a single commit in the PR :-)

First, rebase your changes on top of the main branch. From your working directory:

git fetch origin         # Download the latest code we have here
git rebase origin/master # Move your commits on top of the main branch
git push -f              # Send the changes (-f is required because we re-wrote history)

The PR should only show your commits :-)


Then you can squash tour commits. From your working directory:

git rebase -i origin/master # Move your commits on top of the main branch

This will bring your editor. Keep the first line unchanged, and replace pick with squash for all subsequent lines. Save and quit, your editor will allow you to combine the commit messages in a single commit message.

Then push your updated branch:

git push -f            # Send the changes (-f is required because we re-wrote history)
oraziobattaglia commented 3 years ago

@smortex thanks for the detailed istructions about the rebase, was a hard work for me :-)! I'm not expert of rebase! Hope now is ok.

root-expert commented 3 years ago

Good job! The only thing is that you have two commits now. You can squash the last one by doing:

git rebase -i HEAD~2

You should see two commits:

On the second one type fixup instead of pick, save and quit your editor. You should end up with just one commit. Then force push it.

git push -f

Thanks again!

oraziobattaglia commented 3 years ago

@root-expert thank you for the suggestion but when I do git rebase -i HEAD~2 I see a lot of commit not mine and only the commit "Added details to host network interface for SNMP". I don't see the commit "Merge branch 'master' into master". I don't know how to procede to squash the 2 commits. The second one "Merge branch 'master' into master" was made by a file conflict resolution made here by GUI. Thank you

root-expert commented 3 years ago

Hey @oraziobattaglia are you able to join on Libera.chat irc on channel #voxpupuli to help you further? Thank you!

oraziobattaglia commented 3 years ago

@root-expert @bastelfreak @smortex thank you for the support and the merge!