Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Added support for description to aci_syslog_group module. (DCNE-373) #763

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4c70cc3
[minor_change] Added support for description to aci_syslog_group module.
Dev-Sinha13 May 20, 2025
1551ff6
[ignore] Updated Test file for aci_syslog_group
Dev-Sinha13 May 21, 2025
b9b4084
[minor_change] Added two new attributes to aci_syslog_group and optio…
Dev-Sinha13 May 30, 2025
a346493
[ignore] Added description for default values of attributes in aci_sy…
Dev-Sinha13 Jun 2, 2025
8ee35df
[ignore] Fixed documentation issues
Dev-Sinha13 Jun 4, 2025
80a8bb6
[minor change] Added enhanced_log option as an alias to rfc
Dev-Sinha13 Jun 11, 2025
c63d851
[ignore] Added a constant for format
Dev-Sinha13 Jun 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 72 additions & 15 deletions plugins/modules/aci_syslog_group.py
View file
Open in desktop
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
Expand All @@ -18,47 +20,70 @@
options:
admin_state:
description:
- Administrative state of the syslog group
- Administrative state of the syslog group.
type: str
choices: [ enabled, disabled ]
console_logging:
description:
- Log events to console
- Log events to console.
type: str
choices: [ enabled, disabled ]
console_log_severity:
description:
- Severity of events to log to console
- Severity of events to log to console.
- If unset during creation, value defaults to C(alerts).
type: str
choices: [ alerts, critical, debugging, emergencies, error, information, notifications, warnings ]
choices: [ alerts, critical, emergencies]
local_file_logging:
description:
- Log to local file
- Log to local file.
type: str
choices: [ enabled, disabled ]
local_file_log_severity:
description:
- Severity of events to log to local file
- Severity of events to log to local file.
- If unset during creation, value defaults to C(alerts).
type: str
choices: [ alerts, critical, debugging, emergencies, error, information, notifications, warnings ]
format:
description:
- Format of the syslog messages. If omitted when creating a group, ACI defaults to using aci format.
- Format of the syslog messages.
- If unset during creation the value defaults to C(aci).
- C(rfc5424-ts) is only available starting from ACI version 5.2(8).
type: str
choices: [ aci, nxos, rfc5424-ts ]
Copy link
Collaborator

Choose a reason for hiding this comment

The 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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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 unset during creation and O(format) is provided then it is set to the same value as format. If O(format) is not provided it is set to C(aci).
- C(rfc5424-ts) is only available starting from ACI version 5.2(8).
type: str
choices: [ aci, nxos ]
choices: [ aci, nxos, rfc5424-ts ]
console_log_format:
description:
- Format of the console log messages.
- If unset during creation and O(format) is provided then it is set to the same value as format. If O(format) is not provided it is set to C(aci).
- C(rfc5424-ts) is only available starting from ACI version 5.2(8).
type: str
choices: [ aci, nxos, rfc5424-ts ]
include_ms:
description:
- Include milliseconds in log timestamps
- Include milliseconds in log timestamps.
type: bool
include_time_zone:
description:
- Include timezone in log timestamps
- Include timezone in log timestamps.
type: bool
name:
description:
- Name of the syslog group
- Name of the syslog group.
type: str
aliases: [ syslog_group, syslog_group_name ]
description:
Copy link
Collaborator

Choose a reason for hiding this comment

The 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 rfc5424-ts, same applies to format in aci_syslog_remote_dest. Please find out what this is and add to choices if applicable.

Copy link
Collaborator

Choose a reason for hiding this comment

The 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?

Copy link
Collaborator

@akinross akinross May 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.)Ok I have added the new option "rfc5424-ts"
2.)Yes it can be configured and exposed a format attribute for syslogConsole and syslogFile
3.)I have addressed the severity options

description:
- Description for the syslog group.
type: str
aliases: [ descr ]
state:
description:
- Use C(present) or C(absent) for adding or removing.
Expand All @@ -76,6 +101,7 @@
link: https://developer.cisco.com/docs/apic-mim-ref/
author:
- Tim Cragg (@timcragg)
- Dev Sinha (@DevSinha13)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update example below with description attribute included

"""

EXAMPLES = r"""
Expand All @@ -89,9 +115,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
Expand Down Expand Up @@ -246,17 +290,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"]),
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.
Instead of removing the values, we could log a warning using module.warn(msg) stating that the choice of severity is no longer valid. Then we revert the choice to the default.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would a user have this configured in a working playbook?

Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator

@akinross akinross Jun 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The 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?

Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator

Choose a reason for hiding this comment

The 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(
Expand All @@ -275,11 +322,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(
Expand All @@ -297,6 +353,7 @@ def main():
class_config = dict(
name=name,
format=format,
descr=description,
includeMilliSeconds=include_ms,
)
if include_time_zone is not None:
Expand All @@ -312,12 +369,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),
),
),
],
Expand Down
Loading
Loading