Skip to content

[6/n] simplify FetchArtifactImpl interface #8040

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.

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

Merged
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion installinator/src/artifact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use tokio::sync::mpsc;
use tufaceous_artifact::{ArtifactHash, ArtifactHashId};
use uuid::Uuid;

use crate::{errors::HttpError, peers::FetchReceiver};
use crate::{errors::HttpError, fetch::FetchReceiver};

#[derive(Clone, Debug, Eq, PartialEq, Args)]
pub(crate) struct ArtifactIdOpts {
Expand Down
50 changes: 26 additions & 24 deletions installinator/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ use tufaceous_lib::ControlPlaneZoneImages;
use update_engine::StepResult;

use crate::{
ArtifactWriter, WriteDestination,
artifact::ArtifactIdOpts,
peers::{DiscoveryMechanism, FetchedArtifact, Peers},
reporter::ProgressReporter,
write::{ArtifactWriter, WriteDestination},
fetch::{FetchArtifactBackend, FetchedArtifact, HttpFetchBackend},
peers::DiscoveryMechanism,
reporter::{HttpProgressBackend, ProgressReporter, ReportProgressBackend},
};

/// Installinator app.
Expand Down Expand Up @@ -90,12 +91,15 @@ struct DebugDiscoverOpts {

impl DebugDiscoverOpts {
async fn exec(self, log: &slog::Logger) -> Result<()> {
let peers = Peers::new(
let backend = FetchArtifactBackend::new(
log,
self.opts.mechanism.discover_peers(log).await?,
Box::new(HttpFetchBackend::new(
&log,
self.opts.mechanism.discover_peers(&log).await?,
)),
Duration::from_secs(10),
);
println!("discovered peers: {}", peers.display());
println!("discovered peers: {}", backend.peers().display());
Ok(())
}
}
Expand Down Expand Up @@ -182,19 +186,14 @@ impl InstallOpts {
let image_id = self.artifact_ids.resolve()?;

let discovery = self.discover_opts.mechanism.clone();
let discovery_log = log.clone();
let (progress_reporter, event_sender) =
ProgressReporter::new(log, image_id.update_id, move || {
let log = discovery_log.clone();
let discovery = discovery.clone();
async move {
Ok(Peers::new(
&log,
discovery.discover_peers(&log).await?,
Duration::from_secs(10),
))
}
});
let (progress_reporter, event_sender) = ProgressReporter::new(
log,
image_id.update_id,
ReportProgressBackend::new(
log,
HttpProgressBackend::new(log, discovery),
),
);
let progress_handle = progress_reporter.start();
let discovery = &self.discover_opts.mechanism;

Expand Down Expand Up @@ -234,7 +233,7 @@ impl InstallOpts {
)
.await?;

let address = host_phase_2_artifact.addr;
let address = host_phase_2_artifact.peer.address();

StepSuccess::new(host_phase_2_artifact)
.with_metadata(
Expand Down Expand Up @@ -273,7 +272,7 @@ impl InstallOpts {
)
.await?;

let address = control_plane_artifact.addr;
let address = control_plane_artifact.peer.address();

StepSuccess::new(control_plane_artifact)
.with_metadata(
Expand Down Expand Up @@ -493,9 +492,12 @@ async fn fetch_artifact(
cx,
&log,
|| async {
Ok(Peers::new(
Ok(FetchArtifactBackend::new(
&log,
discovery.discover_peers(&log).await?,
Box::new(HttpFetchBackend::new(
&log,
discovery.discover_peers(&log).await?,
)),
Duration::from_secs(10),
))
},
Expand All @@ -508,7 +510,7 @@ async fn fetch_artifact(
log,
"fetched {} bytes from {}",
artifact.artifact.num_bytes(),
artifact.addr,
artifact.peer,
);

Ok(artifact)
Expand Down
9 changes: 7 additions & 2 deletions installinator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,13 @@ pub(crate) enum DiscoverPeersError {
#[allow(unused)]
Retry(#[source] anyhow::Error),

#[error("failed to discover peers (no more retries left, will abort)")]
#[allow(unused)]
/// Abort further discovery.
///
/// The installinator must keep retrying until it has completed, which is why
/// there's no abort case here in the not cfg(test) case. However, we test
/// some abort-related functionality in tests.
#[cfg(test)]
#[error("failed to discover peers (will abort)")]
Abort(#[source] anyhow::Error),
}

Expand Down
Loading
Loading