Started treating the efs-utils config dir stateful and also handles the static files installed at image build time #212
Conversation
This reverts commit 70a181d.
k8s-ci-robot
added
cncf-cla: yes
|
Hi @jqmichael. 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
the
size/L
jqmichael
changed the title
|
/ok-to-test |
jqmichael
force-pushed
the
makeConfigDirStateful
branch
from
196d946 to
ef73e41
Compare
k8s-ci-robot
added
size/XL
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
jqmichael
force-pushed
the
makeConfigDirStateful
branch
from
ef73e41 to
fbe58a9
Compare
| version = flag.Bool("version", false, "Print the version and exit") | ||
| endpoint = flag.String("endpoint", "unix://tmp/csi.sock", "CSI Endpoint") | ||
| version = flag.Bool("version", false, "Print the version and exit") | ||
| efsUtilsCfgDirPath = flag.String("efs-utils-config-dir-path", "/etc/amazon/efs/", "The path to efs-utils config directory") |
There was a problem hiding this comment.
hmmm are these flags needed, would a user ever want to change them?
There was a problem hiding this comment.
I was expecting users would like to change them when running the driver locally for testing (in order to do that we should not talk to IMDS always, instead passing instanceId and region from a flag).
There was a problem hiding this comment.
efs-utils will always read from /etc/amazon/efs, I think this is just going to confuse people because changing it doesn't actually do anything. All that happens is the driver will copy the files from static path to a different path. But I only know that from looking at the code, a user is not going to know or care
jqmichael
force-pushed
the
makeConfigDirStateful
branch
from
fbe58a9 to
2859933
Compare
|
lgtm, just having second thoughts about the flags if they are needed only for debugging purposes or what |
jqmichael
force-pushed
the
makeConfigDirStateful
branch
from
2859933 to
277a5c9
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jqmichael, 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
Is this a bug fix or adding new feature?
/feature
What is this PR about? / Why do we need it?
This PR added #196 back by only patching node.yaml in the
devoverlay.This PR also start treating the efs-utils config directory stateful and handles static files installed at image build time.
At image build time, static files installed by efs-utils in the config directory, i.e. CAs file, need to be saved in another place so that the other stateful files created at runtime, i.e. private key for client certificate, in the same config directory can be persisted to host with a host path volume. Otherwise creating a host path volume for that directory will clean up everything inside at the first time. Those static files need to be copied back to the config directory when the driver starts up.
What testing is done?
Tested by verifying both tls and non-tls mount installed by 0.3.0 driver can be safely upgraded to the latest driver code without much interruption (other than during the downtime of the old driver).