feat: unify mayastor-client address/port flags

Unifies the `-a` and `-p` flags to a `-h` flag which takes a full url (optionally) without a scheme. Further, this makes the flag global, so it can be used in any position: ```bash RUST_BACKTRACE=1 RUST_LOG=mayastor=trace cargo run -- nexus list cargo run -- --bind 127.0.0.1 nexus list cargo run -- --bind 127.0.0.1:10124 nexus list cargo run -- -b http://127.0.0.1:10124 nexus list cargo run -- --bind http://127.0.0.1 nexus list cargo run -- nexus list --bind 127.0.0.1 cargo run -- nexus list --bind 127.0.0.1:10124 cargo run -- nexus list -b http://127.0.0.1:10124 cargo run -- nexus list --bind http://127.0.0.1 ``` Signed-off-by: Ana Hobden <[email protected]>
openebs · Feb 23, 2021 · 7f0a580 · 7f0a580
1 parent 239fe46
commit 7f0a580
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 49 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/mayastor/Cargo.toml b/mayastor/Cargo.toml
@@ -40,7 +40,7 @@ async-trait = "0.1.36"
 atty = "0.2"
 bincode = "1.2"
 byte-unit = "3.0.1"
-bytes = "0.4.12"
+bytes = "0.4"  # We are blocked on updating http until we update tonic.
 chrono = "0.4"
 clap = "2.33.0"
 colored_json = "*"
@@ -51,6 +51,7 @@ env_logger = "0.7"
 futures = "0.3"
 futures-timer = "2.0"
 git-version = "0.3"
+http = "0.1" # We are blocked on updating http until we update tonic.
 io-uring = "0.4.0"
 ioctl-gen = "0.1.1"
 jsonrpc = { path = "../jsonrpc"}

diff --git a/mayastor/src/bin/mayastor-client/context.rs b/mayastor/src/bin/mayastor-client/context.rs
@@ -1,7 +1,35 @@
 use crate::{BdevClient, JsonClient, MayaClient};
 use byte_unit::Byte;
+use bytes::Bytes;
 use clap::ArgMatches;
-use std::cmp::max;
+use http::uri::{Authority, PathAndQuery, Scheme, Uri};
+use snafu::{Backtrace, ResultExt, Snafu};
+use std::{cmp::max, str::FromStr};
+use tonic::transport::Endpoint;
+
+#[derive(Debug, Snafu)]
+pub enum Error {
+    #[snafu(display("Invalid URI bytes"))]
+    InvalidUriBytes {
+        source: http::uri::InvalidUriBytes,
+        backtrace: Backtrace,
+    },
+    #[snafu(display("Invalid URI parts"))]
+    InvalidUriParts {
+        source: http::uri::InvalidUriParts,
+        backtrace: Backtrace,
+    },
+    #[snafu(display("Invalid URI"))]
+    TonicInvalidUri {
+        source: tonic::codegen::http::uri::InvalidUri,
+        backtrace: Backtrace,
+    },
+    #[snafu(display("Invalid URI"))]
+    InvalidUri {
+        source: http::uri::InvalidUri,
+        backtrace: Backtrace,
+    },
+}
 
 pub struct Context {
     pub(crate) client: MayaClient,
@@ -12,7 +40,7 @@ pub struct Context {
 }
 
 impl Context {
-    pub(crate) async fn new(matches: &ArgMatches<'_>) -> Self {
+    pub(crate) async fn new(matches: &ArgMatches<'_>) -> Result<Self, Error> {
         let verbosity = if matches.is_present("quiet") {
             0
         } else {
@@ -22,27 +50,48 @@ impl Context {
             .value_of("units")
             .and_then(|u| u.chars().next())
             .unwrap_or('b');
-        let endpoint = {
-            let addr = matches.value_of("address").unwrap_or("127.0.0.1");
-            let port = value_t!(matches.value_of("port"), u16).unwrap_or(10124);
-            format!("{}:{}", addr, port)
+        // Ensure the provided host is defaulted & normalized to what we expect.
+        // TODO: This can be significantly cleaned up when we update tonic 0.1
+        // and its deps.
+        let host = if let Some(host) = matches.value_of("bind") {
+            let uri =
+                Uri::from_shared(Bytes::from(host)).context(InvalidUriBytes)?;
+            let mut parts = uri.into_parts();
+            if parts.scheme.is_none() {
+                parts.scheme = Scheme::from_str("http").ok();
+            }
+            if let Some(ref mut authority) = parts.authority {
+                if authority.port_part().is_none() {
+                    parts.authority = Authority::from_shared(Bytes::from(
+                        format!("{}:{}", authority.host(), 10124),
+                    ))
+                    .ok()
+                }
+            }
+            if parts.path_and_query.is_none() {
+                parts.path_and_query = PathAndQuery::from_str("/").ok();
+            }
+            let uri = Uri::from_parts(parts).context(InvalidUriParts)?;
+            Endpoint::from_shared(uri.to_string()).context(TonicInvalidUri)?
+        } else {
+            Endpoint::from_static("http://127.0.0.1:10124")
         };
-        let uri = format!("http://{}", endpoint);
+
         if verbosity > 1 {
-            println!("Connecting to {}", uri);
+            println!("Connecting to {:?}", host);
         }
 
-        let client = MayaClient::connect(uri.clone()).await.unwrap();
-        let bdev = BdevClient::connect(uri.clone()).await.unwrap();
-        let json = JsonClient::connect(uri).await.unwrap();
+        let client = MayaClient::connect(host.clone()).await.unwrap();
+        let bdev = BdevClient::connect(host.clone()).await.unwrap();
+        let json = JsonClient::connect(host).await.unwrap();
 
-        Context {
+        Ok(Context {
             client,
             bdev,
             json,
             verbosity,
             units,
-        }
+        })
     }
     pub(crate) fn v1(&self, s: &str) {
         if self.verbosity > 0 {

diff --git a/mayastor/src/bin/mayastor-client/main.rs b/mayastor/src/bin/mayastor-client/main.rs
@@ -1,18 +1,15 @@
-#[macro_use]
-extern crate clap;
-
 use byte_unit::Byte;
 use clap::{App, AppSettings, Arg};
-use tonic::{transport::Channel, Status};
+use snafu::{Backtrace, ResultExt, Snafu};
+use tonic::transport::Channel;
 
+use crate::context::Context;
 use ::rpc::mayastor::{
     bdev_rpc_client::BdevRpcClient,
     json_rpc_client::JsonRpcClient,
     mayastor_client::MayastorClient,
 };
 
-use crate::context::Context;
-
 mod bdev_cli;
 mod context;
 mod device_cli;
@@ -29,12 +26,28 @@ type MayaClient = MayastorClient<Channel>;
 type BdevClient = BdevRpcClient<Channel>;
 type JsonClient = JsonRpcClient<Channel>;
 
+#[derive(Debug, Snafu)]
+pub enum Error {
+    #[snafu(display("gRPC status: {}", source))]
+    GrpcStatus {
+        source: tonic::Status,
+        backtrace: Backtrace,
+    },
+    #[snafu(display("Context building error: {}", source))]
+    ContextError {
+        source: context::Error,
+        backtrace: Backtrace,
+    },
+}
+
+pub type Result<T, E = Error> = std::result::Result<T, E>;
+
 pub(crate) fn parse_size(src: &str) -> Result<Byte, String> {
     Byte::from_str(src).map_err(|_| src.to_string())
 }
 
 #[tokio::main(max_threads = 2)]
-async fn main() -> Result<(), Status> {
+async fn main() -> crate::Result<()> {
     env_logger::init();
 
     let matches = App::new("Mayastor CLI")
@@ -45,18 +58,13 @@ async fn main() -> Result<(), Status> {
             AppSettings::ColorAlways])
         .about("CLI utility for Mayastor")
         .arg(
-            Arg::with_name("address")
-                .short("a")
-                .long("address")
-                .default_value("127.0.0.1")
+            Arg::with_name("bind")
+                .short("b")
+                .long("bind")
+                .default_value("http://127.0.0.1:10124")
                 .value_name("HOST")
-                .help("IP address of mayastor instance"))
-        .arg(
-            Arg::with_name("port")
-                .short("p")
-                .long("port")
-                .default_value("10124").value_name("NUMBER")
-                .help("Port number of mayastor server"))
+                .help("The URI of mayastor instance")
+                .global(true))
         .arg(
             Arg::with_name("quiet")
                 .short("q")
@@ -68,7 +76,8 @@ async fn main() -> Result<(), Status> {
                 .long("verbose")
                 .multiple(true)
                 .help("Verbose output")
-                .conflicts_with("quiet"))
+                .conflicts_with("quiet")
+                .global(true))
         .arg(
             Arg::with_name("units")
                 .short("u")
@@ -89,22 +98,19 @@ async fn main() -> Result<(), Status> {
         .subcommand(jsonrpc_cli::subcommands())
         .get_matches();
 
-    let ctx = Context::new(&matches).await;
-
-    match matches.subcommand() {
-        ("bdev", Some(args)) => bdev_cli::handler(ctx, args).await?,
-        ("device", Some(args)) => device_cli::handler(ctx, args).await?,
-        ("nexus", Some(args)) => nexus_cli::handler(ctx, args).await?,
-        ("perf", Some(args)) => perf_cli::handler(ctx, args).await?,
-        ("pool", Some(args)) => pool_cli::handler(ctx, args).await?,
-        ("replica", Some(args)) => replica_cli::handler(ctx, args).await?,
-        ("rebuild", Some(args)) => rebuild_cli::handler(ctx, args).await?,
-        ("snapshot", Some(args)) => snapshot_cli::handler(ctx, args).await?,
-        ("jsonrpc", Some(args)) => {
-            jsonrpc_cli::json_rpc_call(ctx, args).await?
-        }
+    let ctx = Context::new(&matches).await.context(ContextError)?;
 
-        _ => eprintln!("Internal Error: Not implemented"),
+    let status = match matches.subcommand() {
+        ("bdev", Some(args)) => bdev_cli::handler(ctx, args).await,
+        ("device", Some(args)) => device_cli::handler(ctx, args).await,
+        ("nexus", Some(args)) => nexus_cli::handler(ctx, args).await,
+        ("perf", Some(args)) => perf_cli::handler(ctx, args).await,
+        ("pool", Some(args)) => pool_cli::handler(ctx, args).await,
+        ("replica", Some(args)) => replica_cli::handler(ctx, args).await,
+        ("rebuild", Some(args)) => rebuild_cli::handler(ctx, args).await,
+        ("snapshot", Some(args)) => snapshot_cli::handler(ctx, args).await,
+        ("jsonrpc", Some(args)) => jsonrpc_cli::json_rpc_call(ctx, args).await,
+        _ => panic!("Command not found"),
     };
-    Ok(())
+    status.context(GrpcStatus)
 }