[node-patches] Change in ovirt-node[master]: If miss custom Syslog port, using default syslog port "514" ...

[rbarry at redhat.com]

Ryan Barry has posted comments on this change.

Change subject: If miss custom Syslog port, using default syslog port "514" to set
......................................................................


Patch Set 1: Code-Review+1

(3 comments)

http://gerrit.ovirt.org/#/c/28346/1/src/ovirtnode/log.py
File src/ovirtnode/log.py:

Line 146: def syslog_auto():
Line 147:     host = ""
Line 148:     port = ""
Line 149:     if (not "OVIRT_SYSLOG_SERVER" in _functions.OVIRT_VARS and
Line 150:         not "OVIRT_SYSLOG_PORT" in _functions.OVIRT_VARS):
Changing this bars users from using DNS discovery through SRV records (_functions.find_srv(...)), which appears to be all this if statement is doing. No change should be needed here
Line 151:         logger.info("Attempting to locate remote syslog server...")
Line 152:         try:
Line 153:             host, port = _functions.find_srv("syslog", "udp")
Line 154:         except:


Line 152:         try:
Line 153:             host, port = _functions.find_srv("syslog", "udp")
Line 154:         except:
Line 155:             pass
Line 156:         if not host is "" and not port is "":
I haven't tested, but I'm guessing things are going wrong here.

find_srv returns False if it can't find it, so neither will be empty strings and this check will pass.

Changing this to "if not host and not port" may resolve.
Line 157:             logger.info("Found! Using syslog server " + host + ":" + port)
Line 158:             ovirt_rsyslog(host, port, "udp")
Line 159:             return True
Line 160:         else:


Line 161:             logger.warn("Syslog server not found!")
Line 162:             return False
Line 163:     elif ("OVIRT_SYSLOG_SERVER" in _functions.OVIRT_VARS and
Line 164:           not "OVIRT_SYSLOG_PORT" in _functions.OVIRT_VARS):
Line 165:         logger.info("Using custom syslog server and default port" +
For consistency, we're ok just saying "Using default syslog port" without any mention of a specified server
Line 166:                     _functions.OVIRT_VARS["OVIRT_SYSLOG_SERVER"] + ":" +
Line 167:                     "514" + ".")
Line 168:         ovirt_rsyslog(_functions.OVIRT_VARS["OVIRT_SYSLOG_SERVER"],
Line 169:                       "514", "udp")


-- 
To view, visit http://gerrit.ovirt.org/28346
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id1eadbac0749d630f67a3bcd60f2f7791fffd5f2
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: hadong <hadong0720 at gmail.com>
Gerrit-Reviewer: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes