Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion bazel/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
load("@bazel_skylib//rules:common_settings.bzl", "bool_flag", "string_flag")
load("@buildifier_prebuilt//:rules.bzl", "buildifier")
load("//bazel:defs.bzl", "write_info_file_var")
load("//bazel:defs.bzl", "volatile_status", "write_info_file_var")

bool_flag(
name = "enable_malicious_code",
Expand Down Expand Up @@ -72,6 +72,47 @@ write_info_file_var(
visibility = ["//visibility:public"],
)

# This is the contents of the ctx.version_file, i.e. all non-STABLE_,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be compacted a bit, it's a lot to read. I think it's enough to point out that volatile-status.txt has to be a direct dependency, and when it is so then it behaves somewhat magically.

# volatile key value pairs outputted from ./workspace_status.sh.
#
# A note on the cache semantics:
# Say we have the following target that just saves the output of
# :volatile-status.txt to its output file:
#
# genrule(
# name = "what-is-volatile-status",
# outs = ["what-is-volatile-status.txt"],
# cmd = "cat $(location :volatile-status.txt) > $@",
# tools = [":volatile-status.txt"],
# )
#
# Building this in a fresh container yields:
# ./ci/container/container-run.sh bash -c \
# 'NODE_NAME=dm1-dll01 bazel build //bazel:what-is-volatile-status && cat bazel-bin/bazel/what-is-volatile-status.txt'
# BUILD_TIMESTAMP 1778503046
# DC dm1
# FORMATTED_DATE 2026 May 11 12 37 26 Mon
#
# The //bazel:what-is-volatile-status target should now be cached so changing the volatile keys
# should not change the output when we build it again (note we changed NODE_NAME to zh1):
# ./ci/container/container-run.sh bash -c \
# 'NODE_NAME=zh1-dll01 bazel build //bazel:what-is-volatile-status && cat bazel-bin/bazel/what-is-volatile-status.txt'
# BUILD_TIMESTAMP 1778503046
# DC dm1
# FORMATTED_DATE 2026 May 11 12 37 26 Mon
#
# However, let's say we change the what-is-volatile-status target such that it has to run again
# and use the new volatile keys. Then we would expect the output to change:
# ./ci/container/container-run.sh bash -c \
# 'NODE_NAME=zh1-dll01 bazel build //bazel:what-is-volatile-status && cat bazel-bin/bazel/what-is-volatile-status.txt'
# BUILD_TIMESTAMP 1778503130
# DC zh1
# FORMATTED_DATE 2026 May 11 12 38 50 Mon
volatile_status(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep write_info_file_var and this consistent: either we use volatile/non-volatile for both, or we use info_file/version_file for both

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean here. WDYM with "both"? Note that we use the volatile status for both the DC and the FARM_METADATA.

name = "volatile-status.txt",
visibility = ["//visibility:public"],
)

exports_files(
[
"prost_generator.sh",
Expand Down
10 changes: 10 additions & 0 deletions bazel/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,16 @@ write_info_file_var = rule(
},
)

def _volatile_status_impl(ctx):
return [DefaultInfo(
files = depset([ctx.version_file]),
runfiles = ctx.runfiles(files = [ctx.version_file]),
)]

volatile_status = rule(
implementation = _volatile_status_impl,
)

def file_size_check(
name,
file,
Expand Down
10 changes: 7 additions & 3 deletions bazel/workspace_status.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ else
fi

# Used as farm metadata
STABLE_FARM_METADATA="USER=${USER:-${HOSTUSER:-$(whoami)}}"
FARM_METADATA="USER=${USER:-${HOSTUSER:-$(whoami)}}"
if [ -n "${CI_JOB_NAME:-}" ]; then
STABLE_FARM_METADATA="$STABLE_FARM_METADATA;JOB_NAME=$CI_JOB_NAME"
FARM_METADATA="$FARM_METADATA;JOB_NAME=$CI_JOB_NAME"
fi
echo "STABLE_FARM_METADATA $STABLE_FARM_METADATA"
echo "FARM_METADATA $FARM_METADATA"

# Used for allocating a Farm testnet to the local DC in CI (Search for allocate_testnet_to_local_dc)
NODE_NAME="${NODE_NAME:-unknown}"
echo "DC ${NODE_NAME%%-*}"
Comment on lines +31 to +33
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes that the node name is always prefixed with <DC>-, correct? worth adding a note. Even better: I'd export NODE_NAME and do the transformation for the one file that needs the DC

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the assumption breaks for unknown though I think then DC will happen to be unknown

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I can add a node that the DC will be everything up to the first -.

7 changes: 0 additions & 7 deletions rs/tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ load("@rules_distroless//apt:defs.bzl", "dpkg_status")
load("@rules_distroless//distroless:defs.bzl", "passwd")
load("@rules_oci//oci:defs.bzl", "oci_image")
load("@rules_shell//shell:sh_binary.bzl", "sh_binary")
load("//bazel:defs.bzl", "write_info_file_var")
load(":system_tests.bzl", "oci_tar", "uvm_config_image")

package(default_visibility = ["//rs:system-tests-pkg"])
Expand Down Expand Up @@ -278,9 +277,3 @@ genrule(
outs = ["version-test.txt"],
cmd = "sed < $< 's/$$/-test/' > $@",
)

write_info_file_var(
name = "farm_metadata.txt",
varname = "STABLE_FARM_METADATA",
visibility = ["//visibility:public"],
)
10 changes: 5 additions & 5 deletions rs/tests/driver/src/driver/farm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::driver::ic::{AmountOfMemoryKiB, ImageSizeGiB, NrOfVCPUs, VmAllocation
use crate::driver::log_events;
use crate::driver::test_env::TestEnv;
use crate::driver::test_env::{RequiredHostFeaturesFromCmdLine, TestEnvAttribute};
use crate::driver::test_env_api::HasFarmUrl;
use crate::driver::test_env_api::{HasFarmUrl, read_var_from_volatile_status_file};
use anyhow::Result;
use chrono::{DateTime, Utc};
use ic_crypto_sha2::Sha256;
Expand Down Expand Up @@ -435,7 +435,7 @@ impl GroupSpec {
pub fn add_meta(mut self, group_base_name: &str) -> Self {
// Read injected farm metadata. We expect 'USER' and 'JOB_NAME' to be set and
// use sensible defaults if unset.
let mut runtime_args_map = parse_farm_metadata_env();
let mut runtime_args_map = parse_farm_metadata();
let user = runtime_args_map.remove("USER").unwrap_or("CI".to_string());
let job_schedule = runtime_args_map
.remove("JOB_NAME")
Expand All @@ -460,10 +460,10 @@ pub struct GroupMetadata {
pub test_name: String,
}

fn parse_farm_metadata_env() -> HashMap<String, String> {
// Read the FARM_METADATA environment variable containing ';' separated key pairs:
fn parse_farm_metadata() -> HashMap<String, String> {
// Read the FARM_METADATA variable from the volatile status file containing ';' separated key pairs:
// 'FARM_METADATA=FOO=bar;BAZ=quux'
let farm_metadata = std::env::var("FARM_METADATA")
let farm_metadata = read_var_from_volatile_status_file("FARM_METADATA")
.expect("Expected the environment variable FARM_METADATA to be defined!");
let mut map = HashMap::new();
let pairs = farm_metadata.split(';');
Expand Down
13 changes: 12 additions & 1 deletion rs/tests/driver/src/driver/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,7 @@ impl SystemTestSubGroup {
}

pub struct SystemTestGroup {
allocate_testnet_to_local_dc: bool,
setup: Option<Box<dyn PotSetupFn>>,
teardowns: Vec<Box<dyn PotSetupFn>>,
tests: Vec<SystemTestSubGroup>,
Expand Down Expand Up @@ -692,6 +693,7 @@ impl TestEnvAttribute for CliArguments {
impl SystemTestGroup {
pub fn new() -> Self {
Self {
allocate_testnet_to_local_dc: false,
setup: Default::default(),
teardowns: Default::default(),
tests: Default::default(),
Expand Down Expand Up @@ -744,6 +746,11 @@ impl SystemTestGroup {
self
}

pub fn allocate_testnet_to_local_dc(mut self) -> Self {
self.allocate_testnet_to_local_dc = true;
self
}

pub fn with_setup<F: PotSetupFn>(mut self, setup: F) -> Self {
self.setup = Some(Box::new(setup));
self
Expand Down Expand Up @@ -1296,7 +1303,11 @@ impl SystemTestGroup {
}
InfraProvider::Farm.write_attribute(&root_env);
if with_farm {
root_env.create_group_setup(group_ctx.group_base_name.clone(), args.no_group_ttl);
root_env.create_group_setup(
group_ctx.group_base_name.clone(),
self.allocate_testnet_to_local_dc,
args.no_group_ttl,
);
}
debug!(group_ctx.log(), "Created group context: {:?}", group_ctx);
}
Expand Down
47 changes: 43 additions & 4 deletions rs/tests/driver/src/driver/test_env_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@
use super::{
config::NODES_INFO,
driver_setup::SSH_AUTHORIZED_PRIV_KEYS_DIR,
farm::{DnsRecord, PlaynetCertificate},
farm::{DnsRecord, HostFeature, PlaynetCertificate},
test_setup::{GroupSetup, InfraProvider},
};
use crate::{
Expand Down Expand Up @@ -1482,11 +1482,21 @@ pub fn get_build_setupos_config_image_tool() -> PathBuf {
}

pub trait HasGroupSetup {
fn create_group_setup(&self, group_base_name: String, no_group_ttl: bool);
fn create_group_setup(
&self,
group_base_name: String,
allocate_testnet_to_local_dc: bool,
no_group_ttl: bool,
);
}

impl HasGroupSetup for TestEnv {
fn create_group_setup(&self, group_base_name: String, no_group_ttl: bool) {
fn create_group_setup(
&self,
group_base_name: String,
allocate_testnet_to_local_dc: bool,
no_group_ttl: bool,
) {
let log = self.logger();
if GroupSetup::attribute_exists(self) {
let group_setup = GroupSetup::read_attribute(self);
Expand All @@ -1501,11 +1511,25 @@ impl HasGroupSetup for TestEnv {
let group_setup = GroupSetup::new(group_base_name.clone(), timeout);
match InfraProvider::read_attribute(self) {
InfraProvider::Farm => {
let required_host_features = allocate_testnet_to_local_dc
.then(|| {
read_var_from_volatile_status_file("DC").filter(|dc| dc != "unknown")
})
.flatten()
.map(|dc| vec![HostFeature::DC(dc)])
.unwrap_or_default();
info!(
log,
"Creating group {} with required_host_features: {:?} ...",
group_setup.infra_group_name,
required_host_features
);

let farm_base_url = FarmBaseUrl::read_attribute(self);
let farm = Farm::new(farm_base_url.into(), self.logger());
let group_spec = GroupSpec {
vm_allocation: None,
required_host_features: vec![],
required_host_features,
preferred_network: None,
metadata: None,
};
Expand Down Expand Up @@ -1612,6 +1636,21 @@ pub fn read_dependency_from_env_to_string(v: &str) -> Result<String> {
read_dependency_to_string(path_from_env)
}

pub fn read_var_from_volatile_status_file(var_name: &str) -> Option<String> {
let volatile_status_path = get_dependency_path_from_env("VOLATILE_STATUS_FILE");
let content = fs::read_to_string(&volatile_status_path).unwrap_or_else(|e| {
panic!("Couldn't read content of the {volatile_status_path:?} file: {e:?}")
});
for line in content.lines() {
if let Some((name, value)) = line.split_once(' ')
&& name.trim() == var_name
{
return Some(value.trim().to_string());
}
}
None
}

pub fn load_wasm<P: AsRef<Path>>(p: P) -> Vec<u8> {
let mut wasm_bytes = std::fs::read(get_dependency_path(&p))
.unwrap_or_else(|e| panic!("Could not read WASM from {:?}: {e:?}", p.as_ref()));
Expand Down
1 change: 1 addition & 0 deletions rs/tests/nested/guestos_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use nested::{

fn main() -> Result<()> {
SystemTestGroup::new()
.allocate_testnet_to_local_dc()
.with_setup(nested::setup)
.add_test(systest!(upgrade_guestos))
.with_timeout_per_test(Duration::from_secs(30 * 60))
Expand Down
1 change: 1 addition & 0 deletions rs/tests/nested/hostos_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use nested::util::{

fn main() -> Result<()> {
SystemTestGroup::new()
.allocate_testnet_to_local_dc()
.with_setup(nested::setup)
.add_test(systest!(upgrade_hostos))
.with_timeout_per_test(Duration::from_secs(30 * 60))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use std::time::Duration;

fn main() -> Result<()> {
SystemTestGroup::new()
.allocate_testnet_to_local_dc()
.with_setup(|env| {
setup(
env,
Expand Down
1 change: 1 addition & 0 deletions rs/tests/nested/nns_recovery/nr_broken_dfinity_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use std::time::Duration;

fn main() -> Result<()> {
SystemTestGroup::new()
.allocate_testnet_to_local_dc()
.with_setup(|env| {
setup(
env,
Expand Down
1 change: 1 addition & 0 deletions rs/tests/nested/nns_recovery/nr_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use std::time::Duration;

fn main() -> Result<()> {
SystemTestGroup::new()
.allocate_testnet_to_local_dc()
.with_setup(|env| {
setup(
env,
Expand Down
1 change: 1 addition & 0 deletions rs/tests/nested/nns_recovery/nr_no_bless_fix_like_np.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use std::time::Duration;

fn main() -> Result<()> {
SystemTestGroup::new()
.allocate_testnet_to_local_dc()
.with_setup(|env| {
setup(
env,
Expand Down
1 change: 1 addition & 0 deletions rs/tests/nested/registration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::time::Duration;

fn main() -> Result<()> {
SystemTestGroup::new()
.allocate_testnet_to_local_dc()
.with_setup(nested::setup)
.add_test(systest!(nested::registration))
.with_timeout_per_test(Duration::from_secs(20 * 60))
Expand Down
1 change: 1 addition & 0 deletions rs/tests/run_systest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ done

exec \
env -C "$TEST_TMPDIR" \
VOLATILE_STATUS_FILE="$(realpath "$VOLATILE_STATUS_FILE")" \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point could we already read out the farm metadata and only set that, instead of silently passing the whole volatile status? Same with DC. It keeps everything tight and the inputs declarative.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I guess we could do the parsing in bash rather than in Rust.

"$(realpath $RUN_SCRIPT_TEST_EXECUTABLE)" \
--working-dir "$TEST_TMPDIR" \
"${test_driver_extra_args[@]}" \
Expand Down
13 changes: 8 additions & 5 deletions rs/tests/system_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,6 @@ def system_test(
_runtime_deps |= icos_config.runtime_deps
icos_images |= icos_config.icos_images

env_var_files["FARM_METADATA"] = "//rs/tests:farm_metadata.txt"

extra_args_simple = []
extra_args_colocated = []

Expand Down Expand Up @@ -210,6 +208,14 @@ def system_test(

tags = tags + ["requires-network", "system_test"]

# VOLATILE_STATUS_FILE is read during group creation time to determine the required host features and Farm metadata.
# In colocated tests the group is created in the non-colocated wrapper driver, we shouldn't use runtime_deps
# to depend on the VOLATILE_STATUS_FILE since that will cause the variable to be repointed to a path on the colocated VM
# which won't exist during group creaton time on the non-colocated wrapper. Instead we pass it as a regular environment variable
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you prefix it with RUN_SCRIPT_ and give it the same treatment as the other files read by the wrapper?

# and repoint it to its realpath in ./run_systest.sh.
env["VOLATILE_STATUS_FILE"] = "$(rootpath //bazel:volatile-status.txt)"
data.append("//bazel:volatile-status.txt")

sh_test(
name = test_name,
srcs = ["//rs/tests:run_systest.sh"],
Expand All @@ -225,8 +231,6 @@ def system_test(
visibility = visibility,
)

COLOCATED_RUNTIME_DEP_ENV_VARS = RUN_SCRIPT_RUNTIME_DEP_ENV_VARS

# create a colocated version of the test (marked as manual _unless_ the test is tagged with "colocate")
sh_test(
srcs = ["//rs/tests:run_systest.sh"],
Expand All @@ -239,7 +243,6 @@ def system_test(
env = env | {
"RUN_SCRIPT_TEST_EXECUTABLE": "$(rootpath //rs/tests/idx:colocate_test_bin)",
"RUN_SCRIPT_DRIVER_EXTRA_ARGS": " ".join(extra_args_colocated),
"COLOCATED_RUNTIME_DEP_ENV_VARS": COLOCATED_RUNTIME_DEP_ENV_VARS,
"COLOCATED_UVM_CONFIG_IMAGE_PATH": "$(rootpath //rs/tests:colocate_uvm_config_image)",
"COLOCATED_TEST_NAME": test_name,
"COLOCATED_TEST_DRIVER_VM_REQUIRED_HOST_FEATURES": json.encode(colocated_test_driver_vm_required_host_features),
Expand Down
Loading