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

[BUG] OpenSearch app is not able to pick up empty node.roles= environment variable to run as coordinating node. #3412

Closed
prudhvigodithi opened this issue May 20, 2022 · 14 comments · Fixed by #10625
Labels
>breaking Identifies a breaking change. bug Something isn't working v3.0.0 Issues and PRs related to version 3.0.0

Comments

@prudhvigodithi
Copy link
Contributor

prudhvigodithi commented May 20, 2022

Describe the bug
Coming from the issue opensearch-project/opensearch-build#2129, OpenSearch app is not able to pick up empty node.roles= environment values, to allow to run as coordinating node. This works fine when added to opensearch.yml as node.roles: [], but not able to pick when passed as environment variable. However when passed a specific role as node.roles=master OpenSearch app is able to pick up and pass the role as master.

docker run -e 'node.roles=master,data' opensearchproject/opensearch:1.3.2 |grep role
[2022-05-20T01:31:30,027][INFO ][o.o.n.Node               ] [9a683cb05664] node name [9a683cb05664], node ID [vgu0pnrgQRytmfj2ZC6siw], cluster name [docker-cluster], roles [master, data]

As per the document https://opensearch.org/docs/latest/opensearch/cluster/, passing empty listnode.roles: [] should allow the node to run with coordinating role.

To Reproduce

docker run -e 'node.roles=' opensearchproject/opensearch:1.3.2 |grep role
[2022-05-20T01:26:25,169][INFO ][o.o.n.Node               ] [5e7b9fa617b3] node name [5e7b9fa617b3], node ID [pEkc98u4Q-SdZLr4ojZ-QQ], cluster name [docker-cluster], roles [master, remote_cluster_client, data, ingest]
docker run -e 'node.roles=[]' opensearchproject/opensearch:1.3.2 |grep role
uncaught exception in thread [main]
org.opensearch.bootstrap.StartupException: java.lang.IllegalArgumentException: unknown role [[]]
Caused by: java.lang.IllegalArgumentException: unknown role [[]]
java.lang.IllegalArgumentException: unknown role [[]]
docker run -e 'node.roles=" "' opensearchproject/opensearch:1.3.2 |grep role
uncaught exception in thread [main]
org.opensearch.bootstrap.StartupException: java.lang.IllegalArgumentException: unknown role [" "]
Caused by: java.lang.IllegalArgumentException: unknown role [" "]
java.lang.IllegalArgumentException: unknown role [" "]

Expected behavior
docker run -e 'node.roles=' should allow to run the node as coordinating node.

Host/Environment (please complete the following information):

  • OS: docker

Additional context
N/A

@prudhvigodithi prudhvigodithi added bug Something isn't working untriaged labels May 20, 2022
@prudhvigodithi prudhvigodithi changed the title [BUG] OpenSearch app is not able to pick up empty node.roles= environment values to run as coordinating node, [BUG] OpenSearch app is not able to pick up empty node.roles= environment variable to run as coordinating node, May 20, 2022
@prudhvigodithi prudhvigodithi changed the title [BUG] OpenSearch app is not able to pick up empty node.roles= environment variable to run as coordinating node, [BUG] OpenSearch app is not able to pick up empty node.roles= environment variable to run as coordinating node. May 20, 2022
@prudhvigodithi
Copy link
Contributor Author

prudhvigodithi commented May 20, 2022

Workaround using docker helm
https://github.com/prudhvigodithi/helm-charts/blob/coordinating-role/charts/opensearch/values.yaml#L11
https://github.com/prudhvigodithi/helm-charts/blob/coordinating-role/charts/opensearch/values.yaml#L45

curl -XGET https://localhost:9200/_cat/nodes?h=nodeRole -u 'admin:admin' --insecure shows right role as coordinating node.

would be good to just pass node.roles= empty environment variable and pick up the role as coordinating node.

@smlx @dblock @bbarani @saratvemulapalli

@reta
Copy link
Collaborator

reta commented May 20, 2022

@prudhvigodithi I believe this is the expected behavior: node.roles= means the value is not set (which is different from setting the value to be empty). I think the correct behavior would be to pass empty list instead node.roles=[].

@prudhvigodithi
Copy link
Contributor Author

@prudhvigodithi I believe this is the expected behavior: node.roles= means the value is not set (which is different from setting the value to be empty). I think the correct behavior would be to pass empty list instead node.roles=[].

hey @reta, I have tried as well and added to the above To Reproduce block it says as java.lang.IllegalArgumentException: unknown role [[]]

