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

Wait for kubernetes API to be ready during EKS cluster creation #11426

Closed

Conversation

barryib
Copy link

@barryib barryib commented Dec 26, 2019

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment 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 pull request followers and do not help prioritize the request

When creating an EKS cluster, the API return an ACTIVE status before the kubernetes endpoint become healthy and ready to process request.

This PR, will add the ability to wait for kubernetes to become ready by checking the \healthz endpoint.

This is a kind of proof of concept and I'm opening a PR to get feedback from the community. There is already and opened issue in the AWS's containers-roadmap, but I don't know how they will handle this (if they do).

Relates

@barryib barryib requested a review from a team December 26, 2019 23:52
@ghost ghost added needs-triage Waiting for first response or review from a maintainer. size/M Managed by automation to categorize the size of a PR. service/eks Issues and PRs that pertain to the eks service. documentation Introduces or discusses updates to documentation. labels Dec 26, 2019
@barryib barryib force-pushed the tba-eks-wait-for-endpoint branch from 7e18b45 to 2099b6b Compare December 27, 2019 00:08
@barryib barryib changed the title Wait kubernetes API to be ready before during EKS cluster creation Wait for kubernetes API to be ready during EKS cluster creation Dec 27, 2019
@bflad bflad added proposal Proposes new design or functionality. upstream Addresses functionality related to the cloud provider. and removed needs-triage Waiting for first response or review from a maintainer. labels Jan 2, 2020
@barryib
Copy link
Author

barryib commented Jan 19, 2020

@bflad any chance to get this reviewed ?

@valll94
Copy link

valll94 commented Apr 7, 2020

Any update on this issue ?

@justolka
Copy link

justolka commented Jun 4, 2020

When will the feature be available? I need that so much

@kevinayres
Copy link

We need this to be merged please. 👍

@max-rocket-internet
Copy link

@bflad any chance of a review?

@barryib
Copy link
Author

barryib commented Oct 4, 2020

@bflad @terraform-providers/aws-provider Any chance to review this PR ? It's almost 10 months now since I open this PR without any reaction from maintainers. Is this something we want to handle in this provider or not ?

@bflad
Copy link
Contributor

bflad commented Oct 5, 2020

Hi @barryib 👋 My apologies that I did not leave a note back in January when I added the proposal label on why this is a proposal. This implementation seems fine but the overall design currently falls outside what the Terraform AWS Provider should reasonably handle as this is adding raw HTTP request handling to an AWS API/SDK handling Terraform Provider. We typically make this distinction to ensure the long-term maintainability of the provider codebase (since the maintainers here work primarily with the AWS Go SDK and APIs, but not all underlying functionality of every AWS service), expectations within the community for the behaviors of a single Terraform Provider, and prevent unexpected behaviors for security conscious implementations.

In this case, the expectations here would be that any of the following occurs:

  • The EKS API could appropriately return the correct status of the cluster (e.g. [EKS] [request]: On create: only return ACTIVE when endpoint actually usable aws/containers-roadmap#654).
  • The Terraform HTTP Provider (where raw HTTP request handlers in the Terraform Provider ecosystem should be implemented) could support the ability to retry operations. This feature request would need to be raised to that provider, if it is not already present. For what its worth, this would help get rid of the DataSync and Storage Gateway HTTP handling as well which is its own current maintenance burden and only present due to the lack of this feature.
  • Downstream Terraform Providers (e.g. Terraform Kubernetes Provider) could optionally handle the possibility of a few retries to perform their operations on provider initialization or resource creation. Feature requests should be raised to those providers.
  • Terraform configurations can be split so the Kubernetes handling is separate and globally retryable for errors like these

Given that all of these fall outside any efforts that would occur within the Terraform AWS Provider itself, it means that we would close this out. Again, this is not because this is not an unreasonable implementation, but due to the design considerations. So thank you for putting this together, however I think this better suited handling in other manners. I would encourage those following this to file AWS Support cases denoting the unexpected EKS API behavior or submit/upvote the other Terraform Provider features mentioned above.

@bflad bflad closed this Oct 5, 2020
@barryib
Copy link
Author

barryib commented Oct 5, 2020

Thanks @bflad for your feedback. It make sense. I'll look to the http provider to see if we can handle retry and custom CA certificate.

In the meantime, the issue is still open aws/containers-roadmap#654 and it sounds that AWS is not able for now to reproduce this problem.

@ghost
Copy link

ghost commented Nov 4, 2020

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Nov 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. proposal Proposes new design or functionality. service/eks Issues and PRs that pertain to the eks service. size/M Managed by automation to categorize the size of a PR. upstream Addresses functionality related to the cloud provider.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants