Skip to content

Conversation

@zacharyblasczyk
Copy link
Contributor

@zacharyblasczyk zacharyblasczyk commented Mar 29, 2024

This needs to go in after https://github.com/wandb/terraform-aws-wandb/pull/196/files

Given these vars as a constant:

domain_name   = "sandbox-aws.wandb.ml"
subdomain     = "smooth-operator"
extra_fqdn    = ["rough-operator.sandbox-aws.wandb.ml"]

I went from

module "wandb_infra" {
  source  = "wandb/wandb/aws"
  version = "4.7.0"

  #we have this set for  Woven Japan and wesfarmers
  enable_dummy_dns    = false 
  enable_operator_alb = false

to:

module "wandb_infra" {
  source = "github.com/wandb/terraform-aws-wandb.git?ref=zacharyb/extra-fqdn-operator"

  enable_dummy_dns    = false
  enable_operator_alb = false

with:

No changes. Your infrastructure matches the configuration.
Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.
Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Step 2. Enable

Then I enabled:

  enable_dummy_dns    = true
  enable_operator_alb = true

module.wandb_infra.module.app_lb.aws_route53_record.alb gets replaced.

"smooth-operator.sandbox-aws.wandb.ml" -> "old-smooth-operator.sandbox-aws.wandb.ml"

module.wandb_infra.module.wandb.helm_release.wandb will be updated in-place with the following:

"external-dns.alpha.kubernetes.io/hostname": |
                        smooth-operator.sandbox-aws.wandb.ml\,rough-operator.sandbox-aws.wandb.ml

module.wandb_infra.module.app_eks.module.external_dns.helm_release.external_dns will get

      + set {
          + name  = "domainFilters[1]"
          + value = "rough-operator.sandbox-aws.wandb.ml"
        }

Outcome:

kubectl get ingress 
NAME    CLASS   HOSTS                                                                      ADDRESS                                                         PORTS   AGE
wandb   alb     smooth-operator.sandbox-aws.wandb.ml,rough-operator.sandbox-aws.wandb.ml   smooth-operator-alb-k8s-735018499.us-east-2.elb.amazonaws.com   80      2d21h
kubectl get po -n kube-system external-dns-569b49b6fb-9mz8v -oyaml | grep domain-filter  
    - --domain-filter=sandbox-aws.wandb.ml
    - --domain-filter=rough-operator.sandbox-aws.wandb.ml

@zacharyblasczyk zacharyblasczyk requested a review from gls4 as a code owner March 29, 2024 19:57
length(var.extra_fqdn) > 0 && var.enable_dummy_dns ? {
"external-dns.alpha.kubernetes.io/hostname" = <<-EOF
${local.fqdn}\,${join("\\,", var.extra_fqdn)}\,${local.fqdn}
EOF
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I use the local.fqdn twice is because of a bug with external dns not reading this form of the annotation correctly:

      external-dns.alpha.kubernetes.io/hostname: |
        smooth-operator.sandbox-aws.wandb.ml,rough-operator.sandbox-aws.wandb.ml

In order to get around this, I added ${local.fqdn} to the end which works just perfectly. 😢

time="2024-03-29T21:23:33Z" level=debug msg="Endpoints generated from ingress: default/wandb: [smooth-operator.sandbox-aws.wandb.ml 0 IN CNAME  smooth-operator-alb-k8s-388333795.us-east-2.elb.amazonaws.com [] rough-operator.sandbox-aws.wandb.ml 0 IN CNAME  smooth-operator-alb-k8s-388333795.us-east-2.elb.amazonaws.com [] smooth-operator.sandbox-aws.wandb.ml\n 0 IN CNAME  smooth-operator-alb-k8s-388333795.us-east-2.elb.amazonaws.com []]"

Copy link
Contributor

Choose a reason for hiding this comment

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

That sure doesn't look perfect to me with a duplicated entry in the endpoints generated. Is there something goofy we're doing with writing out these annotations that it ends up in an incompatible format or is it just good ol' externaldns madness?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

External DNS madness. I am actually creating an issue now in https://github.com/kubernetes-sigs/external-dns/issues. Will comment when I create it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@adityachoudhari26 adityachoudhari26 left a comment

Choose a reason for hiding this comment

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

approved but only merge after #196 is in

@zacharyblasczyk
Copy link
Contributor Author

approved but only merge after #196 is in

I believe @gls4 got this resolved a different way.

@zacharyblasczyk zacharyblasczyk changed the title feat: Adding extra_fqdn support for operator fix: Adding missing extra_fqdn support for operator that was supported previously Apr 16, 2024
@zacharyblasczyk zacharyblasczyk requested a review from nfoucha April 16, 2024 20:28
@zacharyblasczyk zacharyblasczyk merged commit 7adf420 into main Apr 17, 2024
@zacharyblasczyk zacharyblasczyk deleted the zacharyb/extra-fqdn-operator branch April 17, 2024 15:16
jsbroks pushed a commit that referenced this pull request Apr 17, 2024
### [4.7.1](v4.7.0...v4.7.1) (2024-04-17)

### Bug Fixes

* Adding missing extra_fqdn support for operator that was supported previously ([#197](#197)) ([7adf420](7adf420))
@jsbroks
Copy link
Member

jsbroks commented Apr 17, 2024

This PR is included in version 4.7.1 🎉

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.

5 participants