Alon Bar-Lev has posted comments on this change. Change subject: gluster: Added a plugin under gluster for nrpe configuration. ......................................................................
Patch Set 1: (8 comments) http://gerrit.ovirt.org/#/c/26956/1/src/plugins/ovirt-host-deploy/gluster/nrpe.py File src/plugins/ovirt-host-deploy/gluster/nrpe.py: Line 32: import socket Line 33: import fileinput Line 34: import sys Line 35: Line 36: NRPE_CONFIG_FILE = "/etc/nagios/nrpe.cfg" Please put this in constants.py::FileLocations Line 37: Line 38: Line 39: @util.export Line 40: class Plugin(plugin.PluginBase): Line 39: @util.export Line 40: class Plugin(plugin.PluginBase): Line 41: def __init__(self, context): Line 42: super(Plugin, self).__init__(context=context) Line 43: self._enabled = False no need for _enabled if you are not having anything complex Line 44: Line 45: @plugin.event( Line 46: stage=plugin.Stages.STAGE_INIT, Line 47: ) Line 45: @plugin.event( Line 46: stage=plugin.Stages.STAGE_INIT, Line 47: ) Line 48: def _setup(self): Line 49: self.environment[odeploycons.GlusterEnv.ENABLE] = False do not touch ENABLE in this module... Line 50: Line 51: @plugin.event( Line 52: stage=plugin.Stages.STAGE_VALIDATION, Line 53: condition=lambda self: ( Line 57: self.environment[odeploycons.GlusterEnv.ENABLE] Line 58: ), Line 59: ) Line 60: def _validation(self): Line 61: self._enabled = True this is not required, just move the condition to the MISC Line 62: Line 63: @plugin.event( Line 64: stage=plugin.Stages.STAGE_MISC, Line 65: condition=lambda self: ( Line 66: os.path.isfile(NRPE_CONFIG_FILE) Line 67: ), Line 68: ) Line 69: def _misc(self): Line 70: host = self.environment[odeploycons.VdsmEnv.ENGINE_HOST] why do you need host temporary variable? also... the engine host setting is obsolete and is not used any more, since we have no need to check connectivity to engine, so please avoid using it. you need explicit configuration to a proxy, including port. Line 71: addr = socket.gethostbyname(host) Line 72: for line in fileinput.input(NRPE_CONFIG_FILE, inplace=True): Line 73: if line.startswith("allowed_hosts"): Line 74: if line.find(str(addr)) == -1: Line 67: ), Line 68: ) Line 69: def _misc(self): Line 70: host = self.environment[odeploycons.VdsmEnv.ENGINE_HOST] Line 71: addr = socket.gethostbyname(host) why do you resolve it to address? Line 72: for line in fileinput.input(NRPE_CONFIG_FILE, inplace=True): Line 73: if line.startswith("allowed_hosts"): Line 74: if line.find(str(addr)) == -1: Line 75: line = line[:-1] + "," + str(addr) Line 71: addr = socket.gethostbyname(host) Line 72: for line in fileinput.input(NRPE_CONFIG_FILE, inplace=True): Line 73: if line.startswith("allowed_hosts"): Line 74: if line.find(str(addr)) == -1: Line 75: line = line[:-1] + "," + str(addr) please use FileTransaction to modify content of files, see example in other places. Line 76: sys.stdout.write(line) Line 77: Line 72: for line in fileinput.input(NRPE_CONFIG_FILE, inplace=True): Line 73: if line.startswith("allowed_hosts"): Line 74: if line.find(str(addr)) == -1: Line 75: line = line[:-1] + "," + str(addr) Line 76: sys.stdout.write(line) stdout? Line 77: -- To view, visit http://gerrit.ovirt.org/26956 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4faa40e2bf382187907db944f7485c1630fcc0ae Gerrit-PatchSet: 1 Gerrit-Project: ovirt-host-deploy Gerrit-Branch: master Gerrit-Owner: Darshan N <dnara...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Darshan N <dnara...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
