Merge pull request juju#10892 from anastasiamac/lp1851849-lp1851852

jujubot · anastasiamac · commit 0bfb87cbdcf5 · 2019-11-11T13:45:48.000+10:00
juju#10892 ## Description of change add-k8s command is frequently piped. However, the behavior of the command has changed to prompt for user input when neither --client nor --controller option was specified to support an ASK-OR-TELL brief. Since the pipe and commands that require user input don't work together, if the pipe was used, the command to fail immediately with a descriptive message. As a drive-by, rename remaining reference to an ephemeral --client-only option. ## QA steps 1. $ microk8s.config | juju add-k8s k8s ``` This operation can be applied to both a copy on this client and to the one on a controller. ERROR The command is piped and Juju cannot prompt to clarify whether the --client or a --controller is to be used. Please clarify by re-running the command with the desired option(s). ``` 2. $ juju add-credential aws (to ensure everything else works) ``` This operation can be applied to both a copy on this client and to the one on a controller. No current controller was detected and there are no registered controllers on this client: either bootstrap one or register one. Do you ONLY want to add a credential to this client? (Y/n): ``` ## Bug reference https://bugs.launchpad.net/juju/+bug/1851849 https://bugs.launchpad.net/juju/+bug/1851852
diff --git a/cmd/juju/caas/add_test.go b/cmd/juju/caas/add_test.go
@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 
 	"github.com/golang/mock/gomock"
@@ -1044,12 +1045,10 @@ func (s *addCAASSuite) TestCorrectPromptOrderFromStdIn(c *gc.C) {
 	c.Assert(stdIn, gc.NotNil)
 	defer stdIn.Close()
 	ctx, err := s.runCommand(c, stdIn, command, "myk8s")
-	c.Assert(err, gc.ErrorMatches, "EOF")
-	c.Assert(cmdtesting.Stdout(ctx), gc.Equals, "Do you want to add k8s cloud myk8s to:\n"+
-		"    1. client only (--client)\n"+
-		"    2. controller \"foo\" only (--controller foo)\n"+
-		"    3. both (--client --controller foo)\n"+
-		"Enter your choice, or type Q|q to quit: ")
+	c.Assert(errors.Cause(err), gc.ErrorMatches, regexp.QuoteMeta(`
+The command is piped and Juju cannot prompt to clarify whether the --client or a --controller is to be used.
+Please clarify by re-running the command with the desired option(s).`[1:]))
+	c.Assert(cmdtesting.Stdout(ctx), gc.Equals, "")
 	c.Assert(cmdtesting.Stderr(ctx), gc.Equals, "This operation can be applied to both a copy on this client and to the one on a controller.\n")
 }
 
diff --git a/cmd/modelcmd/controller.go b/cmd/modelcmd/controller.go
@@ -18,6 +18,7 @@ import (
 	"github.com/juju/juju/api/controller"
 	"github.com/juju/juju/api/modelmanager"
 	"github.com/juju/juju/api/usermanager"
+	jujucmd "github.com/juju/juju/cmd"
 	"github.com/juju/juju/cmd/juju/interact"
 	"github.com/juju/juju/juju/osenv"
 	"github.com/juju/juju/jujuclient"
@@ -414,7 +415,7 @@ func (c *OptionalControllerCommand) SetFlags(f *gnuflag.FlagSet) {
 	f.StringVar(&c.ControllerName, "c", "", "Controller to operate in")
 	f.StringVar(&c.ControllerName, "controller", "", "")
 	// TODO (juju3) remove me
-	f.BoolVar(&c.Local, "local", false, "DEPRECATED (use --client-only): Local operation only; controller not affected")
+	f.BoolVar(&c.Local, "local", false, "DEPRECATED (use --client): Local operation only; controller not affected")
 }
 
 // Init populates the command with the args from the command line.
@@ -451,6 +452,10 @@ func (c *OptionalControllerCommand) MaybePrompt(ctxt *cmd.Context, action string
 	}
 
 	ctxt.Infof("This operation can be applied to both a copy on this client and to the one on a controller.")
+	if jujucmd.IsPiped(ctxt) {
+		return errors.Errorf("The command is piped and Juju cannot prompt to clarify whether the --client or a --controller is to be used.\n" +
+			"Please clarify by re-running the command with the desired option(s).")
+	}
 	if currentController == "" {
 		msg := "No current controller was detected"
 		all, err := c.Store.AllControllers()
diff --git a/cmd/supercommand.go b/cmd/supercommand.go
@@ -13,6 +13,7 @@ import (
 	"github.com/juju/os/series"
 	"github.com/juju/utils/arch"
 	"github.com/juju/version"
+	"golang.org/x/crypto/ssh/terminal"
 
 	"github.com/juju/juju/juju/osenv"
 	jujuversion "github.com/juju/juju/version"
@@ -86,3 +87,10 @@ func Info(i *cmd.Info) *cmd.Info {
 	info.ShowSuperFlags = []string{"show-log", "debug", "logging-config", "verbose", "quiet", "h", "help"}
 	return &info
 }
+
+// IsPiped determines if the command was used in a pipe and,
+// hence, it's stdin is not usable for user input.
+func IsPiped(ctx *cmd.Context) bool {
+	stdIn, ok := ctx.Stdin.(*os.File)
+	return ok && !terminal.IsTerminal(int(stdIn.Fd()))
+}
