Add support for keystore with Kibana 7.9.0 #3566
Conversation
|
jenkins test this please |
| InitContainerConfigVolumeMountPath = "/mnt/elastic-internal/kibana-config-local" | ||
|
|
||
| InternalConfigVolumeName = "elastic-internal-elasticsearch-config" | ||
| InternalConfigVolumeMountPath = "/mnt/elastic-internal/kibana-config" |
There was a problem hiding this comment.
internal is a bit confusing: why is /mnt/elastic-internal/kibana-config "internal" compared to /mnt/elastic-internal/kibana-config-local? Both living in elastic-internal sound like "internal" stuff.
There was a problem hiding this comment.
This is also to be consistent with what is done with Elasticsearch:
- mountPath: /mnt/elastic-internal/elasticsearch-config
name: elastic-internal-elasticsearch-config
readOnly: true
- mountPath: /usr/share/elasticsearch/config
name: elastic-internal-elasticsearch-config-local
| @@ -23,28 +23,43 @@ import ( | |||
|
|
|||
| // Constants to use for the config files in a Kibana pod. | |||
| const ( | |||
| VolumeName = "config" | |||
| VolumeMountPath = "/usr/share/kibana/" + VolumeName | |||
| ConfigVolumeName = "elastic-internal-kibana-config-local" | |||
There was a problem hiding this comment.
Would shared be more explicit than local to express how the volume is shared between init and main container?
There was a problem hiding this comment.
Naming has been chosen to be consistent with what is done with Elasticsearch:
cloud-on-k8s/pkg/controller/elasticsearch/initcontainer/prepare_fs.go
Lines 36 to 40 in b6cb04d
I agree that shared would be more explicit but I'm not sure we want to do this refactoring for a patch release ?
| @@ -287,6 +251,21 @@ func (d *driver) deploymentParams(kb *kbv1.Kibana) (deployment.Params, error) { | |||
| }, nil | |||
| } | |||
|
|
|||
| func (d *driver) buildVolumes(kb *kbv1.Kibana) []commonvolume.VolumeLike { | |||
| volumes := []commonvolume.VolumeLike{DataVolume, ConfigSharedVolume, ConfigVolume(*kb)} | |||
There was a problem hiding this comment.
Are we mounting ConfigVolume() (if I'm not mistaken: the "raw" config from the secret resource) into the main Kibana container? Isn't it only required for the init container?
There was a problem hiding this comment.
The Secret with the configuration is needed because I eventually decided to create some symbolic links. Even if it is not strictly required for Kibana I think that it is easier to reason about something that is close to what is done with Elasticsearch.
| InitConfigScript = `#!/usr/bin/env bash | ||
| set -eux | ||
|
|
||
| init_config_initialized_flag=/usr/share/kibana/config/elastic-internal-init-config.ok |
There was a problem hiding this comment.
is it only to avoid the cost of re-doing ln -sf?
There was a problem hiding this comment.
This is to avoid the content of the script to be run twice, I agree it should be safe to remove it here. But I also want to avoid Setup Kibana configuration to be printed several times in the logs. It's harmless indeed, but I think it can be confusing for the user/support to see those messages while the container is already running for a while.
| @@ -2,34 +2,38 @@ | |||
| // or more contributor license agreements. Licensed under the Elastic License; | |||
| // you may not use this file except in compliance with the Elastic License. | |||
|
|
|||
| package initcontainer | |||
There was a problem hiding this comment.
Nice to see the refactoring here 👍
charith-elastic
added
>enhancement
Starting Kibana 7.9.0 the keystore is located in the configuration directory instead of the data directory.
This PR allows the keystore init container and the Kibana container to share the
configdirectory through an empty directory.For consistency with what is done with Elasticsearch this PR creates symbolic links from the generated configuration to the shared configuration volume.
Fixes #3528