Merge pull request #13491 from SimonRichardson/run-action-wait-for

#13491

The wait flag allows you to specify no-time arguments for run action,
which then allows you to wait forever (it prevents a timer from running).
Also, passing `wait=true` is semantically the same.

The problem with the last changes is it prevented the use of this and
error'd out with the timeout being too small. The fix is to expose if
the flag is in the forever mode.

## QA steps

See: #13477

```sh
$ juju bootstrap microk8s test
$ juju deploy hello-kubecon
$ juju run-action hello-kubecon/0 pull-site --wait
unit-hello-kubecon-0:
 UnitId: hello-kubecon/0
 id: "55"
 results:
 result: site pulled
 status: completed
 timing:
 completed: 2021-11-04 15:46:58 +0000 UTC
 enqueued: 2021-11-04 15:46:57 +0000 UTC
 started: 2021-11-04 15:46:57 +0000 UTC
```

cmd/juju/action/runaction.go

@@ -30,7 +30,6 @@ type runActionCommand struct {
 	ActionCommandBase
 	api           APIClient
 	unitReceivers []string
-	leaders       map[string]string
 	actionName    string
 	paramsYAML    cmd.FileVar
 	parseStrings  bool
@@ -113,11 +112,10 @@ func (c *runActionCommand) Init(args []string) (err error) {
 		return errors.New("no action specified")
 	}
 
-	// force timeout to be greater or equal to 1ms
-	if c.wait.String() != "" {
-		if c.wait.d.Milliseconds() < 1 {
-			return errors.New("timeout must be greater or equal to 1 ms")
-		}
+	// Force the timeout to be greater or equal to 1ms if we're not waiting
+	// forever.
+	if timeout, forever := c.wait.Get(); !forever && (timeout > 0 && timeout.Milliseconds() < 1) {
+		return errors.New("timeout must be greater or equal to 1 ms")
 	}
 
 	// Parse CLI key-value args if they exist.
@@ -263,7 +261,7 @@ func (c *runActionCommand) Run(ctx *cmd.Context) error {
 	if c.wait.d.Nanoseconds() <= 0 {
 		// Indefinite wait. Discard the tick.
 		wait = time.NewTimer(0 * time.Second)
-		_ = <-wait.C
+		<-wait.C
 	} else {
 		wait = time.NewTimer(c.wait.d)
 	}

cmd/juju/action/runaction_test.go

@@ -185,6 +185,23 @@ func (s *RunActionSuite) TestInit(c *gc.C) {
 		should:      "reject timeouts smaller than 1 ms",
 		args:        []string{"mysql/leader", "valid-action-name", "--wait=99ns"},
 		expectError: "timeout must be greater or equal to 1 ms",
+	}, {
+		should: "do not reject forever timeouts",
+		args: []string{
+			validUnitId,
+			"valid-action-name",
+			"foo.bar=2",
+			"foo.baz.bo=y",
+			"bar.foo=hello",
+			"--wait",
+		},
+		expectUnits:  []string{validUnitId},
+		expectAction: "valid-action-name",
+		expectKVArgs: [][]string{
+			{"foo", "bar", "2"},
+			{"foo", "baz", "bo", "y"},
+			{"bar", "foo", "hello"},
+		},
 	}}
 
 	for i, t := range tests {

cmd/juju/action/waitflag.go

@@ -8,8 +8,8 @@ import (
 )
 
 // A gnuflag.Value for the --wait command line argument. If called
-// as a boolean  with no arguments, the forever flag is set to true.
-// If called  with an argument, d is set to the result of
+// as a boolean with no arguments, the forever flag is set to true.
+// If called with an argument, d is set to the result of
 // time.ParseDuration().
 // eg:
 //   --wait
@@ -32,6 +32,15 @@ func (f *waitFlag) Set(s string) error {
 	return nil
 }
 
+// Get returns the time.Duration for a given wait flag or if we've been told
+// to wait forever.
+func (f *waitFlag) Get() (time.Duration, bool) {
+	if f.forever {
+		return f.d, true
+	}
+	return f.d, false
+}
+
 func (f *waitFlag) String() string {
 	if f.forever {
 		return "true"

cmd/juju/action/waitflag_test.go

@@ -0,0 +1,41 @@
+// Copyright 2021 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package action
+
+import (
+	"time"
+
+	jc "github.com/juju/testing/checkers"
+	gc "gopkg.in/check.v1"
+)
+
+type WaitFlagSuite struct {
+}
+
+var _ = gc.Suite(&WaitFlagSuite{})
+
+func (s *WaitFlagSuite) TestEmpty(c *gc.C) {
+	var flag waitFlag
+	timeout, forever := flag.Get()
+	c.Assert(forever, gc.Equals, false)
+	c.Assert(timeout, gc.Equals, time.Duration(0))
+}
+
+func (s *WaitFlagSuite) TestForever(c *gc.C) {
+	var flag waitFlag
+	err := flag.Set("true")
+	c.Assert(err, jc.ErrorIsNil)
+	timeout, forever := flag.Get()
+	c.Assert(forever, gc.Equals, true)
+	c.Assert(timeout, gc.Equals, time.Duration(0))
+}
+
+func (s *WaitFlagSuite) TestDuration(c *gc.C) {
+	var flag waitFlag
+	err := flag.Set("1ms")
+	c.Assert(err, jc.ErrorIsNil)
+	timeout, forever := flag.Get()
+	c.Assert(forever, gc.Equals, false)
+	c.Assert(timeout, gc.Equals, time.Millisecond)
+}