common: add support for read affinity options per cluster-id from configmap

[Rakshith-R]

Describe the feature you'd like to have

Currently, read affinity options need to be passed through cmdline args and apply to volumes of all the clusters.

Since cephcsi handles multiple cluster which have may different set of crush location key-value pairs, we need to move
these options to be consumed per cluster-id basis from configmap.

What is the value to the end user? (why is it a priority?)

Users can configure different crush location for different cluster.
Ex: crush location labels for internal ceph cluster maybe different from a external ceph cluster.

How will we know we have a good solution? (acceptance criteria)

Concerned options are successfully picked up from configmap per cluster-id.

[Madhu-1]

@Rakshith-R This is a duplicate/part of #3649?

[Rakshith-R]

@Rakshith-R This is a duplicate/part of #3649?

Yea, I think we can call it a subset of that epic.
I think its fine to open issues for sub tasks so they can tracker better and completed independently.
Added related issues in the description.

[Madhu-1]

Sounds good 👍🏻 lets keep adding the trackers to the main one and close it when all is done

[iPraveenParihar]

from meet -

• support for read affinity options per cluster-id from ConfigMap
• users can still pass read affinity options from command line
• If not specified in ConfigMap, should read from command line
• If specified in ConfigMap and command line, options from ConfigMap should be taken as priority.

@Rakshith-R @Madhu-1

[Madhu-1]

from meet -

• support for read affinity options per cluster-id from ConfigMap
• users can still pass read affinity options from command line
• If not specified in ConfigMap, should read from command line
• If specified in ConfigMap and command line, options from ConfigMap should be taken as priority.

@Rakshith-R @Madhu-1

LGTM

[travisn]

What will the format of the configmap be with these settings?

[iPraveenParihar]

What will the format of the configmap be with these settings?

I was thinking something like this -

config.json: |-
    [
      {
        "clusterID": "<cluster-id-1>",
         ...
        "readAffinity": {
          "enabled": true,
          "crushLocationLabels": [
            "topology.kubernetes.io/region",
            "topology.kubernetes.io/zone"
          ]
        }
      },
      {
        "clusterID": "<cluster-id-2>",
         ...
        "readAffinity": {
          "enabled": true,
          "crushLocationLabels": [
            "topology.kubernetes.io/region",
            "topology.kubernetes.io/zone"
          ]
        }
      }
    ]

[travisn]

Looks good. A few thoughts for Rook clusters...

• Since Rook generates the configmap, it could always add all the topology labels that Rook supports.
• Rook could enable the read affinity by default (any reason not to?)
• A setting could be in the cephcluster CR to disable the read affinity

[Rakshith-R]

Looks good. A few thoughts for Rook clusters...

• Since Rook generates the configmap, it could always add all the topology labels that Rook supports.

👍
The current flow is the almost the same too https://github.com/rook/rook/blob/master/pkg/operator/ceph/csi/csi.go#L96 .

• Rook could enable the read affinity by default (any reason not to?)

There is a side affect of increased OSD memory consumption.
Before, request for same data block from clients in different zones would go to the same OSD.
With read affinity, OSDs in each zone would be required to serve the same data block separately leading
to increased OSD memory usage.
IMO, it would not be worth the trade off if the nodes are close by and inter zone network communication is not
that expensive.

• A setting could be in the cephcluster CR to disable the read affinity

I would prefer to keep the feature disabled by default for the above reason.

[travisn]

Sounds good to keep the feature disabled by default. If we can be clear about the amount of increased memory usage expected, it will help users to know when it is worth enabling it, or what additional resources are needed for the OSDs.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.