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

Remove duplicate EC2 cleanup jobs #1151

Merged
merged 2 commits into from
Apr 3, 2023
Merged

Remove duplicate EC2 cleanup jobs #1151

merged 2 commits into from
Apr 3, 2023

Conversation

achantavy
Copy link
Contributor

Fast follow of #1146 - removes duplicate calls to cleanup jobs. All these manual jobs will soon be deprecated as we refactor modules to the new data model.

@@ -1,20 +0,0 @@
{
"statements": [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is covered by autocleanups in models.aws.ec2.keypairs

Copy link
Collaborator

Choose a reason for hiding this comment

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

Really awesome that we can do this partial refactor

@@ -1,10 +1,5 @@
{
"statements": [
{
"query": "MATCH (n:EC2SecurityGroup)<-[:RESOURCE]-(:AWSAccount{id: $AWS_ID}) WHERE n.lastupdated <> $UPDATE_TAG WITH n LIMIT $LIMIT_SIZE DETACH DELETE (n)",
Copy link
Contributor Author

@achantavy achantavy Apr 3, 2023

Choose a reason for hiding this comment

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

This is covered by autocleanups in models.aws.ec2.securitygroups. Additionally the autocleanups also handle a previously unhandled case where we need to cleanup stale rels between SGs and instances.

@@ -1,20 +0,0 @@
{
"statements": [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are covered by autocleanups in models.aws.ec2.volumes.

@@ -20,25 +20,10 @@
"iterationsize": 100,
"iterative": true
},
{
"query": "MATCH (:AWSAccount{id: $AWS_ID})-[:RESOURCE]->(:EC2Instance)-[r:PART_OF_SUBNET]->(:EC2Subnet) WHERE r.lastupdated <> $UPDATE_TAG WITH r LIMIT $LIMIT_SIZE DELETE (r)",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is covered by autocleanups in models.aws.ec2.subnets.

{
"query": "MATCH (:AWSAccount{id: $AWS_ID})-[:RESOURCE]->(:LoadBalancer)-[r:PART_OF_SUBNET]->(:EC2Subnet) WHERE r.lastupdated <> $UPDATE_TAG WITH r LIMIT $LIMIT_SIZE DELETE (r)",
"iterationsize": 100,
"iterative": true
},
{
"query": "MATCH (:AWSAccount{id: $AWS_ID})-[:RESOURCE]->(n:NetworkInterface) WHERE n.lastupdated <> $UPDATE_TAG WITH n LIMIT $LIMIT_SIZE DETACH DELETE (n)",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is covered by autocleanups in models.aws.ec2.networkinterfaces.

@@ -308,11 +308,6 @@ def cleanup(neo4j_session: neo4j.Session, common_job_parameters: Dict[str, Any])
logger.debug("Running EC2 instance cleanup")
GraphJob.from_node_schema(EC2ReservationSchema(), common_job_parameters).run(neo4j_session)
GraphJob.from_node_schema(EC2InstanceSchema(), common_job_parameters).run(neo4j_session)
GraphJob.from_node_schema(EC2SubnetSchema(), common_job_parameters).run(neo4j_session)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are all moved from here to their respective modules. Specifically,

  • EC2SubnetSchema -> intel.aws.ec2.subnets.cleanup_subnets
  • EC2SecurityGroupSchema -> intel.aws.ec2.security_groups.cleanup_ec2_security_groupinfo
  • EC2KeyPairSchema -> intel.aws.ec2.key_pairs.cleanup_ec2_key_pairs
  • EC2NetworkInterfaceSchema -> intel.aws.ec2.network_interfaces.cleanup_network_interfaces
  • EBSVolumeSchema -> intel.aws.ec2.cleanup_volumes

@@ -1,20 +0,0 @@
{
"statements": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Really awesome that we can do this partial refactor

@@ -146,6 +148,7 @@ def cleanup_ec2_security_groupinfo(neo4j_session: neo4j.Session, common_job_para
neo4j_session,
common_job_parameters,
)
GraphJob.from_node_schema(EC2SecurityGroupSchema(), common_job_parameters).run(neo4j_session)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We still need to keep both, for now, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this is intentional because the other resources cleaned up in aws_import_ec2_security_groupinfo_cleanup.json have not been refactored to the new model yet.

@achantavy achantavy merged commit 672b5fe into master Apr 3, 2023
@achantavy achantavy deleted the fixcleanupec2 branch April 3, 2023 21:18
chandanchowdhury pushed a commit to juju4/cartography that referenced this pull request Jun 26, 2024
Fast follow of cartography-cncf#1146 - removes
duplicate calls to cleanup jobs. All these manual jobs will soon be
deprecated as we refactor modules to the new data model.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants