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

Update AWS ALB plan-patches #597

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jochenehret
Copy link
Contributor

  • this set of patch files works with the recent bbl version 9.0.17
  • note that there is still an ELB being created because you can't remove tf resources in _override files
  • the Route 53 DNS record points to the new ALB

* this set of patch files works with the recent bbl version 9.0.17
* note that there is still an ELB being created because you can't remove tf resources in _override files
* the Route 53 DNS record points to the new ALB
@jpalermo jpalermo requested review from a team, danielfor and nouseforaname and removed request for a team February 15, 2024 15:55
@nouseforaname
Copy link

hmm I think you should be able to use the _override to set the count for the replaced LB ( and all it's dependent resources) to 0 which should avoid creating it?

@jochenehret
Copy link
Contributor Author

I've tried to use count = 0, but then you have to use an index wherever cf_router_lb is used. (cf_router_lb[0]). That would complicate the _override file approach.

A better solution would be to enhance the --lb-type command line parameter. It currently accepts "cf" or "concourse" as values. We could add a new value "cf-alb" and clearly separate the AWS ELB Terraform templates from the new ALB templates here:

@ramonskie
Copy link
Contributor

i agree with the solution that we use --lb-type as this way its backwards compatible as well.
and its the least complicated thing to implement without refactoring most of the terraform files.

i do think we should update the README and docs for this so its know that a --lb-type is now required

@ramonskie
Copy link
Contributor

@jochenehret any update on this from your side?

@jochenehret
Copy link
Contributor Author

No update currently from my side. I can't promise that I can fully implement this feature (with the --lb-type option)...

@rkoster rkoster marked this pull request as draft May 16, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Pending Review | Discussion
Development

Successfully merging this pull request may close these issues.

4 participants