Alon Bar-Lev has posted comments on this change. Change subject: tools: support snmp trap as a notification method. ......................................................................
Patch Set 11: (5 comments) http://gerrit.ovirt.org/#/c/22909/11/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/sender/snmp/SNMPEventSender.java File backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/sender/snmp/SNMPEventSender.java: Line 62: Line 63: OID trapOID = SnmpConstants.getTrapOID( Line 64: OVIRT_ENGINE_ENTERPRISE_TRAP_OID, Line 65: 6, Line 66: auditLogTypeVal > each AuditLogType has an id. I am unsure this will be usable as the specific, I tend to expect that the severity is better here, as there should be explicit definition for each specific at manager. Line 67: ); Line 68: v2pdu.add(new VariableBinding(SnmpConstants.snmpTrapOID, trapOID)); Line 69: v2pdu.add(new VariableBinding( Line 70: new OID(trapOID).append(0), Line 86: target.setRetries(2); Line 87: target.setTimeout(5000); Line 88: snmp.send(v2pdu, target); Line 89: final EventSenderResult eventSenderResult = new EventSenderResult(); Line 90: eventSenderResult.setSent(true); > I spent some time thinking about this. you can never know if the other side received a trap, hence marking it as anything in this transport is wrong as long as you have other transports that are reliable. Line 91: return eventSenderResult; Line 92: } catch (IOException e) { Line 93: throw new NotificationServiceException("Could not distribute snmp notification.", e); Line 94: } http://gerrit.ovirt.org/#/c/22909/11/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationProperties.java File backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationProperties.java: Line 77: Line 78: /** Line 79: * SMNP Trap parameters Line 80: */ Line 81: public static final String SNMP_MANAGER = "SNMP_MANAGER"; > So what you suggest? comma/space whatever. Line 82: Line 83: public static final String SNMP_PORT = "SNMP_PORT"; Line 84: Line 85: public static final String SNMP_COMMUNITY = "SNMP_COMMUNITY"; Line 79: * SMNP Trap parameters Line 80: */ Line 81: public static final String SNMP_MANAGER = "SNMP_MANAGER"; Line 82: Line 83: public static final String SNMP_PORT = "SNMP_PORT"; here too... so better to have SNMP_MANAGERS=host:port host:port ... Line 84: Line 85: public static final String SNMP_COMMUNITY = "SNMP_COMMUNITY"; Line 86: Line 87: public static final String SNMP_OID = "SNMP_OID"; http://gerrit.ovirt.org/#/c/22909/11/packaging/services/ovirt-engine-notifier/ovirt-engine-notifier.conf.in File packaging/services/ovirt-engine-notifier/ovirt-engine-notifier.conf.in: Line 116: Line 117: # notify on all but these comma separated list of events: Line 118: BLACKLIST="" Line 119: Line 120: # note: if both WHITELIST and BLACKLIST are defined only WHITELIST is considered. > But empty is a meaningful value: a '' blacklist means all events. if you want to disable snmp, then just disable it. our configuration should not distinct missing and empty value. it is shell like configuration syntax in which missing == empty. Line 121: Line 122: #----------------------------------# Line 123: # Engine Monitoring Configuration: # Line 124: #----------------------------------# -- To view, visit http://gerrit.ovirt.org/22909 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cd22d022ae535f45e046b09a2cbfadd837b465c Gerrit-PatchSet: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: mooli tayer <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Liran Zelkha <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: mooli tayer <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
