Skip to content
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

Parallel describe #1086

Merged
merged 2 commits into from
Aug 31, 2020
Merged

Parallel describe #1086

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c0d4475
pubsys: check for copied AMIs in parallel
tjkirch Aug 23, 2020
aac917b
Add info logging before service requests to help explain timing
tjkirch Aug 27, 2020
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
39 changes: 27 additions & 12 deletions tools/pubsys/src/aws/ami/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ async fn _run(args: &Args, ami_args: &AmiArgs) -> Result<HashMap<String, Image>>

// First we need to find the account IDs for any given roles, so we can grant access to those
// accounts to copy the AMI and snapshots.
info!("Getting account IDs for target regions so we can grant access to copy source AMI");
let mut account_ids = get_account_ids(&regions, &base_region, &aws).await?;

// Get the account ID used in the base region; we don't need to grant to it so we can remove it
Expand All @@ -235,6 +236,7 @@ async fn _run(args: &Args, ami_args: &AmiArgs) -> Result<HashMap<String, Image>>

// If we have any accounts other than the base account, grant them access.
if !account_ids.is_empty() {
info!("Granting access to target accounts so we can copy the AMI");
let account_id_vec: Vec<_> = account_ids.into_iter().collect();

modify_snapshots(
Expand Down Expand Up @@ -278,17 +280,28 @@ async fn _run(args: &Args, ami_args: &AmiArgs) -> Result<HashMap<String, Image>>
ec2_clients.insert(region.clone(), ec2_client);
}

let mut copy_requests = Vec::with_capacity(regions.len());
// First, we check if the AMI already exists in each region.
info!("Checking whether AMIs already exist in target regions");
let mut get_requests = Vec::with_capacity(regions.len());
for region in regions.iter() {
let ec2_client = &ec2_clients[region];
if let Some(id) = get_ami_id(&ami_args.name, &ami_args.arch, region.name(), ec2_client)
.await
.context(error::GetAmiId {
name: &ami_args.name,
arch: &ami_args.arch,
region: region.name(),
})?
{
let get_request = get_ami_id(&ami_args.name, &ami_args.arch, region.name(), ec2_client);
let info_future = ready(region.clone());
get_requests.push(join(info_future, get_request));
}
let request_stream = stream::iter(get_requests).buffer_unordered(4);
let get_responses: Vec<(Region, std::result::Result<Option<String>, register::Error>)> =
request_stream.collect().await;

// If an AMI already existed, just add it to our list, otherwise prepare a copy request.
let mut copy_requests = Vec::with_capacity(regions.len());
for (region, get_response) in get_responses {
let get_response = get_response.context(error::GetAmiId {
name: &ami_args.name,
arch: &ami_args.arch,
region: region.name(),
})?;
if let Some(id) = get_response {
info!(
"Found '{}' already registered in {}: {}",
ami_args.name,
Expand All @@ -298,14 +311,16 @@ async fn _run(args: &Args, ami_args: &AmiArgs) -> Result<HashMap<String, Image>>
amis.insert(region.name().to_string(), Image::new(&id, &ami_args.name));
continue;
}
let request = CopyImageRequest {

let ec2_client = &ec2_clients[&region];
let copy_request = CopyImageRequest {
description: ami_args.description.clone(),
name: ami_args.name.clone(),
source_image_id: ids_of_image.image_id.clone(),
source_region: base_region.name().to_string(),
..Default::default()
};
let response_future = ec2_client.copy_image(request);
let copy_future = ec2_client.copy_image(copy_request);

let base_region_name = base_region.name();
// Store the region so we can output it to the user
Expand All @@ -318,7 +333,7 @@ async fn _run(args: &Args, ami_args: &AmiArgs) -> Result<HashMap<String, Image>>
region.name()
)
});
copy_requests.push(message_future.then(|_| join(region_future, response_future)));
copy_requests.push(message_future.then(|_| join(region_future, copy_future)));
}

// If all target regions already have the AMI, we're done.
Expand Down