Skip to content

[perf-cleanup] remove redundant tests and update mcast polling #9547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
zeeshanlakhani wants to merge 1 commit into
base: zl/mcast-implicit-lifecycle
Choose a base branch
from
Draft

[perf-cleanup] remove redundant tests and update mcast polling #9547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
235 changes: 6 additions & 229 deletions nexus/tests/integration_tests/multicast/api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,13 @@ async fn test_join_by_ip_ssm_with_sources(cptestctx: &ControlPlaneTestContext) {
}

/// Test SSM join-by-IP without sources should fail.
///
/// SSM addresses (232.0.0.0/8) require source IPs for implicit creation.
///
/// This is the canonical test for SSM source validation. The validation
/// code path is shared regardless of how you join (by IP, name, or ID) -
/// all routes converge on the same `instance_multicast_group_join` logic
/// that checks `is_ssm_address()` and rejects joins without sources.
#[nexus_test]
async fn test_join_by_ip_ssm_without_sources_fails(
cptestctx: &ControlPlaneTestContext,
Expand Down Expand Up @@ -516,158 +522,6 @@ async fn test_join_by_ip_ssm_without_sources_fails(
cleanup_instances(cptestctx, client, project_name, &[instance_name]).await;
}

/// Test joining an existing SSM group by ID without sources should fail.
///
/// This tests the SSM validation for join-by-ID path: if an SSM group exists
/// (created by first instance with sources), a second instance cannot join
/// by group ID without providing sources.
#[nexus_test]
async fn test_join_existing_ssm_group_by_id_without_sources_fails(
cptestctx: &ControlPlaneTestContext,
) {
let client = &cptestctx.external_client;
let project_name = "ssm-id-fail-project";

// Setup: SSM pool
let (_, _, _ssm_pool) = ops::join3(
create_project(client, project_name),
create_default_ip_pool(client),
create_multicast_ip_pool_with_range(
client,
"ssm-id-fail-pool",
(232, 40, 0, 1),
(232, 40, 0, 255),
),
)
.await;

create_instance(client, project_name, "ssm-id-inst-1").await;
create_instance(client, project_name, "ssm-id-inst-2").await;

// First instance creates SSM group with sources
let ssm_ip = "232.40.0.100";
let source_ip: IpAddr = "10.40.0.1".parse().unwrap();
let join_url_1 = format!(
"/v1/instances/ssm-id-inst-1/multicast-groups/{ssm_ip}?project={project_name}"
);

let join_body_1 =
InstanceMulticastGroupJoin { source_ips: Some(vec![source_ip]) };
let member_1: MulticastGroupMember =
put_upsert(client, &join_url_1, &join_body_1).await;

let group_id = member_1.multicast_group_id;

// Second instance tries to join by group ID WITHOUT sources - should fail
let join_url_by_id = format!(
"/v1/instances/ssm-id-inst-2/multicast-groups/{group_id}?project={project_name}"
);

let error = NexusRequest::new(
RequestBuilder::new(client, Method::PUT, &join_url_by_id)
.body(Some(&InstanceMulticastGroupJoin {
source_ips: None, // No sources!
}))
.expect_status(Some(StatusCode::BAD_REQUEST)),
)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.expect("Join by ID without sources should fail for SSM group");

let error_body: dropshot::HttpErrorResponseBody =
error.parsed_body().unwrap();
assert!(
error_body.message.contains("SSM")
|| error_body.message.contains("source"),
"Error should mention SSM or source IPs: {}",
error_body.message
);

let expected_group_name = format!("mcast-{}", ssm_ip.replace('.', "-"));
cleanup_instances(
cptestctx,
client,
project_name,
&["ssm-id-inst-1", "ssm-id-inst-2"],
)
.await;
wait_for_group_deleted(client, &expected_group_name).await;
}

/// Test joining an existing SSM group by NAME without sources should fail.
#[nexus_test]
async fn test_join_existing_ssm_group_by_name_without_sources_fails(
cptestctx: &ControlPlaneTestContext,
) {
let client = &cptestctx.external_client;
let project_name = "ssm-name-fail-project";

// Setup: SSM pool
let (_, _, _ssm_pool) = ops::join3(
create_project(client, project_name),
create_default_ip_pool(client),
create_multicast_ip_pool_with_range(
client,
"ssm-name-fail-pool",
(232, 45, 0, 1),
(232, 45, 0, 100),
),
)
.await;

create_instance(client, project_name, "ssm-name-inst-1").await;
create_instance(client, project_name, "ssm-name-inst-2").await;

// First instance creates SSM group with sources
let ssm_ip = "232.45.0.50";
let join_url = format!(
"/v1/instances/ssm-name-inst-1/multicast-groups/{ssm_ip}?project={project_name}"
);
let join_body = InstanceMulticastGroupJoin {
source_ips: Some(vec!["10.0.0.1".parse().unwrap()]),
};

put_upsert::<_, MulticastGroupMember>(client, &join_url, &join_body).await;

// Get the group's auto-generated name
let expected_group_name = format!("mcast-{}", ssm_ip.replace('.', "-"));

// Second instance tries to join by NAME without sources - should fail
let join_by_name_url = format!(
"/v1/instances/ssm-name-inst-2/multicast-groups/{expected_group_name}?project={project_name}"
);
let join_body_no_sources = InstanceMulticastGroupJoin { source_ips: None };

let error = NexusRequest::new(
RequestBuilder::new(client, Method::PUT, &join_by_name_url)
.body(Some(&join_body_no_sources))
.expect_status(Some(StatusCode::BAD_REQUEST)),
)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.expect("Join by name without sources should fail for SSM group");

let error_body: dropshot::HttpErrorResponseBody =
error.parsed_body().unwrap();
assert!(
error_body.message.contains("SSM")
|| error_body.message.contains("source"),
"Error should mention SSM or source IPs: {}",
error_body.message
);

cleanup_instances(
cptestctx,
client,
project_name,
&["ssm-name-inst-1", "ssm-name-inst-2"],
)
.await;
wait_for_group_deleted(client, &expected_group_name).await;
}

/// Test that SSM join-by-IP with empty sources array fails.
///
/// `source_ips: Some(vec![])` (empty array) is treated the same as
Expand Down Expand Up @@ -725,83 +579,6 @@ async fn test_ssm_with_empty_sources_array_fails(
cleanup_instances(cptestctx, client, project_name, &[instance_name]).await;
}

/// Test joining an existing SSM group by IP without sources fails.
///
/// When an SSM group already exists (created by first instance with sources),
/// a second instance joining by IP should still fail without sources since
/// the group is SSM.
#[nexus_test]
async fn test_join_existing_ssm_group_by_ip_without_sources_fails(
cptestctx: &ControlPlaneTestContext,
) {
let client = &cptestctx.external_client;
let project_name = "ssm-ip-existing-fail-project";

// Setup: SSM pool
let (_, _, _ssm_pool) = ops::join3(
create_project(client, project_name),
create_default_ip_pool(client),
create_multicast_ip_pool_with_range(
client,
"ssm-ip-existing-fail-pool",
(232, 47, 0, 1),
(232, 47, 0, 100),
),
)
.await;

create_instance(client, project_name, "ssm-ip-inst-1").await;
create_instance(client, project_name, "ssm-ip-inst-2").await;

// First instance creates SSM group with sources
let ssm_ip = "232.47.0.50";
let join_url = format!(
"/v1/instances/ssm-ip-inst-1/multicast-groups/{ssm_ip}?project={project_name}"
);
let join_body = InstanceMulticastGroupJoin {
source_ips: Some(vec!["10.0.0.1".parse().unwrap()]),
};

put_upsert::<_, MulticastGroupMember>(client, &join_url, &join_body).await;

let expected_group_name = format!("mcast-{}", ssm_ip.replace('.', "-"));

// Second instance tries to join by IP without sources - should fail
// Even though the group exists, SSM still requires sources
let join_url_2 = format!(
"/v1/instances/ssm-ip-inst-2/multicast-groups/{ssm_ip}?project={project_name}"
);
let join_body_no_sources = InstanceMulticastGroupJoin { source_ips: None };

let error = NexusRequest::new(
RequestBuilder::new(client, Method::PUT, &join_url_2)
.body(Some(&join_body_no_sources))
.expect_status(Some(StatusCode::BAD_REQUEST)),
)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.expect("Join existing SSM group by IP without sources should fail");

let error_body: dropshot::HttpErrorResponseBody =
error.parsed_body().unwrap();
assert!(
error_body.message.contains("SSM")
|| error_body.message.contains("source"),
"Error should mention SSM or source IPs: {}",
error_body.message
);

cleanup_instances(
cptestctx,
client,
project_name,
&["ssm-ip-inst-1", "ssm-ip-inst-2"],
)
.await;
wait_for_group_deleted(client, &expected_group_name).await;
}

/// Test join-by-IP with IP not in any pool should fail.
#[nexus_test]
async fn test_join_by_ip_not_in_pool_fails(
Expand Down
26 changes: 11 additions & 15 deletions nexus/tests/integration_tests/multicast/cache_invalidation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,15 @@ async fn test_cache_ttl_driven_refresh() {
"test_cache_ttl_driven_refresh",
)
.customize_nexus_config(&|config| {
// Set short cache TTLs for testing (2 seconds for sled cache)
// Set short cache TTLs for testing
config.pkg.background_tasks.multicast_reconciler.sled_cache_ttl_secs =
chrono::TimeDelta::seconds(2).to_std().unwrap();
chrono::TimeDelta::milliseconds(500).to_std().unwrap();
config
.pkg
.background_tasks
.multicast_reconciler
.backplane_cache_ttl_secs =
chrono::TimeDelta::seconds(1).to_std().unwrap();
chrono::TimeDelta::milliseconds(250).to_std().unwrap();

// Ensure multicast is enabled
config.pkg.multicast.enabled = true;
Expand Down Expand Up @@ -435,9 +435,8 @@ async fn test_cache_ttl_driven_refresh() {
.await
.expect("Should insert new inventory collection");

// Wait for cache TTL to expire (sled_cache_ttl = 1 second)
// Sleep for 1.5 seconds to ensure TTL has expired
tokio::time::sleep(std::time::Duration::from_millis(1500)).await;
// Wait for cache TTL to expire (sled_cache_ttl = 500ms)
tokio::time::sleep(std::time::Duration::from_millis(600)).await;

wait_for_condition_with_reconciler(
&cptestctx.lockstep_client,
Expand Down Expand Up @@ -487,19 +486,17 @@ async fn test_backplane_cache_ttl_expiry() {
"test_backplane_cache_ttl_expiry",
)
.customize_nexus_config(&|config| {
// Set backplane cache TTL to 1 second (shorter than sled cache to test
// independently)
// Set backplane cache TTL short (shorter than sled cache to test independently)
config
.pkg
.background_tasks
.multicast_reconciler
.backplane_cache_ttl_secs =
chrono::TimeDelta::seconds(1).to_std().unwrap();
chrono::TimeDelta::milliseconds(250).to_std().unwrap();

// Keep sled cache TTL longer to ensure we're testing backplane cache
// expiry
// Keep sled cache TTL longer to ensure we're testing backplane cache expiry
config.pkg.background_tasks.multicast_reconciler.sled_cache_ttl_secs =
chrono::TimeDelta::seconds(10).to_std().unwrap();
chrono::TimeDelta::seconds(2).to_std().unwrap();

// Ensure multicast is enabled
config.pkg.multicast.enabled = true;
Expand Down Expand Up @@ -550,9 +547,8 @@ async fn test_backplane_cache_ttl_expiry() {
.await
.expect("Should verify initial port mapping");

// Wait for backplane cache TTL to expire (500ms) but not sled cache (5 seconds)
// Sleep for 1 second to ensure backplane TTL has expired
tokio::time::sleep(std::time::Duration::from_secs(1)).await;
// Wait for backplane cache TTL to expire (250ms) but not sled cache (2 seconds)
tokio::time::sleep(std::time::Duration::from_millis(300)).await;

// Force cache access by triggering reconciler
// This will cause the reconciler to check backplane cache, find it expired,
Expand Down
Loading
Loading