FIX: Check ZooKeeper with ruok over SSH before deletion

cmd/root.go

@@ -26,8 +26,10 @@ var versionCmd = &cobra.Command{
 }
 
 var rootCmd = &cobra.Command{
-	Use:  "arcusctl",
-	Long: `arcusctl is a CLI tool for managing and operating ARCUS cache clusters.`,
+	Use:           "arcusctl",
+	Long:          `arcusctl is a CLI tool for managing and operating Arcus cache clusters.`,
+	SilenceUsage:  true,
+	SilenceErrors: true,
 }
 
 func init() {
@@ -45,7 +47,7 @@ func init() {
 
 func Execute() {
 	if err := rootCmd.Execute(); err != nil {
-		fmt.Fprintln(os.Stderr, err)
+		fmt.Fprintln(os.Stderr, "Error:", err)
 		os.Exit(1)
 	}
 }

cmd/zk/delete.go

@@ -9,11 +9,12 @@ var deleteCmd = &cobra.Command{
 	Use:   "delete <ensemble-name>",
 	Short: "Delete a ZooKeeper ensemble",
 	Args:  cobra.ExactArgs(1),
-	Run: func(cmd *cobra.Command, args []string) {
+	RunE: func(cmd *cobra.Command, args []string) error {
 		purge, _ := cmd.Flags().GetBool("purge")
 		if err := zk.Delete(args[0], purge); err != nil {
-			panic(err)
+			return err
 		}
+		return nil
 	},
 }
 

internal/ssh/ssh.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"strings"
 )
 
 func withOptions(args ...string) []string {
@@ -43,3 +44,21 @@ func FileExists(host string, remotePath string) (bool, error) {
 
 	return false, err
 }
+
+// Quote wraps a value in shell quotes so spaces and special characters
+// are treated literally as part of a single argument.
+func Quote(s string) string {
+	return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
+}
+
+// RunOutput executes a command on a remote host via SSH and returns its output as a string.
+func RunOutput(host string, command string) (string, error) {
+	cmd := exec.Command("ssh", withOptions(host, command)...)
+	cmd.Stderr = os.Stderr
+
+	output, err := cmd.Output()
+	if err != nil {
+		return string(output), fmt.Errorf("execute command on %s: %w", host, err)
+	}
+	return string(output), nil
+}

internal/ssh/ssh_test.go

@@ -0,0 +1,67 @@
+package ssh
+
+import "testing"
+
+func TestQuote(t *testing.T) {
+	tests := []struct {
+		name  string
+		input string
+		want  string
+	}{
+		{
+			name:  "empty",
+			input: "",
+			want:  "''",
+		},
+		{
+			name:  "plain path",
+			input: "/opt/zookeeper",
+			want:  "'/opt/zookeeper'",
+		},
+		{
+			name:  "spaces",
+			input: "/opt/zookeeper data",
+			want:  "'/opt/zookeeper data'",
+		},
+		{
+			name:  "single quote",
+			input: "/opt/user's data",
+			want:  `'/opt/user'\''s data'`,
+		},
+		{
+			name:  "multiple single quotes",
+			input: "a'b'c",
+			want:  `'a'\''b'\''c'`,
+		},
+		{
+			name:  "only single quote",
+			input: "'",
+			want:  `''\'''`,
+		},
+		{
+			name:  "double quotes",
+			input: `/opt/"data"`,
+			want:  `'/opt/"data"'`,
+		},
+		{
+			name:  "shell metacharacters",
+			input: "/opt/$HOME/$(whoami);*",
+			want:  "'/opt/$HOME/$(whoami);*'",
+		},
+		{
+			name:  "newline",
+			input: "/opt/data\nlogs",
+			want:  "'/opt/data\nlogs'",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := Quote(tt.input)
+			if got != tt.want {
+				t.Errorf("Quote(%q) = %q, want %q",
+					tt.input, got, tt.want)
+			}
+		})
+	}
+}

internal/zk/delete.go

@@ -1,7 +1,9 @@
 package zk
 
 import (
+	"errors"
 	"fmt"
+	"os/exec"
 	"path"
 	"strings"
 
@@ -11,19 +13,18 @@ import (
 	"github.com/jam2in/arcusctl/internal/topology"
 )
 
-const removeCommandTemplate = "rm -rf %s"
+const (
+	removeCommandTemplate = "rm -rf %s"
+	exitNCFailed          = 1
+)
 
 func Delete(ensembleName string, purge bool) error {
 	meta, topo, err := loadEnsemble(ensembleName)
 	if err != nil {
 		return err
 	}
 
-	if err := verifyTopology(topo.Servers, topo.Path, topo.Name); err != nil {
-		return err
-	}
-
-	if err := verifyAllStopped(topo.Servers, topo.Path, topo.Name); err != nil {
+	if err := verifyAllStopped(topo); err != nil {
 		return err
 	}
 
@@ -37,7 +38,7 @@ func Delete(ensembleName string, purge bool) error {
 	for host, servers := range hostsMap {
 		fmt.Printf("Removing files on %s...\n", host)
 		if err := removeHostFiles(host, servers, topo.Path, topo.Name); err != nil {
-			return fmt.Errorf("remove files on %s: %w", host, err)
+			return err
 		}
 	}
 
@@ -57,34 +58,42 @@ func Delete(ensembleName string, purge bool) error {
 	return nil
 }
 
-func verifyTopology(
-	servers []topology.ZKServer,
-	topoPath string,
-	ensembleName string,
-) error {
-	for _, server := range servers {
-		confDir := zkConfigDir(topoPath, ensembleName, server.MyID)
+func verifyAllStopped(topo *topology.ZKTopology) error {
+	for _, server := range topo.Servers {
+		output, err := runRuok(server)
 
-		if err := ssh.Run(server.Host(), fmt.Sprintf("test -d %s", confDir)); err != nil {
-			return fmt.Errorf("topology mismatch: %s not found on %s", confDir, server.Host())
+		// If the command succeeded, it means the ZooKeeper is still running and accepting connections.
+		if err == nil {
+			return fmt.Errorf(
+				"ZooKeeper myid=%d on %s is accepting connections;\n"+
+					"stop the ensemble first: arcusctl zk stop %s",
+				server.MyID, server.Host(), topo.Name,
+			)
 		}
-	}
-	return nil
-}
 
-func verifyAllStopped(
-	servers []topology.ZKServer,
-	topoPath string,
-	ensembleName string,
-) error {
-	for _, server := range servers {
-		confDir := zkConfigDir(topoPath, ensembleName, server.MyID)
-		cmd := fmt.Sprintf("pgrep -f '[Q]uorumPeerMain.*%s' > /dev/null 2>&1", confDir)
-		if err := ssh.Run(server.Host(), cmd); err == nil {
-			return fmt.Errorf("server %s (myid=%d) is still running. stop the ensemble before delete",
-				server.Host(), server.MyID)
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) &&
+			exitErr.ExitCode() == exitNCFailed &&
+			strings.Contains(output, "Connection refused") {
+			continue
 		}
+
+		detail := strings.TrimSpace(output)
+		if detail != "" {
+			return fmt.Errorf(
+				"cannot check ZooKeeper myid=%d on %s: %s;\n"+
+					"check SSH access, nc availability, and network connectivity: %w",
+				server.MyID, server.Host(), detail, err,
+			)
+		}
+
+		return fmt.Errorf(
+			"cannot check ZooKeeper myid=%d on %s;\n"+
+				"check SSH access and network connectivity: %w",
+			server.MyID, server.Host(), err,
+		)
 	}
+
 	return nil
 }
 
@@ -104,16 +113,26 @@ func removeHostFiles(
 ) error {
 	var removePaths []string
 
+	// Remove configuration and data directories for each server on the host.
 	for _, server := range servers {
-		// Remove conf/<ensembleName>/zk<myid>.cfg and data/log directories
-		removePaths = append(
-			removePaths,
-			zkConfigDir(topoPath, ensembleName, server.MyID),
-		)
-		removePaths = append(removePaths, nodeDataPaths(server)...)
+		confDir := zkConfigDir(topoPath, ensembleName, server.MyID)
+		removePaths = append(removePaths, ssh.Quote(confDir))
+
+		for _, dataPath := range nodeDataPaths(server) {
+			removePaths = append(removePaths, ssh.Quote(dataPath))
+		}
 	}
 
-	return ssh.Run(host, "rm -rf "+strings.Join(removePaths, " "))
+	cmd := fmt.Sprintf(
+		removeCommandTemplate,
+		strings.Join(removePaths, " "),
+	)
+
+	if err := ssh.Run(host, cmd); err != nil {
+		return fmt.Errorf("remove files on %s: %w", host, err)
+	}
+
+	return nil
 }
 
 func removeInstallationDirs(
@@ -138,7 +157,8 @@ func removeInstallationDirs(
 		}
 
 		fmt.Printf("Removing installation directory on %s...\n", host)
-		if err := ssh.Run(host, fmt.Sprintf(removeCommandTemplate, installPath)); err != nil {
+		cmd := fmt.Sprintf(removeCommandTemplate, ssh.Quote(installPath))
+		if err := ssh.Run(host, cmd); err != nil {
 			return fmt.Errorf("remove installation on %s: %w", host, err)
 		}
 	}

internal/zk/ruok.go

@@ -0,0 +1,26 @@
+package zk
+
+import (
+	"fmt"
+
+	"github.com/jam2in/arcusctl/internal/ssh"
+	"github.com/jam2in/arcusctl/internal/topology"
+)
+
+const (
+	ruokTimeout         = 3
+	ruokCommandTemplate = "echo ruok | nc -v -w %d %s %s 2>&1"
+)
+
+func runRuok(server topology.ZKServer) (string, error) {
+	host, clientPort, _, _ := server.ParseAddress()
+
+	cmd := fmt.Sprintf(
+		ruokCommandTemplate,
+		ruokTimeout,
+		ssh.Quote(host),
+		ssh.Quote(clientPort),
+	)
+
+	return ssh.RunOutput(server.Host(), cmd)
+}