-
Notifications
You must be signed in to change notification settings - Fork 23.7k
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
Define module for managing E-Series email alerts #42643
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shipit
The test
|
The sanity failure is unrelated and has been fixed. I've restarted CI. |
- Enable/disable the sending of email-based alerts. | ||
default: enabled | ||
required: false | ||
choices: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this just be enabled/disabled?
What's present/absent mean?
- A fully qualified domain name, IPv4 address, or IPv6 address of a mail server. | ||
- To use a fully qualified domain name, you must configure a DNS server on both controllers using | ||
M(netapp_e_mgmt_interface). | ||
- Required when I(state=enabled/present). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's confusing having enabled and present
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was to have consistency with all of the other places that state is utilized in Ansible modules. state=present/absent seems to be a common convention elsewhere (hence why prior developers used it in our volume module, for instance).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, though the issue is you are supporting both present and enabled. What's the difference?
Just pick one pair and use that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enabled/disabled for this as we are talking about a service (email)
type: bool | ||
log_path: | ||
description: | ||
- A local path to a file to be used for debug logging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Local to what?
The Ansible Controller?
ssid: | ||
required: true | ||
description: | ||
- The ID of the array to manage. This value must be unique for each array. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this needs updating do it in it's own PR
Email alerts can be enabled for an E-Series system to provide information to interested users by email when a warning or critical level event occurs on the system. This module will allow a system owner to configure whether or not system alerts are enabled, and who will receive them.
@gundalow The requested changes have been completed. Let me know if I missed something. |
SUMMARY
Email alerts can be enabled for an E-Series system to provide
information to interested users by email when a warning or critical
level event occurs on the system. This module will allow a system owner
to configure whether or not system alerts are enabled, and who will
receive them.
ISSUE TYPE
COMPONENT NAME
netapp_e_alerts
ANSIBLE VERSION
ADDITIONAL INFORMATION