remove prev_cockroachdb_version/CRDB_SEED_USE_PREV

.github/buildomat/build-and-test.sh

@@ -83,23 +83,6 @@ ptime -m cargo build -Z unstable-options --timings=json --workspace --tests --lo
 banner test
 ptime -m timeout 2h cargo nextest run --profile ci --locked --verbose
 
-#
-# If upgraded CockroachDB clusters are running on a different cluster version
-# than newly-initialized ones, run database-related tests from the older cluster
-# version.
-#
-if cmp tools/cockroachdb_version tools/prev_cockroachdb_version; then
-    # files are the same
-    true
-else
-    # if return code is > 1, cmp failed
-    [[ $? -gt 1 ]] && exit 2
-    # otherwise, files differ
-    cargo xtask download cockroach-prev
-    CRDB_SEED_USE_PREV=yes ptime -m timeout 1h cargo nextest run --profile ci --locked --verbose \
-        --filter-expr 'rdeps(nexus-test-utils) - package(omicron-dev)'
-fi
-
 #
 # https://github.com/nextest-rs/nextest/issues/16
 #

dev-tools/downloader/src/lib.rs

@@ -26,7 +26,6 @@ use strum::IntoEnumIterator;
 use tar::Archive;
 use tokio::io::{AsyncReadExt, AsyncWriteExt};
 use tokio::process::Command;
-use tokio::sync::Mutex;
 
 const BUILDOMAT_URL: &'static str =
     "https://buildomat.eng.oxide.computer/public/file";
@@ -61,9 +60,6 @@ enum Target {
     /// CockroachDB binary
     Cockroach,
 
-    /// CockroachDB binary (previous major version)
-    CockroachPrev,
-
     /// Web console assets
     Console,
 
@@ -140,8 +136,7 @@ pub async fn run_cmd(args: DownloadArgs) -> Result<()> {
                     }
                     Target::CargoHack => downloader.download_cargo_hack().await,
                     Target::Clickhouse => downloader.download_clickhouse().await,
-                    Target::Cockroach => downloader.download_cockroach("").await,
-                    Target::CockroachPrev => downloader.download_cockroach("prev_").await,
+                    Target::Cockroach => downloader.download_cockroach().await,
                     Target::Console => downloader.download_console().await,
                     Target::DendriteOpenapi => {
                         downloader.download_dendrite_openapi().await
@@ -571,23 +566,20 @@ impl<'a> Downloader<'a> {
         Ok(())
     }
 
-    async fn download_cockroach(&self, prefix: &str) -> Result<()> {
+    async fn download_cockroach(&self) -> Result<()> {
         let os = os_name()?;
 
         let download_dir = self.output_dir.join("downloads");
-        let destination_dir =
-            self.output_dir.join(format!("{prefix}cockroachdb"));
+        let destination_dir = self.output_dir.join("cockroachdb");
 
-        let checksums_path =
-            self.versions_dir.join(format!("{prefix}cockroachdb_checksums"));
+        let checksums_path = self.versions_dir.join("cockroachdb_checksums");
         let [checksum] = get_values_from_file(
             [&format!("CIDL_SHA256_{}", os.env_name())],
             &checksums_path,
         )
         .await?;
 
-        let versions_path =
-            self.versions_dir.join(format!("{prefix}cockroachdb_version"));
+        let versions_path = self.versions_dir.join("cockroachdb_version");
         let version = tokio::fs::read_to_string(&versions_path)
             .await
             .context("Failed to read version from {versions_path}")?;
@@ -613,11 +605,6 @@ impl<'a> Downloader<'a> {
 
         let tarball_path = download_dir.join(tarball_filename);
 
-        // Ensure that the download and unpack steps, which might write to the
-        // same paths, only run one at a time.
-        static MUTEX: Mutex<()> = Mutex::const_new(());
-        let mutex_lock = MUTEX.lock().await;
-
         tokio::fs::create_dir_all(&download_dir).await?;
         tokio::fs::create_dir_all(&destination_dir).await?;
 
@@ -635,9 +622,6 @@ impl<'a> Downloader<'a> {
         info!(self.log, "tarball path: {tarball_path}");
         unpack_tarball(&self.log, &tarball_path, &download_dir).await?;
 
-        // We are done writing to potentially shared files
-        drop(mutex_lock);
-
         // This is where the binary will end up eventually
         let cockroach_binary = destination_dir.join("bin/cockroach");
 

nexus/db-queries/src/db/datastore/cockroachdb_settings.rs

@@ -159,8 +159,8 @@ mod test {
             // This is the expected value while running tests normally.
             assert_eq!(version, CockroachDbClusterVersion::NEWLY_INITIALIZED);
         } else if settings.preserve_downgrade == version.to_string() {
-            // This is the expected value if we are running under
-            // `CRDB_SEED_USE_PREV=yes`.
+            // This is the expected value if the cluster was created on a
+            // previous version and `cluster.preserve_downgrade_option` was set.
             assert_eq!(version, CockroachDbClusterVersion::POLICY);
         } else {
             panic!(

nexus/types/src/deployment/planning_input.rs

@@ -911,15 +911,11 @@ mod tests {
             cockroachdb_version
         );
 
-        let prev_cockroachdb_version =
-            include_str!("../../../../tools/prev_cockroachdb_version")
-                .trim_start_matches('v')
-                .rsplit_once('.')
-                .unwrap()
-                .0;
+        // In the next "tick" release, this version will be stored in a
+        // different file.
         assert_eq!(
             CockroachDbClusterVersion::POLICY.to_string(),
-            prev_cockroachdb_version
+            cockroachdb_version
         );
     }
 }

test-utils/src/dev/db.rs

@@ -47,18 +47,10 @@ const COCKROACHDB_USER: &'static str = "root";
 
 /// Path to the CockroachDB binary
 const COCKROACHDB_BIN: &str = "cockroach";
-/// Path to the CockroachDB binary for the previous major version
-const PREV_COCKROACHDB_BIN: &str = concat!(
-    env!("CARGO_MANIFEST_DIR"),
-    "/../out/prev_cockroachdb/bin/cockroach"
-);
 
 /// The expected CockroachDB version
-pub(super) const COCKROACHDB_VERSION: &str =
+const COCKROACHDB_VERSION: &str =
     include_str!("../../../tools/cockroachdb_version");
-/// The expected previous CockroachDB version
-pub(super) const PREV_COCKROACHDB_VERSION: &str =
-    include_str!("../../../tools/prev_cockroachdb_version");
 
 /// Builder for [`CockroachStarter`] that supports setting some command-line
 /// arguments for the `cockroach start-single-node` command
@@ -84,65 +76,30 @@ pub struct CockroachStarterBuilder {
     args: Vec<String>,
     /// describes the command line that we're going to execute
     cmd_builder: tokio::process::Command,
-    /// expected version of CockroachDB
-    expected_version: &'static str,
     /// how long to wait for CockroachDB to report itself listening
     start_timeout: Duration,
     /// redirect stdout and stderr to files
     redirect_stdio: bool,
 }
 
 impl CockroachStarterBuilder {
-    /// Create a starter for `cockroach` on `$PATH`.
     pub fn new() -> CockroachStarterBuilder {
-        CockroachStarterBuilder::new_raw(COCKROACHDB_BIN)
-            .new_setup(COCKROACHDB_VERSION.trim())
-    }
-
-    /// Create a starter for the previous major version of `cockroach`, found in
-    /// `out/prev_cockroachdb/bin`.
-    pub fn new_prev() -> CockroachStarterBuilder {
-        CockroachStarterBuilder::new_raw(PREV_COCKROACHDB_BIN)
-            .new_setup(PREV_COCKROACHDB_VERSION.trim())
-    }
-
-    /// Helper for constructing a `CockroachStarterBuilder` that runs a specific
-    /// command instead of the usual `cockroach` binary
-    ///
-    /// This is used by `new()` as a starting point.  It's also used by the
-    /// tests to trigger failure modes that would be hard to reproduce with
-    /// `cockroach` itself.
-    fn new_raw(cmd: &str) -> CockroachStarterBuilder {
-        CockroachStarterBuilder {
-            store_dir: None,
-            listen_port: COCKROACHDB_DEFAULT_LISTEN_PORT,
-            env: BTreeMap::new(),
-            args: vec![String::from(cmd)],
-            cmd_builder: tokio::process::Command::new(cmd),
-            expected_version: "",
-            start_timeout: COCKROACHDB_START_TIMEOUT_DEFAULT,
-            redirect_stdio: false,
-        }
-    }
-
-    /// Common setup to run after `new_raw`.
-    fn new_setup(mut self, expected_version: &'static str) -> Self {
-        self.expected_version = expected_version;
+        let mut builder = CockroachStarterBuilder::new_raw(COCKROACHDB_BIN);
 
         // Copy the current set of environment variables.  We could instead
         // allow the default behavior of inheriting the current process
         // environment.  But we want to print these out.  If we use the default
         // behavior, it's possible that what we print out wouldn't match what
         // was used if some other thread modified the process environment in
         // between.
-        self.cmd_builder.env_clear();
+        builder.cmd_builder.env_clear();
 
         // Configure Go to generate a core file on fatal error.  This behavior
         // may be overridden by the user if they've set this variable in their
         // environment.
-        self.env("GOTRACEBACK", "crash");
+        builder.env("GOTRACEBACK", "crash");
         for (key, value) in std::env::vars_os() {
-            self.env(key, value);
+            builder.env(key, value);
         }
 
         // We use single-node insecure mode listening only on localhost.  We
@@ -155,9 +112,29 @@ impl CockroachStarterBuilder {
         // If we decide to let callers customize various listening addresses, we
         // should be careful about making it too easy to generate a more
         // insecure configuration.
-        self.arg("start-single-node").arg("--insecure").arg("--http-addr=:0");
+        builder
+            .arg("start-single-node")
+            .arg("--insecure")
+            .arg("--http-addr=:0");
+        builder
+    }
 
-        self
+    /// Helper for constructing a `CockroachStarterBuilder` that runs a specific
+    /// command instead of the usual `cockroach` binary
+    ///
+    /// This is used by `new()` as a starting point.  It's also used by the
+    /// tests to trigger failure modes that would be hard to reproduce with
+    /// `cockroach` itself.
+    fn new_raw(cmd: &str) -> CockroachStarterBuilder {
+        CockroachStarterBuilder {
+            store_dir: None,
+            listen_port: COCKROACHDB_DEFAULT_LISTEN_PORT,
+            env: BTreeMap::new(),
+            args: vec![String::from(cmd)],
+            cmd_builder: tokio::process::Command::new(cmd),
+            start_timeout: COCKROACHDB_START_TIMEOUT_DEFAULT,
+            redirect_stdio: false,
+        }
     }
 
     /// Redirect stdout and stderr for the "cockroach" process to files within
@@ -269,7 +246,6 @@ impl CockroachStarterBuilder {
             args: self.args,
             env: self.env,
             cmd_builder: self.cmd_builder,
-            expected_version: self.expected_version,
             start_timeout: self.start_timeout,
         })
     }
@@ -325,8 +301,6 @@ pub struct CockroachStarter {
     args: Vec<String>,
     /// the command line that we're going to execute
     cmd_builder: tokio::process::Command,
-    /// expected version of CockroachDB
-    expected_version: &'static str,
     /// how long to wait for the listen URL to be written
     start_timeout: Duration,
 }
@@ -367,11 +341,7 @@ impl CockroachStarter {
     pub async fn start(
         mut self,
     ) -> Result<CockroachInstance, CockroachStartError> {
-        check_db_version(
-            self.cmd_builder.as_std().get_program(),
-            self.expected_version,
-        )
-        .await?;
+        check_db_version().await?;
 
         let mut child_process = self.cmd_builder.spawn().map_err(|source| {
             CockroachStartError::BadCmd { cmd: self.args[0].clone(), source }
@@ -733,30 +703,18 @@ pub async fn format_sql(input: &str) -> Result<String, CockroachStartError> {
 }
 
 /// Verify that CockroachDB has the correct version
-async fn check_db_version(
-    bin: &OsStr,
-    expected_version: &str,
-) -> Result<(), CockroachStartError> {
-    if expected_version.is_empty() {
-        // This can happen when `new_raw` is called without running `new_setup`,
-        // which we do in some tests. In that situation, `bin` might not be
-        // CockroachDB!
-        return Ok(());
-    }
-
-    let mut cmd = tokio::process::Command::new(bin);
+pub async fn check_db_version() -> Result<(), CockroachStartError> {
+    let mut cmd = tokio::process::Command::new(COCKROACHDB_BIN);
     cmd.args(&["version", "--build-tag"]);
     cmd.env("GOTRACEBACK", "crash");
-    let output =
-        cmd.output().await.map_err(|source| CockroachStartError::BadCmd {
-            cmd: bin.to_string_lossy().into_owned(),
-            source,
-        })?;
+    let output = cmd.output().await.map_err(|source| {
+        CockroachStartError::BadCmd { cmd: COCKROACHDB_BIN.to_string(), source }
+    })?;
     let stdout = String::from_utf8_lossy(&output.stdout).into_owned();
     let stderr = String::from_utf8_lossy(&output.stderr).into_owned();
     if !output.status.success() {
         return Err(CockroachStartError::BadVersion {
-            expected: expected_version.to_string(),
+            expected: COCKROACHDB_VERSION.trim().to_string(),
             found: Err(anyhow!(
                 "error {:?} when checking CockroachDB version",
                 output.status.code()
@@ -783,10 +741,10 @@ async fn check_db_version(
         version_str
     };
 
-    if version_str != expected_version {
+    if version_str != COCKROACHDB_VERSION.trim() {
         return Err(CockroachStartError::BadVersion {
             found: Ok(version_str.to_string()),
-            expected: expected_version.to_string(),
+            expected: COCKROACHDB_VERSION.trim().to_string(),
             stdout,
             stderr,
         });

test-utils/src/dev/mod.rs

@@ -60,18 +60,16 @@ pub async fn test_setup_database(
     source: StorageSource,
 ) -> db::CockroachInstance {
     usdt::register_probes().expect("Failed to register USDT DTrace probes");
-    setup_database(log, source, db::CockroachStarterBuilder::new())
-        .await
-        .unwrap()
+    setup_database(log, source).await.unwrap()
 }
 
 // TODO: switch to anyhow entirely -- this function is currently a mishmash of
 // anyhow and unwrap/expect calls.
 async fn setup_database(
     log: &Logger,
     storage_source: StorageSource,
-    builder: db::CockroachStarterBuilder,
 ) -> Result<db::CockroachInstance> {
+    let builder = db::CockroachStarterBuilder::new();
     let mut builder = match &storage_source {
         StorageSource::DoNotPopulate | StorageSource::CopyFromSeed { .. } => {
             builder