Limit scope of logs writable by ipamd container #987
Conversation
|
(This PR is based on #986 - will be rebased/delayed/etc, depending on what happens with that PR) Note the default for AWS_VPC_K8S_CNI_LOG_FILE is a bit confusing, since /host/foo assumes a particular container volume setup. Perhaps we should take the opportunity to change the container default, rather than document the existing behaviour. Either way it wouldn't affect anyone using our provided manifests. |
Yes, I totally agree. I would probably prefer to switch to log to |
I could also change the code/image to default to stdout, but then point it back to the existing files in the manifest (so no change for people using the manifest). Let me know what you want me to do here (if anything). |
|
I guess it's safer to just keep the default file for now, and just clean up the mounts. Just needs a rebase to make the changes a bit easier to see. 😄 |
|
(Rebased) |
| path: "/var/log/aws-routed-eni", | ||
| type: "DirectoryOrCreate", |
config/master/manifests.jsonnet
Outdated
| @@ -172,7 +173,7 @@ local awsnode = { | |||
| volumeMounts: [ | |||
| {mountPath: "/host/opt/cni/bin", name: "cni-bin-dir"}, | |||
| {mountPath: "/host/etc/cni/net.d", name: "cni-net-dir"}, | |||
| {mountPath: "/host/var/log", name: "log-dir"}, | |||
| {mountPath: "/var/log/aws-routed-eni", name: "log-dir"}, | |||
There was a problem hiding this comment.
Can we keep the /host/ part here so that both log files end up in the same host folder? With this config, ipamd.log will be inside the container only, in /host/var/log/aws-routed-eni/, and the plugin.log file from the host is visible as /var/log/aws-routed-eni/plugin.log. Makes it more clear that it's actually a host mounted directory.
There was a problem hiding this comment.
Yep, reverted this part of the PR to continue using an explicit /host prefix.
Fwiw, I changed the AWS_VPC_K8S_CNI_LOG_FILE and mountPath in this PR to the same value so ipamd.log was still writing to the same real host /var/log/aws-routed-eni/. I was trying to reduce confusion by using identical paths inside and outside the container, but obviously that didn't work 😛
There was a problem hiding this comment.
Ahh... Thanks. Was too confusing for me for sure 😆
Reduce the logs exposed to ipamd container to just `/var/log/aws-routed-eni/` rather than all of `/var/log` Also correct documented log file defaults: - `AWS_VPC_K8S_PLUGIN_LOG_FILE` defaults to `/var/log/aws-routed-eni/plugin.log` in scripts/entrypoint.sh#L44 - `AWS_VPC_K8S_CNI_LOG_FILE` defaults to `/host/var/log/aws-routed-eni/ipamd.log` in utils/logger/config.go#L47
Reduce the logs exposed to ipamd container to just
/var/log/aws-routed-eni/rather than all of/var/log(!)Also correct documented log file defaults:
AWS_VPC_K8S_PLUGIN_LOG_FILEdefaults to/var/log/aws-routed-eni/plugin.login scripts/entrypoint.sh#L44AWS_VPC_K8S_CNI_LOG_FILEdefaults to/host/var/log/aws-routed-eni/ipamd.login utils/logger/config.go#L47By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.