Use human readable name as cluster's integration_id

[shtripat]

Currently in tendrl we use an UUID as cluster's integration_id and if multiple clusters maintained by tendrl it becomes tough to identify the clusters.

There is a suggestion to use human readable name as integration_id for clusters. So the flow to use a human readable name integration_id for cluster would be something like below

1. While node agent detects a cluster which is ready for import, it creates a UUID as temporary detected integration_id for cluster and creates an object /clusters/{detected-integration-id} in etcd with is_managed value as no
2. When UI triggers the import for this cluster the dialog thrown for import cluster would list this detected_cluster_id and user needs to provide a human readable integration_id value in the dialog. If user doesnt provide a human readable name in dialog the existing detected_integration_id remains as final integration_id for the cluster then onwards. May be UI should shown a warning on the dialog regarding the same saying If no human readable name provided the detected_integration_id would remain as identifier for the cluster and later it wont be possible to change.
3. UI submits the import cluster request with a flag saying if the detected_cluster_id was replaced with human readable name and the new integration_id. API layer adds a check to find if a cluster with given new integration_id in request, already exists. If so API raises an error saying cluster with this name already exists. This check should happen only if UI send the flag detected_integration_id_overriden as yes
4. In back-end the import cluster flow adds a pre check atom to do the check for pre-existence of cluster with new integration_id in parameters and fail if so. This pre check also runs only if detected_integration_id_overriden is set as yes in request.
5. The import cluster flow replaces the object /clusters/{detected_integration_id} with /clusters/{human_readable_integration_id} in etcd based on detected_integration_id_overriden field set as yes. If the field detected_integration_id_overriden is not set as yes in request the old /clusters/{detected_integartion_id} remains as is.
6. After this import flow runs as usual and continues.

With this there are no other changes required at back-end or any other components and it should be seamlessly working with human readable ntegration_id for cluster.

@r0h4n @nthomas-redhat @gnehapk @shirshendu @GowthamShanmugam @cloudbehl comments/thoughts?

@julienlim if we go ahead with this model, we would need UX changes for the import cluster dialog to capture the human readable integration_id for cluster.

Lets have all the discussion points captured here and go ahead based on the decision taken.

[r0h4n]

Thanks @shtripat , Here's my take
UI:

• Validate the "Cluster.short_name" and ensure it contains only alphabets
• Calls the ImportCluster API with this additional param.

API:

• Validates "Cluster.short_name" based on definitions (not required now)
• Creates a Tendrl job for "ImportCluster" flow

Backend:

• If "Cluster.short_name" found in "ImportCluster" flow then do below steps
• This attribute should be called "Cluster.short_name" (validation, short_name will be only alphabets, no other symbols, due to graphite/etcd limitations) (validate, uniqueness)
• If "Cluster.short_name" then ask provisioner to set Cluster.integration_id == Cluster.short_name and also set the value of etcd tag "indexes/detected_cluster_id_to_integration_id/:detected_cluster_id" == Cluster.short_name
• Delete old "clusters/:integration_id"
• Now lock the cluster with new integration_id and proceed with import as usual with this new integration_id

[shtripat]

Thanks @r0h4n this makes clearer now.
Also with new integration set with short_name, I feel we should set NS.tendrl_context.integartion_id with new value. Now only issue I have is how this would reflect for other modules like tendrl-node-agent, monitoring-integration, notifier etc as those services would be already running by the time import cluster starts.

[julienlim]

@shtripat @r0h4n @nthomas-redhat @gnehapk @shirshendu @GowthamShanmugam @cloudbehl @mcarrano

It should be fairly straightforward to add to the Import Cluster page/dialog a way for user to defined a user-friendly name and let user know if he/she does not that System will default to the system generated cluster integration ID.

Regarding the user-friendly name, the name should support what DNS names allow for, i.e. not just alphabets. Specifically, RFC 1123 says it permits "A" to "Z", "a" to "z", "0" to "9", and the hyphen (-). We may want to also include support for "." (dot) as well. You can also look at https://en.wikipedia.org/wiki/Hostname for additional recommendations and https://kubernetes.io/docs/concepts/overview/working-with-objects/names/.

We've updated the Import Cluster mockups to show the prompt for the user-friendly name -- see https://redhat.invisionapp.com/share/HVGA7O575AZ#/247445416_Import_Clusters.

[shtripat]

@julienlim we are planning to allow only alphanumeric for cluster name as . might create issue in graphite because in graphite a period character is used as separator for directory based hierarchy.

@r0h4n comments above tells to allow only alphabets though.

[julienlim]

Ack @shtripat on not going to allow for "." (dots) and will allow for alphanumeric. What about "-" (hyphen / dash)?

[shtripat]

Personally I feel hyphon could be allowed. @nthomas-redhat @r0h4n comments?

[cloudbehl]

@shtripat We now follow the ""(underscore) for replacing the dot(.). I think it's better we the same function in graphite to change space(" ") to ("").

[cloudbehl]

@shirshendu and I discussed this problem and found a solution which will have a less impact.

We follow two Tree structures in collectd
tendrl -> Cluster -> Volume -> Node -> Brick -> Block Device
tendrl -> Cluster -> Node -> Brick -> Block Device

In which clusters have cluster_id and volume has volume_id and so on.

What we can do is we can add another folder at "clusters" level in the hierarchy named "short_name" or "name"(whatever we want to call it) and inside we will have symlink created from cluster "name" to cluster id in clusters.

See screenshots for more clarity.

Created the "name" folder

created the symlink using.
$ ln -s source destinantion

I have tried this in my setup and it works fine. The grafana shows the name of a cluster as "ankush123" and the metrics are also getting populated. The only change required is, earlier metrics and templating variable used to start with tendrl.clusters.{cluster_id} and now the metrics will tendrl.name.{short_name}.

Discussing the scenarios.

1. This short_name or name folder will be created in all the scenario(if a user gives the name of a cluster at the time of import or not). This is because of the tendrl metrics will now start with "tendrl.name".
2. If a user gives the name the symlink will be from a name(the name provided by a user) to the {cluster_id} in clusters otherwise the name will default be integration id.

Discussed and shown the POC to @shirshendu @r0h4n

@shtripat @nthomas-redhat @julienlim @GowthamShanmugam any comments?

[shtripat]

@r0h4n Just by setting indexes/detected_cluster_id_to_integration_id/:detected_cluster_id to short_name doesnt change the NS.tendrl_context.integration on nodes and so during configure monitoring colletcd gets old integration_id only. Also even if we delete /clusters/{old integration id} other nodes get that cluster entry back as detected cluster again.

How can we set the NS.tendrl_context with new integration_id (short_name) for all the nodes without restart of node-agent? I feel this will resolve most of the issue. I have seen GI started on the nodes also finds the old value of NS.tendrl_context.integration. Can you help on this front?

[r0h4n]

@shtripat we dont need to replace Cluster.integration_id, check full comment #888 (comment)