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

Ability to add retries on data "aws_lb" #26026

Closed
domeales-paloit opened this issue Jul 28, 2022 · 4 comments · Fixed by #26121
Closed

Ability to add retries on data "aws_lb" #26026

domeales-paloit opened this issue Jul 28, 2022 · 4 comments · Fixed by #26121
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/elbv2 Issues and PRs that pertain to the elbv2 service.
Milestone

Comments

@domeales-paloit
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Terraform is rarely the only system/tool that we use for managing infrastructure and when multiple systems are running independently it can be good to link their processes together. I would like to suggest that aws_lb data resources can have an optional retry period to allow the Terraform apply to "wait" for a resource that is being provisioned by another tool/process.

For my use-case, I am attempting to coordinate the creation of the Route53 record for a Load Balancer that is created by an EKS Service. The process for the deployment of the Ingress controller into EKS (and therefore creation of the Load Balancer) is loosely triggered by the Terraform project, but it is difficult to know when exactly the Load Balancer will be ready. Once the Load Balancer is ready it is easy to configure the Route53 record using the data "aws_lb" resource.

There can be timeouts for resources, but it seems not yet for data resources.

Using something like time_sleep is inefficient since it waits the total time every apply.

I would like to add the ability to do retries (via a timeout) on the aws_lb data resource to wait until it is has been provisioned by the parallel process.

New or Affected Resource(s)

  • aws_lb (data resource)

Potential Terraform Configuration

data "aws_lb" "ingress_load_balancer" {
  tags = {
    "service.k8s.aws/resource" = "LoadBalancer"
    "service.k8s.aws/stack"    = "ingress-nginx/ingress-nginx-controller"
    "elbv2.k8s.aws/cluster"    = "my-eks-cluster"
  }

  timeouts {
    read = "10m"
  }
}

Result: Blocks of type "timeouts" are not expected here.

References

@domeales-paloit domeales-paloit added the enhancement Requests to existing resources that expand the functionality or scope. label Jul 28, 2022
@github-actions github-actions bot added needs-triage Waiting for first response or review from a maintainer. service/elbv2 Issues and PRs that pertain to the elbv2 service. labels Jul 28, 2022
@justinretzolk
Copy link
Member

justinretzolk commented Jul 28, 2022

Hey @domeales-paloit 👋 Thank you for taking the time to raise this! My initial thought is that this could be accomplished by using something like the external data source or a null_resource with a local-exec provisioner to run a script to check for when the load balance has been created (or the external job has finished up), and then adding the appropriate values to a depends_on block within the aws_lb data source. This would set up the dependency for the aws_lb data source using currently available tools, and ensure that the data source runs as soon as it's appropriate.

To expand on this thought, I'm not certain that adding the timeout to the data source would necessarily achieve the functionality you're looking for. In the case where the LB never comes into existence, or if the external process took longer than the time defined in the timeout, it would only delay the inevitable error being thrown. With that said, I'd like to leave this issue open in case anyone from the provider team or community has any additional thoughts that I might not be considering. In the meantime, I hope the above suggestions help!

For tracking:
Related: #11342

@justinretzolk justinretzolk removed the needs-triage Waiting for first response or review from a maintainer. label Jul 28, 2022
@domeales-paloit
Copy link
Author

domeales-paloit commented Jul 28, 2022

Hey @justinretzolk - Thanks for your response!

I think that everyone who has had this issue has used either null_resource or external_data_source. Both of these approaches have their problems (additionally null_data_resource is pretty much useless for "modern" Terraform).

For null_resource, the resource is managed as a resource and not a data source. It is not easy to cleanly extract data from the process, and it has the problems of having a resource lifecycle (when what is needed is data, and the retrieval of data). Additionally, being a resource (rather than a data source) it is hard to get the triggers to work as expected. If triggering every time, then it shows up in the plan, and results of what it does can not be used by Terraform to logically make decisions about changes to dependent resources (eg. Route53). Therefore, it shows that the Route53 record might change every time.

For external_data_source, the design of this data resource it hard to use. It requires simple JSON input and output and does not have the execution environment features of null_resource (ie. environment variables). The requirement for JSON input and output enforces the use of another language/interpreter to achieve the desired result. (This is on top of the requirement for aws or kubectl tools to be available.)

With both external_data_source and null_resource we need to pass in the credential needed to access the resources. For AWS this can be non-trivial, and for EKS this means creating a KUBECONFIG YAML. Additionally, with both these resources additional executables are required to achieve the outcome.

In regards to your point that the timeout might not necessarily achieve the outcome, you are correct. This is not an issue of the proposed solution, it is rather an issue of the use of a timeout in general. Obviously it is a design decision when using the aws_lb data resource with a timeout to choose an appropriate amount of time to achieve the coupling between the two processes.

@github-actions
Copy link

github-actions bot commented Aug 4, 2022

This functionality has been released in v4.25.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

github-actions bot commented Sep 4, 2022

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/elbv2 Issues and PRs that pertain to the elbv2 service.
Projects
None yet
2 participants