Skip to content

Commit

Permalink
Make CCM config dirs temporary and randomly named.
Browse files Browse the repository at this point in the history
Tests were not passing because tests tried to use the same dir
concurrently.
  • Loading branch information
Lorak-mmk committed Feb 10, 2025
1 parent bbc4403 commit 3cafd22
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 39 deletions.
1 change: 1 addition & 0 deletions scylla/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ anyhow = "1.0.95"
tokio = { version = "1.34", features = ["test-util", "fs", "process"] }
serde_yaml = {version = "0.9.34+deprecated"}
time = "0.3.36"
tempfile = "3.16"

[[bench]]
name = "benchmark"
Expand Down
81 changes: 48 additions & 33 deletions scylla/tests/ccm_integration/ccm/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ use anyhow::{Context, Error};
use std::collections::{HashMap, HashSet};
use std::fmt::Display;
use std::net::{AddrParseError, IpAddr, Ipv4Addr, Ipv6Addr};
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::process::Command;
use std::str::FromStr;
use std::sync::Arc;
use tempfile::TempDir;
use tokio::fs::{metadata, File};
use tokio::io::{AsyncBufReadExt, BufReader};
use tokio::sync::RwLock;
use tracing::debug;

#[derive(Debug, Clone, PartialEq)]
pub(crate) enum DBType {
Expand Down Expand Up @@ -133,7 +135,7 @@ pub(crate) struct Node {
config: NodeConfig,
logged_cmd: Arc<LoggedCmd>,
/// A `--config-dir` for ccm
config_dir: String,
config_dir: PathBuf,
}

#[allow(dead_code)]
Expand All @@ -142,7 +144,7 @@ impl Node {
opts: NodeOptions,
config: NodeConfig,
logged_cmd: Arc<LoggedCmd>,
config_dir: String,
config_dir: PathBuf,
) -> Self {
Node {
opts,
Expand Down Expand Up @@ -205,7 +207,7 @@ impl Node {
self.opts.name(),
"start".to_string(),
"--config-dir".to_string(),
self.config_dir.clone(),
self.config_dir.to_string_lossy().to_string(),
];
for opt in opts.unwrap_or(&[
NodeStartOption::WaitForBinaryProto,
Expand All @@ -232,7 +234,7 @@ impl Node {
self.opts.name(),
"stop".to_string(),
"--config-dir".to_string(),
self.config_dir.clone(),
self.config_dir.to_string_lossy().to_string(),
];
for opt in opts.unwrap_or(&[]) {
match opt {
Expand All @@ -255,7 +257,7 @@ impl Node {
self.opts.name(),
"remove".to_string(),
"--config-dir".to_string(),
self.config_dir.clone(),
self.config_dir.to_string_lossy().to_string(),
];
self.logged_cmd
.run_command("ccm", &args, RunOptions::new())
Expand All @@ -269,7 +271,7 @@ impl Node {
self.opts.name(),
"updateconf".to_string(),
"--config-dir".to_string(),
self.config_dir.clone(),
self.config_dir.to_string_lossy().to_string(),
];

// converting config to space separated list of `key:value`
Expand Down Expand Up @@ -355,8 +357,8 @@ pub(crate) struct Cluster {
destroyed: bool,
logged_cmd: Arc<LoggedCmd>,
opts: ClusterOptions,
ccm_dir: String,
cluster_dir: String,
ccm_dir: TempDir,
cluster_dir: PathBuf,
}

impl Drop for Cluster {
Expand All @@ -375,12 +377,12 @@ impl Cluster {
/// A `--config-dir` for ccm
/// Since ccm does not support parallel access to different cluster in the same `config-dir`
/// we had to isolate each cluster into its own config directory
fn config_dir(&self) -> &str {
&self.ccm_dir
fn config_dir(&self) -> &Path {
self.ccm_dir.path()
}

/// A directory that is created by CCM where it stores this cluster
fn cluster_dir(&self) -> &str {
fn cluster_dir(&self) -> &Path {
&self.cluster_dir
}

Expand Down Expand Up @@ -448,7 +450,7 @@ impl Cluster {
"--remote-debug-port".to_string(),
node.debug_port().to_string(),
"--config-dir".to_string(),
node.config_dir.clone(),
node.config_dir.to_string_lossy().to_string(),
];
match node.opts.db_type {
DBType::Scylla => {
Expand All @@ -473,7 +475,7 @@ impl Cluster {
},
self.opts.config.clone(),
self.logged_cmd.clone(),
self.config_dir().to_string(),
self.config_dir().to_owned(),
);
let node = Arc::new(RwLock::new(node));
self.nodes.push(node.clone());
Expand All @@ -486,29 +488,33 @@ impl Cluster {
opts.ip_prefix = Self::sniff_ipv4_prefix().await?
};

let ccm_dir = format!("{}/{}", *ROOT_CCM_DIR, opts.name);
let cluster_dir = format!("{}/{}", ccm_dir, opts.name);
let ccm_dir = TempDir::with_prefix_in(&opts.name, &*ROOT_CCM_DIR)
.context("Failed to create temp dir for the cluster")?;
let ccm_dir_path = ccm_dir.path();
let cluster_dir = ccm_dir.path().join(&opts.name);

println!("Config dir: {}", ccm_dir);
println!("Config dir: {:?}", ccm_dir.path());

match metadata(ccm_dir.as_str()).await {
match metadata(ccm_dir_path).await {
Ok(mt) => {
if !mt.is_dir() {
return Err(Error::msg(format!(
"{} already exists and it is not a directory",
ccm_dir
"{:?} already exists and it is not a directory",
ccm_dir_path
)));
}
}
Err(err) => match err.kind() {
std::io::ErrorKind::NotFound => {
tokio::fs::create_dir_all(ccm_dir.as_str())
.await
.with_context(|| format! {"failed to create root directory {}", ccm_dir})?;
tokio::fs::create_dir_all(ccm_dir_path).await.with_context(
|| format! {"failed to create root directory {:?}", ccm_dir_path},
)?;
}
_ => {
return Err(Error::from(err)
.context(format!("failed to create root directory {}", ccm_dir)));
return Err(Error::from(err).context(format!(
"failed to create root directory {:?}",
ccm_dir_path
)));
}
},
}
Expand All @@ -534,8 +540,12 @@ impl Cluster {
}

pub(crate) async fn init(&mut self) -> Result<(), Error> {
let cluster_dir = PathBuf::from(self.opts.cluster_dir());
let config_dir = self.opts.config_dir();
let cluster_dir = PathBuf::from(self.cluster_dir());
let config_dir = self.config_dir();
debug!(
"Init cluster, cluster_dir: {:?}, config_dir: {:?}",
cluster_dir, config_dir
);
if cluster_dir.exists() {
tokio::fs::remove_dir_all(&cluster_dir)
.await
Expand All @@ -554,7 +564,7 @@ impl Cluster {
"-i".to_string(),
self.opts.ip_prefix.to_string(),
"--config-dir".to_string(),
config_dir.to_string(),
config_dir.to_string_lossy().into_owned(),
];
if self.opts.db_type == DBType::Scylla {
args.push("--scylla".to_string());
Expand All @@ -577,7 +587,7 @@ impl Cluster {
"-n".to_string(),
nodes_str,
"--config-dir".to_string(),
config_dir.to_string(),
config_dir.to_string_lossy().into_owned(),
];
self.logged_cmd
.run_command("ccm", &args, RunOptions::new())
Expand All @@ -598,7 +608,7 @@ impl Cluster {
let mut args = vec![
"start".to_string(),
"--config-dir".to_string(),
self.config_dir().to_string(),
self.config_dir().to_string_lossy().into_owned(),
];
for opt in opts.unwrap_or(&[
NodeStartOption::WaitForBinaryProto,
Expand Down Expand Up @@ -632,7 +642,12 @@ impl Cluster {
.logged_cmd
.run_command(
"ccm",
&["stop", &self.opts.name, "--config-dir", &self.config_dir()],
&[
"stop",
&self.opts.name,
"--config-dir",
&self.config_dir().to_string_lossy().into_owned(),
],
RunOptions::new(),
)
.await
Expand All @@ -656,7 +671,7 @@ impl Cluster {
"remove",
&self.opts.name,
"--config-dir",
&self.config_dir(),
&self.config_dir().to_string_lossy().into_owned(),
])
.output()
{
Expand All @@ -681,7 +696,7 @@ impl Cluster {
"remove",
&self.opts.name,
"--config-dir",
&self.config_dir(),
&self.config_dir().to_string_lossy().into_owned(),
],
RunOptions::new(),
)
Expand Down
20 changes: 14 additions & 6 deletions scylla/tests/ccm_integration/ccm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,20 @@ pub(crate) mod cluster;
mod logged_cmd;
pub(crate) mod node_config;

use std::cell::LazyCell;
use std::ops::AsyncFnOnce;
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::LazyLock;

use cluster::Cluster;
use cluster::ClusterOptions;
use tokio::sync::Mutex;
use tracing::info;

pub(crate) const CLUSTER_VERSION: LazyCell<String> =
LazyCell::new(|| std::env::var("SCYLLA_TEST_CLUSTER").unwrap_or("release:6.2.2".to_string()));
pub(crate) static CLUSTER_VERSION: LazyLock<String> =
LazyLock::new(|| std::env::var("SCYLLA_TEST_CLUSTER").unwrap_or("release:6.2.2".to_string()));

const TEST_KEEP_CLUSTER_ON_FAILURE: LazyCell<bool> = LazyCell::new(|| {
const TEST_KEEP_CLUSTER_ON_FAILURE: LazyLock<bool> = LazyLock::new(|| {
std::env::var("TEST_KEEP_CLUSTER_ON_FAILURE")
.unwrap_or("".to_string())
.parse::<bool>()
Expand All @@ -24,10 +25,10 @@ const TEST_KEEP_CLUSTER_ON_FAILURE: LazyCell<bool> = LazyCell::new(|| {
/// CCM does not allow to have one active cluster within one config directory
/// To have more than two active CCM cluster at the same time we isolate each cluster into separate
/// config director, each config directory is created in `ROOT_CCM_DIR`.
pub(crate) const ROOT_CCM_DIR: LazyCell<String> = LazyCell::new(|| {
pub(crate) const ROOT_CCM_DIR: LazyLock<String> = LazyLock::new(|| {
let cargo_manifest_dir = env!("CARGO_MANIFEST_DIR");
let ccm_root_dir_env = std::env::var("CCM_ROOT_DIR");
match ccm_root_dir_env {
let ccm_root_dir = match ccm_root_dir_env {
Ok(x) => x,
Err(e) => {
info!(
Expand All @@ -36,7 +37,14 @@ pub(crate) const ROOT_CCM_DIR: LazyCell<String> = LazyCell::new(|| {
);
cargo_manifest_dir.to_string() + "/ccm_data"
}
};
let path = PathBuf::from(&ccm_root_dir);
if !path.try_exists().unwrap() {
info!("Directory {:?} not found, creating", path);
std::fs::create_dir(path).unwrap();
}

ccm_root_dir
});

pub(crate) async fn run_ccm_test<C, T>(make_cluster_options: C, test_body: T)
Expand Down
3 changes: 3 additions & 0 deletions scylla/tests/ccm_integration/test_example.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::sync::Arc;

use crate::ccm::cluster::{Cluster, ClusterOptions};
use crate::ccm::{run_ccm_test, CLUSTER_VERSION};
use crate::common::utils::setup_tracing;

use scylla::client::session::Session;
use scylla::client::session_builder::SessionBuilder;
Expand All @@ -27,6 +28,7 @@ async fn get_session(cluster: &Cluster) -> Session {

#[tokio::test]
async fn test_cluster_lifecycle1() {
setup_tracing();
async fn test(cluster: Arc<Mutex<Cluster>>) -> () {
let cluster = cluster.lock().await;
let session = get_session(&cluster).await;
Expand Down Expand Up @@ -54,6 +56,7 @@ async fn test_cluster_lifecycle1() {

#[tokio::test]
async fn test_cluster_lifecycle2() {
setup_tracing();
async fn test(cluster: Arc<Mutex<Cluster>>) -> () {
let cluster = cluster.lock().await;
let session = get_session(&cluster).await;
Expand Down

0 comments on commit 3cafd22

Please sign in to comment.