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

CoreDNS settings are not appropriate when using fargate profile for kube-system namespace #11327

Closed
ghost opened this issue Dec 17, 2019 · 9 comments
Labels
service/eks Issues and PRs that pertain to the eks service.

Comments

@ghost
Copy link

ghost commented Dec 17, 2019

This issue was originally opened by @sflaherty2009 as hashicorp/terraform#23692. It was migrated here as a result of the provider split. The original body of the issue is below.


Terraform Version

Terraform v0.12.16

Expected Behavior

When spinning up a cluster with AWS EKS with Fargate profiles for kube-system CoreDNS pods appropriately builds and starts appropriately

Actual Behavior

When spinning up a cluster with AWS EKS with Fargate profiles for kube-system CoreDNS pods go to a pending state due to a node not being available with appropriate labels. The label that it is looking for is eks.amazonaws.com/compute-type : ec2. This label does not exist with Fargate instances and must be removed prior to CoreDNS working appropriately. This can be removed with kubectl patch deployment coredns -n kube-system --type json -p='[{"op": "remove", "path": "/spec/template/metadata/annotations/eks.amazonaws.com~1compute-type"}]' as outlined in AWS documentation. https://docs.aws.amazon.com/eks/latest/userguide/fargate-getting-started.html

Steps to Reproduce

Spin up Fargate profile for the kube-system namespace.

Additional Context

References

@IanMoroney
Copy link

This is definitely a problem.
Without having the manual command, the profile won't start.

@bflad bflad added service/eks Issues and PRs that pertain to the eks service. and removed needs-triage Waiting for first response or review from a maintainer. labels Dec 20, 2019
@bflad
Copy link
Contributor

bflad commented Dec 20, 2019

Please see my response to a similar issue here: #11383 (comment)

Most importantly:

Managing Kubernetes itself is beyond the scope of the Terraform AWS Provider and the maintainers knowledge here, however there is the Terraform Kubernetes Provider as one of many options within the Terraform ecosystem.

For additional context, this comment is referring to the available EKS APIs, which the Terraform AWS Provider interacts with the EKS service. Interactions with Kubernetes can be handled via the mentioned Terraform Kubernetes Provider, or potentially solutions such as using the null_resource resource with the local-exec provisioner.

Since this type of functionality is out of scope of this particular codebase unless the service adds API functionality, I'm going to close this issue.

The maintainers here can only suggest that you reach out to AWS via https://github.com/aws/containers-roadmap, a feature request AWS Support case, or talking to your AWS account managers to have the EKS service automatically remove that annotation, implement API functionality to update/remove annotations, or some other mechanism to improve the handling of this problem for automated deployments.

If you do feel there is a deficiency in the Terraform AWS Provider documentation or an actual bug with the Terraform AWS Provider's interaction with the available EKS APIs however, please do create another issue and we'll take another look. 👍

@bflad bflad closed this as completed Dec 20, 2019
@IanMoroney
Copy link

I disagree with this assessment.

This issue is not related to kubernetes management.

This issue is highlighting a bug that states that the annotation that aws_eks_fargate_profile attaches to the resultant pod definitions is wrong.

It is attaching:
eks.amazonaws.com/compute-type: ec2
when it should be attaching
eks.amazonaws.com/compute-type: fargate

This is a bug in the provider, and not a Kubernetes management issue.

@bflad
Copy link
Contributor

bflad commented Dec 20, 2019

Hi again, @IanMoroney

The Terraform AWS Provider is issuing the EKS CreateFargateProfile API call here:

https://github.com/terraform-providers/terraform-provider-aws/blob/144b987a34c2bdd7011687546f322851b0ad79f8/aws/resource_aws_eks_fargate_profile.go#L100-L135

Any logic for the Kubernetes annotations within the Kubernetes deployments of the EKS Cluster is managed by the EKS service itself. The Terraform AWS Provider can only provide what is available in the API and the API parameters do not seem to give any ability to configure those annotations.

If you have a bug report to file with EKS, you'll need to reach out to AWS Support (the Terraform AWS Provider is unaffiliated with AWS except for business relationships behind the scenes). If I am missing something that we can control with the EKS API and AWS Go SDK, please do let us know.

