Added the ec2_run policy #703
Conversation
athiruma
force-pushed
the
run_ec2_policy
branch
3 times, most recently
from
2a9df1c to
da5629c
Compare
|
The best practice is to split this PR to 2:
|
| :rtype: | ||
| """ | ||
| action = "deleted" | ||
| try: |
There was a problem hiding this comment.
I wonder, does python have the switch case? 🤔
| return tag.get('Value').strip() | ||
| return '' | ||
|
|
||
| def get_clean_up_days_count(self, tags: list): |
There was a problem hiding this comment.
Seems it also updates the cleanup days value ?
There was a problem hiding this comment.
No, It returns the clean_up days count. We have another method which updates the days count.
Check here.
| self._s3_client.put_bucket_tagging(Bucket=resource_id, Tagging={'TagSet': tags}) | ||
| elif self._policy == 'empty_roles': | ||
| self._iam_client.tag_role(RoleName=resource_id, Tags=tags) | ||
| elif self._policy in ('ip_unattached', 'unused_nat_gateway', 'zombie_snapshots', 'ebs_unattached', 'ec2_run'): |
There was a problem hiding this comment.
What about new policies that we will add in the future line s3_age, do you need add it also here ?
its very risky to tide to specific policy name !!!
There was a problem hiding this comment.
Yes, I know. I was thinking about how I change the process. As of now, we will stick to it later we can replace it.
| policies_list = self.__get_policies() | ||
| for policy_type, policies in policies_list.items(): | ||
| # @Todo support for all the aws policies, currently supports ec2_run as urgent requirement | ||
| if self.__policy in policies and self.__policy == "ec2_run": |
There was a problem hiding this comment.
Why you ask for specific policy self.__policy == "ec2_run"
There was a problem hiding this comment.
As I specified in the new ADR, this is only for ec2_run. So this should work against only ec2_run. I'll activate other policies later. Its risky to activate all policies without testing.
| @@ -48,7 +52,7 @@ def __init__(self): | |||
| self._environment_variables_dict['account'] = self.get_aws_account_alias_name().upper().replace('OPENSHIFT-', '') | |||
| self._environment_variables_dict['policy'] = EnvironmentVariables.get_env('policy', '') | |||
|
|
|||
| self._environment_variables_dict['aws_non_cluster_policies'] = ['ec2_idle', 'ec2_stop', 'ec2_run', 'ebs_in_use', | |||
| self._environment_variables_dict['aws_non_cluster_policies'] = ['ec2_idle', 'ec2_stop', 'ebs_in_use', | |||
There was a problem hiding this comment.
Why do u remove ec2_run from this list ?
There was a problem hiding this comment.
It is the static approach. It is very bad to have like this.
Look this new ADR Dir structure. I added policy/cleanup, so all cleanup policies will reside in this folder and we will walk through this and run the policies.
| else: | ||
| cleanup_days = 0 | ||
| force_tag_update = self._aws_cleanup_policies.get_force_tag_update() | ||
| self._aws_cleanup_policies.update_resource_tags(resource_id=instance.get('InstanceId'), |
There was a problem hiding this comment.
I think the name should be update_resource_day_counts and not all the tags, do you update only the day count ?
There was a problem hiding this comment.
Yes, I can rename the method. 👍
|
|
||
| if self._es_operations.check_elastic_search_connection(): | ||
| if policy_result: | ||
| if len(policy_result) > 500: |
There was a problem hiding this comment.
can you update what the meaning of 500 ?
| @@ -11,7 +11,7 @@ This tool support the following policies: | |||
|
|
|||
| * Real time Openshift Cluster cost, User cost | |||
| * [ec2_idle](../../cloud_governance/policy/aws/ec2_idle.py): idle ec2 in last 4 days, cpu < 2% & network < 5mb. | |||
| * [ec2_run](../../cloud_governance/policy/aws/ec2_run.py): running ec2. | |||
| * [ec2_run](../../cloud_governance/policy/aws/cleanup/ec2_run.py): running ec2. | |||
There was a problem hiding this comment.
I moved ec2_run to the cleanup folder. So we have all cleanup policies collectively. ADR
I added ADR for the ec2_run policy. It's not the two separate PRs. It was the better time to have the ADR. Thats why I added the major change. |
athiruma
force-pushed
the
run_ec2_policy
branch
6 times, most recently
from
f378d08 to
3bd9819
Compare
athiruma
force-pushed
the
run_ec2_policy
branch
from
3bd9819 to
56732ec
Compare
Type of change
Note: Fill x in []
Description
Added the ec2_run policy.
Created the ADR for the changes
For security reasons, all pull requests need to be approved first before running any automated CI