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

Resource aggregator should handle missing memory settings #5158

Merged

Conversation

barkbay
Copy link
Contributor

@barkbay barkbay commented Dec 14, 2021

This PR fixes a bug where a user might want to set some JVM parameters without JVM memory settings (Xmx). To reproduce create the following Elasticsearch resource:

apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
  name: es
spec:
  version: 6.8.21
  nodeSets:
    - name: master
      config:
        node.master: true
        node.data: true
        node.ingest: true
        node.store.allow_mmap: false
      podTemplate:
        spec:
          containers:
            - name: elasticsearch
              env:
                - name: ES_JAVA_OPTS
                  value: "-Dlog4j2.formatMsgNoLookups=true"
      count: 1

Operator logs:

2021-12-14T10:23:36.616+0100    ERROR   resource        Failed to report licensing information
{
  "service.version": "XXXXXXX",
     "error": "failed to aggregate Elasticsearch memory: cannot extract max jvm heap size from -Dlog4j2.formatMsgNoLookups=true", "errorVerbose": "cannot extract max jvm heap size from -Dlog4j2.formatMsgNoLookups=true\n..."
}

This PR also removes unit tests cases where the JVM would not be able to start (Unrecognized option, Invalid maximum heap size ...)

@barkbay barkbay added the >bug Something isn't working label Dec 14, 2021
Copy link
Contributor

@thbkrkr thbkrkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@barkbay barkbay merged commit 77be8f6 into elastic:master Dec 15, 2021
@barkbay barkbay deleted the bug/resource-aggregator-missing-mem-settings branch December 15, 2021 06:02
@barkbay barkbay added the v2.0.0 label Dec 15, 2021
david-kow pushed a commit to david-kow/cloud-on-k8s that referenced this pull request Dec 15, 2021
@barkbay barkbay added v1.9.1 and removed v2.0.0 labels Dec 15, 2021
david-kow added a commit that referenced this pull request Dec 15, 2021
)

Co-authored-by: Michael Morello <michael.morello@elastic.co>
fantapsody pushed a commit to fantapsody/cloud-on-k8s that referenced this pull request Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working v1.9.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants