-
Notifications
You must be signed in to change notification settings - Fork 554
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
deploy/kubernetes/base/node.yaml: Fixing deployment on Bottlerocket. #247
Conversation
Updated the path where are written efs-utils.conf, efs-utils.crt and privateKey.pem files to /var/amazon/efs which is one of the few writeable file systems in Bottlerocket. This resolves issue #246.
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
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. I understand the commands that are listed here. |
Welcome @tatodorov! |
Hi @tatodorov. 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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tatodorov The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
We rely on this path being persisted in case the driver gets restarted. I am not 100% sure about the details but have observed that if /etc/amazon/efs/privateKey.pem is not persisted and the driver gets restarted then mounts can hang. The private key i used to make tls connections with EFS via stunnel. We do not decide the path, efs-utils does https://github.com/aws/efs-utils/search?q=%2Fetc%2Famazon%2Fefs&type=code |
Since the PR only changes path in the Volume, it shouldn't impact path on the EFS driver container, which is where efs-util is running. |
@juqing pointed out that hostPath path is not the same as mountPath within the container, so the container can still write to its /etc/amazon/efs/ which is bind mounted to wherever on the host. However I am still reluctant to merge this as is because it would break updates if a user has their privateKey stored on host at /etc/amazon/efs, then after the update the driver tries to read it from host at /var/amazon/efs. Thinking of a solution.... |
I think we need a bottlerocket overlay |
Fyi, some folks who are doing migration from regular Amazon Linux 2(AL2) to Bottlerocket will need to run both AL2 and Bottlerocket in parallel for some duration thus needing to deploy the csi driver in both. A separate overlay might full-fill bottlerocket deployment needs but will not be generic! |
If bottlerocket/al2 nodes have a well-known label distinguishing them we could put nodeSelectors in the overlays and instruct users to deploy two DaemonSets. Or instruct users to label the nodes themselves and set nodeSelectors for two DaemonSets accordingly. I want to minimize disruption for users but I think it is unavoidable that they will have to do something special |
I am favoring the idea of users setting specific node-labels on each of the two node types and the overlay being generic enough to use nodeSelector based on the set nodeLabels. |
We had an offline chat. In summary, for the upgrade case where there're already files present at The specific plan is to mount CC: @wongma7 |
/close superseded by #286 which I am working on getting into the next release (don't think I will make this week but shooting for next week) Thanks all for productive discussion. |
@tatodorov: PR needs rebase. 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. |
Updated the path where are written efs-utils.conf, efs-utils.crt and
privateKey.pem files to /var/amazon/efs which is one of the few
writeable file systems in Bottlerocket.
Is this a bug fix or adding new feature?
This resolves issue #246.
What is this PR about? / Why do we need it?
This allows aws-efs-csi-driver to be deployed also on Bottlerocket OS where /etc is read-only.
What testing is done?
aws-efs-csi-driver was successfully deployed on Amazon Linux 2 and Bottlerocket OS.
on Bottlerocket OS
Logs from efs-plugin:
NOTE
This PR resolves the deployment of aws-efs-csi-driver to Bottlerocket OS, but there is still an issue to attach EFS to a Pod when running on Bottlerocket OS.
Log from efs-plugin
Log from kubelet
on Amazon Linux 2 (EKS optimized)
Logs from efs-plugin: