Skip to content
Merged
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
10 changes: 10 additions & 0 deletions linera-client/src/client_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,16 @@ pub enum NetCommand {
#[arg(long, num_args=0..=1)]
binaries: Option<Option<PathBuf>>,

/// Don't build docker image. This assumes that the image is already built.
#[cfg(feature = "kubernetes")]
#[arg(long, default_value = "false")]
no_build: bool,

/// The name of the docker image to use.
#[cfg(feature = "kubernetes")]
#[arg(long, default_value = "linera:latest")]
docker_image_name: String,

Comment on lines +1007 to +1016
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the link with the kubernetes feature and why --kubernetes in the title of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--kubernetes calls the kubernetes version of linera net up, which needs to be compiled with the kubernetes feature. But if you don't compile with the kubernetes feature, the --kubernetes option is not even available.
You can still compile with the kubernetes feature and run just linera net up without --kubernetes, and it won't run the kubernetes version

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. I misread the test plan (which could have mentioned these extra arguments)

Copy link
Contributor

@ma2bd ma2bd Dec 2, 2024

Choose a reason for hiding this comment

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

nit: It's hard to understand the context because the motivation section doesn't mention kubernetes (and locally superficially seems contradictory although now I remember we have kind).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I edited the motivation, hopefully it's clearer now

/// Run with a specific path where the wallet and validator input files are.
/// If none, then a temporary directory is created.
#[arg(long)]
Expand Down
9 changes: 2 additions & 7 deletions linera-service/src/cli_wrappers/docker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ impl DockerImage {
&self.name
}

pub async fn build(name: String, binaries: &BuildArg, github_root: &PathBuf) -> Result<Self> {
pub async fn build(name: &String, binaries: &BuildArg, github_root: &PathBuf) -> Result<Self> {
Copy link
Contributor

Choose a reason for hiding this comment

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

&str?

let build_arg = match binaries {
BuildArg::Directory(bin_path) => {
// Get the binaries from the specified path
Expand Down Expand Up @@ -73,12 +73,7 @@ impl DockerImage {
),
]);

command
.arg(".")
.args(["-t", &name])
.spawn_and_wait()
.await?;

command.arg(".").args(["-t", name]).spawn_and_wait().await?;
Ok(docker_image)
}
}
Expand Down
23 changes: 20 additions & 3 deletions linera-service/src/cli_wrappers/local_kubernetes_net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ pub struct LocalKubernetesNetConfig {
pub num_initial_validators: usize,
pub num_shards: usize,
pub binaries: BuildArg,
pub no_build: bool,
pub docker_image_name: String,
pub policy: ResourceControlPolicy,
}

Expand All @@ -62,6 +64,8 @@ pub struct LocalKubernetesNet {
next_client_id: usize,
tmp_dir: Arc<TempDir>,
binaries: BuildArg,
no_build: bool,
docker_image_name: String,
kubectl_instance: Arc<Mutex<KubectlInstance>>,
kind_clusters: Vec<KindCluster>,
num_initial_validators: usize,
Expand Down Expand Up @@ -96,6 +100,8 @@ impl SharedLocalKubernetesNetTestingConfig {
num_initial_validators: 4,
num_shards: 4,
binaries,
no_build: false,
docker_image_name: String::from("linera:latest"),
policy: ResourceControlPolicy::devnet(),
})
}
Expand All @@ -122,6 +128,8 @@ impl LineraNetConfig for LocalKubernetesNetConfig {
self.network,
self.testing_prng_seed,
self.binaries,
self.no_build,
self.docker_image_name,
KubectlInstance::new(Vec::new()),
clusters,
self.num_initial_validators,
Expand Down Expand Up @@ -300,10 +308,13 @@ impl LineraNet for LocalKubernetesNet {
}

impl LocalKubernetesNet {
#[allow(clippy::too_many_arguments)]
fn new(
network: Network,
testing_prng_seed: Option<u64>,
binaries: BuildArg,
no_build: bool,
docker_image_name: String,
Copy link
Contributor

@christos-h christos-h Dec 3, 2024

Choose a reason for hiding this comment

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

If this is &str (I don't think you need ownership) wouldn't it make life easier (no cloning everywhere)?

All the futures are joined at the end of the function so there are no borrows which will outlive the reference (I think).

kubectl_instance: KubectlInstance,
kind_clusters: Vec<KindCluster>,
num_initial_validators: usize,
Expand All @@ -315,6 +326,8 @@ impl LocalKubernetesNet {
next_client_id: 0,
tmp_dir: Arc::new(tempdir()?),
binaries,
no_build,
docker_image_name,
kubectl_instance: Arc::new(Mutex::new(kubectl_instance)),
kind_clusters,
num_initial_validators,
Expand Down Expand Up @@ -394,8 +407,12 @@ impl LocalKubernetesNet {
async fn run(&mut self) -> Result<()> {
let github_root = get_github_root().await?;
// Build Docker image
let docker_image =
DockerImage::build(String::from("linera:latest"), &self.binaries, &github_root).await?;
let docker_image_name = if self.no_build {
self.docker_image_name.clone()
} else {
DockerImage::build(&self.docker_image_name, &self.binaries, &github_root).await?;
self.docker_image_name.clone()
};

let base_dir = github_root
.join("kubernetes")
Expand All @@ -412,13 +429,13 @@ impl LocalKubernetesNet {

let mut validators_initialization_futures = Vec::new();
for (i, kind_cluster) in self.kind_clusters.iter().cloned().enumerate() {
let docker_image_name = docker_image.name().to_string();
let base_dir = base_dir.clone();
let github_root = github_root.clone();

let kubectl_instance = kubectl_instance_clone.clone();
let tmp_dir_path = tmp_dir_path_clone.clone();

let docker_image_name = docker_image_name.clone();
let future = async move {
let cluster_id = kind_cluster.id();
kind_cluster.load_docker_image(&docker_image_name).await?;
Expand Down
4 changes: 4 additions & 0 deletions linera-service/src/linera/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1592,6 +1592,8 @@ async fn run(options: &ClientOptions) -> Result<i32, anyhow::Error> {
policy_config,
kubernetes: true,
binaries,
no_build,
docker_image_name,
path: _,
storage: _,
external_protocol: _,
Expand All @@ -1604,6 +1606,8 @@ async fn run(options: &ClientOptions) -> Result<i32, anyhow::Error> {
*shards,
*testing_prng_seed,
binaries,
*no_build,
docker_image_name.clone(),
policy_config.into_policy(),
)
.boxed()
Expand Down
4 changes: 4 additions & 0 deletions linera-service/src/linera/net_up_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ pub async fn handle_net_up_kubernetes(
num_shards: usize,
testing_prng_seed: Option<u64>,
binaries: &Option<Option<PathBuf>>,
no_build: bool,
docker_image_name: String,
policy: ResourceControlPolicy,
) -> anyhow::Result<()> {
if num_initial_validators < 1 {
Expand All @@ -130,6 +132,8 @@ pub async fn handle_net_up_kubernetes(
num_initial_validators,
num_shards,
binaries: binaries.clone().into(),
no_build,
docker_image_name,
policy,
};
let (mut net, client1) = config.instantiate().await?;
Expand Down
Loading