Skip to content
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

POL-1410 - Improve Scheduled Instance Notifications #2817

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

Conversation

bryankaraffa
Copy link
Contributor

Description

Improved notifications related for the 3 Scheduled Instance Policy Templates

Issues Resolved

https://flexera.atlassian.net/browse/POL-1410

Link to Example Applied Policy

https://app.flexera.com/orgs/6/automation/applied-policies/projects/7954
The 3 "Scheduled Instance" Applied Policies running in there
image

Contribution Check List

  • New functionality includes testing.
  • New functionality has been documented in the README if applicable
  • New functionality has been documented in CHANGELOG.MD

@bryankaraffa bryankaraffa added enhancement New feature or request DO-NOT-MERGE labels Nov 8, 2024
@bryankaraffa bryankaraffa requested a review from a team as a code owner November 8, 2024 00:16
Copy link
Contributor

github-actions bot commented Nov 8, 2024

3 Errors
🚫

cost/aws/schedule_instance/aws_schedule_instance.pt

Policy Template has console.log() statements. These are used for debugging and should not be present in catalog policy templates:

Line 516

🚫

cost/azure/schedule_instance/azure_schedule_instance.pt

Policy Template has console.log() statements. These are used for debugging and should not be present in catalog policy templates:

Line 554

🚫

cost/google/schedule_instance/google_schedule_instance.pt

Policy Template has console.log() statements. These are used for debugging and should not be present in catalog policy templates:

Line 499

3 Warnings
⚠️

cost/aws/schedule_instance/aws_schedule_instance.pt

Policy template updated but associated README.md file has not been. Please verify that any necessary changes have been made to the README.


Datasources and scripts with mismatched names found. These names should match; for example, a datasource named ds_currency should be paired with a script named js_currency. This convention should only be ignored when the same script is called by multiple datasources. The following datasource/script pairs have mismatched names:

Line 217: ds_policy_incident_action_failed_details / js_policy_incident_action_fail_details


Possible invalid spacing between comma-separated items found:

Line 512: // schedule="08:00-17:30;MO,TU,WE,TH,FR"

Line 513: // schedule="08:00-17:30;MO,TU,WE,TH,FR;America/Los_Angeles"

Line 514: var schedule_is_valid = /([0-9]{1,2}.*)-([0-9]{1,2}.*);([A-Za-z,]*).*/.test(schedule)

Comma separated items should be organized as follows, with a single space following each comma: apple, banana, pear

⚠️

cost/azure/schedule_instance/azure_schedule_instance.pt

Policy template updated but associated README.md file has not been. Please verify that any necessary changes have been made to the README.


Possible invalid spacing between comma-separated items found:

Line 550: // schedule="08:00-17:30;MO,TU,WE,TH,FR"

Line 551: // schedule="08:00-17:30;MO,TU,WE,TH,FR;America/Los_Angeles"

Line 552: var schedule_is_valid = /([0-9]{1,2}.*)-([0-9]{1,2}.*);([A-Za-z,]*).*/.test(schedule)

Line 589: days = "SU,MO,TU,WE,TH,FR,SA"

Comma separated items should be organized as follows, with a single space following each comma: apple, banana, pear

⚠️

cost/google/schedule_instance/google_schedule_instance.pt

Policy template updated but associated README.md file has not been. Please verify that any necessary changes have been made to the README.


Possible invalid spacing between comma-separated items found:

Line 497: var schedule_is_valid = /([0-9]{1,2}.*)-([0-9]{1,2}.*)_([a-z\-]*).*/.test(schedule)

Comma separated items should be organized as follows, with a single space following each comma: apple, banana, pear

Generated by 🚫 Danger

@nia-vf1 nia-vf1 self-assigned this Nov 8, 2024
// schedule="08:00-17:30;MO,TU,WE,TH,FR;America/Los_Angeles"
var schedule_is_valid = /([0-9]{1,2}.*)-([0-9]{1,2}.*);([A-Za-z,]*).*/.test(schedule)
if ((_.isString(schedule)) && (schedule_is_valid == false)) {
console.log("Skipping instance " + instance['instanceId'] + " with invalid schedule '" + schedule +"'");
Copy link
Contributor

@nia-vf1 nia-vf1 Nov 8, 2024

Choose a reason for hiding this comment

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

Do we need this console.log() for this template meant for the Catalog?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using this thread to discuss:
#2817 (comment)

// schedule="0900-0930_mo-tu-we-th-fr_america-los_angeles"
var schedule_is_valid = /([0-9]{1,2}.*)-([0-9]{1,2}.*)_([a-z\-]*).*/.test(schedule)
if ((_.isString(schedule)) && (schedule_is_valid == false)) {
console.log("Skipping instance " + instance['instanceId'] + " with invalid schedule '" + schedule +"'");
Copy link
Contributor

@nia-vf1 nia-vf1 Nov 8, 2024

Choose a reason for hiding this comment

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

Same as console.log() comments on AWS and Azure policies above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using this thread to discuss:
#2817 (comment)

// schedule="08:00-17:30;MO,TU,WE,TH,FR;America/Los_Angeles"
var schedule_is_valid = /([0-9]{1,2}.*)-([0-9]{1,2}.*);([A-Za-z,]*).*/.test(schedule)
if ((_.isString(schedule)) && (schedule_is_valid == false)) {
console.log("Skipping instance " + instance['resourceID'] + " with invalid schedule '" + schedule +"'");
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as console.log() comments on AWS policy above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going to use this thread to discuss.. I am was thinking this would be a valid piece of information to have in the logs. Another way we could return this is including the instance in the results, but with a value for "schedule_error" field or something -- and only resources that have no schedule_error value would be valid and have it's schedule executed.

Or, we leave this as a console.log and ignore this test warning... any thoughts @XOmniverse ? We've discussed console.log exemptions before

Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably have a broader conversation about this. I originally added that test because I frequently (mostly in older and poorly maintained policy templates) found simple debugging console.log() statements that were left over without being properly removed from the production template.

If we believe there are things we would want logged via console.log() that are not for debugging during policy development, that test can be disabled.

@nia-vf1 nia-vf1 removed their assignment Nov 8, 2024
Copy link
Contributor Author

@bryankaraffa bryankaraffa left a comment

Choose a reason for hiding this comment

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

Using this thread to discuss:
#2817 (comment)

@bryankaraffa bryankaraffa requested a review from a team November 11, 2024 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request READY-FOR-REVIEW
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants