-
Notifications
You must be signed in to change notification settings - Fork 104
Added support for description to aci_syslog_group module. (DCNE-373) #763
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
4c70cc3
1551ff6
b9b4084
a346493
8ee35df
80a8bb6
c63d851
d8907e4
4267662
c4ef871
d592f60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
#!/usr/bin/python | ||
# -*- coding: utf-8 -*- | ||
|
||
# Copyright: (c) 2021, Tim Cragg (@timcragg) | ||
# Copyright: (c) 2025, Dev Sinha (@DevSinha13) | ||
# GNU General Public License v3.0+ (see LICENSE or https://www.gnu.org/licenses/gpl-3.0.txt) | ||
|
||
from __future__ import absolute_import, division, print_function | ||
|
@@ -30,7 +32,7 @@ | |
description: | ||
- Severity of events to log to console | ||
type: str | ||
choices: [ alerts, critical, debugging, emergencies, error, information, notifications, warnings ] | ||
choices: [ alerts, critical, emergencies] | ||
local_file_logging: | ||
description: | ||
- Log to local file | ||
|
@@ -44,8 +46,21 @@ | |
format: | ||
description: | ||
- Format of the syslog messages. If omitted when creating a group, ACI defaults to using aci format. | ||
- C(rfc5424-ts) is only availible starting from version ACI 5.2(8). | ||
samiib marked this conversation as resolved.
Show resolved
Hide resolved
|
||
type: str | ||
choices: [ aci, nxos, rfc5424-ts ] | ||
samiib marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the UI of 6.0.9d rfc5424-ts is mentioned as enhanced_log, should we provide this as input and map it to the correct value in the payload? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The APIC that I am using and have been using for testing this module is 6.0(2h). Is there a way I can check what 6.0.9d would be like? |
||
local_file_log_format: | ||
description: | ||
- The format of the local file log messages. If ommitted then uses same format as the format set for syslog messages. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what are local_file_log_format and console_log_format set to when O(format) is not provided? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there is no value for O(format) then the APIC sets its value to "aci" automatically so by default even if O(format) isn't provided the value for local_file_log_format and console_log_format is "aci" aswell. |
||
- C(rfc5424-ts) is only availible starting from version ACI 5.2(8). | ||
samiib marked this conversation as resolved.
Show resolved
Hide resolved
|
||
type: str | ||
choices: [ aci, nxos ] | ||
choices: [ aci, nxos, rfc5424-ts ] | ||
console_log_format: | ||
description: | ||
- Format of the console log messages. If ommitted then uses the same format as the format set for syslog messages. | ||
- C(rfc5424-ts) is only availible starting from version ACI 5.2(8). | ||
samiib marked this conversation as resolved.
Show resolved
Hide resolved
|
||
type: str | ||
choices: [ aci, nxos, rfc5424-ts ] | ||
include_ms: | ||
description: | ||
- Include milliseconds in log timestamps | ||
|
@@ -59,6 +74,11 @@ | |
- Name of the syslog group | ||
type: str | ||
aliases: [ syslog_group, syslog_group_name ] | ||
description: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the format attribute in latest version it is also possible to configure Enhanced Log option There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also the children syslogConsole and syslogFile seem to have format also, is this something that can be configured? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In UI and model 6.0.9d the severity is restricted to only 3 option for console, please check if we should remove existing values if they do not work? Also verify the models of earlier versions of ACI. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1.)Ok I have added the new option "rfc5424-ts" |
||
description: | ||
- Description for the syslog group. | ||
type: str | ||
aliases: [ descr ] | ||
state: | ||
description: | ||
- Use C(present) or C(absent) for adding or removing. | ||
|
@@ -76,6 +96,7 @@ | |
link: https://developer.cisco.com/docs/apic-mim-ref/ | ||
author: | ||
- Tim Cragg (@timcragg) | ||
- Dev Sinha (@DevSinha13) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. update example below with description attribute included |
||
""" | ||
|
||
EXAMPLES = r""" | ||
|
@@ -89,9 +110,27 @@ | |
local_file_log_severity: warnings | ||
console_logging: enabled | ||
console_log_severity: critical | ||
description: syslog group | ||
state: present | ||
delegate_to: localhost | ||
|
||
- name: Create a syslog group with local_file_log_format and console_log_format | ||
cisco.aci.aci_syslog_group: | ||
host: apic | ||
username: admin | ||
password: SomeSecretPassword | ||
format: aci | ||
name: my_syslog_group | ||
local_file_logging: enabled | ||
local_file_log_severity: warnings | ||
console_logging: enabled | ||
console_log_severity: critical | ||
local_file_log_format: rfc5424-ts | ||
console_log_format: rfc5424-ts | ||
description: syslog group | ||
state: present | ||
|
||
|
||
- name: Disable logging to local file | ||
cisco.aci.aci_syslog_group: | ||
host: apic | ||
|
@@ -246,17 +285,20 @@ def main(): | |
argument_spec.update(aci_annotation_spec()) | ||
argument_spec.update( | ||
name=dict(type="str", aliases=["syslog_group", "syslog_group_name"]), | ||
format=dict(type="str", choices=["aci", "nxos"]), | ||
format=dict(type="str", choices=["aci", "nxos", "rfc5424-ts"]), | ||
local_file_log_format=dict(type="str", choices=["aci", "nxos", "rfc5424-ts"]), | ||
console_log_format=dict(type="str", choices=["aci", "nxos", "rfc5424-ts"]), | ||
admin_state=dict(type="str", choices=["enabled", "disabled"]), | ||
console_logging=dict(type="str", choices=["enabled", "disabled"]), | ||
console_log_severity=dict(type="str", choices=["alerts", "critical", "debugging", "emergencies", "error", "information", "notifications", "warnings"]), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since some choices here are removed this would be classified as a breaking change. Even though they may not be valid severity levels some users could have these configured in their playbooks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would a user have this configured in a working playbook? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, the full set of severity levels only applies to file log and there was always only ever 3 options for console log in the meta. Makes sense to remove them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, these options were never exposed in any version of ACI. I checked the meta files back to 3.0 version. Any usage of these attributes would result in APIC 120 error response that the input is not allowed. We could consider labelling it a bugfix? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would still label the PR as a minor change due to the other changes and additional options. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought that the information was taken from the commit messages? Or is it from PR title? The commit messages are marked with [minor_change]. |
||
console_log_severity=dict(type="str", choices=["alerts", "critical", "emergencies"]), | ||
local_file_logging=dict(type="str", choices=["enabled", "disabled"]), | ||
local_file_log_severity=dict( | ||
type="str", choices=["alerts", "critical", "debugging", "emergencies", "error", "information", "notifications", "warnings"] | ||
), | ||
include_ms=dict(type="bool"), | ||
include_time_zone=dict(type="bool"), | ||
state=dict(type="str", default="present", choices=["absent", "present", "query"]), | ||
description=dict(type="str", aliases=["descr"]), | ||
) | ||
|
||
module = AnsibleModule( | ||
|
@@ -275,11 +317,20 @@ def main(): | |
admin_state = module.params.get("admin_state") | ||
console_logging = module.params.get("console_logging") | ||
console_log_severity = module.params.get("console_log_severity") | ||
console_log_format = module.params.get("console_log_format") | ||
local_file_log_format = module.params.get("local_file_log_format") | ||
local_file_logging = module.params.get("local_file_logging") | ||
local_file_log_severity = module.params.get("local_file_log_severity") | ||
include_ms = aci.boolean(module.params.get("include_ms")) | ||
include_time_zone = aci.boolean(module.params.get("include_time_zone")) | ||
state = module.params.get("state") | ||
description = module.params.get("description") | ||
|
||
if console_log_format is None: | ||
console_log_format = format | ||
|
||
if local_file_log_format is None: | ||
local_file_log_format = format | ||
|
||
aci.construct_url( | ||
root_class=dict( | ||
|
@@ -297,6 +348,7 @@ def main(): | |
class_config = dict( | ||
name=name, | ||
format=format, | ||
descr=description, | ||
includeMilliSeconds=include_ms, | ||
) | ||
if include_time_zone is not None: | ||
|
@@ -312,12 +364,12 @@ def main(): | |
), | ||
dict( | ||
syslogFile=dict( | ||
attributes=dict(adminState=local_file_logging, format=format, severity=local_file_log_severity), | ||
attributes=dict(adminState=local_file_logging, format=local_file_log_format, severity=local_file_log_severity), | ||
), | ||
), | ||
dict( | ||
syslogConsole=dict( | ||
attributes=dict(adminState=console_logging, format=format, severity=console_log_severity), | ||
attributes=dict(adminState=console_logging, format=console_log_format, severity=console_log_severity), | ||
), | ||
), | ||
], | ||
|
Uh oh!
There was an error while loading. Please reload this page.