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

Skip the termination of ec2_stop instances #764

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

athiruma
Copy link
Collaborator

Type of change

Note: Fill x in []

  • bug
  • enhancement
  • documentation
  • dependencies

Description

For security reasons, all pull requests need to be approved first before running any automated CI

@athiruma athiruma added bug Something isn't working ok-to-test PR ok to test labels Apr 26, 2024
@athiruma athiruma requested a review from ebattat April 26, 2024 16:46
@athiruma athiruma self-assigned this Apr 26, 2024
@@ -92,7 +92,7 @@ def __fetch_stop_instance(self, instance_days: int, delete_instance_days: int, s
delta_charge = self.get_ebs_cost(resource=block_device_mappings[instance_id], resource_type='ec2', resource_hours=(self.DAILY_HOURS * (self.DELETE_INSTANCE_DAYS-self.DAYS_TO_NOTIFY_ADMINS)))
stop_cost = self.get_ebs_cost(resource=block_device_mappings[instance_id], resource_type='ec2', resource_hours=(self.DAILY_HOURS * days))
self.__trigger_mail(tags=tags, stopped_time=stopped_time, days=days, resource_id=instance_id, image_id=ami_id, ec2_type=ec2_types[instance_id], instance_id=instance_id, message_type='delete', stop_cost=stop_cost, delta_charge=delta_charge)
self._ec2_client.terminate_instances(InstanceIds=[instance_id])
# self._ec2_client.terminate_instances(InstanceIds=[instance_id])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we going to do snapshot backup ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, commented ami too.

@athiruma athiruma force-pushed the not_action_ec2_stop branch from 0b193b3 to d2303c4 Compare April 26, 2024 17:05
@@ -87,15 +87,15 @@ def __fetch_stop_instance(self, instance_days: int, delete_instance_days: int, s
tag_specifications = [{'ResourceType': 'image', 'Tags': tags}]
if sign == ge:
tag_specifications.append({'ResourceType': 'snapshot', 'Tags': tags})
try:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pls add also comment why we comment out this part, so we will know the reason.
"Comment out termination code to reduce the risk of unintended terminations"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@athiruma athiruma force-pushed the not_action_ec2_stop branch from d2303c4 to b2d9ec9 Compare April 26, 2024 17:27
@ebattat ebattat merged commit 18eba29 into redhat-performance:main Apr 26, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ok-to-test PR ok to test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants