From c2dcd1e90c66f10bf7d1e65406b796bfab0d9ac1 Mon Sep 17 00:00:00 2001
From: Igor Pashev <pashev.igor@gmail.com>
Date: Thu, 2 Jul 2020 19:02:24 +0200
Subject: [PATCH] Improve and unify server operations in CLI (#2040)

This affects commands:

  1. iml server add
  2. iml server remove
  3. iml server force-remove

All of them now use multiple (one and more) hostlist expressions,
for example:

    iml server remove mds[1,2].local c1.local
    iml server add -p stratagem_client c1.local

Signed-off-by: Igor Pashev <pashev.igor@gmail.com>
---
 iml-manager-cli/src/server.rs        | 47 ++++++++++++++++++----------
 iml-system-test-utils/src/vagrant.rs |  2 +-
 vagrant/scripts/deploy_hosts.sh      |  2 +-
 3 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/iml-manager-cli/src/server.rs b/iml-manager-cli/src/server.rs
index c7d35b1527..bac1e1ee8c 100644
--- a/iml-manager-cli/src/server.rs
+++ b/iml-manager-cli/src/server.rs
@@ -28,18 +28,8 @@ use std::{
 use structopt::StructOpt;
 use tokio::io::{stdin, AsyncReadExt};
 
-#[derive(StructOpt, Debug)]
-pub struct RemoveHosts {
-    /// The host(s) to remove. Takes a hostlist expression
-    #[structopt(short = "d", long = "delete_hosts")]
-    hosts: String,
-}
-
 #[derive(StructOpt, Debug)]
 pub struct AddHosts {
-    /// The host(s) to update. Takes a hostlist expression
-    #[structopt(short = "h", long = "hosts")]
-    hosts: String,
     /// The profile to deploy to each host
     #[structopt(short = "p", long = "profile")]
     profile: String,
@@ -58,6 +48,10 @@ pub struct AddHosts {
     /// Password for "password" or password for "private" key
     #[structopt(long, short = "P", required_if("auth", "password"))]
     password: Option<String>,
+
+    /// Hostlist expressions, e. g. mds[1,2].local
+    #[structopt(required = true, min_values = 1)]
+    hosts: Vec<String>,
 }
 
 #[derive(Debug, StructOpt)]
@@ -79,11 +73,19 @@ pub enum ServerCommand {
     Add(AddHosts),
     /// Remove servers from IML
     #[structopt(name = "remove")]
-    Remove(RemoveHosts),
+    Remove {
+        /// Hostlist expressions, e. g. mds[1,2].local
+        #[structopt(required = true, min_values = 1)]
+        hosts: Vec<String>,
+    },
     /// Remove servers from IML DB, but leave agents in place.
     /// Takes a hostlist expression of servers to remove.
     #[structopt(name = "force-remove")]
-    ForceRemove { hosts: String },
+    ForceRemove {
+        /// Hostlist expressions, e. g. mds[1,2].local
+        #[structopt(required = true, min_values = 1)]
+        hosts: Vec<String>,
+    },
     /// Work with Server Profiles
     #[structopt(name = "profile")]
     Profile {
@@ -183,6 +185,19 @@ pub struct Objects<T> {
     objects: Vec<T>,
 }
 
+fn parse_hosts(hosts: &[String]) -> Result<BTreeSet<String>, ImlManagerCliError> {
+    let parsed: Vec<BTreeSet<String>> = hosts
+        .iter()
+        .map(|x| hostlist_parser::parse(x))
+        .collect::<Result<_, _>>()?;
+
+    let union = parsed
+        .into_iter()
+        .fold(BTreeSet::new(), |acc, h| acc.union(&h).cloned().collect());
+
+    Ok(union)
+}
+
 /// Given an expanded hostlist and a list of API host objects
 /// returns a tuple of hosts that match a hostlist item, and the remaining hostlist items
 /// that did not match anything.
@@ -492,7 +507,7 @@ async fn deploy_agents(
 async fn add_server(config: AddHosts) -> Result<(), ImlManagerCliError> {
     let term = Term::stdout();
 
-    let new_hosts = hostlist_parser::parse(&config.hosts)?;
+    let new_hosts = parse_hosts(&config.hosts)?;
 
     tracing::debug!("Parsed hosts {:?}", new_hosts);
 
@@ -528,7 +543,7 @@ pub async fn server_cli(command: ServerCommand) -> Result<(), ImlManagerCliError
         }
         ServerCommand::Add(config) => add_server(config).await?,
         ServerCommand::ForceRemove { hosts } => {
-            let remove_hosts = hostlist_parser::parse(&hosts)?;
+            let remove_hosts = parse_hosts(&hosts)?;
 
             tracing::debug!("Parsed hosts {:?}", remove_hosts);
 
@@ -597,8 +612,8 @@ pub async fn server_cli(command: ServerCommand) -> Result<(), ImlManagerCliError
 
             wait_for_cmds_success(&[command]).await?;
         }
-        ServerCommand::Remove(config) => {
-            let remove_hosts = hostlist_parser::parse(&config.hosts)?;
+        ServerCommand::Remove { hosts } => {
+            let remove_hosts = parse_hosts(&hosts)?;
 
             tracing::debug!("Parsed hosts {:?}", remove_hosts);
 
diff --git a/iml-system-test-utils/src/vagrant.rs b/iml-system-test-utils/src/vagrant.rs
index b22f26f0c1..93e40ebbd0 100644
--- a/iml-system-test-utils/src/vagrant.rs
+++ b/iml-system-test-utils/src/vagrant.rs
@@ -381,7 +381,7 @@ pub async fn setup_deploy_servers(
 
         run_vm_command(
             config.manager,
-            &format!("iml server add -h {} -p {}", hosts.join(","), profile),
+            &format!("iml server add -p {} {}", profile, hosts.join(",")),
         )
         .await?
         .checked_status()
diff --git a/vagrant/scripts/deploy_hosts.sh b/vagrant/scripts/deploy_hosts.sh
index cefff58cd0..4441e0d01b 100644
--- a/vagrant/scripts/deploy_hosts.sh
+++ b/vagrant/scripts/deploy_hosts.sh
@@ -1,3 +1,3 @@
 #!/bin/bash
 
-iml server add --hosts mds[1,2].local,oss[1,2].local --profile $1
+iml server add --profile $1 mds[1,2].local oss[1,2].local