Merge #1743

1743: feat(io-engine): listen on IPv6 unspecified by default r=dsharma-dc a=michaelbeaumont

Not able to fully build/test this because of SPDK issues yet but looks good in editor.

I removed the indirection with macros and strings, not sure if there was a particular need for that.

See #1731. This will need changes to the helm chart after being merged.

Co-authored-by: Mike Beaumont <[email protected]>

io-engine/src/bin/io-engine.rs

@@ -58,7 +58,7 @@ macro_rules! print_feature {
 
 io_engine::CPS_INIT!();
 fn start_tokio_runtime(args: &MayastorCliArgs) {
-    let grpc_address = grpc::endpoint(args.grpc_endpoint.clone());
+    let grpc_socket_addr = args.grpc_endpoint();
     let registration_addr = args.registration_endpoint.clone();
     let rpc_address = args.rpc_address.clone();
     let api_versions = args.api_versions.clone();
@@ -155,7 +155,7 @@ fn start_tokio_runtime(args: &MayastorCliArgs) {
                 grpc::MayastorGrpcServer::run(
                     &node_name,
                     &node_nqn,
-                    grpc_address,
+                    grpc_socket_addr,
                     rpc_address,
                     api_versions.clone(),
                 )
@@ -166,7 +166,7 @@ fn start_tokio_runtime(args: &MayastorCliArgs) {
                 Registration::init(
                     &node_name,
                     &node_nqn,
-                    &grpc_address.to_string(),
+                    &grpc_socket_addr.to_string(),
                     registration_addr,
                     api_versions,
                 );

io-engine/src/bin/nvmet.rs

@@ -27,7 +27,7 @@ const NEXUS: &str = "nexus-e1e27668-fbe1-4c8a-9108-513f6e44d342";
 fn start_tokio_runtime(args: &MayastorCliArgs) {
     let node_name = grpc::node_name(&args.node_name);
     let node_nqn = args.make_hostnqn();
-    let grpc_endpoint = grpc::endpoint(args.grpc_endpoint.clone());
+    let grpc_endpoint = args.grpc_endpoint();
     let rpc_address = args.rpc_address.clone();
     let api_versions = args.api_versions.clone();
 

io-engine/src/core/env.rs

@@ -137,9 +137,16 @@ fn parse_crdt(src: &str) -> Result<[u16; TARGET_CRDT_LEN], String> {
     version = version_info_str!(),
 )]
 pub struct MayastorCliArgs {
-    #[clap(short = 'g', default_value = grpc::default_endpoint_str())]
+    #[clap(short = 'g', long = "grpc-endpoint")]
+    #[deprecated = "Use grpc_ip and grpc_port instead"]
     /// IP address and port (optional) for the gRPC server to listen on.
-    pub grpc_endpoint: String,
+    pub deprecated_grpc_endpoint: Option<String>,
+    #[clap(default_value_t = std::net::IpAddr::V6(std::net::Ipv6Addr::UNSPECIFIED))]
+    /// IP address for the gRPC server to listen on.
+    pub grpc_ip: std::net::IpAddr,
+    #[clap(default_value_t = 10124)]
+    /// Port for the gRPC server to listen on.
+    pub grpc_port: u16,
     #[clap(short = 'R')]
     /// Registration grpc endpoint
     pub registration_endpoint: Option<Uri>,
@@ -310,8 +317,11 @@ impl MayastorFeatures {
 /// Defaults are redefined here in case of using it during tests
 impl Default for MayastorCliArgs {
     fn default() -> Self {
+        #[allow(deprecated)]
         Self {
-            grpc_endpoint: grpc::default_endpoint().to_string(),
+            deprecated_grpc_endpoint: None,
+            grpc_ip: std::net::IpAddr::V6(std::net::Ipv6Addr::UNSPECIFIED),
+            grpc_port: 10124,
             ps_endpoint: None,
             ps_timeout: Duration::from_secs(10),
             ps_retries: 30,
@@ -355,6 +365,15 @@ impl MayastorCliArgs {
     pub fn make_hostnqn(&self) -> Option<String> {
         make_hostnqn(self.node_name.as_ref())
     }
+
+    pub fn grpc_endpoint(&self) -> std::net::SocketAddr {
+        #[allow(deprecated)]
+        if let Some(deprecated_endpoint) = &self.deprecated_grpc_endpoint {
+            grpc::endpoint_from_str(deprecated_endpoint, self.grpc_port)
+        } else {
+            std::net::SocketAddr::new(self.grpc_ip, self.grpc_port)
+        }
+    }
 }
 
 /// Global exit code of the program, initially set to -1 to capture double
@@ -588,7 +607,7 @@ static MAYASTOR_DEFAULT_ENV: OnceCell<parking_lot::Mutex<MayastorEnvironment>> =
 impl MayastorEnvironment {
     pub fn new(args: MayastorCliArgs) -> Self {
         Self {
-            grpc_endpoint: Some(grpc::endpoint(args.grpc_endpoint)),
+            grpc_endpoint: Some(args.grpc_endpoint()),
             registration_endpoint: args.registration_endpoint,
             ps_endpoint: args.ps_endpoint,
             ps_timeout: args.ps_timeout,

io-engine/src/grpc/mod.rs

@@ -216,43 +216,13 @@ pub async fn acquire_subsystem_lock<'a>(
     }
 }
 
-macro_rules! default_ip {
-    () => {
-        "0.0.0.0"
-    };
-}
-
-macro_rules! default_port {
-    () => {
-        10124
-    };
-}
-
-/// Default server port
-pub fn default_port() -> u16 {
-    default_port!()
-}
-
-/// Default endpoint - ip:port
-pub fn default_endpoint_str() -> &'static str {
-    concat!(default_ip!(), ":", default_port!())
-}
-
-/// Default endpoint - ip:port
-pub fn default_endpoint() -> std::net::SocketAddr {
-    default_endpoint_str()
-        .parse()
-        .expect("Expected a valid endpoint")
-}
-
 /// If endpoint is missing a port number then add the default one.
-pub fn endpoint(endpoint: String) -> std::net::SocketAddr {
+pub fn endpoint_from_str(endpoint: &str, port: u16) -> std::net::SocketAddr {
     (if endpoint.contains(':') {
-        endpoint
+        endpoint.parse()
     } else {
-        format!("{}:{}", endpoint, default_port())
+        format!("{}:{}", endpoint, port).parse()
     })
-    .parse()
     .expect("Invalid gRPC endpoint")
 }
 

io-engine/tests/nexus_add_remove.rs

@@ -250,7 +250,6 @@ async fn nexus_add_remove() {
         log_components: vec!["all".into()],
         reactor_mask: "0x3".to_string(),
         no_pci: true,
-        grpc_endpoint: "0.0.0.0".to_string(),
         ..Default::default()
     });
 

io-engine/tests/nexus_children_add_remove.rs

@@ -35,7 +35,6 @@ pub fn mayastor() -> &'static MayastorTest<'static> {
         MayastorTest::new(MayastorCliArgs {
             reactor_mask: "0x2".to_string(),
             no_pci: true,
-            grpc_endpoint: "0.0.0.0".to_string(),
             ..Default::default()
         })
     })