Add fargate support in the EFS CSI driver #418
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
Welcome @anonymint! |
|
Hi @anonymint. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
needs-ok-to-test
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
| @@ -214,7 +219,9 @@ func (w *execWatchdog) updateConfig(efsClientSource string) error { | |||
| return fmt.Errorf("cannot create config file %s for efs-utils. Error: %v", w.efsUtilsCfgPath, err) | |||
| } | |||
| defer f.Close() | |||
| efsCfg := efsUtilsConfig{EfsClientSource: efsClientSource} | |||
| // used on Fargate, IMDS queries suffice otherwise | |||
| region := os.Getenv("AWS_REGION") | |||
There was a problem hiding this comment.
@kbasv i wonder if we could add support for querying fargate task metadata to efs-utils? https://github.com/aws/efs-utils/blob/c5abeed1a90a20c0ac0ad1a592296f795cd33e7c/src/mount_efs/__init__.py#L293 will create issue later
There was a problem hiding this comment.
Should be possible since efs-utils queries ECS metadata for credentials - https://github.com/aws/efs-utils/blob/c5abeed1a90a20c0ac0ad1a592296f795cd33e7c/src/mount_efs/__init__.py#L442
There was a problem hiding this comment.
@anonymint the comment "// used on Fargate, IMDS queries suffice otherwise" is not 100% clear, are you saying it is guaranteed that AWS_REGION will be set if this application is run as a fargate task/pod?
There was a problem hiding this comment.
@wongma7 ythere is another PR not merged regarding this and I would change this to be $AWS_DEFAULT_REGION as current setup in Fargate today, will update accordingly and send you package that Fargate uses and guarantee to have it.
on the side note, IMHOM It would be cleaner to add in attached efs-utils as you mention, there should be similar way/location to pull this region.
|
/lgtm thank you |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: anonymint, wongma7 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
For driver deployment in Fargate tasks environment
Is this a bug fix or adding new feature?
New Feature
What is this PR about? / Why do we need it?
Current driver is aiming toward running on EC2 using IMDS which is not available in Fargate, so this PR is to add Fargate support and check if in Fargate environment we can use Task Metadata Endpoint as a way to pull data similar to IMDS.
What testing is done?