mdn / infra

(Deprecated) MDN Web Docs Infrastructure scripts and configuration
Mozilla Public License 2.0
53 stars 32 forks source link

SE-1773 - fix unnecessary diff with target-health #511

Closed bkochendorfer closed 3 years ago

bkochendorfer commented 3 years ago

I was applying the changes to upgrade the lambda runtime for mdn.dev and noticed this unnecessary diff. Removes the following from the terraform plan for this product.

  # module.dns-mdn-dev.aws_route53_record.main will be updated in-place                                                        
  ~ resource "aws_route53_record" "main" {                                                                                     
        allow_overwrite = true                                                                                                 
        fqdn            = "mdn.dev"                                                                                            
        id              = "Z2MZJ35C5P93MD_mdn.dev_A"           
        name            = "mdn.dev"                                                                                            
        records         = []                                                                                                   
        ttl             = 0                                                                                                                                                                                                                                   
        type            = "A"                                                                                                  
        zone_id         = "Z2MZJ35C5P93MD" 

      - alias {                                                                                                                
          - evaluate_target_health = false -> null                                                                             
          - name                   = "d2pgkrr49jx816.cloudfront.net" -> null
          - zone_id                = "Z2FDTNDATAQYW2" -> null                                                                  
        }                                                                                                                      
      + alias {                                                                                                                
          + evaluate_target_health = true                                                                                      
          + name                   = "d2pgkrr49jx816.cloudfront.net"                                                                                                                                                                                          
          + zone_id                = "Z2FDTNDATAQYW2"                                                                          
        }                                                                                                                      
    } 
escattone commented 3 years ago

Oops, I'm sorry @floatingatoll @duallain ! In my haste, I forgot to check for additional reviewers! 🙏

duallain commented 3 years ago

Oops, I'm sorry @floatingatoll @duallain ! In my haste, I forgot to check for additional reviewers! 🙏

All good, I'm mostly just reviewing everything to try to keep up with all the changes. If you're good with it, no worries merging.

floatingatoll commented 3 years ago

I wish there was a terraform comment explaining why we're disabling health checks, so that we can capture the "because dev doesn't need health checks" in case someday that changes. But r+ to making terraform match reality besides that.

On Mon, Aug 23, 2021 at 2:32 PM Alan @.***> wrote:

Oops, I'm sorry @floatingatoll https://github.com/floatingatoll @duallain https://github.com/duallain ! In my haste, I forgot to check for additional reviewers! 🙏

All good, I'm mostly just reviewing everything to try to keep up with all the changes. If you're good with it, no worries merging.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mdn/infra/pull/511#issuecomment-904148143, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAWUDC2LR4FI4DOQDKD6OTT6K47NANCNFSM5CVOCK6A .