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

Expose EFS toggle #109

Closed
blancharda opened this issue Mar 17, 2023 · 9 comments · Fixed by #129
Closed

Expose EFS toggle #109

blancharda opened this issue Mar 17, 2023 · 9 comments · Fixed by #129
Assignees

Comments

@blancharda
Copy link

The upstream blueprint module for EKS addons exposes enable_aws_efs_csi_driver as an input.

It would be nice to be able to toggle it on in our module, similar to EBS.

@blancharda
Copy link
Author

We may also want to consider setting the helm chart config to add a storage class.
Pretty sure this can be accomplished with something like:

aws_efs_csi_driver_helm_config = {
    storageClasses = [
        {
            name = "efs-dyn-sc"
            ...
        }
   ]
}

@blancharda
Copy link
Author

For reference, @ntwkninja found an example usage of it here.

Note the addition of some supplemental resources (namely, a storage class, and some security group goodness):

#####################
# Supporting Resources
#####################

resource "kubernetes_storage_class_v1" "efs" {
  metadata {
    name = "efs-dyn-sc"
  }

  storage_provisioner = "efs.csi.aws.com"
  parameters = {
    provisioningMode = "efs-ap" # Dynamic provisioning
    fileSystemId     = module.efs.id
    directoryPerms   = "700"
  }

  mount_options = [
    "iam"
  ]

  depends_on = [
    module.eks
  ]
}

module "efs" {
  source  = "terraform-aws-modules/efs/aws"
  version = "~> 1.0"

  creation_token = local.name
  name           = local.name

  # Mount targets / security group
  mount_targets = {
    for k, v in zipmap(module.vpc.azs, module.vpc.private_subnets) : k => { subnet_id = v }
  }
  security_group_description = "${local.name} EFS security group"
  security_group_vpc_id      = module.vpc.vpc_id
  security_group_rules = {
    vpc = {
      # relying on the defaults provdied for EFS/NFS (2049/TCP + ingress)
      description = "NFS ingress from VPC private subnets"
      cidr_blocks = module.vpc.private_subnets_cidr_blocks
    }
  }

  tags = local.tags
}

Note, the above snippet has been adjusted to match the naming/structure of our examples.

@blancharda
Copy link
Author

One last(™️) one..
Might be worth also adding the EFS endpoint to the VPC module like so:

    efs = {
      service             = "elasticfilesystem"
      private_dns_enabled = true
      subnet_ids          = module.vpc.private_subnets
      security_group_ids  = [aws_security_group.vpc_tls.id]
    },

@JaseKoonce JaseKoonce self-assigned this Mar 28, 2023
@JaseKoonce JaseKoonce linked a pull request Mar 30, 2023 that will close this issue
@JaseKoonce
Copy link
Contributor

@blancharda, was there a use case you had in mind for adding the EFS endpoint?

@blancharda
Copy link
Author

blancharda commented Mar 30, 2023

This isn't just the endpoint, it installs the EFS storage driver (as an alternative to EBS) which allows provisioning flexible storage backends and volumes that support RWM mounts -- if I understand correctly, enabling the endpoint would just allow us to access the EFS api from our private VPC without traversing out to the public internet.

@JaseKoonce
Copy link
Contributor

@blancharda Thanks for explaining the endpoint purpose. My understanding (could definitely be flawed) is that because the mount targets point towards the EKS AZs and also because they live in the same VPC the traffic should all stay private/not need to leave the VPC, but I will look into that to verify.

@blancharda
Copy link
Author

(Very possible I don't understand the purpose of the endpoint correctly 😆 -- but it seems worth looking into)

@JaseKoonce
Copy link
Contributor

@blancharda, It definitely looks like you are correct about traffic traversing the public internet to reach the efs api without the vpc endpoint . I will go ahead and add that into the branch

@blancharda
Copy link
Author

blancharda commented Apr 11, 2023

The supporting resources (particularly resource "kubernetes_storage_class_v1" "efs") should include a way to adjust the reclaim_policy.

reclaim_policy is a field in the storage class resource

JaseKoonce added a commit that referenced this issue Apr 18, 2023
* WIP/base efs configs

Signed-off-by: jase koonce <[email protected]>

* tested/WIP

Signed-off-by: jase koonce <[email protected]>

* Add role-duration-seconds to aws creds step in workflow (#121)

* fix race condition for destroying manifests and addons (#123)

* fix race condition for destroying manifests and addons

* delete extra sg configs (#118)

* disable dynamoDB for loki, add configs to S3 (#117)

* disable dynamoDB for loki, add configs to S3

* removed the count for enabling versioning

---------

Co-authored-by: Gabe <[email protected]>

* S3 output role arn (#126)

* Output for IRSA Role ARN

Signed-off-by: Tom Runyon <[email protected]>

* refrence resource with count correctly

Signed-off-by: Tom Runyon <[email protected]>

* refrence resource with count correctly

Signed-off-by: Tom Runyon <[email protected]>

* precommit updates

---------

Signed-off-by: Tom Runyon <[email protected]>
Co-authored-by: Gabe <[email protected]>
Co-authored-by: Gabe Scarberry <[email protected]>

* WIP/base efs configs

Signed-off-by: jase koonce <[email protected]>

* adding vpc endpoint/randomizing efs names

Signed-off-by: jase koonce <[email protected]>

* fix race condition for destroying manifests and addons (#123)

* fix race condition for destroying manifests and addons

* WIP/base efs configs

Signed-off-by: jase koonce <[email protected]>

* duplicate variables fix/rebase

Signed-off-by: jase koonce <[email protected]>

* double argument fix

Signed-off-by: jase koonce <[email protected]>

* test fix

Signed-off-by: jase koonce <[email protected]>

* Add Provider Plugin Cache to automated testing (#130)

* Update eks-addons.tf to fix Ondat issue (#131)

* WIP/base efs configs

Signed-off-by: jase koonce <[email protected]>

* rebase for Ondat fix

Signed-off-by: jase koonce <[email protected]>

* rebase for Ondat fix

Signed-off-by: jase koonce <[email protected]>

* pre-commit

Signed-off-by: jase koonce <[email protected]>

* security group adjustment/tested

Signed-off-by: jase koonce <[email protected]>

* adding reclaim policy variable to efs storage class

Signed-off-by: jase koonce <[email protected]>

* pre-commit

Signed-off-by: jase koonce <[email protected]>

* Set enable efs to true in fixtures.common

Signed-off-by: jase koonce <[email protected]>

* Pre-commit

Signed-off-by: jase koonce <[email protected]>

---------

Signed-off-by: jase koonce <[email protected]>
Signed-off-by: Tom Runyon <[email protected]>
Signed-off-by: JaseKoonce <[email protected]>
Signed-off-by: Gabe <[email protected]>
Co-authored-by: Andy Roth <[email protected]>
Co-authored-by: Zack A <[email protected]>
Co-authored-by: Gabe <[email protected]>
Co-authored-by: brianrexrode <[email protected]>
Co-authored-by: Thomas Runyon <[email protected]>
Co-authored-by: Gabe Scarberry <[email protected]>
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 a pull request may close this issue.

2 participants