Merge pull request #2404 from askb/2349_rm_confirm

2349 rm should ask for confirmation before proceeding

commands/commands.go

@@ -239,6 +239,10 @@ var Commands = []cli.Command{
 				Name:  "force, f",
 				Usage: "Remove local configuration even if machine cannot be removed",
 			},
+			cli.BoolFlag{
+				Name:  "y",
+				Usage: "Assumes automatic yes to proceed with remove, without prompting further user confirmation",
+			},
 		},
 		Name:        "rm",
 		Usage:       "Remove a machine",

commands/rm.go

@@ -14,6 +14,7 @@ func cmdRm(c CommandLine, api libmachine.API) error {
 	}
 
 	force := c.Bool("force")
+	confirm := c.Bool("y")
 
 	for _, hostName := range c.Args() {
 		h, err := api.Load(hostName)
@@ -21,6 +22,13 @@ func cmdRm(c CommandLine, api libmachine.API) error {
 			return fmt.Errorf("Error removing host %q: %s", hostName, err)
 		}
 
+		if !confirm && !force {
+			userinput, err := confirmInput(fmt.Sprintf("Do you really want to remove %q?", hostName))
+			if !userinput {
+				return err
+			}
+		}
+
 		if err := h.Driver.Remove(); err != nil {
 			if !force {
 				log.Errorf("Provider error removing machine %q: %s", hostName, err)

commands/rm_test.go

@@ -23,6 +23,11 @@ func TestCmdRmMissingMachineName(t *testing.T) {
 func TestCmdRm(t *testing.T) {
 	commandLine := &commandstest.FakeCommandLine{
 		CliArgs: []string{"machineToRemove1", "machineToRemove2"},
+		LocalFlags: &commandstest.FakeFlagger{
+			Data: map[string]interface{}{
+				"y": true,
+			},
+		},
 	}
 	api := &libmachinetest.FakeAPI{
 		Hosts: []*host.Host{

docs/get-started-cloud.md

@@ -82,6 +82,11 @@ the last section. If we look at `docker-machine ls`, we'll see it is now the
 To remove a host and all of its containers and images, use `docker-machine rm`:
 
     $ docker-machine rm dev staging
+    Do you really want to remove "dev"? (y/n): y
+    Successfully removed dev
+    Do you really want to remove "staging"? (y/n): y
+    Successfully removed staging
+
     $ docker-machine ls
     NAME      ACTIVE   DRIVER       STATE     URL
 

docs/reference/rm.md

@@ -18,7 +18,11 @@ on the cloud provider or virtualization management platform.
     NAME   ACTIVE   DRIVER       STATE     URL
     foo0   -        virtualbox   Running   tcp://192.168.99.105:2376
     foo1   -        virtualbox   Running   tcp://192.168.99.106:2376
+
     $ docker-machine rm foo1
+    Do you really want to remove "foo1"? (y/n): y
+    Successfully removed foo1
+
     $ docker-machine ls
     NAME   ACTIVE   DRIVER       STATE     URL
     foo0   -        virtualbox   Running   tcp://192.168.99.105:2376

test/integration/cli/create-rm.bats

@@ -64,31 +64,53 @@ load ${BASE_TEST_DIR}/helpers.bash
 }
 
 @test "none: rm with no name fails 'machine rm'" {
-  run machine rm
+  run machine rm -y
   last=$(expr ${#lines[@]} - 1)
   [ "$status" -eq 1 ]
   [[ ${lines[$last]} == "Error: Expected to get one or more machine names as arguments" ]]
 }
 
 @test "none: rm non existent machine fails 'machine rm ∞'" {
-  run machine rm ∞
+  run machine rm ∞ -y
   [ "$status" -eq 1 ]
   [[ ${lines[0]} == "Error removing host \"∞\": Host does not exist: \"∞\"" ]]
 }
 
 @test "none: rm is successful 'machine rm 0'" {
-  run machine rm 0
+  run machine rm 0 -y
   [ "$status" -eq 0 ]
 }
 
+@test "none: rm ask user confirmation when -y is not provided 'echo y | machine rm ba'" {
+  run machine create -d none --url none ba
+  [ "$status" -eq 0 ]
+  run bash -c "echo y | machine rm ba"
+  [ "$status" -eq 0 ]
+}
+
+@test "none: rm deny user confirmation when -y is not provided 'echo n | machine rm ab'" {
+  run machine create -d none --url none ab
+  [ "$status" -eq 0 ]
+  run bash -c "echo n | machine rm ab"
+  [ "$status" -eq 0 ]
+}
+
+@test "none: rm never prompt user confirmation with -f is provided 'echo n | machine rm -f ab'" {
+  run machine create -d none --url none c
+  [ "$status" -eq 0 ]
+  run bash -c "machine rm -f c"
+  [ "$status" -eq 0 ]
+  [[ ${lines[0]} == "Successfully removed c" ]]
+}
+
 # Should be replaced by the test below
 @test "none: rm is successful 'machine rm a'" {
-  run machine rm a
+  run machine rm a -y
   [ "$status" -eq 0 ]
 }
 
 @test "none: rm is case insensitive 'machine rm A'" {
   skip
-  run machine rm A
+  run machine rm A -y
   [ "$status" -eq 0 ]
 }

test/integration/cli/ls.bats

@@ -9,7 +9,7 @@ setup () {
 }
 
 teardown () {
-  machine rm $(machine ls -q)
+  machine rm -y $(machine ls -q)
   echo_to_log
 }
 

test/integration/run-bats.sh

@@ -15,7 +15,7 @@ function quiet_run () {
 
 function cleanup_machines() {
     if [[ $(machine ls -q | wc -l) -ne 0 ]]; then
-        quiet_run machine rm -f $(machine ls -q)
+        quiet_run machine rm -y -f $(machine ls -q)
     fi
 }