search

voxpupuli / puppet-squid

Puppet module for configuration of squid caching proxy.
https://forge.puppet.com/puppet/squid
Other
12 stars 54 forks source link

specify IP to listen for SNMP #85

Closed SourceDoctor closed 6 years ago

SourceDoctor commented 6 years ago

Limit SNMP Listening to specified IP

alexjfisher commented 6 years ago

I'm not 100% sure looking at the manual, but is it even valid to have multiple snmp_listen_ip lines in the squid config? If not... instead of changing the type from a string to a hash to fix the issue with using create_resources, maybe it was actually create_resources that needed changing to just be a simple class declaration? Sorry if we're going round in circles! @traylenator ?

SourceDoctor commented 6 years ago

i tested it now myself. If snmp_incoming_address is specified, it listens only to the last specified IP-Address in Configfile. So in puppet it must be a Class (again). Now i'm asking you how to change the test so it runs successfull ;-)

SourceDoctor commented 6 years ago

@alexjfisher ok don't know how to build tests for classes, if you don't know also i'm thinking about integrate this Option into Header class so tests run successfully und Feature is included. What do you think about it?

SourceDoctor commented 6 years ago

@alexjfisher so tests run well now

alexjfisher commented 6 years ago

I'm getting really confused. We did decide that you can actually only have a single snmp_incoming_address in a squid config file, so from a defined type, you went back to using a class. I think that is correct. But... you're still asking the user for a hash and using create resources to declare the class. That's not so good.

SourceDoctor commented 6 years ago

so now it's an optional string, and it's no more a create resource. It's a class which will be called then.

alexjfisher commented 6 years ago

Great! If you update the README, I'll try to get this merged tomorrow.

SourceDoctor commented 6 years ago

Readme updated ... what about my other Pull request? ;-)

alexjfisher commented 6 years ago

@SourceDoctor I finally had chance to take a deeper look into snmp and squid. I came up with https://github.com/voxpupuli/puppet-squid/pull/86 It's a fair bit simpler. Let me know if it meets your needs.

@traylenator I wonder if we should also simplify snmp_ports at some point? The squid documentation is pretty poor. You can specify a separate snmp_port per worker process, but it doesn't achieve anything. The worker that you query communicates with all other workers before returning the joint statistics.

From http://lists.squid-cache.org/pipermail/squid-users/2015-March/002844.html

SNMP is on the list of SMP-aware features.

The worker receiving the SNMP request will contact other workers to fetch the data for producing the SNMP response. This may take some time.

My testing confirms this. The only reason to wrap the snmp_port in an if $process = might be to suppress warnings when the second worker tries to open the same port?? (I think I saw something about this whilst googling!).

SourceDoctor commented 6 years ago

@alexjfisher sure no Problem take #86 the result will fetch my Needs also. So im fine with it.

alexjfisher commented 6 years ago

@SourceDoctor Excellent. I've not forgotten your other PRs (and thanks for working on them). It just takes time to properly go through the squid documentation sometimes.