Skip to content
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

Allow init container modifications #2306

Closed
charith-elastic opened this issue Dec 19, 2019 · 5 comments · Fixed by #3336
Closed

Allow init container modifications #2306

charith-elastic opened this issue Dec 19, 2019 · 5 comments · Fixed by #3336
Assignees
Labels
>enhancement Enhancement of existing functionality v1.3.0

Comments

@charith-elastic
Copy link
Contributor

charith-elastic commented Dec 19, 2019

Currently, if a user wants to change a small aspect of an init container (e.g. setting custom resource limits), they have to provide the full definition of the init container in the podTemplate.

Presumably we do this to give users full control over how init containers are executed. However, there are a couple of UX issues worth considering:

  • Most users are probably only interested in minor tweaks such as changing the resource limits of the container
  • The way the operator treats initContainers of the podTemplate is confusing. (Other sections in the podTemplate get merged while initContainers simply get replaced if the container name matches a built-in container)
@charith-elastic charith-elastic added the discuss We need to figure this out label Dec 19, 2019
@ferozsalam
Copy link

ferozsalam commented Jan 29, 2020

The elastic-internal-init-filesystem init container is OOMKilled on my machine when I use the default ECK resource definitions (following the Quickstart guide for ECK). I found this issue, which led me to the workaround, however I do think being able to increase the default resources in a less verbose manner would be quite useful.

@sebgl
Copy link
Contributor

sebgl commented Jan 29, 2020

@ferozsalam interesting. The current memory limit is set to 20Mi (#2186 and #2241). Which should normally be enough for that init container, afaik that's the first time we hear it's not.

What memory limit are you applying as a workaround? Can you give more details about your Kubernetes environment?

@ferozsalam
Copy link

@sebgl I've done some testing and it's fine with 30Mi (potentially, lower, I didn't bother going below 10Mi increments).

Regarding my k8s setup - it's a minikube instance on a desktop. Resources are ample though, and I'm typically able to run several dozen containers on it simultaneously. I have attached the output of kubectl describe po and kubectl logs for the relevant container, if this is of any use:
describe_po.txt
logs.txt

@sebgl
Copy link
Contributor

sebgl commented Jan 31, 2020

@ferozsalam thanks, that's very useful.
It seems to crash while copying Elasticsearch bin/ content to the emptyDir:

Copying /usr/share/elasticsearch/bin/* to /mnt/elastic-internal/elasticsearch-bin-local/
'/usr/share/elasticsearch/bin/elasticsearch' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch'
'/usr/share/elasticsearch/bin/elasticsearch-certgen' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-certgen'
'/usr/share/elasticsearch/bin/elasticsearch-certutil' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-certutil'
'/usr/share/elasticsearch/bin/elasticsearch-cli' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-cli'
'/usr/share/elasticsearch/bin/elasticsearch-croneval' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-croneval'
'/usr/share/elasticsearch/bin/elasticsearch-env' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-env'
'/usr/share/elasticsearch/bin/elasticsearch-enve' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-enve'
'/usr/share/elasticsearch/bin/elasticsearch-keystore' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-keystore'
'/usr/share/elasticsearch/bin/elasticsearch-migrate' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-migrate'
'/usr/share/elasticsearch/bin/elasticsearch-node' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-node'
'/usr/share/elasticsearch/bin/elasticsearch-plugin' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-plugin'
'/usr/share/elasticsearch/bin/elasticsearch-saml-metadata' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-saml-metadata'
'/usr/share/elasticsearch/bin/elasticsearch-setup-passwords' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-setup-passwords'
'/usr/share/elasticsearch/bin/elasticsearch-shard' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-shard'
'/usr/share/elasticsearch/bin/elasticsearch-sql-cli' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-sql-cli'
'/usr/share/elasticsearch/bin/elasticsearch-sql-cli-7.5.2.jar' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-sql-cli-7.5.2.jar'
/mnt/elastic-internal/scripts/prepare-fs.sh: line 80:    22 Killed                  cp -av /usr/share/elasticsearch/bin/* /mnt/elastic-internal/elasticsearch-bin-local/

@jgoeres
Copy link

jgoeres commented Mar 6, 2020

I am sometimes observing the exact same problem on my minikube instance. Today, deploying my workload that includes an ECK-powered ES worked several dozen times just fine, now all of a sudden, the elastic-internal-init-filesystem init container gets OOMKilled all the time:

'/usr/share/elasticsearch/bin/elasticsearch-sql-cli' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-sql-cli'
'/usr/share/elasticsearch/bin/elasticsearch-sql-cli-7.6.0.jar' -> '/mnt/elastic-internal/elasticsearch-bin-local/elasticsearch-sql-cli-7.6.0.jar'
/mnt/elastic-internal/scripts/prepare-fs.sh: line 80:    23 Killed                  cp -av /usr/share/elasticsearch/bin/* /mnt/elastic-internal/elasticsearch-bin-local/

Having the option to overwrite selected aspects of the init container instead of having to copy the entire default definition and modify it (and thus possibly miss any changes that might be done by newer versions of the operator) would be really useful.

@barkbay barkbay self-assigned this Jun 11, 2020
@barkbay barkbay added >enhancement Enhancement of existing functionality v1.3.0 and removed discuss We need to figure this out labels Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement Enhancement of existing functionality v1.3.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants