Darshan N has posted comments on this change. Change subject: gluster: Added a plugin under gluster for nrpe configuration. ......................................................................
Patch Set 1: (6 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 Done 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 Done 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... Done 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 Done 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? will use the environment variable to get the fqdn of server as you suggested. 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 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 Done Line 76: sys.stdout.write(line) 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
