From cda502c84fade5ecffaf605e619f81085265be77 Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Tue, 17 Dec 2024 14:16:22 +0000 Subject: [PATCH 1/2] Update snitch/location config selection CASSANDA-19488 deprecates IEndpointSnitch and replaces it with InitialLocationProvider and NodeProximity classes. In Cassandra, the default yaml config retains the endpoint_snitch setting, whereas the alternative cassandra_latest yaml specifies the new options. CCM injects PropertyFileSnitch into config for multi datacenter clusters, but this is problematic as the old and new config options are mutually exclusive. This patch injects both the old and new settings into config when the cluster is populated, then removes whichever one was not present in the original yaml once it has been loaded. This allows dtests to run with both the legacy and latest configs and exercises both code paths. --- ccmlib/cluster.py | 5 +++++ ccmlib/node.py | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/ccmlib/cluster.py b/ccmlib/cluster.py index bca53108..09fb289d 100644 --- a/ccmlib/cluster.py +++ b/ccmlib/cluster.py @@ -275,7 +275,12 @@ def populate(self, nodes, debug=False, tokens=None, use_vnodes=None, ipprefix='1 self.use_vnodes = use_vnodes if isinstance(nodes, list): + # We set both the legacy and modern config options here, although they are mutually exclusive. When we + # actually finalise the config for nodes in the cluster we select which one to keep based on the supplied + # yaml file. One of the two keys must be present in the yaml, so we retain whichever that is. self.set_configuration_options(values={'endpoint_snitch': 'org.apache.cassandra.locator.PropertyFileSnitch'}) + self.set_configuration_options(values={'initial_location_provider': 'org.apache.cassandra.locator.TopologyFileLocationProvider'}) + node_count = 0 i = 0 for c in nodes: diff --git a/ccmlib/node.py b/ccmlib/node.py index a2b1a766..d596bd9d 100644 --- a/ccmlib/node.py +++ b/ccmlib/node.py @@ -1760,12 +1760,21 @@ def _update_yaml(self): if self.cluster.partitioner: data['partitioner'] = self.cluster.partitioner + # Get a map of combined cluster and node configuration with the node # configuration taking precedence. full_options = common.merge_configuration( self.cluster._config_options, self.__config_options, delete_empty=False) + if 'initial_location_provider' in data: + common.debug("yaml contained initial_location_provider, removing endpoint_snitch from merged config") + full_options.pop('endpoint_snitch', None) + elif 'endpoint_snitch' in data: + common.debug("yaml contained endpoint_snitch, removing initial_location_provider from merged config") + full_options.pop('initial_location_provider', None) + full_options.pop('node_proximity', None) + # Merge options with original yaml data. data = common.merge_configuration(data, full_options) From 0e91982141952817773ad08d906b6d3539dfd312 Mon Sep 17 00:00:00 2001 From: Marcus Eriksson Date: Wed, 18 Dec 2024 10:15:57 +0100 Subject: [PATCH 2/2] fix snitch/ilp setting --- ccmlib/cluster.py | 7 +++---- ccmlib/node.py | 21 +++++++++++++-------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/ccmlib/cluster.py b/ccmlib/cluster.py index 09fb289d..cf6607b8 100644 --- a/ccmlib/cluster.py +++ b/ccmlib/cluster.py @@ -275,11 +275,10 @@ def populate(self, nodes, debug=False, tokens=None, use_vnodes=None, ipprefix='1 self.use_vnodes = use_vnodes if isinstance(nodes, list): - # We set both the legacy and modern config options here, although they are mutually exclusive. When we - # actually finalise the config for nodes in the cluster we select which one to keep based on the supplied - # yaml file. One of the two keys must be present in the yaml, so we retain whichever that is. + # We set PFS here as a "marker" that we need to read cassandra-topology.properties for this cluster + # This is then checked in node.py::_update_yaml where we check if initial_location_provider is set in + # the yaml (indicating that modern config is supported) and we set TopologyFileLocationProvider if so self.set_configuration_options(values={'endpoint_snitch': 'org.apache.cassandra.locator.PropertyFileSnitch'}) - self.set_configuration_options(values={'initial_location_provider': 'org.apache.cassandra.locator.TopologyFileLocationProvider'}) node_count = 0 i = 0 diff --git a/ccmlib/node.py b/ccmlib/node.py index d596bd9d..af0be418 100644 --- a/ccmlib/node.py +++ b/ccmlib/node.py @@ -1760,20 +1760,25 @@ def _update_yaml(self): if self.cluster.partitioner: data['partitioner'] = self.cluster.partitioner - # Get a map of combined cluster and node configuration with the node # configuration taking precedence. full_options = common.merge_configuration( self.cluster._config_options, self.__config_options, delete_empty=False) - if 'initial_location_provider' in data: - common.debug("yaml contained initial_location_provider, removing endpoint_snitch from merged config") - full_options.pop('endpoint_snitch', None) - elif 'endpoint_snitch' in data: - common.debug("yaml contained endpoint_snitch, removing initial_location_provider from merged config") - full_options.pop('initial_location_provider', None) - full_options.pop('node_proximity', None) + if 'endpoint_snitch' in full_options and full_options['endpoint_snitch'] == 'org.apache.cassandra.locator.PropertyFileSnitch': + # multi dc cluster, needs to read cassandra-topology.properties - if cassandra.yaml is modern, we use TFLP and unset the endpoint_snitch + if 'initial_location_provider' in data: + data['initial_location_provider'] = 'org.apache.cassandra.locator.TopologyFileLocationProvider' + full_options.pop('endpoint_snitch', None) + else: + # test might set endpoint_snitch: GPFS for example, in this case we need to keep that and unset ILP (or other way round) + if 'initial_location_provider' in full_options: + data.pop('endpoint_snitch', None) + elif 'endpoint_snitch' in full_options: + data.pop('initial_location_provider', None) + data.pop('node_proximity', None) + # Merge options with original yaml data. data = common.merge_configuration(data, full_options)