docker run -e 'node.roles=[]' opensearchproject/opensearch:1.3.2 |grep role
uncaught exception in thread [main]
org.opensearch.bootstrap.StartupException: java.lang.IllegalArgumentException: unknown role [[]]
Caused by: java.lang.IllegalArgumentException: unknown role [[]]
java.lang.IllegalArgumentException: unknown role [[]]

I believe OpenSearch environment variable parsing logic needs to be fixed to look for empty string ?

@reta
Copy link
Collaborator

reta commented May 20, 2022

I believe OpenSearch environment variable parsing logic needs to be fixed to look for empty string ?

I don't think so, it is difficult to distinguish between variable not set (== empty string) and variable not passed at all - in both cases there is no value. May be supporting [] notation to denote empty value would make sense?

@prudhvigodithi
Copy link
Contributor Author

prudhvigodithi commented May 20, 2022

@reta ideally since this works node.roles=master, data (passing as environment variable) and sets right master and data role, setting node.roles= to empty value should allow to set the coordinating role.

May be supporting [] notation to denote empty value would make sense?
yes setting environment variable node.roles=[] should also set as coordinating role, adding this logic would be better

@dblock
Copy link
Member

dblock commented May 20, 2022

Isn't the current behavior that "absence of node.roles = coordinator node"? IMO node.roles= should just produce an error?

@reta
Copy link
Collaborator

reta commented May 20, 2022

Isn't the current behavior that "absence of node.roles = coordinator node"? IMO node.roles= should just produce an error?

@dblock nope (surprisingly), here is how it works [1], citing Elasticsearch documentation here:

 If you don’t set node.roles, the node is assigned the following roles:

    master
    data
    data_content
    data_hot
    data_warm
    data_cold
    data_frozen
    ingest
    ml
    remote_cluster_client
    transform

But:

To create a dedicated coordinating node, set:

node.roles: [ ]

[1] https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-node.html

@prudhvigodithi
Copy link
Contributor Author

prudhvigodithi commented May 20, 2022

Hey @reta yes empty node.roles, passes the default roles to the node, but passing env value node.roles: [ ] or node.roles: (adds default roles [master, remote_cluster_client, data, ingest] ) fails to pick up as coordinating node.

@smlx
Copy link

smlx commented May 23, 2022

Since Opensearch configuration is YAML, the configuration via environment variables should support whatever YAML supports. For environment variables the obvious solution to this is to use JSON encoding since it is almost YAML equivalent while also being whitespace-insensitive which lends itself well to single-line environment variables.

The problem is that for node.roles (and possibly other variables?) Opensearch uses its own poorly defined configuration language which sort of supports arrays by separating bare strings with a comma. However it apparently doesn't have a concept of an empty array.

One solution to this would be to simply switch to using JSON which is well defined and widely supported. However this would be a breaking change. Maybe for backwards compatibility this environment variable should allow the current ad-hoc configuration language but also fall back to JSON? That would make these options all valid:

  • node.roles=master,data - set the node roles to master, and data.
  • node.roles=["master","data"] - set the node roles to master, and data.
  • node.roles=[] - set the node roles to an empty array making it a coordinating node.
  • node.roles= - don't set any node roles, fall back to default values.

@dblock
Copy link
Member

dblock commented May 23, 2022

I think @prudhvigodithi's suggestion of supporting [] as a value is the simplest one. I'd want to see that PR before judging whether there are other side effects.

@coredump17
Copy link

this is also a big issue for me. Do we have any plans to resolve this?

@dblock
Copy link
Member

dblock commented Sep 28, 2023

@mooneym17 I don't think anyone is working on this, please contribute!

@coredump17
Copy link

coredump17 commented Oct 6, 2023

i am new to opensearch, but after looking over the code (untrained eye). I believe setting node.roles=- will set a coordinator node (opensearch-node2).

curl -k -uadmin:admin http://127.0.0.1:9202/_cat/nodes?v
ip heap.percent ram.percent cpu load_1m load_5m load_15m node.role node.roles cluster_manager name
10.89.1.10 19 95 41 4.56 2.95 2.32 dimr cluster_manager,data,ingest,remote_cluster_client * opensearch-node1
10.89.1.11 57 95 38 4.56 2.95 2.32 - - - opensearch-node2

@coredump17
Copy link

thanks. doesnt passing node.roles=- allow for a coordinator as above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>breaking Identifies a breaking change. bug Something isn't working v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants