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

Feature/island fixes #198

Merged
merged 19 commits into from
Jul 21, 2021
Merged

Feature/island fixes #198

merged 19 commits into from
Jul 21, 2021

Conversation

rpoluri
Copy link
Contributor

@rpoluri rpoluri commented Jul 20, 2021

📝 Description

Configuration to use IAM roles for service accounts(IRSA).

🔗 Related Issues

@rpoluri rpoluri requested a review from a team as a code owner July 20, 2021 22:51
@@ -37,6 +37,8 @@ resource "kubernetes_deployment" "apiary_hms_readonly" {
}

spec {
service_account_name = kubernetes_service_account.hms_readonly[0].metadata.0.name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure if we set hms_instance_type to ecs, all references to [0] will fail with this error: hashicorp/terraform#22480

Can we just change them to [count.index]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if that is still happening with current version or setup, if so we probably need to fix it here

name = kubernetes_secret.hms_secrets[0].metadata[0].name

I will test ecs deployment mode in new island.

name = "${local.hms_alias}-readwrite"
namespace = var.k8s_namespace
annotations = {
"eks.amazonaws.com/role-arn" = var.oidc_provider == "" ? "" : aws_iam_role.apiary_hms_readwrite.arn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are using kiam, will this annotation having a value of "" cause any issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested this scenario, IAM using kiam is working as expected when eks.amazonaws.com/role-arn = "", but is not working if configure role as IRSA is overriding kiam

@@ -0,0 +1,23 @@
resource "kubernetes_service_account" "hms_readwrite" {
count = var.hms_instance_type == "k8s" ? 1 : 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we always want to create the service accounts when using k8s, or only when we are not using kiam?

Copy link
Contributor Author

@rpoluri rpoluri Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

creating service accounts when using kiam will not cause any issues, so creating always to keep terraform setup simpler, but we will also probably deprecate kiam as IRSA is working in clusters with Kiam

variables.tf Outdated
default = "monitoring"
}

variable "k8s_namespace" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rename k8s_namespace to metastore_namespace or something more descriptive?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add to VARIABLES.md

variables.tf Show resolved Hide resolved
variables.tf Show resolved Hide resolved
@rpoluri rpoluri merged commit b5a3b26 into master Jul 21, 2021
@rpoluri rpoluri deleted the feature/island_fixes branch July 21, 2021 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants