github-actions[bot] commented on code in PR #63534:
URL: https://github.com/apache/doris/pull/63534#discussion_r3315808718
##########
regression-test/framework/src/main/groovy/org/apache/doris/regression/suite/Suite.groovy:
##########
@@ -384,6 +384,156 @@ class Suite implements GroovyInterceptable {
}
}
+ /**
+ * Create and manage multiple Docker clusters for multi-cluster test
scenarios.
+ *
+ * Usage example:
+ * dockers([
+ * "cluster_1": new ClusterOptions(cloudMode: true, feNum: 1, beNum:
1, msNum: 1),
+ * "cluster_2": new ClusterOptions(cloudMode: true, feNum: 1, beNum:
1, msNum: 0, externalMsCluster: "cluster_1")
+ * ]) { clusters ->
+ * connectWithDockerCluster(clusters.cluster_1) { sql "..." }
+ * connectWithDockerCluster(clusters.cluster_2) { sql "..." }
+ * }
+ *
+ * Important:
+ * - Must use LinkedHashMap to preserve insertion order
+ * - Clusters are created in map insertion order
+ * - Clusters are destroyed in reverse order (dependent clusters first)
+ * - If using externalMsCluster, the referenced cluster must appear
earlier in the map
+ *
+ * @param clusterConfigs LinkedHashMap of cluster name to ClusterOptions
+ * @param manual_init_clusters Set of cluster names to skip automatic
initialization
+ * @param actionSupplier Closure receiving Map<String, SuiteCluster> for
test execution
+ */
+ void dockers(LinkedHashMap<String, ClusterOptions> clusterConfigs,
+ Set<String> manual_init_clusters = new HashSet<>(), Closure
actionSupplier) throws Exception {
+ if (context.config.excludeDockerTest) {
+ logger.info("do not run the docker suite {}, because regression
config excludeDockerTest=true", name)
+ return
+ }
+
+ if (RegressionTest.getGroupExecType(group) !=
RegressionTest.GroupExecType.DOCKER) {
+ throw new Exception("Need to add 'docker' to docker suite's belong
groups, "
+ + "see example demo_p0/docker_action.groovy")
+ }
+
+ if (context.isMultiDockerClusterRunning) {
+ throw new Exception("Nested dockers() calls are not supported")
+ }
+
+ // Validate cluster configs
+ Set<String> clusterNames = new HashSet<>()
+ for (def entry : clusterConfigs.entrySet()) {
+ String clusterName = entry.key
+ ClusterOptions options = entry.value
+
+ if (clusterNames.contains(clusterName)) {
+ throw new Exception("Duplicate cluster name: ${clusterName}")
+ }
+ clusterNames.add(clusterName)
+
+ // Validate externalMsCluster reference
+ if (options.externalMsCluster != null &&
!options.externalMsCluster.isEmpty()) {
+ if (!clusterNames.contains(options.externalMsCluster)) {
+ throw new Exception("Cluster ${clusterName} references
non-existent external MS cluster: ${options.externalMsCluster}")
+ }
+ if (options.msNum > 0) {
+ throw new Exception("Cluster ${clusterName} cannot have
its own MS when using external MS cluster")
+ }
+ }
+ }
+
+ List<String> clusterNamesReversed = new
ArrayList<>(clusterConfigs.keySet())
+ Collections.reverse(clusterNamesReversed)
+
+ // Use LinkedHashMap to preserve order
+ Map<String, SuiteCluster> clusters = new LinkedHashMap<>()
+
+ try {
+ // Create and initialize clusters in order
+ for (def entry : clusterConfigs.entrySet()) {
+ String clusterName = entry.key
+ ClusterOptions options = entry.value
+
+ logger.info("Creating cluster: ${clusterName}")
+ SuiteCluster cluster = new SuiteCluster(clusterName,
context.config)
+
+ clusters.put(clusterName, cluster)
+ }
+
+ for (String clusterName : clusterNamesReversed) {
+ clusters.get(clusterName).destroy(true)
+ }
+
+ for (def entry : clusterConfigs.entrySet()) {
+ String clusterName = entry.key
+ ClusterOptions options = entry.value
+ SuiteCluster cluster = clusters.get(clusterName)
+
+ if (manual_init_clusters.contains(clusterName)) {
+ logger.info("Skipping initialization of cluster:
${clusterName}")
+ continue
+ }
+
+ // Determine cloud mode
+ boolean isCloud = false
+ if (options.cloudMode == null) {
+ // If not specified, use config default or run both modes
+ if (context.config.runMode == RunMode.CLOUD) {
+ isCloud = true
+ } else if (context.config.runMode == RunMode.NOT_CLOUD) {
+ isCloud = false
+ } else {
+ throw new Exception("cloudMode must be specified when
runMode is UNKNOWN for multi-cluster setup")
+ }
+ } else {
+ if (options.cloudMode == true && context.config.runMode ==
RunMode.NOT_CLOUD) {
+ logger.info("Skip cluster ${clusterName} because
cloudMode=true but regression test is in local mode")
+ continue
+ }
+ if (options.cloudMode == false && context.config.runMode
== RunMode.CLOUD) {
+ logger.info("Skip cluster ${clusterName} because
cloudMode=false but regression test is in cloud mode")
+ continue
+ }
+ isCloud = options.cloudMode
+ }
+ logger.info("Initializing cluster ${cluster.name} in ${isCloud
? 'cloud' : 'not_cloud'} mode")
+ cluster.init(options, isCloud)
+ logger.info("Cluster ${clusterName} initialized successfully")
+ }
+
+ // Wait for BE to report
+ Thread.sleep(5000)
+
+ Connection originConnection = context.threadLocalConn.get()
+ context.threadLocalConn.remove()
Review Comment:
`threadLocalConn` stores `ConnectionInfo`, not `Connection` (see
`SuiteContext.threadLocalConn`). If there is an existing connection this cast
will throw at runtime, and even the restore path would put the wrong type back
into the thread local. This should mirror `dockerImpl` and keep the saved value
as `def`/`ConnectionInfo`.
##########
regression-test/framework/src/main/groovy/org/apache/doris/regression/suite/Suite.groovy:
##########
@@ -384,6 +384,156 @@ class Suite implements GroovyInterceptable {
}
}
+ /**
+ * Create and manage multiple Docker clusters for multi-cluster test
scenarios.
+ *
+ * Usage example:
+ * dockers([
+ * "cluster_1": new ClusterOptions(cloudMode: true, feNum: 1, beNum:
1, msNum: 1),
+ * "cluster_2": new ClusterOptions(cloudMode: true, feNum: 1, beNum:
1, msNum: 0, externalMsCluster: "cluster_1")
+ * ]) { clusters ->
+ * connectWithDockerCluster(clusters.cluster_1) { sql "..." }
+ * connectWithDockerCluster(clusters.cluster_2) { sql "..." }
+ * }
+ *
+ * Important:
+ * - Must use LinkedHashMap to preserve insertion order
+ * - Clusters are created in map insertion order
+ * - Clusters are destroyed in reverse order (dependent clusters first)
+ * - If using externalMsCluster, the referenced cluster must appear
earlier in the map
+ *
+ * @param clusterConfigs LinkedHashMap of cluster name to ClusterOptions
+ * @param manual_init_clusters Set of cluster names to skip automatic
initialization
+ * @param actionSupplier Closure receiving Map<String, SuiteCluster> for
test execution
+ */
+ void dockers(LinkedHashMap<String, ClusterOptions> clusterConfigs,
+ Set<String> manual_init_clusters = new HashSet<>(), Closure
actionSupplier) throws Exception {
+ if (context.config.excludeDockerTest) {
+ logger.info("do not run the docker suite {}, because regression
config excludeDockerTest=true", name)
+ return
+ }
+
+ if (RegressionTest.getGroupExecType(group) !=
RegressionTest.GroupExecType.DOCKER) {
+ throw new Exception("Need to add 'docker' to docker suite's belong
groups, "
+ + "see example demo_p0/docker_action.groovy")
+ }
+
+ if (context.isMultiDockerClusterRunning) {
+ throw new Exception("Nested dockers() calls are not supported")
Review Comment:
`SuiteContext` does not define `isMultiDockerClusterRunning`, so the first
property read in `dockers()` throws `MissingPropertyException` before any
cluster is created. Please add a real field to `SuiteContext` (or keep this
guard local) before using it here.
##########
docker/runtime/doris-compose/cluster.py:
##########
@@ -851,6 +888,14 @@ def __init__(self, name, subnet, image, is_cloud,
is_root_user, fe_config,
self.extra_hosts = extra_hosts
self.coverage_dir = coverage_dir
self.cloud_store_config = cloud_store_config
+ self.external_ms_cluster = external_ms_cluster
+ self.instance_id = instance_id
+ if not self.instance_id:
Review Comment:
The configured `instance_id` never reaches the container: `docker_env()`
does not export `INSTANCE_ID`, while `common.sh`'s create/get instance paths
read `${INSTANCE_ID}` and `register_sql_server_cluster` still posts
`default_instance_id`. As a result `--instance-id`/external-MS multi-instance
startup creates or waits for an empty/default instance instead of the requested
one. Please export the chosen instance id and use it consistently in the
registration payloads.
##########
docker/runtime/doris-compose/cluster.py:
##########
@@ -851,6 +888,14 @@ def __init__(self, name, subnet, image, is_cloud,
is_root_user, fe_config,
self.extra_hosts = extra_hosts
self.coverage_dir = coverage_dir
self.cloud_store_config = cloud_store_config
+ self.external_ms_cluster = external_ms_cluster
+ self.instance_id = instance_id
Review Comment:
This stores `external_ms_cluster`, but no code actually uses the referenced
cluster for FDB/MS/recycler. `get_fdb_cluster()` and `get_meta_server_addr()`
still read this cluster's local node groups, and `command.py` still creates
local FDB/MS/recycler nodes. After the already-noted `external_ms_cluster`
NameError is fixed, `--external-ms` will still not use the external service and
can fail when `msNum=0`/`recyclerNum=0` as documented. Please load the
referenced cluster's endpoints and skip local service creation for this mode.
##########
regression-test/framework/src/main/groovy/org/apache/doris/regression/suite/SuiteCluster.groovy:
##########
@@ -356,6 +379,14 @@ class SuiteCluster {
cmd += ['--extra-hosts']
cmd += options.extraHosts
}
+ def envs = new ArrayList<String>(options.environments)
+ if (options.enableStorageVault) {
+ envs.add('ENABLE_STORAGE_VAULT=1')
+ }
+ if (!envs.isEmpty()) {
+ cmd += ['--env']
+ cmd += envs
Review Comment:
`doris-compose up` does not define a `--env` option, so any suite that sets
`options.environments` or `enableStorageVault` will fail in argparse with
`unrecognized arguments: --env ...` before `command.py` can read these values.
Please add the parser option and wire it into the cluster environment, or pass
storage-vault settings through an existing supported option.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
