-
Notifications
You must be signed in to change notification settings - Fork 2
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
AWS EC2 health codebundle #3
base: main
Are you sure you want to change the base?
Conversation
…arse_ebs_results fuc in in Core.py
3652218
to
35e139c
Compare
Tested with age cc @stewartshea |
... cmd=custodian run -r ${AWS_REGION} --output-dir ${OUTPUT_DIR}/aws-c7n-ec2-health ${CURDIR}/old-ec2-instances.yaml --cache-period 0 | ||
... secret__aws_access_key_id=${AWS_ACCESS_KEY_ID} | ||
... secret__aws_secret_access_key=${AWS_SECRET_ACCESS_KEY} | ||
${count}= RW.CLI.Run Cli |
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.
This likely is more complicated that it needs to be. Why evaluate a 1 or a 0 when this is a single check - the total count is a fine metric to push, and then SLI alerts can be configured as needed. This only makes sense as a 1 or a 0 if you want to perform an aggregate score across multiple tasks. Do you plan on adding more tasks?
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.
next code bundle like AutoScaling Group - Verify ASGs have valid configurations
comes under EC2 instance so I was thinking to include in this EC2 health codebundle
|
||
|
||
*** Tasks *** | ||
List old AWS EC2 instances in AWS Region `${AWS_REGION}` in AWS account `${AWS_ACCOUNT_ID}` |
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 we need more than one task here to evaluate "AWS EC2 Health" - otherwise this is just a codebundle that is focused on "AWS EC2 Age" - let's come up with a few more tasks.
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.
Also, what is the idea behind this task? It seems normal for a user to have a "running" instance older than "60 days"... are the defaults here appropriate? Are we checking the right thing? One task might be to look for EC2 instances that are not in a running state and older than a specific age. Another task might be to look for instances that need a refresh (e.g. running over a certain age, since it implies a lack of patch/update?).
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.
Yes, because sometimes instances get patched and restart, and if an instance is quite old, it becomes difficult to patch since it will require numerous updates. This means that old instances can indicate unpatchable systems.
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.
and yes agree we can also look for stopped instances as well that too make sense.
@saurabh3460 I've added a few comments which we can discuss - this was not a thorough review, as I think we need to think through the purpose of this codebundle a little more before a full review can be performed. |
This code bundle contains task to check for old AWS EC2 instances in AWS account in specified region.