Should we rename nodes in Elasticsearch spec

[david-kow]

This came up in a comment for #1463.

I got a feeling that current name ("nodes") implies that it holds a list of nodes, but that's not accurate and might be misleading. It does hold a list, but each item describes a set of identical es nodes and holds:

• name for the set,
• es config to use for all nodes in the set,
• number of nodes in the set,
• pod template to use for all nodes in the set,
• volume claim templates to use for all nodes in the set.

I don't have full context as this was before my time, but seems like this field was renamed already from topologies to topology and then to nodes.

Alternatives that came up: nodeGroups, nodeSets, nodeSpecs, nodeTemplates.

I don't really feel strongly about this, but we decided to ask broader, hence calling @agup006 @jordansissel @Crazybus @zanbel @uric.

[david-kow]

Since there is no feedback so far, I will wait a little more and close this on Monday concluding that we don't want to rename.

[sebgl]

Just to give examples for people looking at this issue.

This is what we currently have:

kind: Elasticsearch
metadata:
  name: elasticsearch-sample
spec:
  version: 7.2.0
  nodes:
  - name: masters
    nodeCount: 3
  - name: data-hot
    nodeCount: 3
  - name: data-warm
    nodeCount: 3

We are wondering whether nodes (and also name and nodeCount) are the right names here.
Example of what could be possible instead:

kind: Elasticsearch
metadata:
  name: elasticsearch-sample
spec:
  version: 7.2.0
  nodeSpecs: # nodeSets, nodeTemplates, nodeGroups, etc.
  - name: masters
    count: 3  # nodeCount, replicas, etc.
  - name: data-hot
    count: 3
  - name: data-warm
    count: 3

[zanbel]

For ES the term nodes seems fine to me and describes what the user is configuring in this section. One thing that I've noticed that when assigning roles there it's done by specifying node.ingest: true while setting the node name or count doesn't use the same node.name: master or node.count, for example. Not saying it's right or wrong, just something I've noticed.

Not directly related this issue that seems to request feedback about ES specifically, but I can share that for other node types such as Kibana, APM, App Search, etc. the term node doesn't seem as appropriate. This also came up in similar discussion in ECE.
We ended up using instance configuration and the specific step in the wizard is titled Instance types. For ECK I don't we face the same challenge since there are different yaml files for each type.

Bottom line, I also don't have a strong opinion, but for ES nodes seems like a good term IMO.

[anyasabo]

nodes confuses me every time I interact with it. How do I describe a single item of nodes? node is what I would assume, but it does not describe just a single node. it is a description of potentially many identical nodes. For me that is the main argument for changing it to nodeSets or nodeGroups (or nodeses 😄 ).

[sebgl]

Pasting here some feedbacks we received through another communication channel:

I think nodes is fine.

I agree that this https://github.com/elastic/cloud-on-k8s/blob/master/operators/pkg/apis/elasticsearch/v1alpha1/elasticsearch_types.go#L35-L36 doesn't feels 100% right
NodeSets and NodeGroups inmho sound better than NodeTemplate (plural vs single)

For me personally nodes sounds fine and isn’t confusing at all. Asking people who are familiar with Kubernetes and Elasticsearch is probably the wrong target audience though. I remember when working on the helm-charts we all agreed that the values names sounded good but then got lots of “this name is confusing” from people who knew Elasticsearch but not Kubernetes which I think will be a large target audience for this.

[pebrc]

I think I am 👍 on changing it to something other than nodes because it is imprecise. As to which one we should choose:

• nodeSpecs spec is a common term in k8s API conventions, plural is indicating that you can have multiples specs for different groups of nodes each based on one of the specs and spec things often have counts in k8s so that seems idiomatic
• nodeTemplates I liked that originally because it conveys the idea of using what is in each template section to create multiple node instances out of it. But it has the disadvantage that we also have a count and a name in here that are not part of the template making this imprecise as well so no improvement over nodes
• nodeSets or nodeGroups emphasise the formation of groups of nodes with sets hinting at the fact that we are using StatefulSets underneath maybe. But maybe it is not as clear that the user can spec out the nodes here as well

I think nodeSpecs is my personal favourite so far, being the most intuitive and having the least arguments against it.

[pebrc]

We discussed this again and decided to go with nodeSets and count

kind: Elasticsearch
metadata:
  name: elasticsearch-sample
spec:
  version: 7.2.0
  nodeSets:
  - name: masters
    count: 3 
  - name: data-hot
    count: 3
  - name: data-warm
    count: 3

kind: Elasticsearch
metadata:
  name: elasticsearch-sample
spec:
  version: 7.2.0
  nodeSets:
  - name: default
    count: 3  

We will also rename the corresponding structs in the code.