Add the --cert-dir flag to change the directory for Webhook certificate.

[dulltz]

Proposal

Add the --cert-dir flag to pass to the controller-runtime.

In PodSecurityPolicy enabled K8s cluster, read-only root filesystem is common.
I want to to use /tmp path as writable empty-dir because sometimes ECK writes its logs to under the /tmp/.

However, ECK v1.0.0 uses /tmp/k8s-webhook-server/serving-certs for the elastic-webhook-server-cert secret.

It is not good to overwrap mount paths, so I hope to add --cert-dir flag to ECK for changing the cert-dir.

It seems to be changeable by passing to controller-runtime.
https://github.com/kubernetes-sigs/controller-runtime/blob/a7c8a93c1cf395974911ef0ece977a7d27b2bf4b/pkg/manager/manager.go#L175

Environment

• ECK version: v1.0.0
• Kubernetes information:
  • v1.16
  • On premise
  • PodSecurityPolicy enabled

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"16+", GitVersion:"v1.16.6-beta.0", GitCommit:"e7f962ba86f4ce7033828210ca3556393c377bcc", GitTreeState:"clean", BuildDate:"2020-01-15T08:26:26Z", GoVersion:"go1.13.5", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"16+", GitVersion:"v1.16.6-beta.0", GitCommit:"e7f962ba86f4ce7033828210ca3556393c377bcc", GitTreeState:"archive", BuildDate:"2020-01-17T02:54:55Z", GoVersion:"go1.13.4", Compiler:"gc", Platform:"linux/amd64"}

[sebgl]

It sounds like a legitimate request 👍
We probably want to make the flag name a bit more explicit to contrast with other flags and other certificates managed by ECK: --webhook-cert-dir.

I have a few remarks and questions, to make sure I understand completely your use case:

• If overriding the --webhook-cert-dir flag, you would also have to tweak the secret volume mount path in the ECK manifest. It is currently set to mountPath: /tmp/k8s-webhook-server/serving-certs by default.

• sometimes ECK writes its logs to under the /tmp/.. The this surprises me a little bit. Can you give more details? When does ECK write its logs to /tmp?

[dulltz]

@sebgl
Thank you for your replying 😄

• If overriding the --webhook-cert-dir flag, you would also have to tweak the secret volume mount path in the ECK manifest. It is currently set to mountPath: /tmp/k8s-webhook-server/serving-certs by default.

Yes, I think so.
If ECK supports --webhook-cert-dir, the flag value will be passed by valueFrom.fieldRef.fieldPath from volumeMount spec.
https://kubernetes.io/docs/tasks/inject-data-application/environment-variable-expose-pod-information/

• sometimes ECK writes its logs to under the /tmp/.. The this surprises me a little bit. Can you give more details? When does ECK write its logs to /tmp?

I don't know when ECK writes under /tmp, but I found that the elastic-operator terminated with the following log.
It seems that some k8s native package writes its log under /tmp because this log filename contains "{{statefulset}}.{{pod}}.{{container}}".

log: exiting because of error: log: cannot create log: open /tmp/elastic-operator.elastic-operator-0.elastic.log.ERROR.20200114-112606.1: read-only file system

[ymmt2005]

Maybe the culprit is klog. It needs to be initialized explicitly. An example is here:
https://github.com/cybozu/neco-containers/blob/master/admission/cmd/root.go#L79-L80

[sebgl]

Could be yeah, it looks like there is some history around klog/glog initialization (example fix in CoreDNS).

@ymmt2005 @dulltz are you able to easily reproduce the log: exiting because of error: log: cannot create log: open /tmp message (I guess with a custom operator manifest)? I'd like to reproduce it myself in order to make sure we fix the log initialization correctly.

[ymmt2005]

@sebgl
Unfortunately I am not sure how to reproduce it.
We have experienced many similar problems and successfully fixed it by properly initializing glog, though.

[sebgl]

#2476 adds the flag --webhook-cert-dir, which can also be used as an environment variable WEBHOOK_CERT_DIR if more convenient.

[sebgl]

I've spent some time investigating around the klog potential error. Even with a read-only filesystem and while logging with klog manually I cannot reproduce the error.

Calling klog.InitFlags(flagset) seems to only be useful for propagating non-default settings:
https://github.com/kubernetes/klog/blob/12be8a0d907ae8c384359251205ba2099b1e85ed/klog.go#L416-L436
The default settings already take care of not writing on a file: https://github.com/kubernetes/klog/blob/12be8a0d907ae8c384359251205ba2099b1e85ed/klog.go#L401-L414

I double-checked in our dependencies, I don't think glog is used over klog anywhere.

We did some changes in the way we handle logging a while ago. Did you observe the error message on ECK v1.0.0 (stable)?

[ymmt2005]

Ah, right. The default was changed to set appropriately in init() since v1.0.0 of klog.
kubernetes/klog@9007a3f#diff-fb778b051a785bbb2eea40d6a00a3a21