Roundrobin DNS for a shared hostname #4
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This pull request introduces round-robin DNS functionality for shared hostnames in Auto Scaling Groups, updating the module to Terraform 0.12+ syntax and adding multi-host handling capabilities.
- Added new
lambda/multihost/multihost.pyfor processing SNS events and managing round-robin DNS entries - Updated
main.tfwith new SNS topic and Lambda function for multi-host handling, using Terraform 0.12+ syntax - Modified
README.mdto include usage instructions for single DNS name configurations and round-robin DNS - Introduced
use_public_ipvariable and updated Python runtime for Lambda functions to 3.8 - Added new output
multihost_handling_sns_topic_arninoutputs.tffor the multi-host feature
5 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings
| try: | ||
| ip_address = ec2_response['PublicIpAddress'] | ||
| logger.info("Found public IP for instance-id %s: %s", instance_id, ip_address) | ||
| except: |
There was a problem hiding this comment.
style: Consider using a more specific exception type instead of a bare except clause. This will help in catching and handling specific errors.
| try: | ||
| ip_address = ec2_response['PrivateIpAddress'] | ||
| logger.info("Found private IP for instance-id %s: %s", instance_id, ip_address) | ||
| except: |
There was a problem hiding this comment.
style: Similar to the previous comment, use a specific exception type here as well.
| ip_address = route53.list_resource_record_sets( | ||
| HostedZoneId=zone_id, | ||
| StartRecordName=hostname, | ||
| StartRecordType='A', | ||
| MaxItems='1' | ||
| )['ResourceRecordSets'][0]['ResourceRecords'][0]['Value'] |
There was a problem hiding this comment.
logic: This API call assumes the record exists. Add error handling for cases where the record is not found.
| logger.info("Finishing ASG action") | ||
| message = json.loads(record['Sns']['Message']) | ||
| if LIFECYCLE_KEY in message and ASG_KEY in message : |
There was a problem hiding this comment.
logic: This code assumes 'record' is still in scope from the previous loop. Move it inside the loop to ensure it's always defined.
|
|
||
| filename = data.archive_file.multihost.output_path | ||
| function_name = format("%s-%s-multi", var.vpc_name, var.autoscale_handler_unique_identifier) | ||
| role = aws_iam_role.autoscale_handling.arn |
There was a problem hiding this comment.
style: Reuse of autoscale_handling role for multihost. Consider creating a separate role for better separation of concerns
Creating DNS for multihost entries aka roundrobin DNS.
Updated to TF 12
This addresses meltwater#33
For the repo owner
If you want people such as myself raising PR's please consider adding
CODEOWNERSandContributing.mdfiles.The
CODEOWNERSwill auto add reviewers andContributing.mdwould detail any other process / code quality etc that submitters should follow