Removing Dependency on IMDS, allowing hostNetwork: true to be removed

[jonathanrainer]

Is this a bug fix or adding new feature?
This PR is a fix that allows the driver to keep functioning even in situations where IMDS is not available. It does this by falling back to the Kubernetes API to get the required information if it cannot get it from the EC2Metadata Service.

What is this PR about? / Why do we need it?
This PR is in response to #313, and will allow the driver to function in environments that do not have access to IMDS. The code was based heavily on kubernetes-sigs/aws-ebs-csi-driver#907, but refactored slightly to use a provider pattern for the metadata and factor out each part into their own file to allow more ease of testing.

What testing is done?
Have written additional unit tests to cover the change but need some help running the E2E tests as running them on my local setup has been fraught with problems. Any guidance on this (can we run them on the usual E2E infrastructure as a one off perhaps?) would be greatly appreciated.

fixes #313

[k8s-ci-robot]

Hi @jonathanrainer. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

[wongma7]

/ok-to-test

[Ashley-wenyizha]

e2e auto-run tests looking good

[Ashley-wenyizha]

/lgtm

[k8s-ci-robot]

@Ashley-wenyizha: changing LGTM is restricted to collaborators

In response to this:

/lgtm

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.

[jonathanrainer]

/retest

[jonathanrainer]

Have tested this in my own EKS cluster. Did a standard install of the Helm Chart plus my changes and the controller booted fine and could serve both static and dynamically provisioned PVs. The IPs also were pod IPs rather than Node IPs as can be evidenced in the below by comparing the pods IPs to the Node Names.

[jonathanrainer]

/retest

[jonathanrainer]

/retest

[wongma7]

with latest helm chart, should be controller.serviceAccount not serviceAccount.controller similar to the EBS one.
https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/master/hack/values_eksctl.yaml

Also the enalbeVolumeSnapshot line can be removed

[wongma7]

Once kubernetes/test-infra#26573 merges we can see if the CI passes /test pull-aws-efs-csi-driver-external-test-eks as well. I really want CI to create a cluster with eksctl --disable-pod-imds before we release this. (Because in EBS we had a big incident where we claimed to have fixed this IMDS dependency but actually nobody tested it on EKS and the CI only exercised kops....)

[jonathanrainer]

@wongma7 Had a look through this and I think the majority of the errors are because of service accounts that don't have permissions, I'll try and have a look into it but if you could too that would be great

[wongma7]

@jonathanrainer need to add https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/master/charts/aws-ebs-csi-driver/templates/controller.yaml#L93-L96 to the template (and kustomize , by running make generate-kustomize)

            - name: CSI_NODE_NAME
              valueFrom:
                fieldRef:
                  fieldPath: spec.nodeName

###
## Printing pod efs-csi-controller-695dc45655-tr8rc efs-plugin container logs
#
I0615 08:52:33.911293       1 config_dir.go:62] Mounted directories do not exist, creating directory at '/etc/amazon/efs'
I0615 08:52:33.912260       1 metadata.go:63] getting MetadataService...
I0615 08:52:37.089439       1 metadata.go:71] retrieving metadata from Kubernetes API
F0615 08:52:37.089492       1 driver.go:56] could not get metadata from AWS: CST_NODE_NAME env var not set 

this is good in sense that the CI test is working as intended...metadata is not available so it's looking for nodeName

[jonathanrainer]

Thanks that's really useful to know I can pull the pod logs out of prow now, pretty sure I'll be much faster/self-sufficient in future!

[jonathanrainer]

@wongma7 Yay it all passed, thanks a lot for your help

[wongma7]

THX lgtm, could you squash the commits and i'll drop the "real" lgtm. Ideally one commit with vendor changes and one commit with the rest, but putting it all together in 1 commit is fine as well.

[jonathanrainer]

@wongma7 All squashed and ready to go

[wongma7]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jonathanrainer, 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:

• OWNERS [wongma7]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment