Merge pull request #8437 from jameinel/2.3-into-develop

#8437

## Description of change

Bring develop up to the tip of 2.3.

 prdesc Merge pull request #8435 from anastasiamac/token-used-msg-lp1751849
 prdesc Merge pull request #8433 from manadart/inc-2.3.5
 prdesc Merge pull request #8420 from jameinel/2.3-run-hook-intermittent-1749320
 prdesc Merge pull request #8431 from anastasiamac/block-duplicate-msg-fix-lp1751903
 prdesc Merge pull request #8430 from howbazaar/2.3-fix-pubsub-race

## QA steps

See individual PR.

## Documentation changes

None.

## Bug reference

 prdesc https://bugs.launchpad.net/juju/+bug/1749320
 prdesc https://bugs.launchpad.net/juju/+bug/1751849
 prdesc https://bugs.launchpad.net/juju/+bug/1751903

cmd/juju/block/protection.go

@@ -4,6 +4,8 @@
 package block
 
 import (
+	"fmt"
+
 	"github.com/juju/errors"
 	"github.com/juju/loggo"
 
@@ -91,9 +93,9 @@ func ProcessBlockedError(err error, block Block) error {
 		return nil
 	}
 	if params.IsCodeOperationBlocked(err) {
-		msg := blockedMessages[block]
-		logger.Errorf("%v\n%v", err, msg)
-		return errors.New(msg)
+		msg := fmt.Sprintf("%v\n%v", err, blockedMessages[block])
+		logger.Infof(msg)
+		return errors.Errorf(msg)
 	}
 	return err
 }

cmd/juju/controller/register.go

@@ -270,6 +270,12 @@ func (c *registerCommand) nonPublicControllerDetails(ctx *cmd.Context, registrat
 	}
 	resp, err := c.secretKeyLogin(registrationParams.controllerAddrs, req, controllerName)
 	if err != nil {
+		// If we got here and got an error, the registration token supplied
+		// will be expired.
+		// Log the error as it will be useful for debugging, but give user a
+		// suggestion for the way forward instead of error details.
+		logger.Infof("while validating secret key: %v", err)
+		err = errors.Errorf("Provided registration token may have been expired.\nA controller administrator must reset your user to issue a new token.\nSee %q for more information.", "juju help change-user-password")
 		return errRet(errors.Trace(err))
 	}
 

cmd/juju/controller/register_test.go

@@ -413,7 +413,11 @@ Enter a name for this controller: »foo
 	defer prompter.CheckDone()
 	s.apiOpenError = errors.New("open failed")
 	err := s.run(c, prompter, registrationData)
-	c.Assert(err, gc.ErrorMatches, `open failed`)
+	c.Assert(c.GetTestLog(), gc.Matches, "(.|\n)*open failed(.|\n)*")
+	c.Assert(err, gc.ErrorMatches, `
+Provided registration token may have been expired.
+A controller administrator must reset your user to issue a new token.
+See "juju help change-user-password" for more information.`[1:])
 }
 
 func (s *RegisterSuite) TestRegisterServerError(c *gc.C) {
@@ -440,7 +444,11 @@ Enter a name for this controller: »foo
 		SecretKey: mockSecretKey,
 	})
 	err = s.run(c, prompter, registrationData)
-	c.Assert(err, gc.ErrorMatches, "xyz")
+	c.Assert(c.GetTestLog(), gc.Matches, "(.|\n)* xyz(.|\n)*")
+	c.Assert(err, gc.ErrorMatches, `
+Provided registration token may have been expired.
+A controller administrator must reset your user to issue a new token.
+See "juju help change-user-password" for more information.`[1:])
 
 	// Check that the controller hasn't been added.
 	_, err = s.store.ControllerByName("controller-name")

featuretests/block_test.go

@@ -4,11 +4,15 @@
 package featuretests
 
 import (
+	"fmt"
+
+	"github.com/juju/cmd/cmdtesting"
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
 
 	"github.com/juju/juju/api/block"
 	jujutesting "github.com/juju/juju/juju/testing"
+	"github.com/juju/juju/state/multiwatcher"
 )
 
 type blockSuite struct {
@@ -30,3 +34,25 @@ func (s *blockSuite) TestBlockFacadeCall(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(found, gc.HasLen, 0)
 }
+
+func (s *blockSuite) TestBlockedMessage(c *gc.C) {
+	// Block operation
+	s.blockClient.SwitchBlockOn(fmt.Sprintf("%v", multiwatcher.BlockChange), "TestBlockedMessage")
+
+	ctx, err := runCommand(c, "resolved", "multi-series/2")
+
+	// Whenever Juju blocks are enabled, the operations that are blocked will be expected to err
+	// out silently.
+	c.Assert(err, gc.ErrorMatches, "cmd: error out silently")
+	c.Assert(cmdtesting.Stdout(ctx), gc.Equals, "")
+	c.Assert(cmdtesting.Stderr(ctx), gc.Equals, `
+ERROR TestBlockedMessage (operation is blocked)
+
+All operations that change model have been disabled for the current model.
+To enable changes, run
+
+    juju enable-command all
+
+
+`[1:])
+}

worker/pubsub/remoteserver.go

@@ -191,7 +191,7 @@ func (r *remoteServer) nextMessage() *params.PubSubMessage {
 	return val.(*params.PubSubMessage)
 }
 
-func (r *remoteServer) connect() MessageWriter {
+func (r *remoteServer) connect() bool {
 	stop := make(chan struct{})
 	r.mutex.Lock()
 	r.stopConnecting = stop
@@ -220,9 +220,10 @@ func (r *remoteServer) connect() MessageWriter {
 
 	r.mutex.Lock()
 	r.stopConnecting = nil
-	r.mutex.Unlock()
+	defer r.mutex.Unlock()
 
 	if connection != nil {
+		r.connection = connection
 		r.logger.Infof("forwarding connected %s -> %s", r.origin, r.target)
 		_, err := r.hub.Publish(
 			forwarder.ConnectedTopic,
@@ -231,8 +232,9 @@ func (r *remoteServer) connect() MessageWriter {
 		if err != nil {
 			r.logger.Errorf("%v", err)
 		}
+		return true
 	}
-	return connection
+	return false
 }
 
 func (r *remoteServer) loop() error {
@@ -246,10 +248,7 @@ func (r *remoteServer) loop() error {
 	for {
 		if r.connection == nil {
 			// If we don't have a current connection, try to get one.
-			if connection := r.connect(); connection != nil {
-				r.mutex.Lock()
-				r.connection = connection
-				r.mutex.Unlock()
+			if r.connect() {
 				delay = nil
 			} else {
 				// Skip through the select to try to reconnect.

worker/uniter/runner/debug/server_test.go

@@ -202,11 +202,16 @@ func (s *DebugHooksServerSuite) TestRunHook(c *gc.C) {
 	timeout := time.After(testing.LongWait)
 	envsh := filepath.Join(debugDir, "env.sh")
 	for {
-		if _, err := os.Stat(envsh); err == nil {
-			break
+		// Wait for env.sh to show up, and have some content. If it exists and
+		// is size 0, we managed to see it at exactly the time it is being
+		// written.
+		if st, err := os.Stat(envsh); err == nil {
+			if st.Size() != 0 {
+				break
+			}
 		}
 		select {
-		case <-time.After(testing.ShortWait):
+		case <-time.After(time.Millisecond):
 		case <-timeout:
 			c.Fatal("timed out waiting for env.sh to be written")
 		}