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

Environment variable substitution not substituted as expected #90823

Closed
dadez73 opened this issue Oct 12, 2022 · 6 comments
Closed

Environment variable substitution not substituted as expected #90823

dadez73 opened this issue Oct 12, 2022 · 6 comments
Labels
>bug :Core/Infra/Settings Settings infrastructure and APIs :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Core/Infra Meta label for core/infra team Team:Delivery Meta label for Delivery team team-discuss

Comments

@dadez73
Copy link

dadez73 commented Oct 12, 2022

Elasticsearch Version

8.2.0

Installed Plugins

No response

Java Version

bundled

OS Version

Linux elasticsearch-es-elk-data-nodes-0 5.4.0-113-generic #127-Ubuntu SMP Wed May 18 14:30:56 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Problem Description

We are tring to upgrade from version 8.1.3 to 8.2.3 Elasticsearch cluster in Kubernetes. During the start, the container hangs with :
Exception in thread "main" java.lang.IllegalArgumentException: Could not resolve placeholder 'WORKER_NODE_NAME'

Where the configuration is set with environment variable:
env:

  • name: WORKER_NODE_NAME
    valueFrom:
    fieldRef:
    fieldPath: "spec.nodeName"
  • name: node.name
    value: master-${WORKER_NODE_NAME}

The variable is pushed in the container correctly

My suspect is that this commit changed the behavior
Allow yaml values for dynamic node settings

Steps to Reproduce

Perform a rolling update of the Elasticsearch nodes from version <=8.1.3 to >=8.2.0

Logs (if relevant)

Wed, Oct 12 2022 10:26:23 am | Exception in thread "main" java.lang.IllegalArgumentException: Could not resolve placeholder 'WORKER_NODE_NAME'
Wed, Oct 12 2022 10:26:23 am | at org.elasticsearch.common.settings.PropertyPlaceholder.parseStringValue(PropertyPlaceholder.java:102)
Wed, Oct 12 2022 10:26:23 am | at org.elasticsearch.common.settings.PropertyPlaceholder.replacePlaceholders(PropertyPlaceholder.java:57)
Wed, Oct 12 2022 10:26:23 am | at org.elasticsearch.common.settings.Settings$Builder.replacePropertyPlaceholders(Settings.java:1259)
Wed, Oct 12 2022 10:26:23 am | at org.elasticsearch.node.InternalSettingsPreparer.prepareEnvironment(InternalSettingsPreparer.java:56)
Wed, Oct 12 2022 10:26:23 am | at org.elasticsearch.common.cli.EnvironmentAwareCommand.createEnv(EnvironmentAwareCommand.java:110)
Wed, Oct 12 2022 10:26:23 am | at org.elasticsearch.common.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:54)
Wed, Oct 12 2022 10:26:23 am | at org.elasticsearch.cli.Command.mainWithoutErrorHandling(Command.java:85)
Wed, Oct 12 2022 10:26:23 am | at org.elasticsearch.cli.Command.main(Command.java:50)
Wed, Oct 12 2022 10:26:23 am | at org.elasticsearch.launcher.CliToolLauncher.main(CliToolLauncher.java:64)

@dadez73 dadez73 added >bug needs:triage Requires assignment of a team area label labels Oct 12, 2022
@arteam arteam added :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts and removed needs:triage Requires assignment of a team area label labels Oct 17, 2022
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

@elasticsearchmachine elasticsearchmachine added the Team:Delivery Meta label for Delivery team label Oct 17, 2022
@mark-vieira mark-vieira added the :Core/Infra/Settings Settings infrastructure and APIs label Oct 17, 2022
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Oct 17, 2022
@mark-vieira
Copy link
Contributor

So it looks like what you're doing is substituting a setting set as an environment variable with another environment variable. It's definitely possible this is a regression related to the PR you mention. Variable substitution is specifically documented around YAML-based settings. If it used to work in environment variables as well that might have been purely coincidence as I don't think we have any specific coverage there. Perhaps someone from @elastic/es-core-infra could elaborate here.

@williamrandolph
Copy link
Contributor

williamrandolph commented Nov 4, 2022

I've reproduced the issue without Kubernetes.

$ export WORKER_NODE_NAME="foobar"
$ export NODE_NAME='master-${WORKER_NODE_NAME}'
$ echo $NODE_NAME
master-${WORKER_NODE_NAME}
$ eval "echo $NODE_NAME"
master-foobar

Then, in elasticsearch.yml:

node.name: ${MY_NODE}

Elasticsearch 8.1.1 starts up with a node named master-foobar, while 8.2.1 fails with:

Exception in thread "main" java.lang.IllegalArgumentException: Could not resolve placeholder 'WORKER_NODE_NAME'
        at org.elasticsearch.common.settings.PropertyPlaceholder.parseStringValue(PropertyPlaceholder.java:102)
        at org.elasticsearch.common.settings.PropertyPlaceholder.replacePlaceholders(PropertyPlaceholder.java:57)
        at org.elasticsearch.common.settings.Settings$Builder.replacePropertyPlaceholders(Settings.java:1258)
        at org.elasticsearch.node.InternalSettingsPreparer.initializeSettings(InternalSettingsPreparer.java:100)
        at org.elasticsearch.node.InternalSettingsPreparer.prepareEnvironment(InternalSettingsPreparer.java:63)
        at org.elasticsearch.common.cli.EnvironmentAwareCommand.createEnv(EnvironmentAwareCommand.java:95)
        at org.elasticsearch.common.cli.EnvironmentAwareCommand.createEnv(EnvironmentAwareCommand.java:86)
        at org.elasticsearch.common.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:81)
        at org.elasticsearch.cli.Command.mainWithoutErrorHandling(Command.java:112)
        at org.elasticsearch.cli.Command.main(Command.java:77)
        at org.elasticsearch.xpack.security.cli.AutoConfigureNode.main(AutoConfigureNode.java:157)

We didn't have any previous coverage of the test case, and only documented the "single substitution" case, rather than the double-substitution in this example. I'd regard the previous behavior as a coincidence as well, but I'll raise it with the team to see if there's a way to handle it more gracefully or improve the error message.

@mark-vieira
Copy link
Contributor

mark-vieira commented Nov 5, 2022

@dadez73 FWIW, kubernetes supports this natively via the $() syntax, so there's actually no need to rely on Elasticsearch to do this subsitution. Just configure like so:

env:
    - name: WORKER_NODE_NAME
      valueFrom:
      fieldRef:
      fieldPath: "spec.nodeName"
    - name: node.name
      value: master-$(WORKER_NODE_NAME)

@dadez73
Copy link
Author

dadez73 commented Dec 12, 2022

hi @mark-vieira , it worked for me! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Settings Settings infrastructure and APIs :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Core/Infra Meta label for core/infra team Team:Delivery Meta label for Delivery team team-discuss
Projects
None yet
Development

No branches or pull requests

5 participants