@IanMoroney
Copy link

eksctl handles this on their side in order to ensure that coredns gets scheduled correctly:

https://github.com/weaveworks/eksctl/blob/master/pkg/fargate/coredns/coredns.go

They set compute-type to be fargate in their logic. We should be doing the same to ensure that coredns gets scheduled properly.

@bflad
Copy link
Contributor

bflad commented Dec 20, 2019

https://github.com/weaveworks/eksctl/blob/master/pkg/fargate/coredns/coredns.go

This file references the Kubernetes Go client (e.g. kubeclient "k8s.io/client-go/kubernetes") for managing Kubernetes, not the AWS Go SDK (e.g. "github.com/aws/aws-sdk-go/service/eks") which would talk to the EKS API. I apologize if this answer is unsatisfactory, but we cannot afford to maintain Kubernetes Go client support within this codebase. Connecting to and configuring Kubernetes clusters may seem like an easy task at first, but to do it properly is a large undertaking, and supporting even one piece of Kubernetes functionality within this Terraform Provider sets an unmaintainable precedent.

Terraform Providers are generally designed around managing one underlying "API client". We enforce this separation of concerns so each Terraform Provider can be separately maintained on its own development cycle with experts in that particular functionality (see also the list of available Terraform Providers). The Terraform AWS Provider boundary is the AWS API functionality provided by the AWS service APIs. We explicitly draw boundaries on the functionality provided by the Terraform AWS Provider since AWS' abilities cover many varying infrastructure components (e.g. many types of databases with RDS, Kafka with MSK, Kubernetes with EKS, etc.) and we do not intend to support all the underlying infrastructure management (e.g. MySQL client, Kafka client, Kubernetes client, etc.) within this one Terraform Provider as it would be unsustainable and cause a sub-par experience.

The Terraform Kubernetes Provider is one of the canonical Terraform Providers designed and maintained against the Kubernetes Go client. That codebase and its published provider should have the functionality being requested to connect to Kubernetes and configure it appropriately. If not, please create a feature request in the Terraform Kubernetes Provider repository.

@IanMoroney
Copy link

Ah ok, I see what you're saying now.
So, it's not possible to set the compute-type on profile creation... it has to be created in a "broken" fashion, then a post deployment command used to correct the annotations.

I understand why this wouldn't want to be baked into the provider.

I was under the impression that because the launch-type was being set to ec2 in the first place, that it was something the provider or the API had control over, but it seems like that's not the case.

Thank you for the explanation @bflad

@antonosmond
Copy link

FWIW if you REALLY want to do this in terraform and you know kubectl will be available locally, you can do something like this:

data "aws_eks_cluster_auth" "main" {
  name = aws_eks_cluster.main.name
}

data "template_file" "kubeconfig" {
  template = <<EOF
apiVersion: v1
kind: Config
current-context: terraform
clusters:
- name: main
  cluster:
    certificate-authority-data: ${aws_eks_cluster.main.certificate_authority.0.data}
    server: ${aws_eks_cluster.main.endpoint}
contexts:
- name: terraform
  context:
    cluster: main
    user: terraform
users:
- name: terraform
  user:
    token: ${data.aws_eks_cluster_auth.main.token}
EOF
}

resource "null_resource" "coredns_patch" {
  provisioner "local-exec" {
    interpreter = ["/bin/bash", "-c"]
    command     = <<EOF
kubectl --kubeconfig=<(echo '${data.template_file.kubeconfig.rendered}') \
  patch deployment coredns \
  --namespace kube-system \
  --type=json \
  -p='[{"op": "remove", "path": "/spec/template/metadata/annotations", "value": "eks.amazonaws.com/compute-type"}]'
EOF
  }
}

I didn't promise it'd be pretty 😝, but it does work and also uses the config directly from the cluster you're creating without relying on a local kubeconfig.

@ghost
Copy link
Author

ghost commented Feb 14, 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 and limited conversation to collaborators Feb 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
service/eks Issues and PRs that pertain to the eks service.
Projects
None yet
Development

No branches or pull requests

3 participants