Skip to content

Update AWS ALB plan-patches #597

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.

Sign up for GitHub

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

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

Update AWS ALB plan-patches #597

wants to merge 1 commit into from
+162 −148

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

@jochenehret
Update AWS ALB plan-patches
0061023 
* 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:

bosh-bootloader/terraform/aws/template_generator.go

Line 42 in d1a959a

switch state.LB.Type {

@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
@ramonskie ramonskie mentioned this pull request Mar 8, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielfor danielfor Awaiting requested review from danielfor danielfor was automatically assigned from cloudfoundry/wg-foundational-infrastructure-vm-deployment-lifecycle-bosh-approvers

@nouseforaname nouseforaname Awaiting requested review from nouseforaname nouseforaname was automatically assigned from cloudfoundry/wg-foundational-infrastructure-vm-deployment-lifecycle-bosh-approvers

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
unscheduled
Projects
Foundational Infrastructure Working Group
Status: Pending Review | Discussion
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jochenehret @nouseforaname @ramonskie @cf-gitbot