Fix node discovery logic to properly handle dedicated cluster managers (#769)

Fixes dedicated cluster manager detection by implementing proper role constants
and upstream-compatible node filtering logic. Addresses the core issue where
nodes with only cluster management roles were incorrectly being included in
the connection pool for client requests.

Key changes:
- Add proper role constants matching OpenSearch server definitions
- Implement isDedicatedClusterManager with role-based logic matching Java client
- Add IncludeDedicatedClusterManagers configuration option
- Remove client-side role validation (handled server-side)

BEHAVIOR CHANGE: Dedicated cluster manager nodes are now excluded from
client request routing by default to match upstream Java client behavior.
Set IncludeDedicatedClusterManagers: true to preserve legacy behavior.

Fixes: #765

Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>

Author: sean-

CHANGELOG.md

@@ -6,11 +6,15 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ### Added
 - Enhanced cluster readiness checking for improved test reliability: `ostest.NewClient()` now includes readiness validation (health + cluster state + nodes info)
+- Configuration option `IncludeDedicatedClusterManagers` for controlling cluster manager node routing ([#765](https://github.com/opensearch-project/opensearch-go/issues/765))
 
 ### Changed
 - Refactor Client struct to use embedded mutex pattern for improved thread safety ([#775](https://github.com/opensearch-project/opensearch-go/pull/775))
 - Refactor metrics struct to use atomic counters for lock-free request/failure tracking ([#776](https://github.com/opensearch-project/opensearch-go/pull/776))
 - Test against Opensearch 2.19.4, 3.1, 3.3, and 3.4 ([#782](https://github.com/opensearch-project/opensearch-go/pull/782))
+- **BREAKING**: Enhanced node discovery to match OpenSearch server behavior ([#765](https://github.com/opensearch-project/opensearch-go/issues/765))
+  - Dedicated cluster manager nodes are now excluded from client request routing by default (best practice)
+  - Node selection logic now matches Java client `NodeSelector.SKIP_DEDICATED_CLUSTER_MASTERS` behavior
 
 ### Deprecated
 

opensearchtransport/discovery.go

@@ -34,22 +34,128 @@ import (
 	"net"
 	"net/http"
 	"net/url"
+	"slices"
 	"strings"
 	"sync"
 	"time"
 )
 
+// Node role constants to match upstream OpenSearch server definitions
+const (
+	// RoleData nodes store and retrieve data, perform indexing, searching, and
+	// aggregating operations on local shards. Available since OpenSearch 1.0.
+	// See: https://docs.opensearch.org/latest/install-and-configure/configuring-opensearch/configuration-system/
+	RoleData = "data"
+
+	// RoleIngest nodes pre-process data before storing via ingest pipelines.
+	// Available since OpenSearch 1.0.
+	// See: https://docs.opensearch.org/latest/install-and-configure/configuring-opensearch/configuration-system/
+	RoleIngest = "ingest"
+
+	// RoleClusterManager nodes manage overall cluster operations, cluster state,
+	// index creation/deletion, node health checks, and shard allocation.
+	// Available since OpenSearch 1.0.
+	// See: https://docs.opensearch.org/latest/install-and-configure/configuring-opensearch/configuration-system/
+	RoleClusterManager = "cluster_manager"
+
+	// RoleRemoteClusterClient nodes can act as cross-cluster clients and connect
+	// to remote clusters. Available since OpenSearch 1.0 (based on Elasticsearch 7.8.0).
+	// See: https://docs.opensearch.org/latest/install-and-configure/configuring-opensearch/configuration-system/
+	RoleRemoteClusterClient = "remote_cluster_client"
+
+	// RoleSearch nodes are dedicated to hosting search replica shards, allowing
+	// separation of search workloads from indexing workloads. Added in OpenSearch 3.0.0-beta1.
+	//
+	// IMPORTANT: This role cannot be combined with any other node role. This restriction
+	// has been enforced since OpenSearch 3.0.0-beta1.
+	//
+	// For searchable snapshots, use RoleWarm instead (recommended in OpenSearch 3.0+).
+	// See: https://docs.opensearch.org/latest/tuning-your-cluster/separate-index-and-search-workloads/
+	RoleSearch = "search"
+
+	// RoleWarm nodes provide access to warm indices and searchable snapshots.
+	// Added in OpenSearch 2.4. In OpenSearch 3.0+, warm role replaces search role
+	// for searchable snapshot functionality.
+	// See: https://docs.opensearch.org/latest/tuning-your-cluster/index/
+	RoleWarm = "warm"
+
+	// RoleML nodes are dedicated to running machine learning tasks and models.
+	// This is a dynamic role added by the ML Commons plugin, not a built-in server role.
+	// Available when ML Commons plugin is installed (typically OpenSearch 1.3+).
+	// See: https://docs.opensearch.org/latest/ml-commons-plugin/cluster-settings/
+	RoleML = "ml"
+
+	// RoleCoordinatingOnly represents nodes with no explicit roles (node.roles: []).
+	// These nodes delegate client requests to shards on data nodes and aggregate results.
+	// This is not a built-in role but a derived state when no roles are specified.
+	// Available since OpenSearch 1.0 as a configuration pattern.
+	// See: https://docs.opensearch.org/latest/install-and-configure/configuring-opensearch/configuration-system/
+	RoleCoordinatingOnly = "coordinating_only"
+
+	// RoleMaster is Deprecated: Use RoleClusterManager instead for inclusive language.
+	// Both roles are functionally identical but master role is deprecated.
+	// See: https://docs.opensearch.org/latest/install-and-configure/configuring-opensearch/configuration-system/
+	RoleMaster = "master"
+)
+
+// roleSet represents a set of node roles for efficient O(1) role lookups.
+type roleSet map[string]struct{}
+
+// newRoleSet creates a roleSet from a slice of role names.
+func newRoleSet(roles []string) roleSet {
+	rs := make(roleSet, len(roles))
+	for _, role := range roles {
+		rs[role] = struct{}{}
+		if role == RoleMaster {
+			// Alias deprecated "master" role to "cluster_manager" for internal checks,
+			// so we only need to perform a single check for "cluster_manager" elsewhere in the library.
+			rs[RoleClusterManager] = struct{}{}
+		}
+	}
+	return rs
+}
+
+// has checks if the roleSet contains a specific role using O(1) map lookup.
+func (rs roleSet) has(roleName string) bool {
+	_, exists := rs[roleName]
+	return exists
+}
+
+// isDedicatedClusterManager implements the logic from upstream Java client
+// NodeSelector.SKIP_DEDICATED_CLUSTER_MASTERS to determine if a node should be skipped.
+// It returns true for nodes that are cluster-manager eligible but have no "work" roles
+// (i.e., roles that actually process/store data or handle requests).
+// This matches OpenSearch server's SniffConnectionStrategy.DEFAULT_NODE_PREDICATE behavior.
+func (rs roleSet) isDedicatedClusterManager() bool {
+	// Must be cluster manager eligible first
+	if !rs.has(RoleClusterManager) {
+		return false
+	}
+
+	// Check if it has any "work" roles that make it non-dedicated
+	workRoles := []string{
+		RoleData,   // stores and retrieves data
+		RoleIngest, // processes incoming data
+		RoleWarm,   // handles warm/cold data storage
+		RoleSearch, // dedicated search processing
+		RoleML,     // machine learning tasks
+	}
+
+	return !slices.ContainsFunc(workRoles, rs.has)
+}
+
 // Discoverable defines the interface for transports supporting node discovery.
 type Discoverable interface {
 	DiscoverNodes() error
 }
 
 // nodeInfo represents the information about node in a cluster.
 type nodeInfo struct {
-	ID         string         `json:"id"`
-	Name       string         `json:"name"`
-	URL        *url.URL       `json:"url"`
-	Roles      []string       `json:"roles"`
+	ID         string   `json:"id"`
+	Name       string   `json:"name"`
+	URL        *url.URL `json:"url"`
+	Roles      []string `json:"roles"`
+	roleSet    roleSet
 	Attributes map[string]any `json:"attributes"`
 	HTTP       struct {
 		PublishAddress string `json:"publish_address"`
@@ -70,24 +176,23 @@ func (c *Client) DiscoverNodes() error {
 	}
 
 	for _, node := range nodes {
-		var isClusterManagerOnlyNode bool
+		// Build role set for efficient O(1) lookups
+		node.roleSet = newRoleSet(node.Roles)
 
-		if len(node.Roles) == 1 && (node.Roles[0] == "master" || node.Roles[0] == "cluster_manager") {
-			isClusterManagerOnlyNode = true
-		}
+		// Skip this node if the user wants to exclude cluster managers (default) and this node is a dedicated cluster master.
+		shouldSkip := !c.includeDedicatedClusterManagers && node.roleSet.isDedicatedClusterManager()
 
 		if debugLogger != nil {
 			var skip string
-			if isClusterManagerOnlyNode {
-				skip = "; [SKIP]"
+			if shouldSkip {
+				skip = "; [SKIP: dedicated cluster manager]"
 			}
 
-			debugLogger.Logf("Discovered node [%s]; %s; roles=%s%s\n", node.Name, node.URL, node.Roles, skip)
+			debugLogger.Logf("Discovered node %q; %s; roles=%v%s\n", node.Name, node.URL, node.Roles, skip)
 		}
 
-		// Skip cluster_manager only nodes
-		// TODO: Move logic to Selector?
-		if isClusterManagerOnlyNode {
+		// Skip dedicated cluster managers (matching upstream Java client behavior)
+		if shouldSkip {
 			continue
 		}