-
Notifications
You must be signed in to change notification settings - Fork 7
Add kubernetes module for aws #236
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
Conversation
1abce85
to
17b6577
Compare
78e95f4
to
a0f49e5
Compare
60afe87
to
15ac1f9
Compare
This reverts commit 3d452bb.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looking good. Left some suggestions and comments.
modules/aws/kubernetes/vars.tf
Outdated
variable "kubernetes_cluster" { | ||
type = map | ||
default = {} | ||
description = "Custom definition kubernetes properties that include name of the cluster, kubernetes version, etc.." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description = "Custom definition kubernetes properties that include name of the cluster, kubernetes version, etc.." | |
description = "Custom definition kubernetes properties that include the name of the cluster, kubernetes version, etc.." |
} | ||
|
||
variable "kubernetes_node_groups" { | ||
description = "Map of map of node groups to create" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really a map of map? or typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, It's a map of maps.
@@ -0,0 +1,96 @@ | |||
resource "aws_iam_role" "eks_cluster" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to add a subsection for privileges in EKS deployment?
https://github.com/scalar-labs/scalar-terraform/blob/master/docs/CloudPrivileges.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, We need some privileges like below.
https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/docs/iam-permissions.md
@@ -0,0 +1,45 @@ | |||
# Optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As with the other examples, the default region should be here at the top of example.tfvars
.
(I didn't use the default ap-northeast-1
region and I got lost when terraform apply
couldn't find the VPC that the network module created.)
# Optional | |
region = "ap-northeast-1" | |
# Optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! 👍
Also added backend.tf.s3
and remote.tf.s3
in 41f7f45
@tei-k Question. I got the following kube_config output after clusters:
- cluster:
server: https://D0BE5C2FAD77ED66ED29817523BB7182.yl4.us-west-1.eks.amazonaws.com
certificate-authority-data: LS0tLS1CRUdJTiBDRV... but the server host in that file resolves to private IP addresses. Because of this $ host d0be5c2fad77ed66ed29817523bb7182.yl4.us-west-1.eks.amazonaws.com
d0be5c2fad77ed66ed29817523bb7182.yl4.us-west-1.eks.amazonaws.com has address 10.42.1.144
d0be5c2fad77ed66ed29817523bb7182.yl4.us-west-1.eks.amazonaws.com has address 10.42.41.117 Can you tell what is happening? |
Good point! 👍 Sorry for missing of explanation on this point. Because Can you first create environment with example.tfvars
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Please create a PR for the update in scalardl-custom-values.yaml
once this PR is merged.
Also, please write a getting started doc once Yusuke's doc reorganization is completed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It worked. Thanks. LGTM!
Description
https://scalar-labs.atlassian.net/browse/DLT-7400
Done
Confirm
scalar-k8/conf
)${eks_cluster_name}_kubeconfig
)scalar-k8/conf/scalardl-custom-values.yaml
scalar-k8/
)Ref