Merge branch 'all-at-once' into 'main'

All at once

See merge request larswirzenius/vmadm!27

8 files changed, 65 insertions, 10 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 344d589..71c0ed2 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -516,7 +516,7 @@ dependencies = [
 
 [[package]]
 name = "vmadm"
-version = "0.2.1"
+version = "0.3.0"
 dependencies = [
  "anyhow",
  "bytesize",
diff --git a/src/cmd/delete.rs b/src/cmd/delete.rs
index 66eb23f..fbb6ab5 100644
--- a/src/cmd/delete.rs
+++ b/src/cmd/delete.rs
@@ -10,7 +10,12 @@ use log::info;
 pub fn delete(specs: &[Specification]) -> Result<(), VirtError> {
     let libvirt = Libvirt::connect("qemu:///system")?;
     for spec in specs {
+        info!("asking virtual machine {} to shutdown", spec.name);
+        libvirt.trigger_shutdown(&spec.name)?;
+    }
+    for spec in specs {
         info!("deleting virtual machine {}", spec.name);
+        libvirt.shutdown(&spec.name)?;
         libvirt.delete(&spec.name, &spec.image)?;
     }
     Ok(())
diff --git a/src/cmd/new.rs b/src/cmd/new.rs
index 39f54c9..4b6a4a3 100644
--- a/src/cmd/new.rs
+++ b/src/cmd/new.rs
@@ -64,7 +64,9 @@ pub fn new(specs: &[Specification]) -> Result<(), NewError> {
         args.set_memory(spec.memory_mib);
         args.set_vcpus(spec.cpus);
         virt_install(&args, &iso)?;
+    }
 
+    for spec in specs {
         info!("waiting for {} to open its SSH port", spec.name);
         wait_for_ssh(&spec.name);
 
diff --git a/src/cmd/shutdown.rs b/src/cmd/shutdown.rs
index b53ebd3..cdd3069 100644
--- a/src/cmd/shutdown.rs
+++ b/src/cmd/shutdown.rs
@@ -9,7 +9,7 @@ pub fn shutdown(specs: &[Specification]) -> Result<(), VirtError> {
     let libvirt = Libvirt::connect("qemu:///system")?;
     for spec in specs {
         info!("shutting down virtual machine {}", spec.name);
-        libvirt.shutdown(&spec.name)?;
+        libvirt.trigger_shutdown(&spec.name)?;
     }
     for spec in specs {
         debug!("waiting for {} to become inactive", spec.name);
diff --git a/src/cmd/start.rs b/src/cmd/start.rs
index a69f54c..8e74c49 100644
--- a/src/cmd/start.rs
+++ b/src/cmd/start.rs
@@ -22,7 +22,7 @@ pub fn start(specs: &[Specification]) -> Result<(), StartError> {
     let libvirt = Libvirt::connect("qemu:///system")?;
     for spec in specs {
         info!("starting virtual machine {}", spec.name);
-        libvirt.start(&spec.name)?;
+        libvirt.trigger_start(&spec.name)?;
     }
     for spec in specs {
         wait_for_ssh(&spec.name);
diff --git a/src/libvirt.rs b/src/libvirt.rs
index c93bd9f..2162f3c 100644
--- a/src/libvirt.rs
+++ b/src/libvirt.rs
@@ -87,17 +87,38 @@ impl Libvirt {
         Ok(())
     }
 
-    pub fn start(&self, name: &str) -> Result<(), VirtError> {
+    pub fn trigger_start(&self, name: &str) -> Result<(), VirtError> {
         if let Some(domain) = self.get_domain(name)? {
-            domain.create()?;
+            if !domain.is_active()? {
+                debug!("starting {}", name);
+                domain.create()?;
+            }
+        }
+        Ok(())
+    }
+
+    pub fn start(&self, name: &str) -> Result<(), VirtError> {
+        if let Some(_) = self.get_domain(name)? {
+            self.trigger_start(name)?;
             wait_for_ssh(name);
         }
         Ok(())
     }
 
+    pub fn trigger_shutdown(&self, name: &str) -> Result<(), VirtError> {
+        if let Some(domain) = self.get_domain(name)? {
+            debug!("asking {} to shut down", name);
+            if domain.is_active()? {
+                domain.shutdown()?;
+            }
+        }
+        Ok(())
+    }
+
     pub fn shutdown(&self, name: &str) -> Result<(), VirtError> {
         if let Some(domain) = self.get_domain(name)? {
-            domain.shutdown()?;
+            debug!("shutting down {}", name);
+            self.trigger_shutdown(name)?;
             wait_until_inactive(&domain, name);
         }
         Ok(())
@@ -105,10 +126,7 @@ impl Libvirt {
 
     pub fn delete(&self, name: &str, image: &Path) -> Result<(), VirtError> {
         if let Some(domain) = self.get_domain(name)? {
-            debug!("shutting down {}", name);
-            domain.shutdown().ok();
-
-            wait_until_inactive(&domain, name);
+            self.shutdown(name)?;
 
             debug!("undefine {}", name);
             domain.undefine()?;
diff --git a/src/util.rs b/src/util.rs
index 9933f03..f3e104b 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -1,11 +1,13 @@
 //! Utilities.
 
+use log::debug;
 use std::net::TcpStream;
 
 const SSH_PORT: i32 = 22;
 
 // Wait for a virtual machine to have opened its SSH port.
 pub fn wait_for_ssh(name: &str) {
+    debug!("waiting for {} to respond to SSH", name);
     let addr = format!("{}:{}", name, SSH_PORT);
     loop {
         if TcpStream::connect(&addr).is_ok() {
diff --git a/vmadm.md b/vmadm.md
index aa3a704..bfd09b4 100644
--- a/vmadm.md
+++ b/vmadm.md
@@ -142,6 +142,8 @@ that the user can log into it as root via SSH after it has booted.
 This requires that the environment it set up so that virtual machines
 can be addressed by name.
 
+First some setup.
+
 ~~~scenario
 given an installed vmadm
 given a Debian 10 OpenStack cloud image
@@ -152,6 +154,11 @@ given file .ssh/id_rsa from ssh_key
 given file .ssh/id_rsa.pub from ssh_key_pub
 given file .ssh/config from ssh_config
 given file .ssh/known_hosts from known_hosts
+~~~
+
+Then we create the VM, and verify that we can log in.
+
+~~~
 when I invoke vmadm new --config config.yaml smoke.yaml
 when I invoke ssh -F .ssh/config debian@smoke hostname
 then stdout contains "smoke"
@@ -159,9 +166,30 @@ when I invoke ssh -F .ssh/config debian@smoke df -h /
 then stdout contains "4.9G"
 when I invoke ssh -F .ssh/config debian@smoke free -m
 then stdout contains "1997"
+~~~
+
+Then we shut it down, twice. The second time is to verify shutting
+down works even if the VM is already shut down.
+
+~~~
+when I invoke vmadm shutdown --config config.yaml smoke.yaml
 when I invoke vmadm shutdown --config config.yaml smoke.yaml
+~~~
+
+Then we start it back up again and verify we can log in. Then we start
+it again, while it's already running, to verify that that works.
+
+~~~
 when I invoke vmadm start --config config.yaml smoke.yaml
 when I invoke ssh -F .ssh/config debian@smoke hostname
+when I invoke vmadm start --config config.yaml smoke.yaml
+~~~
+
+Finally, we delete it twice.
+
+~~~
+when I invoke vmadm delete --config config.yaml smoke.yaml
+when I invoke vmadm delete --config config.yaml smoke.yaml
 ~~~