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

Mute common error when deleting VPC #404

Merged
merged 2 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions ocw/lib/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from ocw.apps import getScheduler
from webui.PCWConfig import PCWConfig
from ..models import Instance, StateChoice, ProviderChoice, CspInfo
from .emailnotify import send_mail, send_leftover_notification
from .emailnotify import send_mail
from .azure import Azure
from .ec2 import EC2
from .gce import GCE
Expand Down Expand Up @@ -155,7 +155,6 @@ def update_run() -> None:
traceback.format_exc())

auto_delete_instances()
send_leftover_notification()
RUNNING = False
if not error_occured:
LAST_UPDATE = datetime.now(timezone.utc)
Expand Down
7 changes: 6 additions & 1 deletion ocw/lib/ec2.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,12 @@ def vpc_can_be_deleted(self, resource_vpc, vpc_id) -> bool:

def report_cleanup_results(self, vpc_errors: list, vpc_notify: list, vpc_locked: list) -> None:
if len(vpc_errors) > 0:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This if is redundant with the block below.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agree with you but unfortunately I pressed merge button just before your comment popup 😞

send_mail(f'Errors on VPC deletion in [{self._namespace}]', '\n'.join(vpc_errors))
# this is most common error message which we can not fix.
# So no point to spam us with notifications about it
known_error = "An error occurred (DependencyViolation) when calling the DeleteVpc operation"
filtered = [x for x in vpc_errors if known_error not in x]
if len(filtered) > 0:
send_mail(f'Errors on VPC deletion in [{self._namespace}]', '\n'.join(vpc_errors))
if len(vpc_notify) > 0:
send_mail(f'{len(vpc_notify)} VPC\'s should be deleted, skipping due vpc-notify-only=True', ','.join(vpc_notify))
if len(vpc_locked) > 0:
Expand Down
18 changes: 0 additions & 18 deletions ocw/lib/emailnotify.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
from datetime import timedelta
import smtplib
import logging
from email.mime.text import MIMEText
from texttable import Texttable
from django.urls import reverse
from webui.PCWConfig import PCWConfig
from webui.settings import build_absolute_uri
from ..models import Instance

logger = logging.getLogger(__name__)

Expand All @@ -31,22 +29,6 @@ def draw_instance_table(objects):
return table.draw()


def send_leftover_notification():
if PCWConfig.has('notify'):
all_instances = Instance.objects
all_instances = all_instances.filter(active=True, age__gt=timedelta(hours=PCWConfig.get_feature_property(
'notify', 'age-hours'))).exclude(ignore=True)
body_prefix = f"Message from {build_absolute_uri()}\n\n"
# Handle namespaces
for namespace in PCWConfig.get_namespaces_for('notify'):
receiver_email = PCWConfig.get_feature_property('notify', 'to', namespace)
namespace_objects = all_instances.filter(namespace=namespace)
if namespace_objects.filter(notified=False).count() > 0 and receiver_email:
send_mail(f'CSP left overs - {namespace}',
body_prefix + draw_instance_table(namespace_objects), receiver_email=receiver_email)
all_instances.update(notified=True)


def send_cluster_notification(namespace, clusters):
if len(clusters) and PCWConfig.has('notify'):
clusters_str = ''
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Generated by Django 5.0.9 on 2024-09-16 19:49

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('ocw', '0012_rename_vault_namespace_instance_namespace_and_more'),
]

operations = [
migrations.RemoveField(
model_name='instance',
name='notified',
),
migrations.AlterField(
model_name='instance',
name='provider',
field=models.CharField(choices=[('GCE', 'Google'), ('EC2', 'EC2'), ('AZURE', 'Azure')], max_length=8),
),
]
1 change: 0 additions & 1 deletion ocw/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ class Instance(models.Model):
instance_id = models.CharField(max_length=200)
region = models.CharField(max_length=64, default='')
namespace = models.CharField(max_length=64, default='')
notified = models.BooleanField(default=False)
ignore = models.BooleanField(default=False)
TAG_IGNORE = 'pcw_ignore'

Expand Down
14 changes: 0 additions & 14 deletions ocw/tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,6 @@ def render(self, record):
return ""


class MailColumn(tables.BooleanColumn):
@property
def header(self):
return ""

def render(self, value, record, bound_column):
value = self._get_bool_value(record, value, bound_column)
if value:
return format_html('<img alt="Email notification was send" src="{}" width=20 height=20/>',
static('img/notified.png'))
return ""


class TagsColumn(tables.TemplateColumn):

def __init__(self, template_name=None, **extra):
Expand All @@ -58,7 +45,6 @@ def header(self):

class InstanceTable(tables.Table):
tags = TagsColumn()
notified = MailColumn()
type = tables.Column(accessor=A('get_type'))
first_seen = tables.DateTimeColumn(format='M d Y')
last_seen = tables.DateTimeColumn(format='M d Y')
Expand Down
12 changes: 2 additions & 10 deletions tests/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,16 +179,12 @@ def mocked__update_provider(arg1, arg2, arg3):
def mocked_auto_delete_instances():
call_stack.append('auto_delete_instances')

def mocked_send_leftover_notification():
call_stack.append('send_leftover_notification')

monkeypatch.setattr('ocw.lib.db._update_provider', mocked__update_provider)
monkeypatch.setattr('ocw.lib.db.auto_delete_instances', mocked_auto_delete_instances)
monkeypatch.setattr('ocw.lib.db.send_leftover_notification', mocked_send_leftover_notification)

update_run()

assert call_stack == ['_update_provider', 'auto_delete_instances', 'send_leftover_notification']
assert call_stack == ['_update_provider', 'auto_delete_instances']


def test_update_run_update_provider_throw_exception(update_run_patch, monkeypatch):
Expand All @@ -202,20 +198,16 @@ def mocked__update_provider(arg1, arg2, arg3):
def mocked_auto_delete_instances():
call_stack.append('auto_delete_instances')

def mocked_send_leftover_notification():
call_stack.append('send_leftover_notification')

def mocked_send_mail(arg1, arg2):
call_stack.append('send_mail')

monkeypatch.setattr('ocw.lib.db._update_provider', mocked__update_provider)
monkeypatch.setattr('ocw.lib.db.auto_delete_instances', mocked_auto_delete_instances)
monkeypatch.setattr('ocw.lib.db.send_leftover_notification', mocked_send_leftover_notification)
monkeypatch.setattr('ocw.lib.db.send_mail', mocked_send_mail)

update_run()

assert call_stack == ['_update_provider', 'send_mail', 'auto_delete_instances', 'send_leftover_notification']
assert call_stack == ['_update_provider', 'send_mail', 'auto_delete_instances']


def test_delete_instances_azure(monkeypatch):
Expand Down
Loading